Skip to content
Merged
Prev Previous commit
Next Next commit
Fix race condition in provisioner jobs
  • Loading branch information
kylecarbs committed Mar 28, 2022
commit fb9fbb6f121d280b2ab7562503bce76ca7c11e2e
16 changes: 9 additions & 7 deletions cli/cliui/provisionerjob.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ func ProvisionerJob(cmd *cobra.Command, opts ProvisionerJobOptions) error {
printStage := func() {
_, _ = fmt.Fprintf(cmd.OutOrStdout(), Styles.Prompt.Render("⧗")+"%s\n", Styles.Field.Render(currentStage))
}
printStage()

updateStage := func(stage string, startedAt time.Time) {
if currentStage != "" {
Expand Down Expand Up @@ -104,9 +103,9 @@ func ProvisionerJob(cmd *cobra.Command, opts ProvisionerJobOptions) error {
if opts.Cancel != nil {
// Handles ctrl+c to cancel a job.
stopChan := make(chan os.Signal, 1)
defer signal.Stop(stopChan)
signal.Notify(stopChan, os.Interrupt)
go func() {
signal.Notify(stopChan, os.Interrupt)
defer signal.Stop(stopChan)
select {
case <-ctx.Done():
return
Expand All @@ -115,8 +114,6 @@ func ProvisionerJob(cmd *cobra.Command, opts ProvisionerJobOptions) error {
return
}
}
// Stop listening for signals so another one kills it!
signal.Stop(stopChan)
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "\033[2K\r\n"+Styles.FocusedPrompt.String()+Styles.Bold.Render("Gracefully canceling...")+"\n\n")
err := opts.Cancel()
if err != nil {
Expand All @@ -127,6 +124,9 @@ func ProvisionerJob(cmd *cobra.Command, opts ProvisionerJobOptions) error {
}()
}

// The initial stage needs to print after the signal handler has been registered.
printStage()

logs, err := opts.Logs()
if err != nil {
return xerrors.Errorf("logs: %w", err)
Expand Down Expand Up @@ -159,8 +159,9 @@ func ProvisionerJob(cmd *cobra.Command, opts ProvisionerJobOptions) error {
return nil
case codersdk.ProvisionerJobFailed:
}
err = xerrors.New(job.Error)
jobMutex.Unlock()
return xerrors.New(job.Error)
return err
}
output := ""
switch log.Level {
Expand All @@ -173,14 +174,15 @@ func ProvisionerJob(cmd *cobra.Command, opts ProvisionerJobOptions) error {
case database.LogLevelInfo:
output = log.Output
}
jobMutex.Lock()
if log.Stage != currentStage && log.Stage != "" {
jobMutex.Lock()
updateStage(log.Stage, log.CreatedAt)
jobMutex.Unlock()
continue
}
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s %s\n", Styles.Placeholder.Render(" "), output)
didLogBetweenStage = true
jobMutex.Unlock()
}
}
}
8 changes: 4 additions & 4 deletions cli/cliui/provisionerjob_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ func TestProvisionerJob(t *testing.T) {
test.Job.Status = codersdk.ProvisionerJobSucceeded
now = database.Now()
test.Job.CompletedAt = &now
test.JobMutex.Unlock()
close(test.Logs)
test.JobMutex.Unlock()
}()
test.PTY.ExpectMatch("Queued")
test.Next <- struct{}{}
Expand Down Expand Up @@ -67,8 +67,8 @@ func TestProvisionerJob(t *testing.T) {
test.Job.Status = codersdk.ProvisionerJobSucceeded
now = database.Now()
test.Job.CompletedAt = &now
test.JobMutex.Unlock()
close(test.Logs)
test.JobMutex.Unlock()
}()
test.PTY.ExpectMatch("Queued")
test.Next <- struct{}{}
Expand All @@ -79,7 +79,7 @@ func TestProvisionerJob(t *testing.T) {
})

// This cannot be ran in parallel because it uses a signal.
//nolint:paralleltest
// nolint:paralleltest
t.Run("Cancel", func(t *testing.T) {
if runtime.GOOS == "windows" {
// Sending interrupt signal isn't supported on Windows!
Expand All @@ -98,8 +98,8 @@ func TestProvisionerJob(t *testing.T) {
test.Job.Status = codersdk.ProvisionerJobCanceled
now := database.Now()
test.Job.CompletedAt = &now
test.JobMutex.Unlock()
close(test.Logs)
test.JobMutex.Unlock()
}()
test.PTY.ExpectMatch("Queued")
test.Next <- struct{}{}
Expand Down