Skip to content

Commit

Permalink
feat: add an allowFailure field to WorkflowStepSpec
Browse files Browse the repository at this point in the history 
* do not fail the entire workflow if a step labeled with
  allowFailure throws an error

Signed-off-by: Alex Johnson <[email protected]>
  • Loading branch information
Alex Johnson committed Jun 12, 2024
1 parent 57aa5a8 commit f9c9e69
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 21 deletions.
47 changes: 31 additions & 16 deletions core/src/commands/workflow.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -106,6 +106,8 @@ export class WorkflowCommand extends Command<Args, {}> {

const stepErrors: StepErrors = {}

let shouldFail = false

for (const [index, step] of steps.entries()) {
if (shouldBeDropped(index, steps, stepErrors)) {
continue
Expand Down Expand Up @@ -186,7 +188,10 @@ export class WorkflowCommand extends Command<Args, {}> {
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
}

Expand All @@ -204,15 +209,18 @@ export class WorkflowCommand extends Command<Args, {}> {
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
}

garden.events.emit("workflowStepComplete", getStepEndEvent(index, stepStartedAt))
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.)
Expand Down Expand Up @@ -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
}
Expand Down
11 changes: 9 additions & 2 deletions core/src/config/workflow.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ export interface WorkflowStepSpec {
script?: string
skip?: boolean
when?: workflowStepModifier
allowFailure?: boolean
}

export const workflowStepSchema = createSchema({
Expand Down Expand Up @@ -255,21 +256,27 @@ 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.
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",
Expand Down
81 changes: 81 additions & 0 deletions core/test/unit/src/commands/workflow.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
})
})
})

Expand Down
10 changes: 9 additions & 1 deletion docs/reference/commands.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand Down
25 changes: 23 additions & 2 deletions docs/reference/workflow-config.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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:
Expand Down Expand Up @@ -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.
Expand All @@ -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).
Expand Down

0 comments on commit f9c9e69

Please sign in to comment.