fix(cli): handle version mismatch re MatchedProvisioners response (#1…

…5682) * Modifies `MatchedProvisioners` response of `codersdk.TemplateVersion` to be a pointer * CLI now checks for absence of `*MatchedProvisioners` before showing warning regarding provisioners * Extracts logic for warning about provisioners to a function * Improves test coverage for CLI template push with `coder_workspace_tags`. (cherry picked from commit 3014713)
coder · stirby · Dec 16, 2024 · Dec 10, 2024 · Dec 11, 2024 · Nov 28, 2024
commit 6da176031284bb24947a9e721dbfb4ba57fc364a
diff --git a/cli/templatepush.go b/cli/templatepush.go
@@ -416,30 +416,7 @@ func createValidTemplateVersion(inv *serpent.Invocation, args createValidTemplat
 	if err != nil {
 		return nil, err
 	}
-	var tagsJSON strings.Builder
-	if err := json.NewEncoder(&tagsJSON).Encode(version.Job.Tags); err != nil {
-		// Fall back to the less-pretty string representation.
-		tagsJSON.Reset()
-		_, _ = tagsJSON.WriteString(fmt.Sprintf("%v", version.Job.Tags))
-	}
-	if version.MatchedProvisioners.Count == 0 {
-		cliui.Warnf(inv.Stderr, `No provisioners are available to handle the job!
-Please contact your deployment administrator for assistance.
-Details:
-	Provisioner job ID : %s
-	Requested tags     : %s
-`, version.Job.ID, tagsJSON.String())
-	} else if version.MatchedProvisioners.Available == 0 {
-		cliui.Warnf(inv.Stderr, `All available provisioner daemons have been silent for a while.
-Your build will proceed once they become available.
-If this persists, please contact your deployment administrator for assistance.
-Details:
-	Provisioner job ID : %s
-	Requested tags     : %s
-	Most recently seen : %s
-`, version.Job.ID, strings.TrimSpace(tagsJSON.String()), version.MatchedProvisioners.MostRecentlySeen.Time)
-	}
-
+	WarnMatchedProvisioners(inv, version)
 	err = cliui.ProvisionerJob(inv.Context(), inv.Stdout, cliui.ProvisionerJobOptions{
 		Fetch: func() (codersdk.ProvisionerJob, error) {
 			version, err := client.TemplateVersion(inv.Context(), version.ID)
@@ -505,6 +482,41 @@ func ParseProvisionerTags(rawTags []string) (map[string]string, error) {
 	return tags, nil
 }
 
+var (
+	warnNoMatchedProvisioners = `Your build has been enqueued, but there are no provisioners that accept the required tags. Once a compatible provisioner becomes available, your build will continue. Please contact your administrator.
+Details:
+	Provisioner job ID : %s
+	Requested tags     : %s
+`
+	warnNoAvailableProvisioners = `Provisioners that accept the required tags have not responded for longer than expected. This may delay your build. Please contact your administrator if your build does not complete.
+Details:
+	Provisioner job ID : %s
+	Requested tags     : %s
+	Most recently seen : %s
+`
+)
+
+func WarnMatchedProvisioners(inv *serpent.Invocation, tv codersdk.TemplateVersion) {
+	if tv.MatchedProvisioners == nil {
+		// Nothing in the response, nothing to do here!
+		return
+	}
+	var tagsJSON strings.Builder
+	if err := json.NewEncoder(&tagsJSON).Encode(tv.Job.Tags); err != nil {
+		// Fall back to the less-pretty string representation.
+		tagsJSON.Reset()
+		_, _ = tagsJSON.WriteString(fmt.Sprintf("%v", tv.Job.Tags))
+	}
+	if tv.MatchedProvisioners.Count == 0 {
+		cliui.Warnf(inv.Stderr, warnNoMatchedProvisioners, tv.Job.ID, tagsJSON.String())
+		return
+	}
+	if tv.MatchedProvisioners.Available == 0 {
+		cliui.Warnf(inv.Stderr, warnNoAvailableProvisioners, tv.Job.ID, strings.TrimSpace(tagsJSON.String()), tv.MatchedProvisioners.MostRecentlySeen.Time)
+		return
+	}
+}
+
 // prettyDirectoryPath returns a prettified path when inside the users
 // home directory. Falls back to dir if the users home directory cannot
 // discerned. This function calls filepath.Clean on the result.

diff --git a/cli/templatepush_test.go b/cli/templatepush_test.go
@@ -3,6 +3,7 @@ package cli_test
 import (
 	"bytes"
 	"context"
+	"database/sql"
 	"os"
 	"path/filepath"
 	"runtime"
@@ -18,6 +19,7 @@ import (
 	"github.com/coder/coder/v2/coderd/coderdtest"
 	"github.com/coder/coder/v2/coderd/database"
 	"github.com/coder/coder/v2/coderd/database/dbtestutil"
+	"github.com/coder/coder/v2/coderd/database/dbtime"
 	"github.com/coder/coder/v2/coderd/rbac"
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/provisioner/echo"
@@ -412,84 +414,162 @@ func TestTemplatePush(t *testing.T) {
 
 		t.Run("WorkspaceTagsTerraform", func(t *testing.T) {
 			t.Parallel()
-			ctx := testutil.Context(t, testutil.WaitShort)
 
-			// Start an instance **without** a built-in provisioner.
-			// We're not actually testing that the Terraform applies.
-			// What we test is that a provisioner job is created with the expected
-			// tags based on the __content__ of the Terraform.
-			store, ps := dbtestutil.NewDB(t)
-			client := coderdtest.New(t, &coderdtest.Options{
-				Database: store,
-				Pubsub:   ps,
-			})
-
-			owner := coderdtest.CreateFirstUser(t, client)
-			templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin())
-
-			// Create a tar file with some pre-defined content
-			tarFile := testutil.CreateTar(t, map[string]string{
-				"main.tf": `
-variable "a" {
-	type = string
-	default = "1"
-}
-data "coder_parameter" "b" {
-	type = string
-	default = "2"
-}
-resource "null_resource" "test" {}
-data "coder_workspace_tags" "tags" {
-	tags = {
-		"foo": "bar",
-		"a": var.a,
-		"b": data.coder_parameter.b.value,
-	}
-}`,
-			})
-
-			// Write the tar file to disk.
-			tempDir := t.TempDir()
-			err := tfparse.WriteArchive(tarFile, "application/x-tar", tempDir)
-			require.NoError(t, err)
-
-			// Run `coder templates push`
-			templateName := strings.ReplaceAll(testutil.GetRandomName(t), "_", "-")
-			var stdout, stderr strings.Builder
-			inv, root := clitest.New(t, "templates", "push", templateName, "-d", tempDir, "--yes")
-			inv.Stdout = &stdout
-			inv.Stderr = &stderr
-			clitest.SetupConfig(t, templateAdmin, root)
-
-			// Don't forget to clean up!
-			cancelCtx, cancel := context.WithCancel(ctx)
-			t.Cleanup(cancel)
-			done := make(chan error)
-			go func() {
-				done <- inv.WithContext(cancelCtx).Run()
-			}()
-
-			// Assert that a provisioner job was created with the desired tags.
-			wantTags := database.StringMap(provisionersdk.MutateTags(uuid.Nil, map[string]string{
-				"foo": "bar",
-				"a":   "1",
-				"b":   "2",
-			}))
-			require.Eventually(t, func() bool {
-				jobs, err := store.GetProvisionerJobsCreatedAfter(ctx, time.Time{})
-				if !assert.NoError(t, err) {
-					return false
-				}
-				if len(jobs) == 0 {
-					return false
-				}
-				return assert.EqualValues(t, wantTags, jobs[0].Tags)
-			}, testutil.WaitShort, testutil.IntervalSlow)
-
-			cancel()
-			<-done
+			tests := []struct {
+				name         string
+				setupDaemon  func(ctx context.Context, store database.Store, owner codersdk.CreateFirstUserResponse, tags database.StringMap, now time.Time) error
+				expectOutput string
+			}{
+				{
+					name: "no provisioners available",
+					setupDaemon: func(_ context.Context, _ database.Store, _ codersdk.CreateFirstUserResponse, _ database.StringMap, _ time.Time) error {
+						return nil
+					},
+					expectOutput: "there are no provisioners that accept the required tags",
+				},
+				{
+					name: "provisioner stale",
+					setupDaemon: func(ctx context.Context, store database.Store, owner codersdk.CreateFirstUserResponse, tags database.StringMap, now time.Time) error {
+						pk, err := store.InsertProvisionerKey(ctx, database.InsertProvisionerKeyParams{
+							ID:             uuid.New(),
+							CreatedAt:      now,
+							OrganizationID: owner.OrganizationID,
+							Name:           "test",
+							Tags:           tags,
+							HashedSecret:   []byte("secret"),
+						})
+						if err != nil {
+							return err
+						}
+						oneHourAgo := now.Add(-time.Hour)
+						_, err = store.UpsertProvisionerDaemon(ctx, database.UpsertProvisionerDaemonParams{
+							Provisioners:   []database.ProvisionerType{database.ProvisionerTypeTerraform},
+							LastSeenAt:     sql.NullTime{Time: oneHourAgo, Valid: true},
+							CreatedAt:      oneHourAgo,
+							Name:           "test",
+							Tags:           tags,
+							OrganizationID: owner.OrganizationID,
+							KeyID:          pk.ID,
+						})
+						return err
+					},
+					expectOutput: "Provisioners that accept the required tags have not responded for longer than expected",
+				},
+				{
+					name: "active provisioner",
+					setupDaemon: func(ctx context.Context, store database.Store, owner codersdk.CreateFirstUserResponse, tags database.StringMap, now time.Time) error {
+						pk, err := store.InsertProvisionerKey(ctx, database.InsertProvisionerKeyParams{
+							ID:             uuid.New(),
+							CreatedAt:      now,
+							OrganizationID: owner.OrganizationID,
+							Name:           "test",
+							Tags:           tags,
+							HashedSecret:   []byte("secret"),
+						})
+						if err != nil {
+							return err
+						}
+						_, err = store.UpsertProvisionerDaemon(ctx, database.UpsertProvisionerDaemonParams{
+							Provisioners:   []database.ProvisionerType{database.ProvisionerTypeTerraform},
+							LastSeenAt:     sql.NullTime{Time: now, Valid: true},
+							CreatedAt:      now,
+							Name:           "test-active",
+							Tags:           tags,
+							OrganizationID: owner.OrganizationID,
+							KeyID:          pk.ID,
+						})
+						return err
+					},
+					expectOutput: "",
+				},
+			}
 
-			require.Contains(t, stderr.String(), "No provisioners are available to handle the job!")
+			for _, tt := range tests {
+				tt := tt
+				t.Run(tt.name, func(t *testing.T) {
+					t.Parallel()
+
+					// Start an instance **without** a built-in provisioner.
+					// We're not actually testing that the Terraform applies.
+					// What we test is that a provisioner job is created with the expected
+					// tags based on the __content__ of the Terraform.
+					store, ps := dbtestutil.NewDB(t)
+					client := coderdtest.New(t, &coderdtest.Options{
+						Database: store,
+						Pubsub:   ps,
+					})
+
+					owner := coderdtest.CreateFirstUser(t, client)
+					templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin())
+
+					// Create a tar file with some pre-defined content
+					tarFile := testutil.CreateTar(t, map[string]string{
+						"main.tf": `
+							variable "a" {
+								type = string
+								default = "1"
+							}
+							data "coder_parameter" "b" {
+								type = string
+								default = "2"
+							}
+							resource "null_resource" "test" {}
+							data "coder_workspace_tags" "tags" {
+								tags = {
+									"a": var.a,
+									"b": data.coder_parameter.b.value,
+									"test_name": "` + tt.name + `"
+								}
+							}`,
+					})
+
+					// Write the tar file to disk.
+					tempDir := t.TempDir()
+					err := tfparse.WriteArchive(tarFile, "application/x-tar", tempDir)
+					require.NoError(t, err)
+
+					wantTags := database.StringMap(provisionersdk.MutateTags(uuid.Nil, map[string]string{
+						"a":         "1",
+						"b":         "2",
+						"test_name": tt.name,
+					}))
+
+					templateName := strings.ReplaceAll(testutil.GetRandomName(t), "_", "-")
+
+					inv, root := clitest.New(t, "templates", "push", templateName, "-d", tempDir, "--yes")
+					clitest.SetupConfig(t, templateAdmin, root)
+					pty := ptytest.New(t).Attach(inv)
+
+					ctx := testutil.Context(t, testutil.WaitShort)
+					now := dbtime.Now()
+					require.NoError(t, tt.setupDaemon(ctx, store, owner, wantTags, now))
+
+					cancelCtx, cancel := context.WithCancel(ctx)
+					t.Cleanup(cancel)
+					done := make(chan error)
+					go func() {
+						done <- inv.WithContext(cancelCtx).Run()
+					}()
+
+					require.Eventually(t, func() bool {
+						jobs, err := store.GetProvisionerJobsCreatedAfter(ctx, time.Time{})
+						if !assert.NoError(t, err) {
+							return false
+						}
+						if len(jobs) == 0 {
+							return false
+						}
+						return assert.EqualValues(t, wantTags, jobs[0].Tags)
+					}, testutil.WaitShort, testutil.IntervalFast)
+
+					if tt.expectOutput != "" {
+						pty.ExpectMatch(tt.expectOutput)
+					}
+
+					cancel()
+					<-done
+				})
+			}
 		})
 
 		t.Run("ChangeTags", func(t *testing.T) {