feat: when using order group, abort plan/apply if any fail #3323

oysteingraendsen · 2023-04-12T12:20:51Z

what

Break out of order group loop if any command fails. This applies only when using parallel apply/plan and execution order group.

why

Atlantis supports execution groups when using parallel apply/plan, but does not stop if there are any errors
in a prior execution group. This should happen in my opinion.

tests

I have added tests for execution order groups
I have added tests that ensures that execution stops on failed apply for the next execution group

references

Builds on PR feat: add execution order group #2178
closes abort atlantis workflow when earlier execution_order_group fails #3147

jukie · 2023-04-12T16:45:44Z

server/events/project_command_pool_executor.go

@@ -81,6 +81,9 @@ func runProjectCmdsParallelGroups(
 	for _, group := range groups {
 		res := runProjectCmdsParallel(group, runnerFunc, poolSize)
 		results = append(results, res.ProjectResults...)
+		if res.HasErrors() {


Could a check be ran here against an opt-out flag?

I can add a flag if you want that. Should that be added on a repo level? In the issue that I referred to, the suggestion was setting this as the expected behaviour.

I'm not sure I see when you would want to opt out of this as you want it to run in a given order, but I'm all ears for what you guys want 👍

I'm not an official maintainer so my opinion is lower than what @krrrr38 mentioned in the issue 😄
It may not be worth it but my thought process was that it changes existing behavior so if any users don't want this it would break their workflow.
And a repo level flag would probably be the most flexible

I agree with @jukie , repo level will be nicer and allows the user to change the behaviour by a flag

Why would anyone want their execution group to continue if it has errors in between?

runatlantis.io/docs/repo-level-atlantis-yaml.md

GenPage · 2023-04-16T20:24:58Z

There's another PR to address this that I think is more involved.

#3292 solves essentially what you are proposing, I think it might be more prudent to utilize that feature and deprecate execution_order_group

nitrocode · 2023-04-18T13:57:03Z

The features may be able to work together as this comment points out

#3292 (comment)

oysteingraendsen · 2023-04-19T06:34:49Z

The features may be able to work together as this comment points out

#3292 (comment)

I'll just stay put until you figure out if this is going to be useful or not ;)

nitrocode · 2023-04-19T14:12:45Z

Nevermind. I was corrected in the other PR. It looks like the PR author and GenPage plan to deprecate the execution_order_group with the new depends_on proposal.

nitrocode · 2023-04-22T22:35:01Z

On the other hand, the terragrunt-atlantis-config repo is currently setup to use the execution order and people who currently use this also complain that a single error will not stop the pipeline. Even if execution-order will be deprecated, i think we should still fix this bug for current users. Then we can have the depends-on implemented and slowly deprecate execution-order thus giving time for people to migrate.

nitrocode · 2023-04-22T22:35:45Z

cc @jamengual @GenPage thoughts?

We did the same for the whitelist to allowlist deprecations as well. Slow deprecation to allow people to migrate.

jamengual · 2023-04-22T22:37:26Z

yes, we can do the same

GenPage · 2023-04-23T17:36:25Z

Sgtm

nitrocode · 2023-04-27T17:26:10Z

@oysteingraendsen can you confirm that you have tested this, not only in unit tests, but end to end in your own setup? If you could test

with the flag
- with all successful jobs that does not early exist
- with a broken intermediary job that early exits
without the flag
- with all successful jobs
- with a broken intermediary job that does NOT early exit (status quo)

oysteingraendsen · 2023-05-05T09:50:47Z

@oysteingraendsen can you confirm that you have tested this, not only in unit tests, but end to end in your own setup? If you could test
* with the flag
  
  * with all successful jobs that does not early exist
  * with a broken intermediary job that early exits

* without the flag
  
  * with all successful jobs
  * with a broken intermediary job that does NOT early exit (status quo)

Sorry about the delay. I have been a bit caught up lately.

I have tested the points you mentioned and it works for apply, but not for plan. I'll write some tests for plan as well to figure out why it does not work there too.

GenPage · 2023-05-06T04:15:19Z

@oysteingraendsen If you need help troubleshooting the tests, feel free to ping me here or in Slack.

server/events/apply_command_runner_test.go

GenPage

LGTM, other than the nitpick comment

server/events/project_command_pool_executor.go

oysteingraendsen · 2023-05-11T19:26:14Z

I just verified that it now works. It's ready to be merged from my end now, @GenPage

…is#3323) * feat: when using order group, abort plan/apply if any fail * feat: add 'abort_on_execution_order_fail' flag on repo level * feat: use runProjectCmdsParallelGroups in version_command_runner * chore: add plan tests --------- Co-authored-by: nitrocode <7775707+nitrocode@users.noreply.github.com>

oysteingraendsen changed the title ~~feat: when using ordergroup, abort plan/apply if any fail~~ feat: when using order group, abort plan/apply if any fail Apr 12, 2023

oysteingraendsen force-pushed the order-group-abort-on-fail branch from bc1bf17 to 51f7b76 Compare April 12, 2023 12:50

oysteingraendsen marked this pull request as ready for review April 12, 2023 12:51

oysteingraendsen requested a review from a team as a code owner April 12, 2023 12:51

github-actions bot added the go Pull requests that update Go code label Apr 12, 2023

jukie reviewed Apr 12, 2023

View reviewed changes

github-actions bot added the docs Documentation label Apr 14, 2023

oysteingraendsen requested review from jamengual and jukie April 14, 2023 07:51

hedlund reviewed Apr 14, 2023

View reviewed changes

runatlantis.io/docs/repo-level-atlantis-yaml.md Outdated Show resolved Hide resolved

jamengual mentioned this pull request Apr 20, 2023

feat: Add project depends on functionality #3292

Merged

1 task

oysteingraendsen force-pushed the order-group-abort-on-fail branch from 05b5910 to 5de44bf Compare April 27, 2023 08:10

GenPage added this to the v0.24.0 milestone May 6, 2023

GenPage added the needs tests Change requires tests label May 6, 2023

oysteingraendsen force-pushed the order-group-abort-on-fail branch from c89e05f to ec0644b Compare May 9, 2023 11:01

GenPage reviewed May 10, 2023

View reviewed changes

server/events/apply_command_runner_test.go Outdated Show resolved Hide resolved

GenPage previously approved these changes May 10, 2023

View reviewed changes

nitrocode reviewed May 10, 2023

View reviewed changes

server/events/project_command_pool_executor.go Outdated Show resolved Hide resolved

nitrocode reviewed May 10, 2023

View reviewed changes

server/events/project_command_pool_executor.go Show resolved Hide resolved

nitrocode dismissed GenPage’s stale review via 36adc3b May 10, 2023 03:25

nitrocode previously approved these changes May 10, 2023

View reviewed changes

oysteingraendsen dismissed nitrocode’s stale review via 89d5731 May 10, 2023 08:10

oysteingraendsen force-pushed the order-group-abort-on-fail branch from 36adc3b to 89d5731 Compare May 10, 2023 08:10

oysteingraendsen added 4 commits May 11, 2023 20:57

feat: when using order group, abort plan/apply if any fail

921a0dd

feat: add 'abort_on_execution_order_fail' flag on repo level

d4f7609

feat: use runProjectCmdsParallelGroups in version_command_runner

6afcf78

chore: add plan tests

8e4026d

oysteingraendsen force-pushed the order-group-abort-on-fail branch from 89d5731 to 8e4026d Compare May 11, 2023 18:58

Merge branch 'main' into order-group-abort-on-fail

96b01cf

nitrocode approved these changes May 11, 2023

View reviewed changes

GenPage merged commit 4e867fc into runatlantis:main May 12, 2023

oysteingraendsen deleted the order-group-abort-on-fail branch May 12, 2023 17:25

albertorm95 mentioned this pull request May 19, 2023

fix(metrics): add missing stat metric scopes #3416

Merged

grjones mentioned this pull request Jun 16, 2023

Discarding a Plan Causes the Whole Working Directory to be Deleted #3531

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: when using order group, abort plan/apply if any fail #3323

feat: when using order group, abort plan/apply if any fail #3323

oysteingraendsen commented Apr 12, 2023 •

edited

Loading

jukie Apr 12, 2023

oysteingraendsen Apr 12, 2023

jukie Apr 12, 2023 •

edited

Loading

jamengual Apr 12, 2023

nitrocode Apr 18, 2023

GenPage commented Apr 16, 2023

nitrocode commented Apr 18, 2023

oysteingraendsen commented Apr 19, 2023

nitrocode commented Apr 19, 2023

nitrocode commented Apr 22, 2023

nitrocode commented Apr 22, 2023

jamengual commented Apr 22, 2023

GenPage commented Apr 23, 2023

nitrocode commented Apr 27, 2023

oysteingraendsen commented May 5, 2023

GenPage commented May 6, 2023

GenPage left a comment

oysteingraendsen commented May 11, 2023

feat: when using order group, abort plan/apply if any fail #3323

feat: when using order group, abort plan/apply if any fail #3323

Conversation

oysteingraendsen commented Apr 12, 2023 • edited Loading

what

why

tests

references

jukie Apr 12, 2023

Choose a reason for hiding this comment

oysteingraendsen Apr 12, 2023

Choose a reason for hiding this comment

jukie Apr 12, 2023 • edited Loading

Choose a reason for hiding this comment

jamengual Apr 12, 2023

Choose a reason for hiding this comment

nitrocode Apr 18, 2023

Choose a reason for hiding this comment

GenPage commented Apr 16, 2023

nitrocode commented Apr 18, 2023

oysteingraendsen commented Apr 19, 2023

nitrocode commented Apr 19, 2023

nitrocode commented Apr 22, 2023

nitrocode commented Apr 22, 2023

jamengual commented Apr 22, 2023

GenPage commented Apr 23, 2023

nitrocode commented Apr 27, 2023

oysteingraendsen commented May 5, 2023

GenPage commented May 6, 2023

GenPage left a comment

Choose a reason for hiding this comment

oysteingraendsen commented May 11, 2023

oysteingraendsen commented Apr 12, 2023 •

edited

Loading

jukie Apr 12, 2023 •

edited

Loading