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

Long running apply for a single project can trigger automerge #2607

Open
Fauzyy opened this issue Oct 20, 2022 · 2 comments
Open

Long running apply for a single project can trigger automerge #2607

Fauzyy opened this issue Oct 20, 2022 · 2 comments
Labels
bug Something isn't working help wanted Good feature for contributors

Comments

@Fauzyy
Copy link
Contributor

Fauzyy commented Oct 20, 2022

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 noticed an unexpected automerge in one of our pull requests that involve an individual project apply that was long running. Atlantis automerged the PR even though there are still unapplied plans. I believe the root cause of this is how the pull status is being updated in BoltDB.

Reproduction Steps

This issue can be difficult to reproduce as it requires an apply to run long:

  1. Open a pull request with a diff that generates plans for more than one workspace.
  2. Apply any single workspace with: atlantis apply -p <project_name>
  3. Push a commit to trigger another autoplan
  4. Autoplan finishes with new plan results
  5. Apply finishes and the PR is automerged with 1/1 plans applied

Logs

When the apply finishes, it updates the commit status for the previous commit, example:
image
image

Environment details

  • Atlantis version: v0.19.7
  • If not running the latest Atlantis version have you tried to reproduce this issue on the latest version: No

Atlantis server-side config file:

repo-config: /home/atlantis/repos.yaml
data-dir: /home/atlantis/.atlantis
checkout-strategy: merge
parallel-pool-size: 15
disable-repo-locking: true
stats-namespace: atlantis_upstream
enable-regexp-cmd: true
enable-policy-checks: true
enable-diff-markdown-format: true
hide-prev-plan-comments: true

Repo atlantis.yaml file:

version: 2
automerge: true
parallel_plan: true
parallel_apply: true

Additional Context

I did some digging and this seems to stem from how Atlantis is updating the Pull Status when our pull is out of date:

if currStatus == nil || currStatus.Pull.HeadCommit != pull.HeadCommit {
var statuses []models.ProjectStatus
for _, r := range newResults {
statuses = append(statuses, b.projectResultToProject(r))
}
newStatus = models.PullStatus{
Pull: pull,
Projects: statuses,
}

If currStatus.Pull.HeadCommit != pull.HeadCommit we end up overwriting the Pull Status with the results from this apply. The problem here is that Atlantis is trying to overwrite the newer status even for applies that are targeted with -p. For individual project applies, we should still merge our project results with the existing ones even if the commit is out-of-date like we do later in the code:

} else {
// If there's an existing pull at the right commit then we have to
// merge our project results with the existing ones. We do a merge
// because it's possible a user is just applying a single project
// in this command and so we don't want to delete our data about
// other projects that aren't affected by this command.
newStatus = *currStatus
for _, res := range newResults {
// First, check if we should update any existing projects.
updatedExisting := false
for i := range newStatus.Projects {
// NOTE: We're using a reference here because we are
// in-place updating its Status field.
proj := &newStatus.Projects[i]
if res.Workspace == proj.Workspace &&
res.RepoRelDir == proj.RepoRelDir &&
res.ProjectName == proj.ProjectName {
proj.Status = res.PlanStatus()
updatedExisting = true
break
}
}
if !updatedExisting {
// If we didn't update an existing project, then we need to
// add this because it's a new one.
newStatus.Projects = append(newStatus.Projects, b.projectResultToProject(res))
}
}
}

@Fauzyy Fauzyy added the bug Something isn't working label Oct 20, 2022
@Fauzyy
Copy link
Contributor Author

Fauzyy commented Oct 20, 2022

I'd be happy to contribute a fix here

@jamengual
Copy link
Contributor

jamengual commented Oct 20, 2022 via email

@jamengual jamengual added the help wanted Good feature for contributors label Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Good feature for contributors
Projects
None yet
Development

No branches or pull requests

2 participants