Skip to content

Commit

Permalink
Restore unlock action on queued checkruns (#774)
Browse files Browse the repository at this point in the history
When we update checkruns for queued commits (after the blocking commit
gets confirmed or rejected), we need to check if the queue was
previously locked by something else and restore the unlock button for
each commit.

### Sequence of events without this change:
1. Queue is locked due to diverged commits on latest deployment vs
current plan (i.e. if someone previously force applied). All queued
commits should have a unlock button.
2. New commit comes in with a pending confirm/reject action. My other PR
makes it so that all queued checkruns are updated to reflect that the
queue is waiting on this confirm reject action.
<img width="1461" alt="Screenshot 2024-11-08 at 3 08 07 PM"
src="https://github.com/user-attachments/assets/3daf3dc4-a95a-411c-983c-8ad95f53d2e7">
4. Confirm/reject commit gets resolved. All queued commit checkruns get
updated to the default status, without the unlock button. Currently
users would have to click on the commit they want to unlock (first
screenshot below), then click on an older checkrun of that commit
(linked in blue) to access the unlock button (2nd screenshot below).
<img width="877" alt="Screenshot 2024-11-08 at 3 07 08 PM"
src="https://github.com/user-attachments/assets/5c86a489-f18f-45cd-b887-583774ccaa0a">
<img width="1243" alt="Screenshot 2024-11-08 at 3 07 17 PM"
src="https://github.com/user-attachments/assets/0fa5b694-6350-450a-9155-7a3ca7583b48">

### After this change:
4. (Previous step 4) All queued commit checkruns get updated back to its
original status from before the confirm/reject commit was applied,
**including** unlock actions if the queue was previously locked.


### Additional notes:
* LockState is migrated out of the queue package into its own `lock`
package to avoid cyclic imports with terraform module
* bumping max interfacebloat golangci error to allow adding
GetLockState() function to queue interface

### Testing
1. Open PR 1. Force apply PR 1, and verify that the force apply is
successful.
2. Open PR 2. Merge PR 2 to master, and verify that PR 2 deployment
failed and is now locked on PR 1's last commit (there should be an
unlock button and link to the locking commit).
3. Open PR 3. Force apply PR 3, and verify that there is a
Confirm/Reject action.
4. Verify that PR 2's deployment checkrun on master has an updated
summary with link to the pending Confirm/Reject run on PR 3.
5. Reject forced apply on PR 3.
6. Verify that PR 2's deployment checkrun updates again, this time back
to the original checkrun summary from step 2. If the queue was
previously locked (which it was), there should be an unlock button.
  • Loading branch information
tlin4194 authored Dec 3, 2024
1 parent d5ef226 commit d31039c
Show file tree
Hide file tree
Showing 16 changed files with 129 additions and 91 deletions.
2 changes: 1 addition & 1 deletion .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,4 @@ issues:
- dogsled
linters-settings:
interfacebloat:
max: 6
max: 7
15 changes: 15 additions & 0 deletions server/neptune/workflows/internal/deploy/lock/lock.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package lock

type LockStatus int

type LockState struct {
Revision string
Status LockStatus
}

const (
UnlockedStatus LockStatus = iota
LockedStatus

QueueDepthStat = "queue.depth"
)
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/pkg/errors"
"github.com/runatlantis/atlantis/server/neptune/workflows/activities"
"github.com/runatlantis/atlantis/server/neptune/workflows/activities/github"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/lock"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/revision/queue"
"github.com/slack-go/slack"
"go.temporal.io/sdk/workflow"
Expand All @@ -24,7 +25,7 @@ type Slack struct {
func (s *Slack) Notify(ctx workflow.Context) error {
state := s.DeployQueue.GetLockState()

if state.Status == queue.UnlockedStatus {
if state.Status == lock.UnlockedStatus {
return nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"github.com/runatlantis/atlantis/server/neptune/workflows/activities"
"github.com/runatlantis/atlantis/server/neptune/workflows/activities/github"
terraformActivities "github.com/runatlantis/atlantis/server/neptune/workflows/activities/terraform"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/lock"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/revision/notifier"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/revision/queue"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/terraform"
Expand All @@ -20,7 +21,7 @@ import (
)

type request struct {
LockState queue.LockState
LockState lock.LockState
InitialItems []terraform.DeploymentInfo
}

Expand Down Expand Up @@ -51,7 +52,7 @@ func testWorkflow(ctx workflow.Context, request request) error {

func TestNotifier(t *testing.T) {
t.Run("empty queue", func(t *testing.T) {
state := queue.LockState{Status: queue.UnlockedStatus}
state := lock.LockState{Status: lock.UnlockedStatus}
ts := testsuite.WorkflowTestSuite{}
env := ts.NewTestWorkflowEnvironment()

Expand All @@ -68,7 +69,7 @@ func TestNotifier(t *testing.T) {
})

t.Run("locked state", func(t *testing.T) {
state := queue.LockState{Status: queue.LockedStatus}
state := lock.LockState{Status: lock.LockedStatus}
ts := testsuite.WorkflowTestSuite{}
env := ts.NewTestWorkflowEnvironment()

Expand All @@ -85,7 +86,7 @@ func TestNotifier(t *testing.T) {
})

t.Run("no slack config", func(t *testing.T) {
state := queue.LockState{Status: queue.LockedStatus}
state := lock.LockState{Status: lock.LockedStatus}
ts := testsuite.WorkflowTestSuite{}
env := ts.NewTestWorkflowEnvironment()

Expand Down Expand Up @@ -121,7 +122,7 @@ func TestNotifier(t *testing.T) {
})

t.Run("activity called", func(t *testing.T) {
state := queue.LockState{Status: queue.LockedStatus}
state := lock.LockState{Status: lock.LockedStatus}
ts := testsuite.WorkflowTestSuite{}
env := ts.NewTestWorkflowEnvironment()

Expand Down
29 changes: 8 additions & 21 deletions server/neptune/workflows/internal/deploy/revision/queue/queue.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,32 +7,19 @@ import (

"github.com/runatlantis/atlantis/server/neptune/workflows/activities/github"
activity "github.com/runatlantis/atlantis/server/neptune/workflows/activities/terraform"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/lock"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/terraform"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/metrics"
"go.temporal.io/sdk/workflow"
)

type LockStatus int

type LockState struct {
Revision string
Status LockStatus
}

const (
UnlockedStatus LockStatus = iota
LockedStatus

QueueDepthStat = "queue.depth"
)

type Deploy struct {
queue *priority
lockStatusCallback func(workflow.Context, *Deploy)
scope metrics.Scope

// mutable: default is unlocked
lock LockState
lock lock.LockState
}

func NewQueue(callback func(workflow.Context, *Deploy), scope metrics.Scope) *Deploy {
Expand All @@ -43,12 +30,12 @@ func NewQueue(callback func(workflow.Context, *Deploy), scope metrics.Scope) *De
}
}

func (q *Deploy) GetLockState() LockState {
func (q *Deploy) GetLockState() lock.LockState {
return q.lock
}

func (q *Deploy) SetLockForMergedItems(ctx workflow.Context, state LockState) {
if state.Status == LockedStatus {
func (q *Deploy) SetLockForMergedItems(ctx workflow.Context, state lock.LockState) {
if state.Status == lock.LockedStatus {
q.scope.Counter("locked").Inc(1)
} else {
q.scope.Counter("unlocked").Inc(1)
Expand All @@ -58,11 +45,11 @@ func (q *Deploy) SetLockForMergedItems(ctx workflow.Context, state LockState) {
}

func (q *Deploy) CanPop() bool {
return q.queue.HasItemsOfPriority(High) || (q.lock.Status == UnlockedStatus && !q.queue.IsEmpty())
return q.queue.HasItemsOfPriority(High) || (q.lock.Status == lock.UnlockedStatus && !q.queue.IsEmpty())
}

func (q *Deploy) Pop() (terraform.DeploymentInfo, error) {
defer q.scope.Gauge(QueueDepthStat).Update(float64(q.queue.Size()))
defer q.scope.Gauge(lock.QueueDepthStat).Update(float64(q.queue.Size()))
return q.queue.Pop()
}

Expand All @@ -79,7 +66,7 @@ func (q *Deploy) IsEmpty() bool {
}

func (q *Deploy) Push(msg terraform.DeploymentInfo) {
defer q.scope.Gauge(QueueDepthStat).Update(float64(q.queue.Size()))
defer q.scope.Gauge(lock.QueueDepthStat).Update(float64(q.queue.Size()))
if msg.Root.TriggerInfo.Type == activity.ManualTrigger {
q.queue.Push(msg, High)
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (

"github.com/runatlantis/atlantis/server/neptune/workflows/activities/github"
activity "github.com/runatlantis/atlantis/server/neptune/workflows/activities/terraform"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/lock"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/revision/queue"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/terraform"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/metrics"
Expand Down Expand Up @@ -38,8 +39,8 @@ func TestQueue(t *testing.T) {
q := queue.NewQueue(func(ctx workflow.Context, d *queue.Deploy) {
called = true
}, metrics.NewNullableScope())
q.SetLockForMergedItems(test.Background(), queue.LockState{
Status: queue.LockedStatus,
q.SetLockForMergedItems(test.Background(), lock.LockState{
Status: lock.LockedStatus,
})

assert.True(t, called)
Expand All @@ -52,17 +53,17 @@ func TestQueue(t *testing.T) {

t.Run("can pop empty queue locked", func(t *testing.T) {
q := queue.NewQueue(noopCallback, metrics.NewNullableScope())
q.SetLockForMergedItems(test.Background(), queue.LockState{
Status: queue.LockedStatus,
q.SetLockForMergedItems(test.Background(), lock.LockState{
Status: lock.LockedStatus,
})
assert.Equal(t, false, q.CanPop())
})
t.Run("can pop manual trigger locked", func(t *testing.T) {
q := queue.NewQueue(noopCallback, metrics.NewNullableScope())
msg1 := wrap("1", activity.ManualTrigger)
q.Push(msg1)
q.SetLockForMergedItems(test.Background(), queue.LockState{
Status: queue.LockedStatus,
q.SetLockForMergedItems(test.Background(), lock.LockState{
Status: lock.LockedStatus,
})
assert.Equal(t, true, q.CanPop())
})
Expand All @@ -76,8 +77,8 @@ func TestQueue(t *testing.T) {
q := queue.NewQueue(noopCallback, metrics.NewNullableScope())
msg1 := wrap("1", activity.MergeTrigger)
q.Push(msg1)
q.SetLockForMergedItems(test.Background(), queue.LockState{
Status: queue.LockedStatus,
q.SetLockForMergedItems(test.Background(), lock.LockState{
Status: lock.LockedStatus,
})
assert.Equal(t, false, q.CanPop())
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/notifier"

"github.com/runatlantis/atlantis/server/neptune/workflows/activities/github"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/lock"
"go.temporal.io/sdk/workflow"
)

Expand All @@ -20,17 +21,17 @@ type LockStateUpdater struct {
}

func (u *LockStateUpdater) UpdateQueuedRevisions(ctx workflow.Context, queue *Deploy, repoFullName string) {
lock := queue.GetLockState()
queueLock := queue.GetLockState()
infos := queue.GetOrderedMergedItems()

var actions []github.CheckRunAction
var summary string
var revisionsSummary string = queue.GetQueuedRevisionsSummary()
state := github.CheckRunQueued
if lock.Status == LockedStatus {
if queueLock.Status == lock.LockedStatus {
actions = append(actions, github.CreateUnlockAction())
state = github.CheckRunActionRequired
revisionLink := github.BuildRevisionURLMarkdown(repoFullName, lock.Revision)
revisionLink := github.BuildRevisionURLMarkdown(repoFullName, queueLock.Revision)
summary = fmt.Sprintf("This deploy is locked from a manual deployment for revision %s. Unlock to proceed.\n%s", revisionLink, revisionsSummary)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/runatlantis/atlantis/server/neptune/workflows/activities"
"github.com/runatlantis/atlantis/server/neptune/workflows/activities/github"
tfActivity "github.com/runatlantis/atlantis/server/neptune/workflows/activities/terraform"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/lock"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/revision/queue"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/terraform"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/metrics"
Expand Down Expand Up @@ -126,8 +127,8 @@ func TestLockStateUpdater_locked_new_version(t *testing.T) {

env.ExecuteWorkflow(testUpdaterWorkflow, updaterReq{
Queue: []terraform.DeploymentInfo{info},
Lock: queue.LockState{
Status: queue.LockedStatus,
Lock: lock.LockState{
Status: lock.LockedStatus,
Revision: "1234",
},
ExpectedRequest: notifier.GithubCheckRunRequest{
Expand All @@ -152,7 +153,7 @@ func TestLockStateUpdater_locked_new_version(t *testing.T) {

type updaterReq struct {
Queue []terraform.DeploymentInfo
Lock queue.LockState
Lock lock.LockState
ExpectedRequest notifier.GithubCheckRunRequest
ExpectedDeploymentID string
ExpectedT *testing.T
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
internalContext "github.com/runatlantis/atlantis/server/neptune/context"
"github.com/runatlantis/atlantis/server/neptune/workflows/activities/deployment"
tfModel "github.com/runatlantis/atlantis/server/neptune/workflows/activities/terraform"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/lock"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/terraform"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/metrics"
"github.com/runatlantis/atlantis/server/neptune/workflows/plugins"
Expand All @@ -24,9 +25,10 @@ type queue interface {
IsEmpty() bool
CanPop() bool
Pop() (terraform.DeploymentInfo, error)
SetLockForMergedItems(ctx workflow.Context, state LockState)
SetLockForMergedItems(ctx workflow.Context, state lock.LockState)
GetOrderedMergedItems() []terraform.DeploymentInfo
GetQueuedRevisionsSummary() string
GetLockState() lock.LockState
}

type deployer interface {
Expand Down Expand Up @@ -114,8 +116,8 @@ func NewWorker(
// we don't persist lock state anywhere so in the case of workflow completion we need to rebuild
// the lock state
if latestDeployment != nil && latestDeployment.Root.Trigger == string(tfModel.ManualTrigger) {
q.SetLockForMergedItems(ctx, LockState{
Status: LockedStatus,
q.SetLockForMergedItems(ctx, lock.LockState{
Status: lock.LockedStatus,
Revision: latestDeployment.Revision,
})
}
Expand Down Expand Up @@ -181,8 +183,8 @@ func (w *Worker) Work(ctx workflow.Context) {
workflow.GetMetricsHandler(ctx).WithTags(map[string]string{metricNames.SignalNameTag: UnlockSignalName}).
Counter(metricNames.SignalReceive).
Inc(1)
w.Queue.SetLockForMergedItems(ctx, LockState{
Status: UnlockedStatus,
w.Queue.SetLockForMergedItems(ctx, lock.LockState{
Status: lock.UnlockedStatus,
})
continue
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"go.temporal.io/sdk/testsuite"
"go.temporal.io/sdk/workflow"

"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/lock"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/revision/queue"
internalTerraform "github.com/runatlantis/atlantis/server/neptune/workflows/internal/deploy/terraform"
"github.com/runatlantis/atlantis/server/neptune/workflows/internal/metrics"
Expand Down Expand Up @@ -47,7 +48,7 @@ func testStateWorkflow(ctx workflow.Context, r workerRequest) (workerResponse, e
Queue: list.New(),
}

q.SetLockForMergedItems(ctx, queue.LockState{Status: r.InitialLockStatus})
q.SetLockForMergedItems(ctx, lock.LockState{Status: r.InitialLockStatus})

workflow.Go(ctx, func(ctx workflow.Context) {
ch := workflow.GetSignalChannel(ctx, "add-to-queue")
Expand Down Expand Up @@ -109,8 +110,8 @@ func TestWorker_StartsWithEmptyQueue(t *testing.T) {
assert.NoError(t, err)

assert.True(t, q.QueueIsEmpty)
assert.Equal(t, queue.LockState{
Status: queue.UnlockedStatus,
assert.Equal(t, lock.LockState{
Status: lock.UnlockedStatus,
}, q.Lock)
assert.Equal(t, queue.WaitingWorkerState, q.State)
}, 2*time.Second)
Expand Down Expand Up @@ -145,8 +146,8 @@ func TestWorker_StartsWithEmptyQueue(t *testing.T) {
assert.NoError(t, err)

assert.True(t, q.QueueIsEmpty)
assert.Equal(t, queue.LockState{
Status: queue.UnlockedStatus,
assert.Equal(t, lock.LockState{
Status: lock.UnlockedStatus,
}, q.Lock)
assert.Equal(t, queue.WorkingWorkerState, q.State)
}, 6*time.Second)
Expand Down
Loading

0 comments on commit d31039c

Please sign in to comment.