coder · aslilac · Oct 5, 2023 · Oct 2, 2023 · Oct 2, 2023 · Oct 2, 2023
diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go
@@ -760,21 +760,53 @@ func UpdateTemplateVersion(t *testing.T, client *codersdk.Client, organizationID
 	return templateVersion
 }
 
-// AwaitTemplateVersionJobCompleted awaits for an import job to reach completed status.
+// AwaitTemplateVersionJobRunning waits for the build to be picked up by a provisioner.
+func AwaitTemplateVersionJobRunning(t *testing.T, client *codersdk.Client, version uuid.UUID) codersdk.TemplateVersion {
+	t.Helper()
+
+	ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
+	defer cancel()
+
+	t.Logf("waiting for template version %s build job to start", version)
+	var templateVersion codersdk.TemplateVersion
+	require.Eventually(t, func() bool {
+		var err error
+		templateVersion, err = client.TemplateVersion(ctx, version)
+		if err != nil {
+			return false
+		}
+		t.Logf("template version job status: %s", templateVersion.Job.Status)
+		switch templateVersion.Job.Status {
+		case codersdk.ProvisionerJobPending:
+			return false
+		case codersdk.ProvisionerJobRunning:
+			return true
+		default:
+			t.FailNow()
+			return false
+		}
+	}, testutil.WaitShort, testutil.IntervalFast, "make sure you set `IncludeProvisionerDaemon`!")
+	t.Logf("template version %s job has started", version)
+	return templateVersion
+}
+
+// AwaitTemplateVersionJobCompleted waits for the build to be completed. This may result
+// from cancelation, an error, or from completing successfully.
 func AwaitTemplateVersionJobCompleted(t *testing.T, client *codersdk.Client, version uuid.UUID) codersdk.TemplateVersion {
 	t.Helper()
 
 	ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
 	defer cancel()
 
-	t.Logf("waiting for template version job %s", version)
+	t.Logf("waiting for template version %s build job to complete", version)
 	var templateVersion codersdk.TemplateVersion
 	require.Eventually(t, func() bool {
 		var err error
 		templateVersion, err = client.TemplateVersion(ctx, version)
+		t.Logf("template version job status: %s", templateVersion.Job.Status)
 		return assert.NoError(t, err) && templateVersion.Job.CompletedAt != nil
-	}, testutil.WaitLong, testutil.IntervalMedium)
-	t.Logf("got template version job %s", version)
+	}, testutil.WaitLong, testutil.IntervalMedium, "make sure you set `IncludeProvisionerDaemon`!")
+	t.Logf("template version %s job has completed", version)
 	return templateVersion
 }
 

diff --git a/coderd/insights_test.go b/coderd/insights_test.go
@@ -2110,7 +2110,7 @@ func TestTemplateInsights_RBAC(t *testing.T) {
 			t.Run("AsOwner", func(t *testing.T) {
 				t.Parallel()
 
-				client := coderdtest.New(t, &coderdtest.Options{})
+				client := coderdtest.New(t, nil)
 				admin := coderdtest.CreateFirstUser(t, client)
 
 				ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
@@ -2134,7 +2134,7 @@ func TestTemplateInsights_RBAC(t *testing.T) {
 			t.Run("AsTemplateAdmin", func(t *testing.T) {
 				t.Parallel()
 
-				client := coderdtest.New(t, &coderdtest.Options{})
+				client := coderdtest.New(t, nil)
 				admin := coderdtest.CreateFirstUser(t, client)
 
 				templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID, rbac.RoleTemplateAdmin())
@@ -2160,7 +2160,7 @@ func TestTemplateInsights_RBAC(t *testing.T) {
 			t.Run("AsRegularUser", func(t *testing.T) {
 				t.Parallel()
 
-				client := coderdtest.New(t, &coderdtest.Options{})
+				client := coderdtest.New(t, nil)
 				admin := coderdtest.CreateFirstUser(t, client)
 
 				regular, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)
@@ -2239,7 +2239,7 @@ func TestGenericInsights_RBAC(t *testing.T) {
 				t.Run("AsOwner", func(t *testing.T) {
 					t.Parallel()
 
-					client := coderdtest.New(t, &coderdtest.Options{})
+					client := coderdtest.New(t, nil)
 					admin := coderdtest.CreateFirstUser(t, client)
 
 					ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
@@ -2261,7 +2261,7 @@ func TestGenericInsights_RBAC(t *testing.T) {
 				t.Run("AsTemplateAdmin", func(t *testing.T) {
 					t.Parallel()
 
-					client := coderdtest.New(t, &coderdtest.Options{})
+					client := coderdtest.New(t, nil)
 					admin := coderdtest.CreateFirstUser(t, client)
 
 					templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID, rbac.RoleTemplateAdmin())
@@ -2285,7 +2285,7 @@ func TestGenericInsights_RBAC(t *testing.T) {
 				t.Run("AsRegularUser", func(t *testing.T) {
 					t.Parallel()
 
-					client := coderdtest.New(t, &coderdtest.Options{})
+					client := coderdtest.New(t, nil)
 					admin := coderdtest.CreateFirstUser(t, client)
 
 					regular, _ := coderdtest.CreateAnotherUser(t, client, admin.OrganizationID)

diff --git a/coderd/templateversions.go b/coderd/templateversions.go
@@ -20,6 +20,7 @@ import (
 
 	"github.com/coder/coder/v2/coderd/audit"
 	"github.com/coder/coder/v2/coderd/database"
+	"github.com/coder/coder/v2/coderd/database/db2sdk"
 	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/coder/v2/coderd/database/provisionerjobs"
 	"github.com/coder/coder/v2/coderd/externalauth"
@@ -248,7 +249,7 @@ func (api *API) templateVersionRichParameters(rw http.ResponseWriter, r *http.Re
 		return
 	}
 	if !job.CompletedAt.Valid {
-		httpapi.Write(ctx, rw, http.StatusPreconditionFailed, codersdk.Response{
+		httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
 			Message: "Job hasn't completed!",
 		})
 		return
@@ -383,7 +384,7 @@ func (api *API) templateVersionVariables(rw http.ResponseWriter, r *http.Request
 		return
 	}
 	if !job.CompletedAt.Valid {
-		httpapi.Write(ctx, rw, http.StatusPreconditionFailed, codersdk.Response{
+		httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
 			Message: "Job hasn't completed!",
 		})
 		return
@@ -1040,6 +1041,22 @@ func (api *API) patchActiveTemplateVersion(rw http.ResponseWriter, r *http.Reque
 		})
 		return
 	}
+	job, err := api.Database.GetProvisionerJobByID(ctx, version.JobID)
+	if err != nil {
+		httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
+			Message: "Internal error fetching template version job status.",
+			Detail:  err.Error(),
+		})
+		return
+	}
+	jobStatus := db2sdk.ProvisionerJobStatus(job)
+	if jobStatus != codersdk.ProvisionerJobSucceeded {
+		httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{
+			Message: "Only versions that have been built successfully can be promoted.",
+			Detail:  fmt.Sprintf("Attempted to promote a version with a %s build", jobStatus),
+		})
+		return
+	}
 
 	err = api.Database.InTx(func(store database.Store) error {
 		err = store.UpdateTemplateActiveVersionByID(ctx, database.UpdateTemplateActiveVersionByIDParams{

diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go
@@ -241,7 +241,9 @@ func TestPatchCancelTemplateVersion(t *testing.T) {
 	})
 	t.Run("AlreadyCanceled", func(t *testing.T) {
 		t.Parallel()
-		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
+		client := coderdtest.New(t, &coderdtest.Options{
+			IncludeProvisionerDaemon: true,
+		})
 		user := coderdtest.CreateFirstUser(t, client)
 		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
 			Parse: echo.ParseComplete,
@@ -255,15 +257,7 @@ func TestPatchCancelTemplateVersion(t *testing.T) {
 		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
 		defer cancel()
 
-		require.Eventually(t, func() bool {
-			var err error
-			version, err = client.TemplateVersion(ctx, version.ID)
-			if !assert.NoError(t, err) {
-				return false
-			}
-			t.Logf("Status: %s", version.Job.Status)
-			return version.Job.Status == codersdk.ProvisionerJobRunning
-		}, testutil.WaitShort, testutil.IntervalFast)
+		coderdtest.AwaitTemplateVersionJobRunning(t, client, version.ID)
 		err := client.CancelTemplateVersion(ctx, version.ID)
 		require.NoError(t, err)
 		err = client.CancelTemplateVersion(ctx, version.ID)
@@ -280,7 +274,9 @@ func TestPatchCancelTemplateVersion(t *testing.T) {
 	// Running -> Canceling is the best we can do for now.
 	t.Run("Canceling", func(t *testing.T) {
 		t.Parallel()
-		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
+		client := coderdtest.New(t, &coderdtest.Options{
+			IncludeProvisionerDaemon: true,
+		})
 		user := coderdtest.CreateFirstUser(t, client)
 		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, &echo.Responses{
 			Parse: echo.ParseComplete,
@@ -557,13 +553,60 @@ func TestPatchActiveTemplateVersion(t *testing.T) {
 		require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
 	})
 
-	t.Run("DoesNotBelong", func(t *testing.T) {
+	t.Run("CanceledBuild", func(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
 		user := coderdtest.CreateFirstUser(t, client)
 		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
 		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
+		version = coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, nil, template.ID)
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		err := client.CancelTemplateVersion(ctx, version.ID)
+		require.NoError(t, err)
+		coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
+		err = client.UpdateActiveTemplateVersion(ctx, template.ID, codersdk.UpdateActiveTemplateVersion{
+			ID: version.ID,
+		})
+		var apiErr *codersdk.Error
+		require.ErrorAs(t, err, &apiErr)
+		require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
+		require.Contains(t, apiErr.Detail, "canceled")
+	})
+
+	t.Run("PendingBuild", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, nil)
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
+		version = coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, nil, template.ID)
+
+		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
+		defer cancel()
+
+		err := client.UpdateActiveTemplateVersion(ctx, template.ID, codersdk.UpdateActiveTemplateVersion{
+			ID: version.ID,
+		})
+		var apiErr *codersdk.Error
+		require.ErrorAs(t, err, &apiErr)
+		require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
+		require.Contains(t, apiErr.Detail, "pending")
+	})
+
+	t.Run("DoesNotBelong", func(t *testing.T) {
+		t.Parallel()
+		client := coderdtest.New(t, &coderdtest.Options{
+			IncludeProvisionerDaemon: true,
+		})
+		user := coderdtest.CreateFirstUser(t, client)
+		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
 		version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
+		_ = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
+		_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
 
 		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
 		defer cancel()
@@ -576,13 +619,18 @@ func TestPatchActiveTemplateVersion(t *testing.T) {
 		require.Equal(t, http.StatusBadRequest, apiErr.StatusCode())
 	})
 
-	t.Run("Found", func(t *testing.T) {
+	t.Run("SuccessfulBuild", func(t *testing.T) {
 		t.Parallel()
 		auditor := audit.NewMock()
-		client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true, Auditor: auditor})
+		client := coderdtest.New(t, &coderdtest.Options{
+			IncludeProvisionerDaemon: true,
+			Auditor:                  auditor,
+		})
 		user := coderdtest.CreateFirstUser(t, client)
 		version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
 		template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
+		version = coderdtest.UpdateTemplateVersion(t, client, user.OrganizationID, nil, template.ID)
+		_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
 
 		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
 		defer cancel()
@@ -592,8 +640,8 @@ func TestPatchActiveTemplateVersion(t *testing.T) {
 		})
 		require.NoError(t, err)
 
-		require.Len(t, auditor.AuditLogs(), 5)
-		assert.Equal(t, database.AuditActionWrite, auditor.AuditLogs()[4].Action)
+		require.Len(t, auditor.AuditLogs(), 6)
+		assert.Equal(t, database.AuditActionWrite, auditor.AuditLogs()[5].Action)
 	})
 }
 

diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go
@@ -255,11 +255,11 @@ func TestWorkspaceAgent(t *testing.T) {
 				req.TemplateID = template.ID
 			})
 
+		coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
 		err = client.UpdateActiveTemplateVersion(ctx, template.ID, codersdk.UpdateActiveTemplateVersion{
 			ID: version.ID,
 		})
 		require.NoError(t, err)
-		coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
 		// Creating another workspace is just easier.
 		workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
 		build := coderdtest.AwaitWorkspaceBuildJobCompleted(t, client, workspace.LatestBuild.ID)

diff --git a/coderd/workspaces_test.go b/coderd/workspaces_test.go
@@ -428,7 +428,6 @@ func TestPostWorkspacesByOrganization(t *testing.T) {
 		t.Parallel()
 		client := coderdtest.New(t, nil)
 		first := coderdtest.CreateFirstUser(t, client)
-
 		other, _ := coderdtest.CreateAnotherUser(t, client, first.OrganizationID, rbac.RoleMember(), rbac.RoleOwner())
 
 		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)

@@ -32,6 +32,8 @@ export const VersionRow: React.FC<VersionRowProps> = ({
     onClick: () => navigate(version.name),
   });
 
+  const jobStatus = version.job.status;
+
   return (
     <TimelineEntry
       data-testid={`version-${version.id}`}
@@ -78,10 +80,20 @@ export const VersionRow: React.FC<VersionRowProps> = ({
           <Stack direction="row" alignItems="center" spacing={2}>
             {isActive && <Pill text="Active" type="success" />}
             {isLatest && <Pill text="Newest" type="info" />}
+            {jobStatus === "pending" && (
+              <Pill text={<>Pending&hellip;</>} type="warning" lightBorder />
+            )}
+            {jobStatus === "running" && (
+              <Pill text={<>Building&hellip;</>} type="warning" lightBorder />
+            )}
+            {(jobStatus === "canceling" || jobStatus === "canceled") && (
+              <Pill text="Canceled" type="neutral" lightBorder />
+            )}
+            {jobStatus === "failed" && <Pill text="Failed" type="error" />}
             {onPromoteClick && (
               <Button
                 className={styles.promoteButton}
-                disabled={isActive}
+                disabled={isActive || jobStatus !== "succeeded"}
                 onClick={(e) => {
                   e.preventDefault();
                   e.stopPropagation();