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 2bb529b
Showing 1 changed file with 15 additions and 6 deletions.
21 changes: 15 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,13 @@ 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 := ctx
version := workflow.GetVersion(ctx, NotifyAllCheckRunsChangeID, workflow.DefaultVersion, 1)
if version == 1 {
ctx, _ = workflow.NewDisconnectedContext(ctx)
}
p.TFStateReceiver.Receive(ctx, c, roots)
})

Expand Down Expand Up @@ -182,7 +192,6 @@ 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)

for _, rootInfo := range roots {
ctx = workflow.WithRetryPolicy(ctx, temporal.RetryPolicy{
MaximumAttempts: 3,
Expand Down

0 comments on commit 2bb529b

Please sign in to comment.