The "merge" strategy is broken, wants to revert changes with "The branch we're merging into is ahead, it is recommended to pull new commits first."

[bschaeffer]

Community Note

• Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request. Searching for pre-existing feature requests helps us consolidate datapoints for identical requirements into a single place, thank you!
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for issue followers and do not help prioritize the request.
• If you are interested in working on this issue or have submitted a pull request, please leave a comment.

---

Overview of the Issue

We set ATLANTIS_CHECKOUT_STRATEGY=merge in our server. I opened a PR and saw that the lock for the project was held by another PR. No big deal. Once that PR was applied and merged I re-planned my PR only to notice that it wanted to revert the changes just merged to master by the other PR. I see the revert in the plan output and also this comment from atlantis:

⚠️ The branch we're merging into is ahead, it is recommended to pull new commits first.

Reproduction Steps

Open two PRs from the same master commit. PR1 gets the lock, makes a change, merges to master. PR2 now is able to get the lock, but wants to revert the change from PR1. Get this comment:

⚠️ The branch we're merging into is ahead, it is recommended to pull new commits first.

Expected Results

I expect the merge strategy to use a plan based off master AND the changes in my branch. Otherwise what is the point of the locks?

Also, it is recommended is too soft a comment for this issue. More like you will revert the work in master if you do not rebase or merge master into your branch.

Environment details

• Version: 0.18.2
• ATLANTIS_CHECKOUT_STRATEGY=merge

Logs

creating dir "/atlantis-data/repos/<redacted>/default"
ran: git remote add head https://<redacted>@github.com/<redacted>.git. Output: 
ran: git fetch head +refs/heads/braden/prevent-gke-cluster-destory:. Output: From https://github.com/<redacted>
 * branch                  braden/prevent-gke-cluster-destory -> FETCH_HEAD
 * [new branch]            braden/prevent-gke-cluster-destory -> head/braden/prevent-gke-cluster-destory
ran: git merge -q --no-ff -m atlantis-merge FETCH_HEAD. Output: 
clone directory "/atlantis-data/repos/<redacted>/default" already exists, checking if it's at the right commit
repo is at correct commit "127f56f07d6aa152a8488146322779ae922ac559" so will not re-clone

Additional Context

• https://www.runatlantis.io/docs/checkout-strategy.html#branch

[danielgblanco]

Also running 0.18.2 and running into the same issue. After reading the docs, I would expect Atlantis to run the plan with the changes on the branch merged on top of master. The idea being that in a repo with multiple independent teams working on their individual project we could drop the Require branches to be up to date before merging and rely on project locking to guarantee consistency. This would save teams the need to "Update branch" every time another team commits something in an unrelated project.

[jamengual]

is this still happening with v0.19.8?

[modesvops]

We still have this issue on v0.19.8

[bschaeffer]

Hrm... still seems like an issue

[nitrocode]

Thanks @bschaeffer for confirming this is still an issue. Let's reopen it.

[greghensley]

I'm still seeing this behavior in 0.21.0.

Looking at server/events/working_dir.go#Clone(), it appears that Atlantis only clones the repo once each time the PR's HEAD is updated. If a new plan is run, Atlantis will re-use the clone so long as the remote PR branch HEAD matches the commit that was previously fetched. That would be fine with the basic branch strategy, but when the merge strategy is in use Atlantis should be resetting the base branch to the new upstream base and re-merging the PR branch into the new base branch. Instead, Atlantis just observes that the upstream base and local base are no longer the same and prints an error.

Is there any reason why Atlantis cannot just fall back to re-cloning the repo if the upstream base branch has diverged?

[jamengual]

@Fabianoshz something to keep in mind to test in your PR.

no reason @greghensley this could very well be a bug, if you think that it is and you have tested with different settings and repos, please feel free to submit a PR.

[Fabianoshz]

Hey guys, sorry for the late reply, I'm almost finishing my work on #2921 I can certainly look at this, I didn't changed Clone() aside from reusing the code from #2180.

@greghensley could you pinpoint exactly what code are you referring to?

[greghensley]

@Fabianoshz I'm referring to

atlantis/server/events/working_dir.go

Lines 115 to 118 in 677fa8f

	if strings.HasPrefix(currCommit, p.HeadCommit) {
	log.Debug("repo is at correct commit %q so will not re-clone", p.HeadCommit)
	return cloneDir, w.warnDiverged(log, p, headRepo, cloneDir), nil
	}

and suggesting it could be changed to

		if strings.HasPrefix(currCommit, p.HeadCommit) {
			if !w.warnDiverged(log, p, headRepo, cloneDir) {
				log.Debug("repo is at correct commit %q so will not re-clone", p.HeadCommit)
				return cloneDir, false, nil
			}
		}

Such that, when the local base branch has diverged from the upstream base (and we're using the merge strategy), we fall through to

atlantis/server/events/working_dir.go

Line 125 in 677fa8f

return cloneDir, false, w.forceClone(log, cloneDir, headRepo, p)

That results in the same code-path when either the PR branch is out-of-date or the base branch is out-of-date (and the merge strategy is used), ensuring that we always plan against the latest upstream base (+ PR branch) rather than the current behavior of printing a warning after generating a useless (and possibly harmful) plan.

[Fabianoshz]

Ok, that makes sense to me, I can try to implement and test this but I might take some time because I still need to finish some work on the single clone PR, if you wish you can open a PR and I can help reviewing and testing or you can wait until I can fix this.