From 59f16ef86845122f34bea09515fe84bc3ac5e10b Mon Sep 17 00:00:00 2001 From: doodle Date: Thu, 19 Jun 2025 01:23:05 +0200 Subject: [PATCH] fix: preserve Atlantis apply lock after plan (#5570) Signed-off-by: yasinlachiny Signed-off-by: yasinlachiny Co-authored-by: yasinlachiny Co-authored-by: PePe Amengual <2208324+jamengual@users.noreply.github.com> --- server/events/command_runner_test.go | 29 +++++++++++++++++++++++----- server/events/plan_command_runner.go | 24 +++++++++++++++++++---- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/server/events/command_runner_test.go b/server/events/command_runner_test.go index a9e992854..9f05a5517 100644 --- a/server/events/command_runner_test.go +++ b/server/events/command_runner_test.go @@ -17,6 +17,7 @@ import ( "errors" "fmt" "regexp" + "strconv" "strings" "testing" @@ -882,7 +883,6 @@ func TestRunCommentCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_Fals ch.FailOnPreWorkflowHookError = false ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan}) pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp) - lockingLocker.VerifyWasCalledOnce().UnlockByPull(testdata.Pull.BaseRepo.FullName, testdata.Pull.Num) } func TestRunCommentCommand_FailedPreWorkflowHook_FailOnPreWorkflowHookError_True(t *testing.T) { @@ -919,7 +919,22 @@ func TestRunGenericPlanCommand_DeletePlans(t *testing.T) { autoMerger.GlobalAutomerge = true defer func() { autoMerger.GlobalAutomerge = false }() - When(projectCommandRunner.Plan(Any[command.ProjectContext]())).ThenReturn(command.ProjectResult{PlanSuccess: &models.PlanSuccess{}}) + projectCtx := command.ProjectContext{ + CommandName: command.Plan, + ExecutionOrderGroup: 0, + ProjectName: "TestProject", + Workspace: "default", + BaseRepo: testdata.GithubRepo, + Pull: testdata.Pull, + } + When(projectCommandBuilder.BuildPlanCommands(Any[*command.Context](), Any[*events.CommentCommand]())). + ThenReturn([]command.ProjectContext{projectCtx}, nil) + When(projectCommandRunner.Plan(projectCtx)).ThenReturn(command.ProjectResult{ + Command: command.Plan, + PlanSuccess: &models.PlanSuccess{ + TerraformOutput: "true", + }, + }) When(workingDir.GetPullDir(Any[models.Repo](), Any[models.PullRequest]())).ThenReturn(tmp, nil) pull := &github.PullRequest{State: github.Ptr("open")} modelPull := models.PullRequest{BaseRepo: testdata.GithubRepo, State: models.OpenPullState, Num: testdata.Pull.Num} @@ -927,8 +942,13 @@ func TestRunGenericPlanCommand_DeletePlans(t *testing.T) { When(eventParsing.ParseGithubPull(Any[logging.SimpleLogging](), Eq(pull))).ThenReturn(modelPull, modelPull.BaseRepo, testdata.GithubRepo, nil) testdata.Pull.BaseRepo = testdata.GithubRepo ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan}) - pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp) - lockingLocker.VerifyWasCalledOnce().UnlockByPull(testdata.Pull.BaseRepo.FullName, testdata.Pull.Num) + lockingLocker.VerifyWasCalledOnce().Unlock(fmt.Sprintf("%s/%d/%s/%s", testdata.Pull.BaseRepo.FullName, testdata.Pull.Num, projectCtx.ProjectName, projectCtx.Workspace)) + + lockingLocker.VerifyWasCalledOnce(). + Unlock(testdata.Pull.BaseRepo.FullName + "/" + + strconv.Itoa(testdata.Pull.Num) + "/" + + projectCtx.ProjectName + "/" + + projectCtx.Workspace) } func TestRunSpecificPlanCommandDoesnt_DeletePlans(t *testing.T) { @@ -1031,7 +1051,6 @@ func TestRunGenericPlanCommand_DiscardApprovals(t *testing.T) { testdata.Pull.BaseRepo = testdata.GithubRepo ch.RunCommentCommand(testdata.GithubRepo, nil, nil, testdata.User, testdata.Pull.Num, &events.CommentCommand{Name: command.Plan}) pendingPlanFinder.VerifyWasCalledOnce().DeletePlans(tmp) - lockingLocker.VerifyWasCalledOnce().UnlockByPull(testdata.Pull.BaseRepo.FullName, testdata.Pull.Num) vcsClient.VerifyWasCalledOnce().DiscardReviews(Any[logging.SimpleLogging](), Any[models.Repo](), Any[models.PullRequest]()) } diff --git a/server/events/plan_command_runner.go b/server/events/plan_command_runner.go index 5d38a2d4f..cbc29a934 100644 --- a/server/events/plan_command_runner.go +++ b/server/events/plan_command_runner.go @@ -1,6 +1,8 @@ package events import ( + "strconv" + "github.com/runatlantis/atlantis/server/core/locking" "github.com/runatlantis/atlantis/server/events/command" "github.com/runatlantis/atlantis/server/events/models" @@ -239,10 +241,6 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) { if !cmd.IsForSpecificProject() { ctx.Log.Debug("deleting previous plans and locks") p.deletePlans(ctx) - _, err = p.lockingLocker.UnlockByPull(baseRepo.FullName, pull.Num) - if err != nil { - ctx.Log.Err("deleting locks: %s", err) - } } // Only run commands in parallel if enabled @@ -255,6 +253,24 @@ func (p *PlanCommandRunner) run(ctx *command.Context, cmd *CommentCommand) { } ctx.CommandHasErrors = result.HasErrors() + for i, projResult := range result.ProjectResults { + projCtx := projectCmds[i] + + if projResult.PlanStatus() == models.PlannedNoChangesPlanStatus || projResult.PlanStatus() == models.ErroredPlanStatus { + ctx.Log.Info("Keeping lock for project '%s' (no changes or error)", projCtx.ProjectName) + continue + } + + // delete lock only if there are changes + ctx.Log.Info("Deleting lock for project '%s' (changes detected)", projCtx.ProjectName) + lockID := projCtx.BaseRepo.FullName + "/" + strconv.Itoa(projCtx.Pull.Num) + "/" + projCtx.ProjectName + "/" + projCtx.Workspace + + _, err := p.lockingLocker.Unlock(lockID) + if err != nil { + ctx.Log.Err("failed unlocking project '%s': %s", projCtx.ProjectName, err) + } + } + if p.autoMerger.automergeEnabled(projectCmds) && result.HasErrors() { ctx.Log.Info("deleting plans because there were errors and automerge requires all plans succeed") p.deletePlans(ctx)