fix: preserve Atlantis apply lock after plan by yasinlachiny · Pull Request #5570 · runatlantis/atlantis

yasinlachiny · 2025-05-07T21:43:20Z

what

Updated Atlantis behavior to retain the apply lock when re-running plan if there are no changes.

Ensured the lock is only removed when actual changes are detected between re-runs

why

Addresses community-reported issue #5568 where users observed unintended lock removals during no-op plans.

Fixes an issue where Atlantis incorrectly removes the apply lock when a plan is re-run, even if no changes are detected.

Preserves expected workflow and safety in CI/CD pipelines by preventing premature unlocks

tests

✅ Manually tested locally:

Created an apply lock by running plan and apply.

Re-ran plan without making any changes — lock was preserved as expected.

Introduced a change and re-ran plan — lock was correctly removed.

Verified this behavior in a multi-project repository setup.

references

Fixes: #5568

Signed-off-by: yasinlachiny <your-email@example.com>

…kError_False Signed-off-by: yasinlachiny <yasin.lachiny@gmail.com>

yasinlachiny · 2025-06-06T21:45:03Z

Hi @jamengual,

Could you please re-run the test?

go test ./server/events
ok github.com/runatlantis/atlantis/server/events 9.504s

It passes on my local machine:

Thanks in advance!

jamengual

The change looks good to me @yasinlachiny thanks for the contribution

Signed-off-by: yasinlachiny <your-email@example.com> Signed-off-by: yasinlachiny <yasin.lachiny@gmail.com> Co-authored-by: yasinlachiny <your_email@example.com> Co-authored-by: PePe Amengual <2208324+jamengual@users.noreply.github.com> Signed-off-by: PePe Amengual <2208324+jamengual@users.noreply.github.com>

Signed-off-by: yasinlachiny <your-email@example.com> Signed-off-by: yasinlachiny <yasin.lachiny@gmail.com> Co-authored-by: yasinlachiny <your_email@example.com> Co-authored-by: PePe Amengual <2208324+jamengual@users.noreply.github.com> Signed-off-by: Joseph McDonald <jojosr2000@gmail.com>

bobziuchkovski · 2025-06-26T19:18:19Z

We've observed similar behavior where the lock was being removed on re-plan. This is definitely an improvement over what we've currently observed.

However, is "remove the lock if a subsequent plan has changes" really the expected correct behavior? I would expect the lock to be held from the time an apply is made until the PR is merged unless explicitly unlocked.

yasinlachiny · 2025-06-26T19:44:00Z

Holding the lock even after changes are made can lead to a major issue: if we update the PR, the same PR still holds the lock, preventing us from applying again. So, in cases where there are changes, it's necessary to release the lock to allow a new apply to proceed.

bobziuchkovski · 2025-06-26T19:54:08Z

I'm not familiar with the internals here, but I would expect the lock to be associated with the PR and subsequent applies to be allowed for the same PR holding the lock.

yasinlachiny · 2025-06-26T20:23:10Z

atlantis apply doesn't allow this, even within the same PR, because it would enable multiple apply operations to run at the same time.

bobziuchkovski · 2025-06-26T20:32:32Z

Is that true? Doesn't that mean multiple apply operations could run at the same time as-is after removing the lock when new changes are detected since no lock exists?

yasinlachiny · 2025-06-26T20:49:01Z

It removes lock on plan not apply

Signed-off-by: yasinlachiny <your-email@example.com> Signed-off-by: yasinlachiny <yasin.lachiny@gmail.com> Co-authored-by: yasinlachiny <your_email@example.com> Co-authored-by: PePe Amengual <2208324+jamengual@users.noreply.github.com> Signed-off-by: Moritz Wirth <mw@flanga.io>

This reverts commit 59f16ef.

This fixes a critical bug introduced in commit 59f16ef (PR #5570) where the unlock key format didn't match the lock key format, causing locks to never be released properly. The issue: - Locks are created with key format: repo/path/workspace - Unlocks were attempted with format: repo/pullnum/projectname/workspace - This mismatch meant locks were never actually released This particularly affected users with multiple projects in the same directory using different .tfvars files, as they would encounter "workspace is currently locked" errors when trying to apply multiple projects. The fix: - Changed unlock key to match the lock key format: repo/path/workspace - Updated corresponding test to expect the correct format Fixes #5722 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

When PR #5570 added logic to preserve apply locks after plan when no changes are detected, it introduced a bug in the unlock key format. The lock key uses: repo/path/workspace But the unlock key was using: repo/pullnum/projectname/workspace This mismatch caused locks to never be released, blocking subsequent operations on projects in the same directory. This commit fixes the unlock key to match the lock key format. Fixes #5722 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…deletion Remove the premature lock deletion logic introduced in PR #5570 that was causing e2e test failures. This logic incorrectly unlocked projects immediately after plan operations when changes were detected, which broke the intended workflow where locks should remain until PR close/merge. The issue was causing "missing expected replies" errors in e2e tests because: 1. Plan detects changes → locks get deleted immediately after plan 2. PR gets merged → UnlockByPull finds no locks to unlock 3. No unlock comment is generated → tests expect 4 replies but get 3 This fix restores the original behavior where locks persist through the entire PR lifecycle and are only cleaned up when the PR is closed/merged, which generates the expected unlock comment. Fixes the e2e test failures reported in the CI where tests were expecting unlock comments but not receiving them. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>

…behavior Restore the UnlockByPull calls that were removed when PR #5570 introduced individual project unlock logic. These calls are needed in: 1. Generic plan command when deleting previous plans 2. Automerge error handling in both autoplan and plan commands 3. Test expectations reverted to original behavior This ensures locks are properly released when plans are deleted, which the original tests were verifying before PR #5570 changed the behavior. The fix maintains the original behavior where UnlockByPull is called whenever plans are deleted, ensuring proper cleanup of all locks associated with the pull request.

…deletion Remove the premature lock deletion logic introduced in PR #5570 that was causing e2e test failures. This logic incorrectly unlocked projects immediately after plan operations when changes were detected, which broke the intended workflow where locks should remain until PR close/merge. The issue was causing "missing expected replies" errors in e2e tests because: 1. Plan detects changes → locks get deleted immediately after plan 2. PR gets merged → UnlockByPull finds no locks to unlock 3. No unlock comment is generated → tests expect 4 replies but get 3 Changes made: - Remove individual project unlock loop from plan command runner - Restore UnlockByPull calls in generic plan and automerge error handling - Revert test expectations to original pre-PR#5570 behavior This fix restores the original behavior where locks persist through the entire PR lifecycle and are only cleaned up when the PR is closed/merged, which generates the expected unlock comment. Fixes the e2e test failures where tests were expecting unlock comments but not receiving them.

Signed-off-by: yasinlachiny <your-email@example.com> Signed-off-by: yasinlachiny <yasin.lachiny@gmail.com> Co-authored-by: yasinlachiny <your_email@example.com> Co-authored-by: PePe Amengual <2208324+jamengual@users.noreply.github.com> Signed-off-by: dimisjim <dimitris.moraitidis@gmail.com>

github-actions bot added the go Pull requests that update Go code label May 7, 2025

yasinlachiny force-pushed the locking branch from 940d765 to e0e7c31 Compare May 7, 2025 21:45

dosubot bot added the bug Something isn't working label May 7, 2025

yasinlachiny changed the title ~~Locking~~ fix: preserve Atlantis apply lock after plan May 7, 2025

fix: do not unlock after replanning

cb5c683

Signed-off-by: yasinlachiny <your-email@example.com>

yasinlachiny force-pushed the locking branch from 13ce465 to cb5c683 Compare May 22, 2025 18:05

Merge branch 'main' into locking

1ed5308

yasinlachiny force-pushed the locking branch from ab7f4e9 to 3e509de Compare June 6, 2025 19:09

yasinlachiny marked this pull request as draft June 6, 2025 19:11

yasinlachiny force-pushed the locking branch 2 times, most recently from 7711a4f to 559c7a5 Compare June 6, 2025 20:56

fix: TestRunCommentCommand_FailedPreWorkflowHook_FailOnPreWorkflowHoo…

c2047a9

…kError_False Signed-off-by: yasinlachiny <yasin.lachiny@gmail.com>

yasinlachiny force-pushed the locking branch from 559c7a5 to c2047a9 Compare June 6, 2025 21:41

yasinlachiny marked this pull request as ready for review June 6, 2025 21:41

jamengual approved these changes Jun 18, 2025

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 18, 2025

jamengual merged commit 59f16ef into runatlantis:main Jun 18, 2025
60 checks passed

BrewTestBot mentioned this pull request Jun 27, 2025

atlantis 0.35.0 Homebrew/homebrew-core#228358

Merged

tufitko added a commit to tufitko/atlantis that referenced this pull request Aug 19, 2025

Revert "fix: preserve Atlantis apply lock after plan (runatlantis#5570)"

391f292

This reverts commit 59f16ef.

jamengual mentioned this pull request Sep 10, 2025

fix: correct unlock key format in plan command runner #5780

Closed

jamengual mentioned this pull request Sep 10, 2025

fix: correct unlock key format to match lock key format #5781

Merged

4 tasks

jamengual mentioned this pull request Sep 12, 2025

fix: restore unlock comment functionality by removing premature lock deletion #5789

Closed

4 tasks

jamengual mentioned this pull request Sep 12, 2025

fix: restore unlock comment functionality by removing premature lock deletion #5790

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: preserve Atlantis apply lock after plan #5570

fix: preserve Atlantis apply lock after plan #5570
jamengual merged 3 commits intorunatlantis:mainfrom
yasinlachiny:locking

yasinlachiny commented May 7, 2025

Uh oh!

yasinlachiny commented Jun 6, 2025

Uh oh!

jamengual left a comment

Uh oh!

Uh oh!

bobziuchkovski commented Jun 26, 2025

Uh oh!

yasinlachiny commented Jun 26, 2025

Uh oh!

bobziuchkovski commented Jun 26, 2025

Uh oh!

yasinlachiny commented Jun 26, 2025

Uh oh!

bobziuchkovski commented Jun 26, 2025

Uh oh!

yasinlachiny commented Jun 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

yasinlachiny commented May 7, 2025

what

why

tests

references

Uh oh!

yasinlachiny commented Jun 6, 2025

Uh oh!

jamengual left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

bobziuchkovski commented Jun 26, 2025

Uh oh!

yasinlachiny commented Jun 26, 2025

Uh oh!

bobziuchkovski commented Jun 26, 2025

Uh oh!

yasinlachiny commented Jun 26, 2025

Uh oh!

bobziuchkovski commented Jun 26, 2025

Uh oh!

yasinlachiny commented Jun 26, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants