fix: set a failed canceled job status correctly (#3101)

Kira-Pilot · johnstcn · web-flow · commit 0aa8c2efeb38 · 2022-07-21T16:47:06.000-04:00
* set a failed canceled job status correctly resolves #1374 * added unit test for convertProvisionerJob * Update coderd/provisionerjobs_internal_test.go Co-authored-by: Cian Johnston <cian@coder.com> * PR feedback Co-authored-by: Cian Johnston <cian@coder.com>
diff --git a/coderd/provisionerjobs.go b/coderd/provisionerjobs.go
@@ -312,7 +312,11 @@ func convertProvisionerJob(provisionerJob database.ProvisionerJob) codersdk.Prov
 	switch {
 	case provisionerJob.CanceledAt.Valid:
 		if provisionerJob.CompletedAt.Valid {
-			job.Status = codersdk.ProvisionerJobCanceled
+			if job.Error == "" {
+				job.Status = codersdk.ProvisionerJobCanceled
+			} else {
+				job.Status = codersdk.ProvisionerJobFailed
+			}
 		} else {
 			job.Status = codersdk.ProvisionerJobCanceling
 		}
diff --git a/coderd/provisionerjobs_internal_test.go b/coderd/provisionerjobs_internal_test.go
@@ -3,6 +3,7 @@ package coderd
 import (
 	"context"
 	"crypto/sha256"
+	"database/sql"
 	"encoding/json"
 	"net/http/httptest"
 	"net/url"
@@ -146,6 +147,109 @@ func TestProvisionerJobLogs_Unit(t *testing.T) {
 	})
 }
 
+func TestConvertProvisionerJob_Unit(t *testing.T) {
+	t.Parallel()
+	validNullTimeMock := sql.NullTime{
+		Time:  database.Now(),
+		Valid: true,
+	}
+	invalidNullTimeMock := sql.NullTime{}
+	errorMock := sql.NullString{
+		String: "error",
+		Valid:  true,
+	}
+	testCases := []struct {
+		name     string
+		input    database.ProvisionerJob
+		expected codersdk.ProvisionerJob
+	}{
+		{
+			name:  "empty",
+			input: database.ProvisionerJob{},
+			expected: codersdk.ProvisionerJob{
+				Status: codersdk.ProvisionerJobPending,
+			},
+		},
+		{
+			name: "cancellation pending",
+			input: database.ProvisionerJob{
+				CanceledAt:  validNullTimeMock,
+				CompletedAt: invalidNullTimeMock,
+			},
+			expected: codersdk.ProvisionerJob{
+				Status: codersdk.ProvisionerJobCanceling,
+			},
+		},
+		{
+			name: "cancellation failed",
+			input: database.ProvisionerJob{
+				CanceledAt:  validNullTimeMock,
+				CompletedAt: validNullTimeMock,
+				Error:       errorMock,
+			},
+			expected: codersdk.ProvisionerJob{
+				CompletedAt: &validNullTimeMock.Time,
+				Status:      codersdk.ProvisionerJobFailed,
+				Error:       errorMock.String,
+			},
+		},
+		{
+			name: "cancellation succeeded",
+			input: database.ProvisionerJob{
+				CanceledAt:  validNullTimeMock,
+				CompletedAt: validNullTimeMock,
+			},
+			expected: codersdk.ProvisionerJob{
+				CompletedAt: &validNullTimeMock.Time,
+				Status:      codersdk.ProvisionerJobCanceled,
+			},
+		},
+		{
+			name: "job pending",
+			input: database.ProvisionerJob{
+				StartedAt: invalidNullTimeMock,
+			},
+			expected: codersdk.ProvisionerJob{
+				Status: codersdk.ProvisionerJobPending,
+			},
+		},
+		{
+			name: "job failed",
+			input: database.ProvisionerJob{
+				CompletedAt: validNullTimeMock,
+				StartedAt:   validNullTimeMock,
+				Error:       errorMock,
+			},
+			expected: codersdk.ProvisionerJob{
+				CompletedAt: &validNullTimeMock.Time,
+				StartedAt:   &validNullTimeMock.Time,
+				Error:       errorMock.String,
+				Status:      codersdk.ProvisionerJobFailed,
+			},
+		},
+		{
+			name: "job succeeded",
+			input: database.ProvisionerJob{
+				CompletedAt: validNullTimeMock,
+				StartedAt:   validNullTimeMock,
+			},
+			expected: codersdk.ProvisionerJob{
+				CompletedAt: &validNullTimeMock.Time,
+				StartedAt:   &validNullTimeMock.Time,
+				Status:      codersdk.ProvisionerJobSucceeded,
+			},
+		},
+	}
+	for _, testCase := range testCases {
+		testCase := testCase
+		t.Run(testCase.name, func(t *testing.T) {
+			t.Parallel()
+			actual := convertProvisionerJob(testCase.input)
+			assert.Equal(t, testCase.expected, actual)
+		})
+	}
+}
+
 type fakePubSub struct {
 	t        *testing.T
 	cond     *sync.Cond
diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go
@@ -125,6 +125,11 @@ func TestPatchCancelTemplateVersion(t *testing.T) {
 		var apiErr *codersdk.Error
 		require.ErrorAs(t, err, &apiErr)
 		require.Equal(t, http.StatusPreconditionFailed, apiErr.StatusCode())
+		require.Eventually(t, func() bool {
+			var err error
+			version, err = client.TemplateVersion(context.Background(), version.ID)
+			return assert.NoError(t, err) && version.Job.Status == codersdk.ProvisionerJobFailed
+		}, 5*time.Second, 25*time.Millisecond)
 	})
 	// TODO(Cian): until we are able to test cancellation properly, validating
 	// Running -> Canceling is the best we can do for now.
