Skip to content

Commit c525891

Browse files
johnstcnjohnstcn
committed
fix(cli): handle version mismatch re MatchedProvisioners response (#15682)
* 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)
1 parent f1d5f5d commit c525891

6 files changed

+206
-114
lines changed

‎cli/templatepush.go

+36-24
Original file line numberDiff line numberDiff line change
@@ -416,30 +416,7 @@ func createValidTemplateVersion(inv *serpent.Invocation, args createValidTemplat
416416
if err != nil {
417417
return nil, err
418418
}
419-
var tagsJSON strings.Builder
420-
if err := json.NewEncoder(&tagsJSON).Encode(version.Job.Tags); err != nil {
421-
// Fall back to the less-pretty string representation.
422-
tagsJSON.Reset()
423-
_, _ = tagsJSON.WriteString(fmt.Sprintf("%v", version.Job.Tags))
424-
}
425-
if version.MatchedProvisioners.Count == 0 {
426-
cliui.Warnf(inv.Stderr, `No provisioners are available to handle the job!
427-
Please contact your deployment administrator for assistance.
428-
Details:
429-
Provisioner job ID : %s
430-
Requested tags : %s
431-
`, version.Job.ID, tagsJSON.String())
432-
} else if version.MatchedProvisioners.Available == 0 {
433-
cliui.Warnf(inv.Stderr, `All available provisioner daemons have been silent for a while.
434-
Your build will proceed once they become available.
435-
If this persists, please contact your deployment administrator for assistance.
436-
Details:
437-
Provisioner job ID : %s
438-
Requested tags : %s
439-
Most recently seen : %s
440-
`, version.Job.ID, strings.TrimSpace(tagsJSON.String()), version.MatchedProvisioners.MostRecentlySeen.Time)
441-
}
442-
419+
WarnMatchedProvisioners(inv, version)
443420
err = cliui.ProvisionerJob(inv.Context(), inv.Stdout, cliui.ProvisionerJobOptions{
444421
Fetch: func() (codersdk.ProvisionerJob, error) {
445422
version, err := client.TemplateVersion(inv.Context(), version.ID)
@@ -505,6 +482,41 @@ func ParseProvisionerTags(rawTags []string) (map[string]string, error) {
505482
return tags, nil
506483
}
507484

485+
var (
486+
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.
487+
Details:
488+
Provisioner job ID : %s
489+
Requested tags : %s
490+
`
491+
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.
492+
Details:
493+
Provisioner job ID : %s
494+
Requested tags : %s
495+
Most recently seen : %s
496+
`
497+
)
498+
499+
func WarnMatchedProvisioners(inv *serpent.Invocation, tv codersdk.TemplateVersion) {
500+
if tv.MatchedProvisioners == nil {
501+
// Nothing in the response, nothing to do here!
502+
return
503+
}
504+
var tagsJSON strings.Builder
505+
if err := json.NewEncoder(&tagsJSON).Encode(tv.Job.Tags); err != nil {
506+
// Fall back to the less-pretty string representation.
507+
tagsJSON.Reset()
508+
_, _ = tagsJSON.WriteString(fmt.Sprintf("%v", tv.Job.Tags))
509+
}
510+
if tv.MatchedProvisioners.Count == 0 {
511+
cliui.Warnf(inv.Stderr, warnNoMatchedProvisioners, tv.Job.ID, tagsJSON.String())
512+
return
513+
}
514+
if tv.MatchedProvisioners.Available == 0 {
515+
cliui.Warnf(inv.Stderr, warnNoAvailableProvisioners, tv.Job.ID, strings.TrimSpace(tagsJSON.String()), tv.MatchedProvisioners.MostRecentlySeen.Time)
516+
return
517+
}
518+
}
519+
508520
// prettyDirectoryPath returns a prettified path when inside the users
509521
// home directory. Falls back to dir if the users home directory cannot
510522
// discerned. This function calls filepath.Clean on the result.

‎cli/templatepush_test.go

+156-76
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package cli_test
33
import (
44
"bytes"
55
"context"
6+
"database/sql"
67
"os"
78
"path/filepath"
89
"runtime"
@@ -18,6 +19,7 @@ import (
1819
"github.com/coder/coder/v2/coderd/coderdtest"
1920
"github.com/coder/coder/v2/coderd/database"
2021
"github.com/coder/coder/v2/coderd/database/dbtestutil"
22+
"github.com/coder/coder/v2/coderd/database/dbtime"
2123
"github.com/coder/coder/v2/coderd/rbac"
2224
"github.com/coder/coder/v2/codersdk"
2325
"github.com/coder/coder/v2/provisioner/echo"
@@ -412,84 +414,162 @@ func TestTemplatePush(t *testing.T) {
412414

413415
t.Run("WorkspaceTagsTerraform", func(t *testing.T) {
414416
t.Parallel()
415-
ctx := testutil.Context(t, testutil.WaitShort)
416417

417-
// Start an instance **without** a built-in provisioner.
418-
// We're not actually testing that the Terraform applies.
419-
// What we test is that a provisioner job is created with the expected
420-
// tags based on the __content__ of the Terraform.
421-
store, ps := dbtestutil.NewDB(t)
422-
client := coderdtest.New(t, &coderdtest.Options{
423-
Database: store,
424-
Pubsub: ps,
425-
})
426-
427-
owner := coderdtest.CreateFirstUser(t, client)
428-
templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin())
429-
430-
// Create a tar file with some pre-defined content
431-
tarFile := testutil.CreateTar(t, map[string]string{
432-
"main.tf": `
433-
variable "a" {
434-
type = string
435-
default = "1"
436-
}
437-
data "coder_parameter" "b" {
438-
type = string
439-
default = "2"
440-
}
441-
resource "null_resource" "test" {}
442-
data "coder_workspace_tags" "tags" {
443-
tags = {
444-
"foo": "bar",
445-
"a": var.a,
446-
"b": data.coder_parameter.b.value,
447-
}
448-
}`,
449-
})
450-
451-
// Write the tar file to disk.
452-
tempDir := t.TempDir()
453-
err := tfparse.WriteArchive(tarFile, "application/x-tar", tempDir)
454-
require.NoError(t, err)
455-
456-
// Run `coder templates push`
457-
templateName := strings.ReplaceAll(testutil.GetRandomName(t), "_", "-")
458-
var stdout, stderr strings.Builder
459-
inv, root := clitest.New(t, "templates", "push", templateName, "-d", tempDir, "--yes")
460-
inv.Stdout = &stdout
461-
inv.Stderr = &stderr
462-
clitest.SetupConfig(t, templateAdmin, root)
463-
464-
// Don't forget to clean up!
465-
cancelCtx, cancel := context.WithCancel(ctx)
466-
t.Cleanup(cancel)
467-
done := make(chan error)
468-
go func() {
469-
done <- inv.WithContext(cancelCtx).Run()
470-
}()
471-
472-
// Assert that a provisioner job was created with the desired tags.
473-
wantTags := database.StringMap(provisionersdk.MutateTags(uuid.Nil, map[string]string{
474-
"foo": "bar",
475-
"a": "1",
476-
"b": "2",
477-
}))
478-
require.Eventually(t, func() bool {
479-
jobs, err := store.GetProvisionerJobsCreatedAfter(ctx, time.Time{})
480-
if !assert.NoError(t, err) {
481-
return false
482-
}
483-
if len(jobs) == 0 {
484-
return false
485-
}
486-
return assert.EqualValues(t, wantTags, jobs[0].Tags)
487-
}, testutil.WaitShort, testutil.IntervalSlow)
488-
489-
cancel()
490-
<-done
418+
tests := []struct {
419+
name string
420+
setupDaemon func(ctx context.Context, store database.Store, owner codersdk.CreateFirstUserResponse, tags database.StringMap, now time.Time) error
421+
expectOutput string
422+
}{
423+
{
424+
name: "no provisioners available",
425+
setupDaemon: func(_ context.Context, _ database.Store, _ codersdk.CreateFirstUserResponse, _ database.StringMap, _ time.Time) error {
426+
return nil
427+
},
428+
expectOutput: "there are no provisioners that accept the required tags",
429+
},
430+
{
431+
name: "provisioner stale",
432+
setupDaemon: func(ctx context.Context, store database.Store, owner codersdk.CreateFirstUserResponse, tags database.StringMap, now time.Time) error {
433+
pk, err := store.InsertProvisionerKey(ctx, database.InsertProvisionerKeyParams{
434+
ID: uuid.New(),
435+
CreatedAt: now,
436+
OrganizationID: owner.OrganizationID,
437+
Name: "test",
438+
Tags: tags,
439+
HashedSecret: []byte("secret"),
440+
})
441+
if err != nil {
442+
return err
443+
}
444+
oneHourAgo := now.Add(-time.Hour)
445+
_, err = store.UpsertProvisionerDaemon(ctx, database.UpsertProvisionerDaemonParams{
446+
Provisioners: []database.ProvisionerType{database.ProvisionerTypeTerraform},
447+
LastSeenAt: sql.NullTime{Time: oneHourAgo, Valid: true},
448+
CreatedAt: oneHourAgo,
449+
Name: "test",
450+
Tags: tags,
451+
OrganizationID: owner.OrganizationID,
452+
KeyID: pk.ID,
453+
})
454+
return err
455+
},
456+
expectOutput: "Provisioners that accept the required tags have not responded for longer than expected",
457+
},
458+
{
459+
name: "active provisioner",
460+
setupDaemon: func(ctx context.Context, store database.Store, owner codersdk.CreateFirstUserResponse, tags database.StringMap, now time.Time) error {
461+
pk, err := store.InsertProvisionerKey(ctx, database.InsertProvisionerKeyParams{
462+
ID: uuid.New(),
463+
CreatedAt: now,
464+
OrganizationID: owner.OrganizationID,
465+
Name: "test",
466+
Tags: tags,
467+
HashedSecret: []byte("secret"),
468+
})
469+
if err != nil {
470+
return err
471+
}
472+
_, err = store.UpsertProvisionerDaemon(ctx, database.UpsertProvisionerDaemonParams{
473+
Provisioners: []database.ProvisionerType{database.ProvisionerTypeTerraform},
474+
LastSeenAt: sql.NullTime{Time: now, Valid: true},
475+
CreatedAt: now,
476+
Name: "test-active",
477+
Tags: tags,
478+
OrganizationID: owner.OrganizationID,
479+
KeyID: pk.ID,
480+
})
481+
return err
482+
},
483+
expectOutput: "",
484+
},
485+
}
491486

492-
require.Contains(t, stderr.String(), "No provisioners are available to handle the job!")
487+
for _, tt := range tests {
488+
tt := tt
489+
t.Run(tt.name, func(t *testing.T) {
490+
t.Parallel()
491+
492+
// Start an instance **without** a built-in provisioner.
493+
// We're not actually testing that the Terraform applies.
494+
// What we test is that a provisioner job is created with the expected
495+
// tags based on the __content__ of the Terraform.
496+
store, ps := dbtestutil.NewDB(t)
497+
client := coderdtest.New(t, &coderdtest.Options{
498+
Database: store,
499+
Pubsub: ps,
500+
})
501+
502+
owner := coderdtest.CreateFirstUser(t, client)
503+
templateAdmin, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID, rbac.RoleTemplateAdmin())
504+
505+
// Create a tar file with some pre-defined content
506+
tarFile := testutil.CreateTar(t, map[string]string{
507+
"main.tf": `
508+
variable "a" {
509+
type = string
510+
default = "1"
511+
}
512+
data "coder_parameter" "b" {
513+
type = string
514+
default = "2"
515+
}
516+
resource "null_resource" "test" {}
517+
data "coder_workspace_tags" "tags" {
518+
tags = {
519+
"a": var.a,
520+
"b": data.coder_parameter.b.value,
521+
"test_name": "` + tt.name + `"
522+
}
523+
}`,
524+
})
525+
526+
// Write the tar file to disk.
527+
tempDir := t.TempDir()
528+
err := tfparse.WriteArchive(tarFile, "application/x-tar", tempDir)
529+
require.NoError(t, err)
530+
531+
wantTags := database.StringMap(provisionersdk.MutateTags(uuid.Nil, map[string]string{
532+
"a": "1",
533+
"b": "2",
534+
"test_name": tt.name,
535+
}))
536+
537+
templateName := strings.ReplaceAll(testutil.GetRandomName(t), "_", "-")
538+
539+
inv, root := clitest.New(t, "templates", "push", templateName, "-d", tempDir, "--yes")
540+
clitest.SetupConfig(t, templateAdmin, root)
541+
pty := ptytest.New(t).Attach(inv)
542+
543+
ctx := testutil.Context(t, testutil.WaitShort)
544+
now := dbtime.Now()
545+
require.NoError(t, tt.setupDaemon(ctx, store, owner, wantTags, now))
546+
547+
cancelCtx, cancel := context.WithCancel(ctx)
548+
t.Cleanup(cancel)
549+
done := make(chan error)
550+
go func() {
551+
done <- inv.WithContext(cancelCtx).Run()
552+
}()
553+
554+
require.Eventually(t, func() bool {
555+
jobs, err := store.GetProvisionerJobsCreatedAfter(ctx, time.Time{})
556+
if !assert.NoError(t, err) {
557+
return false
558+
}
559+
if len(jobs) == 0 {
560+
return false
561+
}
562+
return assert.EqualValues(t, wantTags, jobs[0].Tags)
563+
}, testutil.WaitShort, testutil.IntervalFast)
564+
565+
if tt.expectOutput != "" {
566+
pty.ExpectMatch(tt.expectOutput)
567+
}
568+
569+
cancel()
570+
<-done
571+
})
572+
}
493573
})
494574

495575
t.Run("ChangeTags", func(t *testing.T) {

0 commit comments

Comments
 (0)