From 7a62534359b9b968b9f5cad41528e24640126b34 Mon Sep 17 00:00:00 2001
From: Dean Sheather <dean@deansheather.com>
Date: Fri, 14 Jul 2023 18:54:57 -0700
Subject: [PATCH] fix: allow unhanger to unhang canceling jobs (#8529)

---
 coderd/unhanger/detector.go      | 19 ++++++---
 coderd/unhanger/detector_test.go | 66 ++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+), 6 deletions(-)

diff --git a/coderd/unhanger/detector.go b/coderd/unhanger/detector.go
index a0b61483ab..fb81311664 100644
--- a/coderd/unhanger/detector.go
+++ b/coderd/unhanger/detector.go
@@ -17,7 +17,6 @@ import (
 	"github.com/coder/coder/coderd/database/db2sdk"
 	"github.com/coder/coder/coderd/database/dbauthz"
 	"github.com/coder/coder/coderd/database/pubsub"
-	"github.com/coder/coder/codersdk"
 	"github.com/coder/coder/provisionersdk"
 )
 
@@ -201,8 +200,10 @@ func (d *Detector) run(t time.Time) Stats {
 		log := d.log.With(slog.F("job_id", job.ID))
 
 		err := unhangJob(ctx, log, d.db, d.pubsub, job.ID)
-		if err != nil && !(xerrors.As(err, &acquireLockError{}) || xerrors.As(err, &jobInelligibleError{})) {
-			log.Error(ctx, "error forcefully terminating hung provisioner job", slog.Error(err))
+		if err != nil {
+			if !(xerrors.As(err, &acquireLockError{}) || xerrors.As(err, &jobInelligibleError{})) {
+				log.Error(ctx, "error forcefully terminating hung provisioner job", slog.Error(err))
+			}
 			continue
 		}
 
@@ -232,10 +233,16 @@ func unhangJob(ctx context.Context, log slog.Logger, db database.Store, pub pubs
 		}
 
 		// Check if we should still unhang it.
-		jobStatus := db2sdk.ProvisionerJobStatus(job)
-		if jobStatus != codersdk.ProvisionerJobRunning {
+		if !job.StartedAt.Valid {
+			// This shouldn't be possible to hit because the query only selects
+			// started and not completed jobs, and a job can't be "un-started".
 			return jobInelligibleError{
-				Err: xerrors.Errorf("job is not running (status %s)", jobStatus),
+				Err: xerrors.New("job is not started"),
+			}
+		}
+		if job.CompletedAt.Valid {
+			return jobInelligibleError{
+				Err: xerrors.Errorf("job is completed (status %s)", db2sdk.ProvisionerJobStatus(job)),
 			}
 		}
 		if job.UpdatedAt.After(time.Now().Add(-HungJobDuration)) {
diff --git a/coderd/unhanger/detector_test.go b/coderd/unhanger/detector_test.go
index 4f98a82153..41d2dd26f2 100644
--- a/coderd/unhanger/detector_test.go
+++ b/coderd/unhanger/detector_test.go
@@ -525,6 +525,72 @@ func TestDetectorHungOtherJobTypes(t *testing.T) {
 	detector.Wait()
 }
 
+func TestDetectorHungCanceledJob(t *testing.T) {
+	t.Parallel()
+
+	var (
+		ctx        = testutil.Context(t, testutil.WaitLong)
+		db, pubsub = dbtestutil.NewDB(t)
+		log        = slogtest.Make(t, nil)
+		tickCh     = make(chan time.Time)
+		statsCh    = make(chan unhanger.Stats)
+	)
+
+	var (
+		now       = time.Now()
+		tenMinAgo = now.Add(-time.Minute * 10)
+		sixMinAgo = now.Add(-time.Minute * 6)
+		org       = dbgen.Organization(t, db, database.Organization{})
+		user      = dbgen.User(t, db, database.User{})
+		file      = dbgen.File(t, db, database.File{})
+
+		// Template import job.
+		templateImportJob = dbgen.ProvisionerJob(t, db, database.ProvisionerJob{
+			CreatedAt: tenMinAgo,
+			CanceledAt: sql.NullTime{
+				Time:  tenMinAgo,
+				Valid: true,
+			},
+			UpdatedAt: sixMinAgo,
+			StartedAt: sql.NullTime{
+				Time:  tenMinAgo,
+				Valid: true,
+			},
+			OrganizationID: org.ID,
+			InitiatorID:    user.ID,
+			Provisioner:    database.ProvisionerTypeEcho,
+			StorageMethod:  database.ProvisionerStorageMethodFile,
+			FileID:         file.ID,
+			Type:           database.ProvisionerJobTypeTemplateVersionImport,
+			Input:          []byte("{}"),
+		})
+	)
+
+	t.Log("template import job ID: ", templateImportJob.ID)
+
+	detector := unhanger.New(ctx, db, pubsub, log, tickCh).WithStatsChannel(statsCh)
+	detector.Start()
+	tickCh <- now
+
+	stats := <-statsCh
+	require.NoError(t, stats.Error)
+	require.Len(t, stats.TerminatedJobIDs, 1)
+	require.Contains(t, stats.TerminatedJobIDs, templateImportJob.ID)
+
+	// Check that the job was updated.
+	job, err := db.GetProvisionerJobByID(ctx, templateImportJob.ID)
+	require.NoError(t, err)
+	require.WithinDuration(t, now, job.UpdatedAt, 30*time.Second)
+	require.True(t, job.CompletedAt.Valid)
+	require.WithinDuration(t, now, job.CompletedAt.Time, 30*time.Second)
+	require.True(t, job.Error.Valid)
+	require.Contains(t, job.Error.String, "Build has been detected as hung")
+	require.False(t, job.ErrorCode.Valid)
+
+	detector.Close()
+	detector.Wait()
+}
+
 func TestDetectorPushesLogs(t *testing.T) {
 	t.Parallel()