From 96d187c07ebf0e80b7b41f71f6b0cf353e4c9e65 Mon Sep 17 00:00:00 2001 From: ammario Date: Thu, 9 Jun 2022 20:35:25 -0500 Subject: [PATCH] Make `coder bump` idempotent Resolves #2223 In addition to solving what's outlined in the issue, I remove the client-side minute check because it had no clear purpose when the API already returns an error. --- cli/bump.go | 38 ++++++++++----------- cli/bump_test.go | 89 +++--------------------------------------------- 2 files changed, 23 insertions(+), 104 deletions(-) diff --git a/cli/bump.go b/cli/bump.go index 54d1b4bf58d26..220f6c406b816 100644 --- a/cli/bump.go +++ b/cli/bump.go @@ -12,55 +12,55 @@ import ( ) const ( - bumpDescriptionLong = `To extend the autostop deadline for a workspace. -If no unit is specified in the duration, we assume minutes.` - defaultBumpDuration = 90 * time.Minute + bumpDescriptionLong = `To extend the autostop deadline for a workspace.` ) func bump() *cobra.Command { bumpCmd := &cobra.Command{ Args: cobra.RangeArgs(1, 2), Annotations: workspaceCommand, - Use: "bump [duration]", + Use: "bump ", Short: "Extend the autostop deadline for a workspace.", Long: bumpDescriptionLong, Example: "coder bump my-workspace 90m", RunE: func(cmd *cobra.Command, args []string) error { - bumpDuration := defaultBumpDuration - if len(args) > 1 { - d, err := tryParseDuration(args[1]) - if err != nil { - return err - } - bumpDuration = d - } - - if bumpDuration < time.Minute { - return xerrors.New("minimum bump duration is 1 minute") + bumpDuration, err := tryParseDuration(args[1]) + if err != nil { + return err } client, err := createClient(cmd) if err != nil { return xerrors.Errorf("create client: %w", err) } + workspace, err := namedWorkspace(cmd, client, args[0]) if err != nil { return xerrors.Errorf("get workspace: %w", err) } - if workspace.LatestBuild.Deadline.IsZero() { - _, _ = fmt.Fprintf(cmd.OutOrStdout(), "no deadline set\n") + newDeadline := time.Now().Add(bumpDuration) + + if newDeadline.Before(workspace.LatestBuild.Deadline) { + _, _ = fmt.Fprintf( + cmd.OutOrStdout(), + "The proposed deadline is %s before the current deadline.\n", + workspace.LatestBuild.Deadline.Sub(newDeadline).Round(time.Minute), + ) return nil } - newDeadline := workspace.LatestBuild.Deadline.Add(bumpDuration) if err := client.PutExtendWorkspace(cmd.Context(), workspace.ID, codersdk.PutExtendWorkspaceRequest{ Deadline: newDeadline, }); err != nil { return err } - _, _ = fmt.Fprintf(cmd.OutOrStdout(), "Workspace %q will now stop at %s\n", workspace.Name, newDeadline.Format(time.RFC3339)) + _, _ = fmt.Fprintf( + cmd.OutOrStdout(), + "Workspace %q will now stop at %s\n", workspace.Name, + newDeadline.Format(time.RFC822), + ) return nil }, diff --git a/cli/bump_test.go b/cli/bump_test.go index 041f8996919cf..635bb0b90956e 100644 --- a/cli/bump_test.go +++ b/cli/bump_test.go @@ -17,47 +17,6 @@ import ( func TestBump(t *testing.T) { t.Parallel() - t.Run("BumpOKDefault", func(t *testing.T) { - t.Parallel() - - // Given: we have a workspace - var ( - err error - ctx = context.Background() - client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) - user = coderdtest.CreateFirstUser(t, client) - version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID) - cmdArgs = []string{"bump", workspace.Name} - stdoutBuf = &bytes.Buffer{} - ) - - // Given: we wait for the workspace to be built - coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - workspace, err = client.Workspace(ctx, workspace.ID) - require.NoError(t, err) - expectedDeadline := workspace.LatestBuild.Deadline.Add(90 * time.Minute) - - // Assert test invariant: workspace build has a deadline set equal to now plus ttl - initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond) - require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline, time.Minute) - - cmd, root := clitest.New(t, cmdArgs...) - clitest.SetupConfig(t, client, root) - cmd.SetOut(stdoutBuf) - - // When: we execute `coder bump ` - err = cmd.ExecuteContext(ctx) - require.NoError(t, err, "unexpected error") - - // Then: the deadline of the latest build is updated - updated, err := client.Workspace(ctx, workspace.ID) - require.NoError(t, err) - require.WithinDuration(t, expectedDeadline, updated.LatestBuild.Deadline, time.Minute) - }) - t.Run("BumpSpecificDuration", func(t *testing.T) { t.Parallel() @@ -71,7 +30,7 @@ func TestBump(t *testing.T) { _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID) - cmdArgs = []string{"bump", workspace.Name, "30"} + cmdArgs = []string{"bump", workspace.Name, "10h"} stdoutBuf = &bytes.Buffer{} ) @@ -79,7 +38,7 @@ func TestBump(t *testing.T) { coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) workspace, err = client.Workspace(ctx, workspace.ID) require.NoError(t, err) - expectedDeadline := workspace.LatestBuild.Deadline.Add(30 * time.Minute) + expectedDeadline := time.Now().Add(10 * time.Hour) // Assert test invariant: workspace build has a deadline set equal to now plus ttl initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond) @@ -150,7 +109,7 @@ func TestBump(t *testing.T) { workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID, func(cwr *codersdk.CreateWorkspaceRequest) { cwr.TTLMillis = nil }) - cmdArgs = []string{"bump", workspace.Name} + cmdArgs = []string{"bump", workspace.Name, "1h"} stdoutBuf = &bytes.Buffer{} ) // Unset the workspace TTL @@ -180,51 +139,11 @@ func TestBump(t *testing.T) { // When: we execute `coder bump workspace`` err = cmd.ExecuteContext(ctx) - require.NoError(t, err) + require.Error(t, err) // Then: nothing happens and the deadline remains unset updated, err := client.Workspace(ctx, workspace.ID) require.NoError(t, err) require.Zero(t, updated.LatestBuild.Deadline) }) - - t.Run("BumpMinimumDuration", func(t *testing.T) { - t.Parallel() - - // Given: we have a workspace with no deadline set - var ( - err error - ctx = context.Background() - client = coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true}) - user = coderdtest.CreateFirstUser(t, client) - version = coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) - _ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) - project = coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) - workspace = coderdtest.CreateWorkspace(t, client, user.OrganizationID, project.ID) - cmdArgs = []string{"bump", workspace.Name, "59s"} - stdoutBuf = &bytes.Buffer{} - ) - - // Given: we wait for the workspace to build - coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID) - workspace, err = client.Workspace(ctx, workspace.ID) - require.NoError(t, err) - - // Assert test invariant: workspace build has a deadline set equal to now plus ttl - initDeadline := time.Now().Add(time.Duration(*workspace.TTLMillis) * time.Millisecond) - require.WithinDuration(t, initDeadline, workspace.LatestBuild.Deadline, time.Minute) - - cmd, root := clitest.New(t, cmdArgs...) - clitest.SetupConfig(t, client, root) - cmd.SetOut(stdoutBuf) - - // When: we execute `coder bump workspace 59s` - err = cmd.ExecuteContext(ctx) - require.ErrorContains(t, err, "minimum bump duration is 1 minute") - - // Then: an error is reported and the deadline remains as before - updated, err := client.Workspace(ctx, workspace.ID) - require.NoError(t, err) - require.WithinDuration(t, workspace.LatestBuild.Deadline, updated.LatestBuild.Deadline, time.Minute) - }) }