-
Notifications
You must be signed in to change notification settings - Fork 1k
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
undiverged not working correctly as a plan requirement #4051
Comments
Did it work with 0.26.0? |
Tested this on 0.27.1 with both parallel plan and apply turned off -- it still goes ahead an happily plans the changes. |
Hi there 👋🏻 I am also interested in the Unfortunately, I was not able to validate its functionality. 😞
How does it work at all? Notes:
Many thanks for your help and explanations 🙏🏻 EDIT Ok I was able to validate it and indeed, it does not work with Atlantis < 0.27.0 |
I just tested on 0.27.1 and didn't work 😢 |
I'm starting to look into this, and one question I have is, should we even support EDIT: To be clear I do agree that there is an issue here, at the very possibly an inconsistency due to a race condition. I'm also happy to work on a fix if we decide that's the right way to go, I just want to understand the use case better. |
@lukemassa yah it's pretty important to get this working IMO. Here is a scenario
Obviously they should be looking at the plan and noticing problems there, but there are many cases where something could be missed. Tired engineer, incompetent engineer, plan that would already have many changes in it and other small changes it is undoing are missed, etc. Ideally the scenario above should have never let them run the plan since it was behind main, and ideally would not let them apply it either. |
My understanding of https://www.runatlantis.io/docs/checkout-strategy.html#merge, and I was able to confirm this at least in a toy example, is that when you go to run that plan, it checks out |
It does sound like that would solve it, but the setting defaults to branch...so anyone unaware of the setting could fall victim to problems with undiverged. I'm not sure in which scenario anyone would even want to use the |
True, however
From https://www.runatlantis.io/docs/command-requirements.html#undiverged I'm not sure we have the ability to detect divergence in a So maybe we remove support for Interested to hear from other commenters here @albertorm95 @xakraz @kpocius ? EDIT: Additionally we maybe should have startup fail if |
Thanks @lukemassa for giving a look at the issue 🙏 I like the startup fail idea but what about repos that use Time does things well :) By the way, I was wondering about the usefulness of the |
There is a problem with the up-to-date branch protection, we have a monorepo of projects so its very hard (basically a race) if we enable that there. |
@lukemassa thanks for all the details. I think the arguments against The last thing I have in favor of |
This feature is very useful for us, any way we can help on the investigation of how this got broken? Thanks! |
Yeah one thing I'm still bit confused on is, if you've chosen the "merge" checkout strategy, you are explicitly indicating to Atlantis that you want it to handle the divergence behind the scenes. If you don't want that then you shouldn't use the checkout strategy. And if instead you use the "branch" checkout strategy, "undiverged" has no effect as documented (and as I mention above, we should probably fail at startup if you try). @albertorm95 could you explain your use case? Are you using the merge or branch strategy? |
Sure, this is our scenario: We have a monorepo where multiple teams collab and manage their infrastructure
Undiverged will help us to trust the plans generated, since in theory it will detect if there are changes in cc: @lukemassa |
The scenario you describe should be consistent with having an apply requirement for Again, when using the I'm still leaning towards just an update to documentation/config validation to clarify this is an apply-only requirement. |
I agree to do a Documentation update to make this clear. |
Community Note
Overview of the Issue
When running
atlantis plan
on an out of date branch, it results in inconsistent behavior as seen below. I suspect the undiverged check is not running early enough. It should probably execute before any plans are run at all? My guess is that parallel planning is throwing it off.Reproduction Steps
Set up a repo that plans multiple paths in parallel.
Make an out of date PR
Run atlantis plan
Environment details
Atlantis server-side config file:
Repo
atlantis.yaml
file:The text was updated successfully, but these errors were encountered: