Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better flow control: Ability to use Waves, set if on individual stages/steps #376

Open
giseburt opened this issue Oct 15, 2022 · 0 comments

Comments

@giseburt
Copy link
Contributor

This continues from the discussion in #365 with the if control.

Looking at the docs for jobs.<job_id>.if, it appears that:

  1. The if statement, if present, can allow a step to build if dependent steps (in jobs.<job_id>.needs) fail, skip, or are cancelled.
  2. The if statement, if omitted, is the same as if: success() 1
  3. The if expression options that allow bypassing failure are: (Same reference.)
    • always() - ignores failure AND cancellation
    • failure() - ONLY on failure
    • cancelled() - ONLY if the workflow was cancelled
    • success() || failure() - unlike always() this allows cancellation of a step
  4. If a previous step is skipped then the if skips dependent steps unless one of the above expressions is added, known issue: Job-level "if" condition not evaluated correctly if job in "needs" property is skipped actions/runner#491
    • The best proposed solution for NOT skipping dependent steps, but also not running the if there's been a failure:
        if: |
          always() &&
          !contains(needs.*.result, 'failure') &&
          !contains(needs.*.result, 'cancelled') &&
          <ACTUAL TEST INTENDED HERE>
  5. Closely related is jobs.<job_id>.continue-on-error - which allows a job to act as if it succeeds even when it fails, making a job that cannot fail

1, 2, and 4 means that we only need the if statement on the build phase, and can skip if statements on publish steps. Depending on how 4 gets "fixed" either an if will need to be added with !(failure() || cancelled()) (As this workflow run demonstrates).

I see no value on adding (customized) if statements on publish jobs in the absence of a way to correlate the asset being deployed with some tangible asset name. Even then the value is dubious, since it'll likely invalidate the deployment since asset publish steps are shared by deployment steps that use those assets.

In a closely related issue, needs is used to handle dependencies, which the only way to make a stage NOT depend on another stage is with pipeline.addWave(), but that doesn't support GitHub options such as this if in the current code.

In the PR i have ready, this code works as expected (explained more below):

    const pipeline = new GitHubWorkflow(app, 'Pipeline', {
      workflowPath: `${dir}/.github/workflows/deploy.yml`,
      synth: new ShellStep('Build', {
        installCommands: ['yarn'],
        commands: ['yarn build'],
      }),
    });

    const stageA = new GitHubStage(app, 'MyStageA', {
      env: { account: '111111111111', region: 'us-east-1' },
      jobSettings: {
        if: "success() && contains(github.event.issue.labels.*.name, 'deployToA')",
      },
    });

    new Stack(stageA, 'MyStackA');

    const stageB = new GitHubStage(app, 'MyStageB', {
      env: { account: '12345678901', region: 'us-east-1' },
      jobSettings: {
        if: "success() && contains(github.event.issue.labels.*.name, 'deployToB')",
      },
    });

    new Stack(stageB, 'MyStackB');

    // Make a wave to have the stages be parallel (not depend on each other)
    const wave = pipeline.addWave('MyWave');
    wave.addStage(stageA);
    wave.addStage(stageB);

This makes four jobs:

  • Build-Build - with if set
  • Assets-FileAsset1 - needs Build-Build but doesn't have if set otherwise
  • MyWave-MyStageA-MyStackA-Deploy - needs Build-Build and Assets-FileAsset1 and has the correct if set
  • MyWave-MyStageB-MyStackB-Deploy - also needs Build-Build and Assets-FileAsset1 and has the correct if set
    • If it were pipeline.addStage(stageA); and pipeline.addStage(stageB); then there would be an additional needs value of MyWave-MyStageA-MyStackA-Deploy

Obviously there's a new construct GitHubStage, and a new GitHubWorkflow.addWave(). We can discuss technical details in the PR about how these work and if we want to maintain those names, etc.

Footnotes

  1. According to the docs here:

    You can use the following status check functions as expressions in if conditionals. A default status check of success() is applied unless you include one of these functions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant