pr feedback

src/model/cloud-runner/providers/aws/aws-client-factory.ts

@@ -13,11 +13,29 @@ export class AwsClientFactory {
   private static cloudWatchLogs: CloudWatchLogs;
   private static s3: S3;
 
+  private static getCredentials() {
+    // Explicitly provide credentials from environment variables for LocalStack compatibility
+    // LocalStack accepts any credentials, but the AWS SDK needs them to be explicitly set
+    const accessKeyId = process.env.AWS_ACCESS_KEY_ID;
+    const secretAccessKey = process.env.AWS_SECRET_ACCESS_KEY;
+
+    if (accessKeyId && secretAccessKey) {
+      return {
+        accessKeyId,
+        secretAccessKey,
+      };
+    }
+
+    // Return undefined to let AWS SDK use default credential chain
+    return undefined;
+  }
+
   static getCloudFormation(): CloudFormation {
     if (!this.cloudFormation) {
       this.cloudFormation = new CloudFormation({
         region: Input.region,
         endpoint: CloudRunnerOptions.awsCloudFormationEndpoint,
+        credentials: AwsClientFactory.getCredentials(),
       });
     }
 
@@ -29,6 +47,7 @@ export class AwsClientFactory {
       this.ecs = new ECS({
         region: Input.region,
         endpoint: CloudRunnerOptions.awsEcsEndpoint,
+        credentials: AwsClientFactory.getCredentials(),
       });
     }
 
@@ -40,6 +59,7 @@ export class AwsClientFactory {
       this.kinesis = new Kinesis({
         region: Input.region,
         endpoint: CloudRunnerOptions.awsKinesisEndpoint,
+        credentials: AwsClientFactory.getCredentials(),
       });
     }
 
@@ -51,6 +71,7 @@ export class AwsClientFactory {
       this.cloudWatchLogs = new CloudWatchLogs({
         region: Input.region,
         endpoint: CloudRunnerOptions.awsCloudWatchLogsEndpoint,
+        credentials: AwsClientFactory.getCredentials(),
       });
     }
 
@@ -63,6 +84,7 @@ export class AwsClientFactory {
         region: Input.region,
         endpoint: CloudRunnerOptions.awsS3Endpoint,
         forcePathStyle: true,
+        credentials: AwsClientFactory.getCredentials(),
       });
     }
 

src/model/cloud-runner/providers/k8s/kubernetes-task-runner.ts

@@ -22,6 +22,8 @@ class KubernetesTaskRunner {
     let shouldReadLogs = true;
     let shouldCleanup = true;
     let retriesAfterFinish = 0;
+    let kubectlLogsFailedCount = 0;
+    const maxKubectlLogsFailures = 3;
     // eslint-disable-next-line no-constant-condition
     while (true) {
       await new Promise((resolve) => setTimeout(resolve, 3000));
@@ -31,16 +33,28 @@ class KubernetesTaskRunner {
       const isRunning = await KubernetesPods.IsPodRunning(podName, namespace, kubeClient);
 
       const callback = (outputChunk: string) => {
+        // Filter out kubectl error messages about being unable to retrieve container logs
+        // These errors pollute the output and don't contain useful information
+        const lowerChunk = outputChunk.toLowerCase();
+        if (lowerChunk.includes('unable to retrieve container logs')) {
+          CloudRunnerLogger.log(`Filtered kubectl error: ${outputChunk.trim()}`);
+          return;
+        }
+
         output += outputChunk;
 
         // split output chunk and handle per line
         for (const chunk of outputChunk.split(`\n`)) {
-          ({ shouldReadLogs, shouldCleanup, output } = FollowLogStreamService.handleIteration(
-            chunk,
-            shouldReadLogs,
-            shouldCleanup,
-            output,
-          ));
+          // Skip empty chunks and kubectl error messages (case-insensitive)
+          const lowerChunk = chunk.toLowerCase();
+          if (chunk.trim() && !lowerChunk.includes('unable to retrieve container logs')) {
+            ({ shouldReadLogs, shouldCleanup, output } = FollowLogStreamService.handleIteration(
+              chunk,
+              shouldReadLogs,
+              shouldCleanup,
+              output,
+            ));
+          }
         }
       };
       try {
@@ -52,11 +66,81 @@ class KubernetesTaskRunner {
           true,
           callback,
         );
+        // Reset failure count on success
+        kubectlLogsFailedCount = 0;
       } catch (error: any) {
+        kubectlLogsFailedCount++;
         await new Promise((resolve) => setTimeout(resolve, 3000));
         const continueStreaming = await KubernetesPods.IsPodRunning(podName, namespace, kubeClient);
         CloudRunnerLogger.log(`K8s logging error ${error} ${continueStreaming}`);
 
+        // Filter out kubectl error messages from the error output
+        const errorMessage = error?.message || error?.toString() || '';
+        const isKubectlLogsError = errorMessage.includes('unable to retrieve container logs for containerd://') || 
+                                   errorMessage.toLowerCase().includes('unable to retrieve container logs');
+        
+        if (isKubectlLogsError) {
+          CloudRunnerLogger.log(`Kubectl unable to retrieve logs, attempt ${kubectlLogsFailedCount}/${maxKubectlLogsFailures}`);
+          
+          // If kubectl logs has failed multiple times, try reading the log file directly from the pod
+          // This works even if the pod is terminated, as long as it hasn't been deleted
+          if (kubectlLogsFailedCount >= maxKubectlLogsFailures && !isRunning && !continueStreaming) {
+            CloudRunnerLogger.log(`Attempting to read log file directly from pod as fallback...`);
+            try {
+              // Try to read the log file from the pod
+              // Use kubectl exec for running pods, or try to access via PVC if pod is terminated
+              let logFileContent = '';
+              
+              if (isRunning) {
+                // Pod is still running, try exec
+                logFileContent = await CloudRunnerSystem.Run(
+                  `kubectl exec ${podName} -c ${containerName} -n ${namespace} -- cat /home/job-log.txt 2>/dev/null || echo ""`,
+                  true,
+                  true,
+                );
+              } else {
+                // Pod is terminated, try to create a temporary pod to read from the PVC
+                // First, check if we can still access the pod's filesystem
+                CloudRunnerLogger.log(`Pod is terminated, attempting to read log file via temporary pod...`);
+                // For terminated pods, we might not be able to exec, so we'll skip this fallback
+                // and rely on the log file being written to the PVC (if mounted)
+                CloudRunnerLogger.logWarning(`Cannot read log file from terminated pod via exec`);
+              }
+              
+              if (logFileContent && logFileContent.trim()) {
+                CloudRunnerLogger.log(`Successfully read log file from pod (${logFileContent.length} chars)`);
+                // Process the log file content line by line
+                for (const line of logFileContent.split(`\n`)) {
+                  const lowerLine = line.toLowerCase();
+                  if (line.trim() && !lowerLine.includes('unable to retrieve container logs')) {
+                    ({ shouldReadLogs, shouldCleanup, output } = FollowLogStreamService.handleIteration(
+                      line,
+                      shouldReadLogs,
+                      shouldCleanup,
+                      output,
+                    ));
+                  }
+                }
+                
+                // Check if we got the end of transmission marker
+                if (FollowLogStreamService.DidReceiveEndOfTransmission) {
+                  CloudRunnerLogger.log('end of log stream (from log file)');
+                  break;
+                }
+              } else {
+                CloudRunnerLogger.logWarning(`Log file read returned empty content, continuing with available logs`);
+                // If we can't read the log file, break out of the loop to return whatever logs we have
+                // This prevents infinite retries when kubectl logs consistently fails
+                break;
+              }
+            } catch (execError: any) {
+              CloudRunnerLogger.logWarning(`Failed to read log file from pod: ${execError}`);
+              // If we've exhausted all options, break to return whatever logs we have
+              break;
+            }
+          }
+        }
+
         // If pod is not running and we tried --previous but it failed, try without --previous
         if (!isRunning && !continueStreaming && error?.message?.includes('previous terminated container')) {
           CloudRunnerLogger.log(`Previous container not found, trying current container logs...`);
@@ -124,7 +208,25 @@ class KubernetesTaskRunner {
       }
     }
 
-    return output;
+    // Filter out kubectl error messages from the final output
+    // These errors can be added via stderr even when kubectl fails
+    // We filter them out so they don't pollute the BuildResults
+    const lines = output.split('\n');
+    const filteredLines = lines.filter(
+      (line) => !line.toLowerCase().includes('unable to retrieve container logs'),
+    );
+    const filteredOutput = filteredLines.join('\n');
+
+    // Log if we filtered out significant content
+    const originalLineCount = lines.length;
+    const filteredLineCount = filteredLines.length;
+    if (originalLineCount > filteredLineCount) {
+      CloudRunnerLogger.log(
+        `Filtered out ${originalLineCount - filteredLineCount} kubectl error message(s) from output`,
+      );
+    }
+
+    return filteredOutput;
   }
 
   static async watchUntilPodRunning(kubeClient: CoreV1Api, podName: string, namespace: string) {

src/model/cloud-runner/tests/cloud-runner-hooks.test.ts

@@ -103,14 +103,18 @@ commands: echo "test"`;
       CloudRunnerLogger.log(`run 2 succeeded`);
 
       const buildContainsBuildSucceeded = results2.includes('Build succeeded');
-      const buildContainsPreBuildHookRunMessage = results2.includes('before-build hook test!');
+      const buildContainsPreBuildHookRunMessage = results2.includes('before-build hook test!!');
       const buildContainsPostBuildHookRunMessage = results2.includes('after-build hook test!');
 
       const buildContainsPreBuildStepMessage = results2.includes('before-build step test!');
       const buildContainsPostBuildStepMessage = results2.includes('after-build step test!');
 
-      // Skip "Build succeeded" check for local-docker when using ubuntu image (Unity doesn't run)
-      if (CloudRunnerOptions.providerStrategy !== 'local' && CloudRunnerOptions.providerStrategy !== 'local-docker') {
+      // Skip "Build succeeded" check for local-docker and aws when using ubuntu image (Unity doesn't run)
+      if (
+        CloudRunnerOptions.providerStrategy !== 'local' &&
+        CloudRunnerOptions.providerStrategy !== 'local-docker' &&
+        CloudRunnerOptions.providerStrategy !== 'aws'
+      ) {
         expect(buildContainsBuildSucceeded).toBeTruthy();
       }
       expect(buildContainsPreBuildHookRunMessage).toBeTruthy();