Checkout Strategy Merge - Merging before a plan #979
Comments
|
Yes, the comment from atlantis did contain a waning about the branch diverging. That's awesome, I didn't know that warning existed. What are your thoughts on adding an option to always force clone on divergence? Maybe adding a new checkout strategy called |
|
I don't think that will work. There's some concerns I had in the original ticket. Basically we're not guaranteed to have the whole PR locked, maybe only some directories, so when it's unlocked some of the directories might be half way through a plan or apply cycle and we can't reclone for those projects. |
|
After looking at the code I see what you're talking about. I wonder if each project should be its own branch off of the PR branch. The branch doesn't really have to exist after the plan is generated 🤔 But that's a decent size change and another conversation. In this case, what are your thoughts on taking the divergence option a step further and having an option to block applies if divergence is detected? This would add an extra safety net preventing users from shooting themselves in the foot if they accidentally don't notice that they are removing objects because they glanced over the warning. |
|
Sure that could work. You could also set "require branches be up to date" in github (or your vcs) and then use the mergeable apply requirement. |
|
I ran into this situation as well. For now I'm simply wondering if any method exists such that a user could make Atlantis reclone when the branch diverges? Here are a couple of methods I suspect might cause a reclone:
If someone could confirm then it would be much appreciated 🙂 |
|
Closing the PR and opening a new one would definitely solve any divergence, especially if the new PR is based on master (but with merge strategy it should work even if the new PR has the same old branch point as the old PR). As far as I know, running |
|
This feels a bit unresolved to me. If atlantis has a lock on PR for a project, and there are other competing projects, shouldn't merging always be a valid strategy. I am struggling to think of a situation where always merging against master is an invalid approach. If PR1 has the lock, PR2 makes changes in the same project, atlantis IS guaranteeing that PR2s changes won't be merged and reverted by a PR1 |
|
I guess I just want a solution where I merge PR1 and PR2 does not try to revert the changes PR1 just landed. How do we configure atlantis to do the right thing and never revert changes that have landed and been applied in master? |
|
Actually, the more I think about it, PR2 cannot be planned at all until PR1 is merged, so the merging and the files changing are guaranteed and it seems like a bug to not rebase against the latest master when planning. |
The current "merge" checkout strategy is unsafe. It merges the PR and the base branch without holding the directory lock(s), so there is a potentially very long window where another PR can be applied and be unexpectedly reverted later. This happens occasionally if a PR causes plans in multiple directories, but is almost _guaranteed_ to happen if the initial plan has to wait until a lock is freed up and a manual "atlantis plan" command is given. Instead of printing a warning when this happens, we now merge again if necessary while holding the lock. Plans are then guaranteed to only be made when merged with the base branch for each directory being planned, and applying later should be safe even if the base branch sees further updates. This fixes/affects runatlantis#804, runatlantis#867, runatlantis#979
The current "merge" checkout strategy is unsafe. It merges the PR and the base branch without holding the directory lock(s), so there is a potentially very long window where another PR can be applied and be unexpectedly reverted later. This happens occasionally if a PR causes plans in multiple directories, but is almost _guaranteed_ to happen if the initial plan has to wait until a lock is freed up and a manual "atlantis plan" command is given. Instead of printing a warning when this happens, we now merge again if necessary while holding the lock. Plans are then guaranteed to only be made when merged with the base branch for each directory being planned, and applying later should be safe even if the base branch sees further updates. This fixes/affects runatlantis#804, runatlantis#867, runatlantis#979
…n base update (#3187) * merge again if base branch has been updated The current "merge" checkout strategy is unsafe. It merges the PR and the base branch without holding the directory lock(s), so there is a potentially very long window where another PR can be applied and be unexpectedly reverted later. This happens occasionally if a PR causes plans in multiple directories, but is almost _guaranteed_ to happen if the initial plan has to wait until a lock is freed up and a manual "atlantis plan" command is given. Instead of printing a warning when this happens, we now merge again if necessary while holding the lock. Plans are then guaranteed to only be made when merged with the base branch for each directory being planned, and applying later should be safe even if the base branch sees further updates. This fixes/affects #804, #867, #979 * Remove diverged test that no longer applies * Reinstate TestClone_MasterHasDiverged test and make it pass * Extend TestClone_MasterHasDiverged to test new merging functionality We now verify that the first Clone with CheckoutMerge=true with a diverged base branch atually clones and merges again. --------- Co-authored-by: PePe Amengual <[email protected]>
…n base update (#3187) * merge again if base branch has been updated The current "merge" checkout strategy is unsafe. It merges the PR and the base branch without holding the directory lock(s), so there is a potentially very long window where another PR can be applied and be unexpectedly reverted later. This happens occasionally if a PR causes plans in multiple directories, but is almost _guaranteed_ to happen if the initial plan has to wait until a lock is freed up and a manual "atlantis plan" command is given. Instead of printing a warning when this happens, we now merge again if necessary while holding the lock. Plans are then guaranteed to only be made when merged with the base branch for each directory being planned, and applying later should be safe even if the base branch sees further updates. This fixes/affects #804, #867, #979 * Remove diverged test that no longer applies * Reinstate TestClone_MasterHasDiverged test and make it pass * Extend TestClone_MasterHasDiverged to test new merging functionality We now verify that the first Clone with CheckoutMerge=true with a diverged base branch atually clones and merges again. --------- Co-authored-by: PePe Amengual <[email protected]>
…n base update (runatlantis#3187) * merge again if base branch has been updated The current "merge" checkout strategy is unsafe. It merges the PR and the base branch without holding the directory lock(s), so there is a potentially very long window where another PR can be applied and be unexpectedly reverted later. This happens occasionally if a PR causes plans in multiple directories, but is almost _guaranteed_ to happen if the initial plan has to wait until a lock is freed up and a manual "atlantis plan" command is given. Instead of printing a warning when this happens, we now merge again if necessary while holding the lock. Plans are then guaranteed to only be made when merged with the base branch for each directory being planned, and applying later should be safe even if the base branch sees further updates. This fixes/affects runatlantis#804, runatlantis#867, runatlantis#979 * Remove diverged test that no longer applies * Reinstate TestClone_MasterHasDiverged test and make it pass * Extend TestClone_MasterHasDiverged to test new merging functionality We now verify that the first Clone with CheckoutMerge=true with a diverged base branch atually clones and merges again. --------- Co-authored-by: PePe Amengual <[email protected]>
…n base update (runatlantis#3187) * merge again if base branch has been updated The current "merge" checkout strategy is unsafe. It merges the PR and the base branch without holding the directory lock(s), so there is a potentially very long window where another PR can be applied and be unexpectedly reverted later. This happens occasionally if a PR causes plans in multiple directories, but is almost _guaranteed_ to happen if the initial plan has to wait until a lock is freed up and a manual "atlantis plan" command is given. Instead of printing a warning when this happens, we now merge again if necessary while holding the lock. Plans are then guaranteed to only be made when merged with the base branch for each directory being planned, and applying later should be safe even if the base branch sees further updates. This fixes/affects runatlantis#804, runatlantis#867, runatlantis#979 * Remove diverged test that no longer applies * Reinstate TestClone_MasterHasDiverged test and make it pass * Extend TestClone_MasterHasDiverged to test new merging functionality We now verify that the first Clone with CheckoutMerge=true with a diverged base branch atually clones and merges again. --------- Co-authored-by: PePe Amengual <[email protected]>
First, huge thanks to the creators and maintainers of this project. Atlantis is helping us create a more autonomous engineering org.
Our Configuration
We have
checkoutStrategy: mergeset in our helm definitionThe Question
In production we had two PRs (same repo) open against the same project/workspace/directory. The locking worked as expected, and locked the first PR (PR1) project. The second PR (PR2) is created and gets a message notifying it that the project is locked.
Work continued on PR1 and eventually we ran
atlantis applyand merged to master. This lifted the lock from PR1.Since the lock was lifted we were now able to run
atlantis planon PR2. The output from the plan showed deletions from the work that was just merged in PR1.Is this the expected behavior, even with the checkoutStrategy set to merge?
The text was updated successfully, but these errors were encountered: