-
Notifications
You must be signed in to change notification settings - Fork 6
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
Restore unlock action on queued checkruns #774
Restore unlock action on queued checkruns #774
Conversation
fa88f07
to
5fbd36d
Compare
@@ -36,6 +37,7 @@ type stateReceiver interface { | |||
type deployQueue interface { | |||
GetOrderedMergedItems() []DeploymentInfo | |||
GetQueuedRevisionsSummary() string | |||
GetLockState() lock.LockState |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason I wanted to migrate LockState to its own package is so that the terraform package can also use this GetLockState function without running into cyclic package dependencies (queue package imports terraform package, but terraform package needs access to functions and structs defined in queue package).
// If the queue is currently locked we need to provide the unlock action. | ||
queueLock := n.Queue.GetLockState() | ||
if queueLock.Status == lock.LockedStatus { | ||
actions = append(actions, github.CreateUnlockAction()) | ||
runState = github.CheckRunActionRequired | ||
revisionLink := github.BuildRevisionURLMarkdown(deploymentInfo.Repo.GetFullName(), queueLock.Revision) | ||
summary = fmt.Sprintf("This deploy is locked from a manual deployment for revision %s. Unlock to proceed.\n%s", revisionLink, revisionsSummary) | ||
} else { | ||
summary = "This deploy is queued and will be processed as soon as possible.\n" + revisionsSummary | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
GetOrderedMergedItems() []terraform.DeploymentInfo | ||
GetQueuedRevisionsSummary() string | ||
GetLockState() lock.LockState |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding these two functions to the interface is breaking the linter.
Solutions:
- separate queue package from the revision package so that queue struct can be shared without interfaces? (similar to what I did for the Lock package but this seems more complicated)
- Keep using interfaces, but split it up into 2 interfaces: one for functions used by deploy worker, one for terraform runner. WIP looking into if this can be done cleanly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edit: resolved by bumping the interfacebloat max setting in golangci.yml instead
565a13a
to
7025d13
Compare
d31039c
into
DEVCON-2524-output-deploy-queue-to-checkrun-summary
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:
After this change:
Additional notes:
lock
package to avoid cyclic imports with terraform moduleTesting