feat: Add lock-acquire-timeout to manage locks

runatlantis · Oct 11, 2024 · 7919060 · 7919060
1 parent d313d5a
commit 7919060
Show file tree

Hide file tree

Showing 11 changed files with 58 additions and 49 deletions.
diff --git a/cmd/server.go b/cmd/server.go
@@ -113,6 +113,7 @@ const (
 	APISecretFlag                    = "api-secret"
 	HidePrevPlanComments             = "hide-prev-plan-comments"
 	QuietPolicyChecks                = "quiet-policy-checks"
+	LockAcquireTimeoutSeconds        = "lock-acquire-timeout"
 	LockingDBType                    = "locking-db-type"
 	LogLevelFlag                     = "log-level"
 	MarkdownTemplateOverridesDirFlag = "markdown-template-overrides-dir"
@@ -173,6 +174,7 @@ const (
 	DefaultGiteaBaseURL                 = "https://gitea.com"
 	DefaultGiteaPageSize                = 30
 	DefaultGitlabHostname               = "gitlab.com"
+	DefaultLockAcquireTimeoutSeconds    = 900
 	DefaultLockingDBType                = "boltdb"
 	DefaultLogLevel                     = "info"
 	DefaultMaxCommentsPerCommand        = 100
@@ -607,6 +609,10 @@ var intFlags = map[string]intFlag{
 			" If merge base is further behind than this number of commits from any of branches heads, full fetch will be performed.",
 		defaultValue: DefaultCheckoutDepth,
 	},
+	LockAcquireTimeoutSeconds: {
+		description:  fmt.Sprintf("The number of seconds to wait for a lock to be acquired before timing out. The default value is %d", DefaultLockAcquireTimeoutSeconds),
+		defaultValue: DefaultLockAcquireTimeoutSeconds,
+	},
 	MaxCommentsPerCommand: {
 		description:  "If non-zero, the maximum number of comments to split command output into before truncating.",
 		defaultValue: DefaultMaxCommentsPerCommand,

diff --git a/server/controllers/events/events_controller_e2e_test.go b/server/controllers/events/events_controller_e2e_test.go
@@ -1347,7 +1347,7 @@ func setupE2E(t *testing.T, repoDir string, opt setupOption) (events_controllers
 	}
 
 	defaultTFVersion := terraformClient.DefaultVersion()
-	locker := events.NewDefaultWorkingDirLocker()
+	locker := events.NewDefaultWorkingDirLocker(1)
 	parser := &config.ParserValidator{}
 
 	globalCfgArgs := valid.GlobalCfgArgs{

diff --git a/server/controllers/locks_controller_test.go b/server/controllers/locks_controller_test.go
@@ -280,7 +280,7 @@ func TestDeleteLock_UpdateProjectStatus(t *testing.T) {
 	cp := vcsmocks.NewMockClient()
 	l := mocks2.NewMockDeleteLockCommand()
 	workingDir := mocks2.NewMockWorkingDir()
-	workingDirLocker := events.NewDefaultWorkingDirLocker()
+	workingDirLocker := events.NewDefaultWorkingDirLocker(1)
 	pull := models.PullRequest{
 		BaseRepo: models.Repo{FullName: repoName},
 	}
@@ -345,7 +345,7 @@ func TestDeleteLock_CommentFailed(t *testing.T) {
 	}, nil)
 	cp := vcsmocks.NewMockClient()
 	workingDir := mocks2.NewMockWorkingDir()
-	workingDirLocker := events.NewDefaultWorkingDirLocker()
+	workingDirLocker := events.NewDefaultWorkingDirLocker(1)
 	var backend locking.Backend
 	tmp := t.TempDir()
 	backend, err := db.New(tmp)
@@ -372,7 +372,7 @@ func TestDeleteLock_CommentSuccess(t *testing.T) {
 	cp := vcsmocks.NewMockClient()
 	dlc := mocks2.NewMockDeleteLockCommand()
 	workingDir := mocks2.NewMockWorkingDir()
-	workingDirLocker := events.NewDefaultWorkingDirLocker()
+	workingDirLocker := events.NewDefaultWorkingDirLocker(1)
 	var backend locking.Backend
 	tmp := t.TempDir()
 	backend, err := db.New(tmp)

diff --git a/server/events/delete_lock_command_test.go b/server/events/delete_lock_command_test.go
@@ -43,7 +43,7 @@ func TestDeleteLock_Success(t *testing.T) {
 	l := lockmocks.NewMockLocker()
 	When(l.Unlock("id")).ThenReturn(&models.ProjectLock{}, nil)
 	workingDir := events.NewMockWorkingDir()
-	workingDirLocker := events.NewDefaultWorkingDirLocker()
+	workingDirLocker := events.NewDefaultWorkingDirLocker(1)
 	workspace := "workspace"
 	path := "path"
 	projectName := ""

diff --git a/server/events/project_command_builder_internal_test.go b/server/events/project_command_builder_internal_test.go
@@ -656,7 +656,7 @@ projects:
 				&DefaultProjectFinder{},
 				vcsClient,
 				workingDir,
-				NewDefaultWorkingDirLocker(),
+				NewDefaultWorkingDirLocker(1),
 				globalCfg,
 				&DefaultPendingPlanFinder{},
 				&CommentParser{ExecutableName: "atlantis"},
@@ -873,7 +873,7 @@ projects:
 				&DefaultProjectFinder{},
 				vcsClient,
 				workingDir,
-				NewDefaultWorkingDirLocker(),
+				NewDefaultWorkingDirLocker(1),
 				globalCfg,
 				&DefaultPendingPlanFinder{},
 				&CommentParser{ExecutableName: "atlantis"},
@@ -1120,7 +1120,7 @@ workflows:
 				&DefaultProjectFinder{},
 				vcsClient,
 				workingDir,
-				NewDefaultWorkingDirLocker(),
+				NewDefaultWorkingDirLocker(1),
 				globalCfg,
 				&DefaultPendingPlanFinder{},
 				&CommentParser{ExecutableName: "atlantis"},
@@ -1272,7 +1272,7 @@ projects:
 				&DefaultProjectFinder{},
 				vcsClient,
 				workingDir,
-				NewDefaultWorkingDirLocker(),
+				NewDefaultWorkingDirLocker(1),
 				globalCfg,
 				&DefaultPendingPlanFinder{},
 				&CommentParser{ExecutableName: "atlantis"},
@@ -1414,7 +1414,7 @@ projects:
 				&DefaultProjectFinder{},
 				vcsClient,
 				workingDir,
-				NewDefaultWorkingDirLocker(),
+				NewDefaultWorkingDirLocker(1),
 				globalCfg,
 				&DefaultPendingPlanFinder{},
 				&CommentParser{ExecutableName: "atlantis"},

diff --git a/server/events/project_command_builder_test.go b/server/events/project_command_builder_test.go
@@ -258,7 +258,7 @@ terraform {
 				&events.DefaultProjectFinder{},
 				vcsClient,
 				workingDir,
-				events.NewDefaultWorkingDirLocker(),
+				events.NewDefaultWorkingDirLocker(1),
 				valid.NewGlobalCfgFromArgs(globalCfgArgs),
 				&events.DefaultPendingPlanFinder{},
 				&events.CommentParser{ExecutableName: "atlantis"},
@@ -624,7 +624,7 @@ projects:
 					&events.DefaultProjectFinder{},
 					vcsClient,
 					workingDir,
-					events.NewDefaultWorkingDirLocker(),
+					events.NewDefaultWorkingDirLocker(1),
 					valid.NewGlobalCfgFromArgs(globalCfgArgs),
 					&events.DefaultPendingPlanFinder{},
 					&events.CommentParser{ExecutableName: "atlantis"},
@@ -812,7 +812,7 @@ projects:
 				&events.DefaultProjectFinder{},
 				vcsClient,
 				workingDir,
-				events.NewDefaultWorkingDirLocker(),
+				events.NewDefaultWorkingDirLocker(1),
 				valid.NewGlobalCfgFromArgs(globalCfgArgs),
 				&events.DefaultPendingPlanFinder{},
 				&events.CommentParser{ExecutableName: "atlantis"},
@@ -1141,7 +1141,7 @@ projects:
 				&events.DefaultProjectFinder{},
 				vcsClient,
 				workingDir,
-				events.NewDefaultWorkingDirLocker(),
+				events.NewDefaultWorkingDirLocker(1),
 				valid.NewGlobalCfgFromArgs(globalCfgArgs),
 				&events.DefaultPendingPlanFinder{},
 				&events.CommentParser{ExecutableName: "atlantis"},
@@ -1239,7 +1239,7 @@ func TestDefaultProjectCommandBuilder_BuildMultiApply(t *testing.T) {
 		&events.DefaultProjectFinder{},
 		nil,
 		workingDir,
-		events.NewDefaultWorkingDirLocker(),
+		events.NewDefaultWorkingDirLocker(1),
 		valid.NewGlobalCfgFromArgs(globalCfgArgs),
 		&events.DefaultPendingPlanFinder{},
 		&events.CommentParser{ExecutableName: "atlantis"},
@@ -1325,7 +1325,7 @@ projects:
 		&events.DefaultProjectFinder{},
 		nil,
 		workingDir,
-		events.NewDefaultWorkingDirLocker(),
+		events.NewDefaultWorkingDirLocker(1),
 		valid.NewGlobalCfgFromArgs(globalCfgArgs),
 		&events.DefaultPendingPlanFinder{},
 		&events.CommentParser{ExecutableName: "atlantis"},
@@ -1413,7 +1413,7 @@ func TestDefaultProjectCommandBuilder_EscapeArgs(t *testing.T) {
 				&events.DefaultProjectFinder{},
 				vcsClient,
 				workingDir,
-				events.NewDefaultWorkingDirLocker(),
+				events.NewDefaultWorkingDirLocker(1),
 				valid.NewGlobalCfgFromArgs(globalCfgArgs),
 				&events.DefaultPendingPlanFinder{},
 				&events.CommentParser{ExecutableName: "atlantis"},
@@ -1575,7 +1575,7 @@ projects:
 				&events.DefaultProjectFinder{},
 				vcsClient,
 				workingDir,
-				events.NewDefaultWorkingDirLocker(),
+				events.NewDefaultWorkingDirLocker(1),
 				valid.NewGlobalCfgFromArgs(globalCfgArgs),
 				&events.DefaultPendingPlanFinder{},
 				&events.CommentParser{ExecutableName: "atlantis"},
@@ -1685,7 +1685,7 @@ projects:
 			&events.DefaultProjectFinder{},
 			vcsClient,
 			workingDir,
-			events.NewDefaultWorkingDirLocker(),
+			events.NewDefaultWorkingDirLocker(1),
 			valid.NewGlobalCfgFromArgs(globalCfgArgs),
 			&events.DefaultPendingPlanFinder{},
 			&events.CommentParser{ExecutableName: "atlantis"},
@@ -1754,7 +1754,7 @@ func TestDefaultProjectCommandBuilder_WithPolicyCheckEnabled_BuildAutoplanComman
 		&events.DefaultProjectFinder{},
 		vcsClient,
 		workingDir,
-		events.NewDefaultWorkingDirLocker(),
+		events.NewDefaultWorkingDirLocker(1),
 		globalCfg,
 		&events.DefaultPendingPlanFinder{},
 		&events.CommentParser{ExecutableName: "atlantis"},
@@ -1842,7 +1842,7 @@ func TestDefaultProjectCommandBuilder_BuildVersionCommand(t *testing.T) {
 		&events.DefaultProjectFinder{},
 		nil,
 		workingDir,
-		events.NewDefaultWorkingDirLocker(),
+		events.NewDefaultWorkingDirLocker(1),
 		valid.NewGlobalCfgFromArgs(globalCfgArgs),
 		&events.DefaultPendingPlanFinder{},
 		&events.CommentParser{ExecutableName: "atlantis"},
@@ -1972,7 +1972,7 @@ func TestDefaultProjectCommandBuilder_BuildPlanCommands_Single_With_RestrictFile
 				&events.DefaultProjectFinder{},
 				vcsClient,
 				workingDir,
-				events.NewDefaultWorkingDirLocker(),
+				events.NewDefaultWorkingDirLocker(1),
 				valid.NewGlobalCfgFromArgs(globalCfgArgs),
 				&events.DefaultPendingPlanFinder{},
 				&events.CommentParser{ExecutableName: "atlantis"},
@@ -2083,7 +2083,7 @@ func TestDefaultProjectCommandBuilder_BuildPlanCommands_with_IncludeGitUntracked
 				&events.DefaultProjectFinder{},
 				vcsClient,
 				workingDir,
-				events.NewDefaultWorkingDirLocker(),
+				events.NewDefaultWorkingDirLocker(1),
 				valid.NewGlobalCfgFromArgs(globalCfgArgs),
 				&events.DefaultPendingPlanFinder{},
 				&events.CommentParser{ExecutableName: "atlantis"},

diff --git a/server/events/project_command_runner_test.go b/server/events/project_command_runner_test.go
@@ -58,7 +58,7 @@ func TestDefaultProjectCommandRunner_Plan(t *testing.T) {
 		PullApprovedChecker:       nil,
 		WorkingDir:                mockWorkingDir,
 		Webhooks:                  nil,
-		WorkingDirLocker:          events.NewDefaultWorkingDirLocker(),
+		WorkingDirLocker:          events.NewDefaultWorkingDirLocker(1),
 		CommandRequirementHandler: mockCommandRequirementHandler,
 	}
 
@@ -248,7 +248,7 @@ func TestDefaultProjectCommandRunner_ApplyNotApproved(t *testing.T) {
 	mockWorkingDir := mocks.NewMockWorkingDir()
 	runner := &events.DefaultProjectCommandRunner{
 		WorkingDir:       mockWorkingDir,
-		WorkingDirLocker: events.NewDefaultWorkingDirLocker(),
+		WorkingDirLocker: events.NewDefaultWorkingDirLocker(1),
 		CommandRequirementHandler: &events.DefaultCommandRequirementHandler{
 			WorkingDir: mockWorkingDir,
 		},
@@ -269,7 +269,7 @@ func TestDefaultProjectCommandRunner_ApplyNotMergeable(t *testing.T) {
 	mockWorkingDir := mocks.NewMockWorkingDir()
 	runner := &events.DefaultProjectCommandRunner{
 		WorkingDir:       mockWorkingDir,
-		WorkingDirLocker: events.NewDefaultWorkingDirLocker(),
+		WorkingDirLocker: events.NewDefaultWorkingDirLocker(1),
 		CommandRequirementHandler: &events.DefaultCommandRequirementHandler{
 			WorkingDir: mockWorkingDir,
 		},
@@ -293,7 +293,7 @@ func TestDefaultProjectCommandRunner_ApplyDiverged(t *testing.T) {
 	mockWorkingDir := mocks.NewMockWorkingDir()
 	runner := &events.DefaultProjectCommandRunner{
 		WorkingDir:       mockWorkingDir,
-		WorkingDirLocker: events.NewDefaultWorkingDirLocker(),
+		WorkingDirLocker: events.NewDefaultWorkingDirLocker(1),
 		CommandRequirementHandler: &events.DefaultCommandRequirementHandler{
 			WorkingDir: mockWorkingDir,
 		},
@@ -415,7 +415,7 @@ func TestDefaultProjectCommandRunner_Apply(t *testing.T) {
 				EnvStepRunner:             mockEnv,
 				WorkingDir:                mockWorkingDir,
 				Webhooks:                  mockSender,
-				WorkingDirLocker:          events.NewDefaultWorkingDirLocker(),
+				WorkingDirLocker:          events.NewDefaultWorkingDirLocker(1),
 				CommandRequirementHandler: applyReqHandler,
 			}
 			repoDir := t.TempDir()
@@ -496,7 +496,7 @@ func TestDefaultProjectCommandRunner_ApplyRunStepFailure(t *testing.T) {
 		LockURLGenerator:          mockURLGenerator{},
 		ApplyStepRunner:           mockApply,
 		WorkingDir:                mockWorkingDir,
-		WorkingDirLocker:          events.NewDefaultWorkingDirLocker(),
+		WorkingDirLocker:          events.NewDefaultWorkingDirLocker(1),
 		CommandRequirementHandler: applyReqHandler,
 		Webhooks:                  mockSender,
 	}
@@ -565,7 +565,7 @@ func TestDefaultProjectCommandRunner_RunEnvSteps(t *testing.T) {
 		EnvStepRunner:             &env,
 		WorkingDir:                mockWorkingDir,
 		Webhooks:                  nil,
-		WorkingDirLocker:          events.NewDefaultWorkingDirLocker(),
+		WorkingDirLocker:          events.NewDefaultWorkingDirLocker(1),
 		CommandRequirementHandler: mockCommandRequirementHandler,
 	}
 
@@ -699,7 +699,7 @@ func TestDefaultProjectCommandRunner_Import(t *testing.T) {
 				StateRmStepRunner:         mockStateRm,
 				WorkingDir:                mockWorkingDir,
 				Webhooks:                  mockSender,
-				WorkingDirLocker:          events.NewDefaultWorkingDirLocker(),
+				WorkingDirLocker:          events.NewDefaultWorkingDirLocker(1),
 				CommandRequirementHandler: applyReqHandler,
 			}
 			ctx := command.ProjectContext{
@@ -1244,7 +1244,7 @@ func TestDefaultProjectCommandRunner_ApprovePolicies(t *testing.T) {
 				EnvStepRunner:    mockEnv,
 				WorkingDir:       mockWorkingDir,
 				Webhooks:         mockSender,
-				WorkingDirLocker: events.NewDefaultWorkingDirLocker(),
+				WorkingDirLocker: events.NewDefaultWorkingDirLocker(1),
 			}
 			repoDir := t.TempDir()
 			When(mockWorkingDir.GetWorkingDir(

diff --git a/server/events/working_dir_locker.go b/server/events/working_dir_locker.go
@@ -50,11 +50,15 @@ type DefaultWorkingDirLocker struct {
 	// matching to determine if something is locked. It's naive but that's okay
 	// because there won't be many locks at one time.
 	locks []string
+
+	lockAcquireTimeoutSeconds int
 }
 
 // NewDefaultWorkingDirLocker is a constructor.
-func NewDefaultWorkingDirLocker() WorkingDirLocker {
-	return &DefaultWorkingDirLocker{}
+func NewDefaultWorkingDirLocker(lockAcquireTimeoutSeconds int) *DefaultWorkingDirLocker {
+	return &DefaultWorkingDirLocker{
+		lockAcquireTimeoutSeconds: lockAcquireTimeoutSeconds,
+	}
 }
 
 func (d *DefaultWorkingDirLocker) TryLockPull(repoFullName string, pullNum int) (func(), error) {
@@ -80,8 +84,7 @@ func (d *DefaultWorkingDirLocker) TryLock(repoFullName string, pullNum int, work
 	defer d.mutex.Unlock()
 
 	ticker := time.NewTicker(time.Second)
-	// TODO: make this configurable
-	timeout := time.NewTimer(1 * time.Second)
+	timeout := time.NewTimer(time.Duration(d.lockAcquireTimeoutSeconds) * time.Second)
 	var acquireLock bool
 
 	for {
@@ -108,7 +111,6 @@ func (d *DefaultWorkingDirLocker) TryLock(repoFullName string, pullNum int, work
 				}
 			}
 		}
-
 	}
 }