From f9c9e69f7899f170039c84f835dbb240054e18c2 Mon Sep 17 00:00:00 2001 From: Alex Johnson Date: Thu, 30 May 2024 23:17:32 -0500 Subject: [PATCH] feat: add an allowFailure field to WorkflowStepSpec * do not fail the entire workflow if a step labeled with allowFailure throws an error Signed-off-by: Alex Johnson --- core/src/commands/workflow.ts | 47 +++++++++----- core/src/config/workflow.ts | 11 +++- core/test/unit/src/commands/workflow.ts | 81 +++++++++++++++++++++++++ docs/reference/commands.md | 10 ++- docs/reference/workflow-config.md | 25 +++++++- 5 files changed, 153 insertions(+), 21 deletions(-) diff --git a/core/src/commands/workflow.ts b/core/src/commands/workflow.ts index 87999ad9466..2a9970c65ce 100644 --- a/core/src/commands/workflow.ts +++ b/core/src/commands/workflow.ts @@ -7,7 +7,7 @@ */ import cloneDeep from "fast-copy" -import { flatten, last, repeat, size } from "lodash-es" +import { flatten, last, repeat } from "lodash-es" import { printHeader, getTerminalWidth, renderMessageWithDivider, renderDuration } from "../logger/util.js" import type { CommandParams, CommandResult } from "./base.js" import { Command } from "./base.js" @@ -106,6 +106,8 @@ export class WorkflowCommand extends Command { const stepErrors: StepErrors = {} + let shouldFail = false + for (const [index, step] of steps.entries()) { if (shouldBeDropped(index, steps, stepErrors)) { continue @@ -186,7 +188,10 @@ export class WorkflowCommand extends Command { stepErrors[index] = [err] printStepDuration({ ...stepParams, success: false }) logErrors(stepBodyLog, [err], index, steps.length, step.description) - // There may be succeeding steps with `when: onError` or `when: always`, so we continue. + if (!step.allowFailure) { + shouldFail = true + } + // There may be succeeding steps with `when: onError`, `when: onFailure`, or `when: always`, so we continue. continue } @@ -204,7 +209,10 @@ export class WorkflowCommand extends Command { garden.events.emit("workflowStepError", getStepEndEvent(index, stepStartedAt)) logErrors(outerLog, stepResult.errors, index, steps.length, step.description) stepErrors[index] = stepResult.errors - // There may be succeeding steps with `when: onError` or `when: always`, so we continue. + if (!step.allowFailure) { + shouldFail = true + } + // There may be succeeding steps with `when: onError`, `when: onFailure`, or `when: always`, so we continue. continue } @@ -212,7 +220,7 @@ export class WorkflowCommand extends Command { printStepDuration({ ...stepParams, success: true }) } - if (size(stepErrors) > 0) { + if (shouldFail) { printResult({ startedAt, log: outerLog, workflow, success: false }) garden.events.emit("workflowError", {}) // TODO: If any of the errors are not instanceof GardenError, we need to log the explanation (with bug report information, etc.) @@ -417,35 +425,42 @@ export function shouldBeDropped(stepIndex: number, steps: WorkflowStepSpec[], st if (step.when === "never") { return true } - const lastErrorIndex = last( + + let lastErrorIndex = last( steps.filter((s, index) => s.when !== "onError" && !!stepErrors[index]).map((_, index) => index) ) - if (step.when === "onError") { + if (step.when === "onFailure") { + lastErrorIndex = last( + steps + .filter((s, index) => !s.allowFailure && s.when !== "onFailure" && !!stepErrors[index]) + .map((_, index) => index) + ) + } + + if (step.when === "onError" || step.when === "onFailure") { if (lastErrorIndex === undefined) { // No error has been thrown yet, so there's no need to run this `onError` step. return true } - const previousOnErrorStepIndexes: number[] = [] + const previousStepIndexes: number[] = [] for (const [index, s] of steps.entries()) { - if (s.when === "onError" && lastErrorIndex < index && index < stepIndex) { - previousOnErrorStepIndexes.push(index) + if (s.when === step.when && lastErrorIndex < index && index < stepIndex) { + previousStepIndexes.push(index) } } /** - * If true, then there is one or more `onError` step between this step and the step that threw the error, and - * there's also a non-`onError`/`never` step in between. That means that it's not up to this sequence of `onError` + * If true, then there is one or more `onError` or `onFailure` steps between this step and the step that threw the error, and + * there's also a non-`onError`/`onFailure`/`never` step in between. That means that it's not up to this sequence of `onError` * steps to "handle" that error. * - * Example: Here, steps a, b and c don't have a `when` modifier, and e1, e2 and e3 have `when: onError`. + * Example: Here, steps a, b and c don't have a `when` modifier, and e1, e2 and e3 have a `when`. * [a, b, e1, e2, c, e3] * If a throws an error, we run e1 and e2, but drop c and e3. */ const errorBelongsToPreviousSequence = - previousOnErrorStepIndexes.find((prevOnErrorIdx) => { - return steps.find( - (s, idx) => !["never", "onError"].includes(s.when || "") && prevOnErrorIdx < idx && idx < stepIndex - ) + previousStepIndexes.find((prevIdx) => { + return steps.find((s, idx) => !["never", step.when].includes(s.when || "") && prevIdx < idx && idx < stepIndex) }) !== undefined return errorBelongsToPreviousSequence } diff --git a/core/src/config/workflow.ts b/core/src/config/workflow.ts index 25ed712d794..b5da4e19aa6 100644 --- a/core/src/config/workflow.ts +++ b/core/src/config/workflow.ts @@ -200,6 +200,7 @@ export interface WorkflowStepSpec { script?: string skip?: boolean when?: workflowStepModifier + allowFailure?: boolean } export const workflowStepSchema = createSchema({ @@ -255,9 +256,12 @@ export const workflowStepSchema = createSchema({ \`onSuccess\` (default): This step will be run if all preceding steps succeeded or were skipped. - \`onError\`: This step will be run if a preceding step failed, or if its preceding step has \`when: onError\`. + \`onError\`: This step will be run if a preceding step throws any error, or if its preceding step has \`when: onError\`. If the next step has \`when: onError\`, it will also be run. Otherwise, all subsequent steps are ignored. + \`onFailure\`: This step will be run if a preceding step throws an error with \`allowFailure: false\`, or if its preceding step has \`when: onFailure\`. + If the next step has \`when: onFailure\`, it will also be run. Otherwise, all subsequent steps are ignored. + \`always\`: This step will always be run, regardless of whether any preceding steps have failed. \`never\`: This step will always be ignored. @@ -265,11 +269,14 @@ export const workflowStepSchema = createSchema({ See the [workflows guide](${DOCS_BASE_URL}/using-garden/workflows#the-skip-and-when-options) for details and examples. `), + allowFailure: joi + .boolean() + .description(`Set to true to allow the step to fail without failing the entire workflow.`), }), xor: [["command", "script"]], }) -export type workflowStepModifier = "onSuccess" | "onError" | "always" | "never" +export type workflowStepModifier = "onSuccess" | "onError" | "onFailure" | "always" | "never" export const triggerEvents = [ "push", diff --git a/core/test/unit/src/commands/workflow.ts b/core/test/unit/src/commands/workflow.ts index fd571b56cf4..45137a46524 100644 --- a/core/test/unit/src/commands/workflow.ts +++ b/core/test/unit/src/commands/workflow.ts @@ -82,6 +82,35 @@ describe("RunWorkflowCommand", () => { expect(result.errors || []).to.eql([]) }) + it("should allow failure for error in step", async () => { + garden.setWorkflowConfigs([ + { + apiVersion: GardenApiVersion.v0, + name: "workflow-a", + kind: "Workflow", + internal: { + basePath: garden.projectRoot, + }, + files: [], + envVars: {}, + resources: defaultWorkflowResources, + steps: [ + { script: "echo error!; exit 1", allowFailure: true }, // <-- error thrown here + { command: ["echo", "success!"] }, + { command: ["echo", "onError!"], when: "onError" }, + ], + }, + ]) + + const { result, errors } = await cmd.action({ ...defaultParams, args: { workflow: "workflow-a" } }) + + expect(result).to.exist + expect(errors).to.not.exist + expect(result?.steps).to.not.have.property("step-1") + expect(result?.steps).to.not.have.property("step-2") + expect(result?.steps).to.have.property("step-3") + }) + it("should add workflowStep metadata to log entries provided to steps", async () => { const _garden = await makeTestGardenA(undefined) // Ensure log entries are empty @@ -765,6 +794,58 @@ describe("RunWorkflowCommand", () => { ] expect(shouldBeDropped(5, steps, { 0: [new Error()] })).to.be.true }) + + it("should be included if a step throws an error with allowFailure = true", () => { + const steps: WorkflowStepSpec[] = [ + { command: ["deploy"], allowFailure: true }, // <-- error thrown here + { command: ["test"] }, + { command: ["build"], when: "onError" }, // <-- checking this step + ] + expect(shouldBeDropped(2, steps, { 0: [new Error()] })).to.be.false + }) + }) + + context("step has when = onFailure", () => { + it("should be dropped if no previous steps have failed", () => { + const steps: WorkflowStepSpec[] = [ + { command: ["deploy"] }, + { command: ["test"] }, + { command: ["build"], when: "onFailure" }, + ] + expect(shouldBeDropped(2, steps, {})).to.be.true + }) + + it("should be included if a step in the current sequence failed", () => { + const steps: WorkflowStepSpec[] = [ + { command: ["deploy"] }, // <-- error thrown here + { command: ["test"] }, + { command: ["build"], when: "onFailure" }, // <-- checking this step + { command: ["test"], when: "onFailure" }, // <-- checking this step + ] + expect(shouldBeDropped(2, steps, { 0: [new Error()] })).to.be.false + expect(shouldBeDropped(3, steps, { 0: [new Error()] })).to.be.false + }) + + it("should be dropped if a step in a preceding sequence failed", () => { + const steps: WorkflowStepSpec[] = [ + { command: ["deploy"] }, // <-- error thrown here + { command: ["test"] }, + { command: ["build"], when: "onFailure" }, + { command: ["test"], when: "onFailure" }, + { command: ["test"] }, + { command: ["test"], when: "onFailure" }, // <-- checking this step + ] + expect(shouldBeDropped(5, steps, { 0: [new Error()] })).to.be.true + }) + + it("should be dropped if a step throws an error with allowFailure = true", () => { + const steps: WorkflowStepSpec[] = [ + { command: ["deploy"], allowFailure: true }, // <-- error thrown here + { command: ["test"] }, + { command: ["build"], when: "onFailure" }, // <-- checking this step + ] + expect(shouldBeDropped(2, steps, { 0: [new Error()] })).to.be.true + }) }) }) diff --git a/docs/reference/commands.md b/docs/reference/commands.md index 1e9d8602db2..c87fa974899 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -2782,9 +2782,14 @@ workflowConfigs: # # `onSuccess` (default): This step will be run if all preceding steps succeeded or were skipped. # - # `onError`: This step will be run if a preceding step failed, or if its preceding step has `when: onError`. + # `onError`: This step will be run if a preceding step throws any error, or if its preceding step has `when: + # onError`. # If the next step has `when: onError`, it will also be run. Otherwise, all subsequent steps are ignored. # + # `onFailure`: This step will be run if a preceding step throws an error with `allowFailure: false`, or if its + # preceding step has `when: onFailure`. + # If the next step has `when: onFailure`, it will also be run. Otherwise, all subsequent steps are ignored. + # # `always`: This step will always be run, regardless of whether any preceding steps have failed. # # `never`: This step will always be ignored. @@ -2794,6 +2799,9 @@ workflowConfigs: # and examples. when: + # Set to true to allow the step to fail without failing the entire workflow. + allowFailure: + # A list of triggers that determine when the workflow should be run, and which environment should be used (Garden # Cloud only). triggers: diff --git a/docs/reference/workflow-config.md b/docs/reference/workflow-config.md index b0537f1fdf6..ba375c69853 100644 --- a/docs/reference/workflow-config.md +++ b/docs/reference/workflow-config.md @@ -118,9 +118,14 @@ steps: # # `onSuccess` (default): This step will be run if all preceding steps succeeded or were skipped. # - # `onError`: This step will be run if a preceding step failed, or if its preceding step has `when: onError`. + # `onError`: This step will be run if a preceding step throws any error, or if its preceding step has `when: + # onError`. # If the next step has `when: onError`, it will also be run. Otherwise, all subsequent steps are ignored. # + # `onFailure`: This step will be run if a preceding step throws an error with `allowFailure: false`, or if its + # preceding step has `when: onFailure`. + # If the next step has `when: onFailure`, it will also be run. Otherwise, all subsequent steps are ignored. + # # `always`: This step will always be run, regardless of whether any preceding steps have failed. # # `never`: This step will always be ignored. @@ -129,6 +134,9 @@ steps: # and examples. when: onSuccess + # Set to true to allow the step to fail without failing the entire workflow. + allowFailure: + # A list of triggers that determine when the workflow should be run, and which environment should be used (Garden # Cloud only). triggers: @@ -470,9 +478,12 @@ If used, this step will be run under the following conditions (may use template `onSuccess` (default): This step will be run if all preceding steps succeeded or were skipped. -`onError`: This step will be run if a preceding step failed, or if its preceding step has `when: onError`. +`onError`: This step will be run if a preceding step throws any error, or if its preceding step has `when: onError`. If the next step has `when: onError`, it will also be run. Otherwise, all subsequent steps are ignored. +`onFailure`: This step will be run if a preceding step throws an error with `allowFailure: false`, or if its preceding step has `when: onFailure`. +If the next step has `when: onFailure`, it will also be run. Otherwise, all subsequent steps are ignored. + `always`: This step will always be run, regardless of whether any preceding steps have failed. `never`: This step will always be ignored. @@ -484,6 +495,16 @@ and examples. | -------- | ------------- | -------- | | `string` | `"onSuccess"` | No | +### `steps[].allowFailure` + +[steps](#steps) > allowFailure + +Set to true to allow the step to fail without failing the entire workflow. + +| Type | Required | +| --------- | -------- | +| `boolean` | No | + ### `triggers[]` A list of triggers that determine when the workflow should be run, and which environment should be used (Garden Cloud only).