From a2529f76f2a39e2525ec6eb07eeaaf4f3f6d8cb3 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 28 Nov 2024 14:05:56 +0000 Subject: [PATCH 1/9] fix(cli): handle version mismatch re MatchedProvisioners response --- cli/templatepush.go | 61 ++++++++++++++++++++------------- coderd/templateversions.go | 2 +- coderd/templateversions_test.go | 9 +++-- codersdk/provisionerdaemons.go | 1 + codersdk/templateversions.go | 2 +- 5 files changed, 45 insertions(+), 30 deletions(-) diff --git a/cli/templatepush.go b/cli/templatepush.go index 8516d7f9c1310..fae3f2bbdf747 100644 --- a/cli/templatepush.go +++ b/cli/templatepush.go @@ -416,29 +416,6 @@ 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) - } err = cliui.ProvisionerJob(inv.Context(), inv.Stdout, cliui.ProvisionerJobOptions{ Fetch: func() (codersdk.ProvisionerJob, error) { @@ -505,6 +482,44 @@ func ParseProvisionerTags(rawTags []string) (map[string]string, error) { return tags, nil } +var ( + warnNoMatchedProvisioners = `No provisioners are available to handle the job! +Please contact your deployment administrator for assistance. +Details: + Provisioner job ID : %s + Requested tags : %s +` + warnNoAvailableProvisioners = `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 +` +) + +func WarnMatchedProvisioners(inv *serpent.Invocation, tv codersdk.TemplateVersion) { + 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 == nil { + // Nothing in the response, nothing to do here! + return + } + 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/coderd/templateversions.go b/coderd/templateversions.go index 3e42879171a45..7946bdd56fc5f 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -1719,7 +1719,7 @@ func convertTemplateVersion(version database.TemplateVersion, job codersdk.Provi }, Archived: version.Archived, Warnings: warnings, - MatchedProvisioners: matchedProvisioners, + MatchedProvisioners: &matchedProvisioners, } } diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 5e96de10d5058..2a13af0bc410a 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -471,14 +471,13 @@ func TestPostTemplateVersionsByOrganization(t *testing.T) { pj, err := store.GetProvisionerJobByID(ctx, tv.Job.ID) require.NoError(t, err) require.EqualValues(t, tt.wantTags, pj.Tags) + // Also assert that we get the expected information back from the API endpoint + require.Zero(t, tv.MatchedProvisioners.Count) + require.Zero(t, tv.MatchedProvisioners.Available) + require.Zero(t, tv.MatchedProvisioners.MostRecentlySeen.Time) } else { require.ErrorContains(t, err, tt.expectError) } - - // Also assert that we get the expected information back from the API endpoint - require.Zero(t, tv.MatchedProvisioners.Count) - require.Zero(t, tv.MatchedProvisioners.Available) - require.Zero(t, tv.MatchedProvisioners.MostRecentlySeen.Time) }) } }) diff --git a/codersdk/provisionerdaemons.go b/codersdk/provisionerdaemons.go index acd0c6955ab7f..27d2766a7cd13 100644 --- a/codersdk/provisionerdaemons.go +++ b/codersdk/provisionerdaemons.go @@ -53,6 +53,7 @@ type ProvisionerDaemon struct { // MatchedProvisioners represents the number of provisioner daemons // available to take a job at a specific point in time. +// Introduced in Coder version 2.18.0. type MatchedProvisioners struct { // Count is the number of provisioner daemons that matched the given // tags. If the count is 0, it means no provisioner daemons matched the diff --git a/codersdk/templateversions.go b/codersdk/templateversions.go index 5bda52daf3dfe..4f07f84074ec4 100644 --- a/codersdk/templateversions.go +++ b/codersdk/templateversions.go @@ -32,7 +32,7 @@ type TemplateVersion struct { Archived bool `json:"archived"` Warnings []TemplateVersionWarning `json:"warnings,omitempty" enums:"DEPRECATED_PARAMETERS"` - MatchedProvisioners MatchedProvisioners `json:"matched_provisioners,omitempty"` + MatchedProvisioners *MatchedProvisioners `json:"matched_provisioners,omitempty"` } type TemplateVersionExternalAuth struct { From a2664550ffb8826c1a478348c3d992bfe247d530 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Thu, 28 Nov 2024 20:17:48 +0000 Subject: [PATCH 2/9] add function --- cli/templatepush.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/templatepush.go b/cli/templatepush.go index fae3f2bbdf747..e7e251a21c248 100644 --- a/cli/templatepush.go +++ b/cli/templatepush.go @@ -416,7 +416,7 @@ func createValidTemplateVersion(inv *serpent.Invocation, args createValidTemplat if err != nil { return nil, err } - + 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) From f94059f40ad1f850e73291f88432236ee1191c5a Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 29 Nov 2024 09:43:16 +0000 Subject: [PATCH 3/9] address PR comments --- cli/templatepush.go | 17 ++-- cli/templatepush_test.go | 203 ++++++++++++++++++++++++--------------- 2 files changed, 134 insertions(+), 86 deletions(-) diff --git a/cli/templatepush.go b/cli/templatepush.go index e7e251a21c248..4f9fc9f7b9e74 100644 --- a/cli/templatepush.go +++ b/cli/templatepush.go @@ -483,33 +483,30 @@ func ParseProvisionerTags(rawTags []string) (map[string]string, error) { } var ( - warnNoMatchedProvisioners = `No provisioners are available to handle the job! -Please contact your deployment administrator for assistance. + 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 = `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. + 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 + Last response : %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 == nil { - // Nothing in the response, nothing to do here! - return - } if tv.MatchedProvisioners.Count == 0 { cliui.Warnf(inv.Stderr, warnNoMatchedProvisioners, tv.Job.ID, tagsJSON.String()) return diff --git a/cli/templatepush_test.go b/cli/templatepush_test.go index a20e3070740a8..38c2b4eb07ede 100644 --- 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,133 @@ func TestTemplatePush(t *testing.T) { t.Run("WorkspaceTagsTerraform", func(t *testing.T) { t.Parallel() - ctx := testutil.Context(t, testutil.WaitShort) + now := dbtime.Now() + oneHourAgo := now.Add(-time.Hour) - // 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) error + expectOutput string + }{ + { + name: "no provisioners available", + setupDaemon: func(_ context.Context, _ database.Store, _ codersdk.CreateFirstUserResponse, _ database.StringMap) 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) error { + _, 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, + }) + 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) error { + _, 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, + }) + 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), "_", "-") + + var output strings.Builder + inv, root := clitest.New(t, "templates", "push", templateName, "-d", tempDir, "--yes") + inv.Stdout = &output + inv.Stderr = &output + clitest.SetupConfig(t, templateAdmin, root) + + ctx := testutil.Context(t, testutil.WaitShort) + require.NoError(t, tt.setupDaemon(ctx, store, owner, wantTags)) + + 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{}) + return assert.NoError(t, err) && len(jobs) == 1 && assert.EqualValues(t, wantTags, jobs[0].Tags) + }, testutil.WaitShort, testutil.IntervalFast) + cancel() + <-done + + if tt.expectOutput != "" { + require.Contains(t, output.String(), tt.expectOutput) + } + }) + } }) t.Run("ChangeTags", func(t *testing.T) { From 6c4eb3220294d39c4ed3e330a0b00a0d92d4a753 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 29 Nov 2024 11:03:57 +0000 Subject: [PATCH 4/9] add provisioner key --- cli/templatepush_test.go | 28 ++++++++++++++++++++++++++-- 1 file changed, 26 insertions(+), 2 deletions(-) diff --git a/cli/templatepush_test.go b/cli/templatepush_test.go index 38c2b4eb07ede..dcc97d7206578 100644 --- a/cli/templatepush_test.go +++ b/cli/templatepush_test.go @@ -432,13 +432,25 @@ func TestTemplatePush(t *testing.T) { { name: "provisioner stale", setupDaemon: func(ctx context.Context, store database.Store, owner codersdk.CreateFirstUserResponse, tags database.StringMap) error { - _, err := store.UpsertProvisionerDaemon(ctx, database.UpsertProvisionerDaemonParams{ + 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: oneHourAgo, Valid: true}, CreatedAt: oneHourAgo, Name: "test", Tags: tags, OrganizationID: owner.OrganizationID, + KeyID: pk.ID, }) return err }, @@ -447,13 +459,25 @@ func TestTemplatePush(t *testing.T) { { name: "active provisioner", setupDaemon: func(ctx context.Context, store database.Store, owner codersdk.CreateFirstUserResponse, tags database.StringMap) error { - _, err := store.UpsertProvisionerDaemon(ctx, database.UpsertProvisionerDaemonParams{ + 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 }, From 738f67c46c7f09843bd46b5b45c26221b71aa4e4 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 29 Nov 2024 11:10:58 +0000 Subject: [PATCH 5/9] partially revert change to message copy --- cli/templatepush.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/templatepush.go b/cli/templatepush.go index 4f9fc9f7b9e74..2576e878edddb 100644 --- a/cli/templatepush.go +++ b/cli/templatepush.go @@ -492,7 +492,7 @@ Details: Details: Provisioner job ID : %s Requested tags : %s - Last response : %s + Most recently seen : %s ` ) From 4118a94aad164adc2469a4b233ce1806a8b91d52 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 29 Nov 2024 11:25:08 +0000 Subject: [PATCH 6/9] adjust condition for checking provisioner jobs --- cli/templatepush_test.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cli/templatepush_test.go b/cli/templatepush_test.go index dcc97d7206578..aab7ffd4549c1 100644 --- a/cli/templatepush_test.go +++ b/cli/templatepush_test.go @@ -555,7 +555,13 @@ func TestTemplatePush(t *testing.T) { require.Eventually(t, func() bool { jobs, err := store.GetProvisionerJobsCreatedAfter(ctx, time.Time{}) - return assert.NoError(t, err) && len(jobs) == 1 && assert.EqualValues(t, wantTags, jobs[0].Tags) + 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) cancel() <-done From ba9c22d310f9ee60440d594055a832280a0d0d06 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 29 Nov 2024 15:03:32 +0000 Subject: [PATCH 7/9] hopefully address test flake --- cli/templatepush_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/cli/templatepush_test.go b/cli/templatepush_test.go index aab7ffd4549c1..5a50ce6f6a684 100644 --- a/cli/templatepush_test.go +++ b/cli/templatepush_test.go @@ -414,24 +414,22 @@ func TestTemplatePush(t *testing.T) { t.Run("WorkspaceTagsTerraform", func(t *testing.T) { t.Parallel() - now := dbtime.Now() - oneHourAgo := now.Add(-time.Hour) tests := []struct { name string - setupDaemon func(ctx context.Context, store database.Store, owner codersdk.CreateFirstUserResponse, tags database.StringMap) error + 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) error { + 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) error { + 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, @@ -443,6 +441,7 @@ func TestTemplatePush(t *testing.T) { 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}, @@ -458,7 +457,7 @@ func TestTemplatePush(t *testing.T) { }, { name: "active provisioner", - setupDaemon: func(ctx context.Context, store database.Store, owner codersdk.CreateFirstUserResponse, tags database.StringMap) error { + 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, @@ -544,7 +543,8 @@ func TestTemplatePush(t *testing.T) { clitest.SetupConfig(t, templateAdmin, root) ctx := testutil.Context(t, testutil.WaitShort) - require.NoError(t, tt.setupDaemon(ctx, store, owner, wantTags)) + now := dbtime.Now() + require.NoError(t, tt.setupDaemon(ctx, store, owner, wantTags, now)) cancelCtx, cancel := context.WithCancel(ctx) t.Cleanup(cancel) From bbb2861fbb47c9cc8f1d689a08e2216b29e60028 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 29 Nov 2024 17:16:37 +0000 Subject: [PATCH 8/9] use ptytest instead of a strings.Builder to assert cli output --- cli/templatepush_test.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/cli/templatepush_test.go b/cli/templatepush_test.go index 5a50ce6f6a684..ae8f60bd9c551 100644 --- a/cli/templatepush_test.go +++ b/cli/templatepush_test.go @@ -536,11 +536,9 @@ func TestTemplatePush(t *testing.T) { templateName := strings.ReplaceAll(testutil.GetRandomName(t), "_", "-") - var output strings.Builder inv, root := clitest.New(t, "templates", "push", templateName, "-d", tempDir, "--yes") - inv.Stdout = &output - inv.Stderr = &output clitest.SetupConfig(t, templateAdmin, root) + pty := ptytest.New(t).Attach(inv) ctx := testutil.Context(t, testutil.WaitShort) now := dbtime.Now() @@ -563,12 +561,13 @@ func TestTemplatePush(t *testing.T) { } return assert.EqualValues(t, wantTags, jobs[0].Tags) }, testutil.WaitShort, testutil.IntervalFast) - cancel() - <-done if tt.expectOutput != "" { - require.Contains(t, output.String(), tt.expectOutput) + pty.ExpectMatch(tt.expectOutput) } + + cancel() + <-done }) } }) From 575e772806a0de9288f4bff93a9d434932c4ad40 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 29 Nov 2024 17:22:14 +0000 Subject: [PATCH 9/9] avoid passing in non-nil codersdk.MatchedProvisioners where not necessary --- coderd/templateversions.go | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/coderd/templateversions.go b/coderd/templateversions.go index 7946bdd56fc5f..004dc6e5a4d6d 100644 --- a/coderd/templateversions.go +++ b/coderd/templateversions.go @@ -77,7 +77,7 @@ func (api *API) templateVersion(rw http.ResponseWriter, r *http.Request) { warnings = append(warnings, codersdk.TemplateVersionWarningUnsupportedWorkspaces) } - httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(templateVersion, convertProvisionerJob(jobs[0]), codersdk.MatchedProvisioners{}, warnings)) + httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(templateVersion, convertProvisionerJob(jobs[0]), nil, warnings)) } // @Summary Patch template version by ID @@ -173,7 +173,7 @@ func (api *API) patchTemplateVersion(rw http.ResponseWriter, r *http.Request) { return } - httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(updatedTemplateVersion, convertProvisionerJob(jobs[0]), codersdk.MatchedProvisioners{}, nil)) + httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(updatedTemplateVersion, convertProvisionerJob(jobs[0]), nil, nil)) } // @Summary Cancel template version by ID @@ -814,7 +814,7 @@ func (api *API) templateVersionsByTemplate(rw http.ResponseWriter, r *http.Reque return err } - apiVersions = append(apiVersions, convertTemplateVersion(version, convertProvisionerJob(job), codersdk.MatchedProvisioners{}, nil)) + apiVersions = append(apiVersions, convertTemplateVersion(version, convertProvisionerJob(job), nil, nil)) } return nil @@ -869,7 +869,7 @@ func (api *API) templateVersionByName(rw http.ResponseWriter, r *http.Request) { return } - httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(templateVersion, convertProvisionerJob(jobs[0]), codersdk.MatchedProvisioners{}, nil)) + httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(templateVersion, convertProvisionerJob(jobs[0]), nil, nil)) } // @Summary Get template version by organization, template, and name @@ -934,7 +934,7 @@ func (api *API) templateVersionByOrganizationTemplateAndName(rw http.ResponseWri return } - httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(templateVersion, convertProvisionerJob(jobs[0]), codersdk.MatchedProvisioners{}, nil)) + httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(templateVersion, convertProvisionerJob(jobs[0]), nil, nil)) } // @Summary Get previous template version by organization, template, and name @@ -1020,7 +1020,7 @@ func (api *API) previousTemplateVersionByOrganizationTemplateAndName(rw http.Res return } - httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(previousTemplateVersion, convertProvisionerJob(jobs[0]), codersdk.MatchedProvisioners{}, nil)) + httpapi.Write(ctx, rw, http.StatusOK, convertTemplateVersion(previousTemplateVersion, convertProvisionerJob(jobs[0]), nil, nil)) } // @Summary Archive template unused versions by template id @@ -1633,7 +1633,7 @@ func (api *API) postTemplateVersionsByOrganization(rw http.ResponseWriter, r *ht ProvisionerJob: provisionerJob, QueuePosition: 0, }), - matchedProvisioners, + &matchedProvisioners, warnings)) } @@ -1701,7 +1701,7 @@ func (api *API) templateVersionLogs(rw http.ResponseWriter, r *http.Request) { api.provisionerJobLogs(rw, r, job) } -func convertTemplateVersion(version database.TemplateVersion, job codersdk.ProvisionerJob, matchedProvisioners codersdk.MatchedProvisioners, warnings []codersdk.TemplateVersionWarning) codersdk.TemplateVersion { +func convertTemplateVersion(version database.TemplateVersion, job codersdk.ProvisionerJob, matchedProvisioners *codersdk.MatchedProvisioners, warnings []codersdk.TemplateVersionWarning) codersdk.TemplateVersion { return codersdk.TemplateVersion{ ID: version.ID, TemplateID: &version.TemplateID.UUID, @@ -1719,7 +1719,7 @@ func convertTemplateVersion(version database.TemplateVersion, job codersdk.Provi }, Archived: version.Archived, Warnings: warnings, - MatchedProvisioners: &matchedProvisioners, + MatchedProvisioners: matchedProvisioners, } }