Skip to content

Commit 956b2e4

Browse files
committed
PR feedback
1 parent 69731ba commit 956b2e4

File tree

8 files changed

+432
-115
lines changed

8 files changed

+432
-115
lines changed

.github/workflows/cloud-runner-integrity.yml

Lines changed: 327 additions & 102 deletions
Large diffs are not rendered by default.

dist/index.js

Lines changed: 43 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

dist/index.js.map

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/model/cloud-runner/providers/k8s/kubernetes-job-spec-factory.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ class KubernetesJobSpecFactory {
6161
backoffLimit: 0,
6262
template: {
6363
spec: {
64+
terminationGracePeriodSeconds: 90, // Give PreStopHook (60s sleep) time to complete
6465
volumes: [
6566
{
6667
name: 'build-mount',

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ class KubernetesPods {
3232
);
3333
}
3434

35+
let containerExitCode: number | undefined;
36+
let containerSucceeded = false;
37+
3538
if (containerStatuses.length > 0) {
3639
containerStatuses.forEach((cs, idx) => {
3740
if (cs.state?.waiting) {
@@ -40,10 +43,15 @@ class KubernetesPods {
4043
);
4144
}
4245
if (cs.state?.terminated) {
46+
const exitCode = cs.state.terminated.exitCode;
47+
containerExitCode = exitCode;
48+
if (exitCode === 0) {
49+
containerSucceeded = true;
50+
}
4351
errorDetails.push(
4452
`Container ${idx} (${cs.name}) terminated: ${cs.state.terminated.reason} - ${
4553
cs.state.terminated.message || ''
46-
} (exit code: ${cs.state.terminated.exitCode})`,
54+
} (exit code: ${exitCode})`,
4755
);
4856
}
4957
});
@@ -53,6 +61,25 @@ class KubernetesPods {
5361
errorDetails.push(`Recent events: ${JSON.stringify(events.slice(-5), undefined, 2)}`);
5462
}
5563

64+
// Check if only PreStopHook failed but container succeeded
65+
const hasPreStopHookFailure = events.some((e) => e.reason === 'FailedPreStopHook');
66+
67+
if (containerSucceeded && containerExitCode === 0) {
68+
// Container succeeded - PreStopHook failure is non-critical
69+
if (hasPreStopHookFailure) {
70+
CloudRunnerLogger.logWarning(
71+
`Pod ${podName} marked as Failed due to PreStopHook failure, but container exited successfully (exit code 0). This is non-fatal.`,
72+
);
73+
} else {
74+
CloudRunnerLogger.log(
75+
`Pod ${podName} container succeeded (exit code 0), but pod phase is Failed. Checking details...`,
76+
);
77+
}
78+
CloudRunnerLogger.log(`Pod details: ${errorDetails.join('\n')}`);
79+
// Don't throw error - container succeeded, PreStopHook failure is non-critical
80+
return false; // Pod is not running, but we don't treat it as a failure
81+
}
82+
5683
const errorMessage = `K8s pod failed\n${errorDetails.join('\n')}`;
5784
CloudRunnerLogger.log(errorMessage);
5885
throw new Error(errorMessage);

src/model/cloud-runner/services/hooks/container-hook-service.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,10 @@ export class ContainerHookService {
334334
if (step.image === undefined) {
335335
step.image = `ubuntu`;
336336
}
337+
// Ensure allowFailure defaults to false if not explicitly set
338+
if (step.allowFailure === undefined) {
339+
step.allowFailure = false;
340+
}
337341
}
338342
if (object === undefined) {
339343
throw new Error(`Failed to parse ${steps}`);

src/model/cloud-runner/services/hooks/container-hook.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,5 @@ export class ContainerHook {
66
public name!: string;
77
public image: string = `ubuntu`;
88
public hook!: string;
9+
public allowFailure: boolean = false; // If true, hook failures won't stop the build
910
}

src/model/cloud-runner/workflows/custom-workflow.ts

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,33 @@ export class CustomWorkflow {
3232
// }
3333
for (const step of steps) {
3434
CloudRunnerLogger.log(`Cloud Runner is running in custom job mode`);
35-
output += await CloudRunner.Provider.runTaskInWorkflow(
36-
CloudRunner.buildParameters.buildGuid,
37-
step.image,
38-
step.commands,
39-
`/${CloudRunnerFolders.buildVolumeFolder}`,
40-
`/${CloudRunnerFolders.projectPathAbsolute}/`,
41-
environmentVariables,
42-
[...secrets, ...step.secrets],
43-
);
35+
try {
36+
const stepOutput = await CloudRunner.Provider.runTaskInWorkflow(
37+
CloudRunner.buildParameters.buildGuid,
38+
step.image,
39+
step.commands,
40+
`/${CloudRunnerFolders.buildVolumeFolder}`,
41+
`/${CloudRunnerFolders.projectPathAbsolute}/`,
42+
environmentVariables,
43+
[...secrets, ...step.secrets],
44+
);
45+
output += stepOutput;
46+
} catch (error: any) {
47+
const allowFailure = step.allowFailure === true;
48+
const stepName = step.name || step.image || 'unknown';
49+
50+
if (allowFailure) {
51+
CloudRunnerLogger.logWarning(
52+
`Hook container "${stepName}" failed but allowFailure is true. Continuing build. Error: ${error?.message || error}`,
53+
);
54+
// Continue to next step
55+
} else {
56+
CloudRunnerLogger.log(
57+
`Hook container "${stepName}" failed and allowFailure is false (default). Stopping build.`,
58+
);
59+
throw error;
60+
}
61+
}
4462
}
4563

4664
return output;

0 commit comments

Comments
 (0)