Skip to content

Commit

Permalink
Always update checkrun status for terraform workflows
Browse files Browse the repository at this point in the history
Why?
* Terraform workflows cannot cancel due to WaitForCancellation, so we might as well try to update the status of the checkrun.
* We also think this will help with the non determinism issues we are seeing where cancellations can occur at different than expected times on replay, causing the checkrun cache to be inaccurate, thus causing wrong calls to either create or update.
  • Loading branch information
asubrocky committed Oct 10, 2023
1 parent ef108a9 commit 7e6fa75
Showing 1 changed file with 12 additions and 6 deletions.
18 changes: 12 additions & 6 deletions server/neptune/workflows/internal/pr/revision/processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import (
)

const (
ReviewSignalID = "pr-review"
CheckRunCancelled = "checkrun was cancelled"
ReviewSignalID = "pr-review"
CheckRunCancelled = "checkrun was cancelled"
NotifyAllCheckRunsChangeID = "notify-all-checkruns-always"
)

type TFWorkflow func(ctx workflow.Context, request terraform.Request) (terraform.Response, error)
Expand Down Expand Up @@ -72,9 +73,11 @@ func (p *Processor) Process(ctx workflow.Context, prRevision Revision) {
// Mark checkruns as aborted if the context was cancelled, this typically happens if revisions arrive in quick succession
defer func() {
if temporal.IsCanceledError(ctx.Err()) {
ctx, _ := workflow.NewDisconnectedContext(ctx)
p.markCheckRunsAborted(ctx, prRevision, roots)
return
version := workflow.GetVersion(ctx, NotifyAllCheckRunsChangeID, workflow.DefaultVersion, 1)
if version == workflow.DefaultVersion {
p.markCheckRunsAborted(ctx, prRevision, roots)
return
}
}

// At this point, all workflows should be successful, and we can mark combined plan check run as success
Expand Down Expand Up @@ -131,6 +134,9 @@ func (p *Processor) awaitChildTerraformWorkflows(ctx workflow.Context, futures [
selector := workflow.NewNamedSelector(ctx, "TerraformChildWorkflow")
ch := workflow.GetSignalChannel(ctx, state.WorkflowStateChangeSignal)
selector.AddReceive(ch, func(c workflow.ReceiveChannel, _ bool) {
// The parent workflow can be cancelled when a new revision is received, but we still would like to notify
// state of any of the terraform workflows which will finish regardless of parent cancellation.
ctx, _ := workflow.NewDisconnectedContext(ctx)
p.TFStateReceiver.Receive(ctx, c, roots)
})

Expand Down Expand Up @@ -182,7 +188,7 @@ func (p *Processor) markCombinedCheckRun(ctx workflow.Context, revision Revision

func (p *Processor) markCheckRunsAborted(ctx workflow.Context, revision Revision, roots map[string]RootInfo) {
p.markCombinedCheckRun(ctx, revision, github.CheckRunCancelled, CheckRunCancelled)

Check failure on line 191 in server/neptune/workflows/internal/pr/revision/processor.go

View workflow job for this annotation

GitHub Actions / runner / golangci-lint

File is not `gofmt`-ed with `-s` (gofmt)
for _, rootInfo := range roots {
ctx = workflow.WithRetryPolicy(ctx, temporal.RetryPolicy{
MaximumAttempts: 3,
Expand Down

0 comments on commit 7e6fa75

Please sign in to comment.