Skip to content

Commit a8f2a8a

Browse files
deansheatherdeansheather
authored
fix(cli): skip dry-run for workspace start/restart commands (#20754)
## Problem The `prepWorkspaceBuild()` function in `cli/create.go` was unconditionally executing dry-runs for **all** workspace actions. This caused unnecessary delays and "Planning workspace..." messages during `coder start` and `coder restart` commands when they should only happen during `coder create` and `coder update`. ## Root Cause The `prepWorkspaceBuild()` function is shared code called by: - **create command** - passes `WorkspaceCreate` action ✅ dry-run IS desired - **update command** - passes `WorkspaceUpdate` action ✅ dry-run IS desired - **start command** - passes `WorkspaceStart` action (or `WorkspaceUpdate` as fallback) ❌ dry-run NOT desired for `WorkspaceStart` - **restart command** - passes `WorkspaceRestart` action ❌ dry-run NOT desired - **scaletest commands** - pass `WorkspaceCreate` action ✅ dry-run IS desired ## Solution Wrapped the dry-run section (lines 580-627) in a conditional that only executes when `args.Action == WorkspaceCreate || args.Action == WorkspaceUpdate`. This skips dry-run for `WorkspaceStart` and `WorkspaceRestart` actions while preserving it for creation and explicit updates. ## Changes - Added conditional check around the entire dry-run logic block - Added clarifying comment explaining the intent - Changed from unconditional execution to: `if args.Action == WorkspaceCreate || args.Action == WorkspaceUpdate { ... }` ## Impact | Command | Action Type | Dry-run Before | Dry-run After | Status | |---------|-------------|----------------|---------------|--------| | `coder create` | `WorkspaceCreate` | ✅ Yes | ✅ Yes | Unchanged | | `coder update` | `WorkspaceUpdate` | ✅ Yes | ✅ Yes | Unchanged | | `coder start` (normal) | `WorkspaceStart` | ❌ Yes (bug) | ✅ No | **Fixed** | | `coder start` (template changed) | `WorkspaceUpdate` | ✅ Yes | ✅ Yes | Unchanged (correct behavior) | | `coder restart` | `WorkspaceRestart` | ❌ Yes (bug) | ✅ No | **Fixed** | | scaletest | `WorkspaceCreate` | ✅ Yes | ✅ Yes | Unchanged | ## Testing ✅ **Code compiles successfully** ```bash go build -o /dev/null ./cli/... ``` ✅ **All relevant tests pass locally** ```bash cd cli && go test -run "TestCreate|TestStart|TestRestart|TestUpdate" -v PASS ok github.com/coder/coder/v2/cli 3.337s ``` ✅ **All CI checks pass** - test-go-pg (ubuntu, macos, windows) ✅ - test-go-pg-17 ✅ - test-go-race-pg ✅ - test-e2e ✅ - All other checks ✅ ## Behavior Changes **Before:** - Users running `coder start` would see "Planning workspace..." and wait for unnecessary dry-run completion - Users running `coder restart` would experience unnecessary dry-run overhead **After:** - `coder start` (simple start) skips dry-run entirely (faster, more intuitive) - `coder start` (with template update) still shows dry-run (correct - user needs to see what's changing) - `coder restart` skips dry-run entirely (faster, more intuitive) - `coder create` maintains existing dry-run behavior (shows "Planning workspace..." and resource preview) - `coder update` maintains existing dry-run behavior (shows "Planning workspace..." and resource preview) ## Verification Manual testing should verify: 1. `coder create` still shows "Planning workspace..." ✅ 2. `coder update` still shows "Planning workspace..." ✅ 3. `coder start` (simple start) does NOT show "Planning workspace..." ✅ 4. `coder restart` does NOT show "Planning workspace..." ✅
1 parent 04727c0 commit a8f2a8a

File tree

1 file changed

+48
-44
lines changed

1 file changed

+48
-44
lines changed

‎cli/create.go‎

Lines changed: 48 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -577,53 +577,57 @@ func prepWorkspaceBuild(inv *serpent.Invocation, client *codersdk.Client, args p
577577
return nil, xerrors.Errorf("template version git auth: %w", err)
578578
}
579579

580-
// Run a dry-run with the given parameters to check correctness
581-
dryRun, err := client.CreateTemplateVersionDryRun(inv.Context(), templateVersion.ID, codersdk.CreateTemplateVersionDryRunRequest{
582-
WorkspaceName: args.NewWorkspaceName,
583-
RichParameterValues: buildParameters,
584-
})
585-
if err != nil {
586-
return nil, xerrors.Errorf("begin workspace dry-run: %w", err)
587-
}
580+
// Only perform dry-run for workspace creation and updates
581+
// Skip for start and restart to avoid unnecessary delays
582+
if args.Action == WorkspaceCreate || args.Action == WorkspaceUpdate {
583+
// Run a dry-run with the given parameters to check correctness
584+
dryRun, err := client.CreateTemplateVersionDryRun(inv.Context(), templateVersion.ID, codersdk.CreateTemplateVersionDryRunRequest{
585+
WorkspaceName: args.NewWorkspaceName,
586+
RichParameterValues: buildParameters,
587+
})
588+
if err != nil {
589+
return nil, xerrors.Errorf("begin workspace dry-run: %w", err)
590+
}
588591

589-
matchedProvisioners, err := client.TemplateVersionDryRunMatchedProvisioners(inv.Context(), templateVersion.ID, dryRun.ID)
590-
if err != nil {
591-
return nil, xerrors.Errorf("get matched provisioners: %w", err)
592-
}
593-
cliutil.WarnMatchedProvisioners(inv.Stdout, &matchedProvisioners, dryRun)
594-
_, _ = fmt.Fprintln(inv.Stdout, "Planning workspace...")
595-
err = cliui.ProvisionerJob(inv.Context(), inv.Stdout, cliui.ProvisionerJobOptions{
596-
Fetch: func() (codersdk.ProvisionerJob, error) {
597-
return client.TemplateVersionDryRun(inv.Context(), templateVersion.ID, dryRun.ID)
598-
},
599-
Cancel: func() error {
600-
return client.CancelTemplateVersionDryRun(inv.Context(), templateVersion.ID, dryRun.ID)
601-
},
602-
Logs: func() (<-chan codersdk.ProvisionerJobLog, io.Closer, error) {
603-
return client.TemplateVersionDryRunLogsAfter(inv.Context(), templateVersion.ID, dryRun.ID, 0)
604-
},
605-
// Don't show log output for the dry-run unless there's an error.
606-
Silent: true,
607-
})
608-
if err != nil {
609-
// TODO (Dean): reprompt for parameter values if we deem it to
610-
// be a validation error
611-
return nil, xerrors.Errorf("dry-run workspace: %w", err)
612-
}
592+
matchedProvisioners, err := client.TemplateVersionDryRunMatchedProvisioners(inv.Context(), templateVersion.ID, dryRun.ID)
593+
if err != nil {
594+
return nil, xerrors.Errorf("get matched provisioners: %w", err)
595+
}
596+
cliutil.WarnMatchedProvisioners(inv.Stdout, &matchedProvisioners, dryRun)
597+
_, _ = fmt.Fprintln(inv.Stdout, "Planning workspace...")
598+
err = cliui.ProvisionerJob(inv.Context(), inv.Stdout, cliui.ProvisionerJobOptions{
599+
Fetch: func() (codersdk.ProvisionerJob, error) {
600+
return client.TemplateVersionDryRun(inv.Context(), templateVersion.ID, dryRun.ID)
601+
},
602+
Cancel: func() error {
603+
return client.CancelTemplateVersionDryRun(inv.Context(), templateVersion.ID, dryRun.ID)
604+
},
605+
Logs: func() (<-chan codersdk.ProvisionerJobLog, io.Closer, error) {
606+
return client.TemplateVersionDryRunLogsAfter(inv.Context(), templateVersion.ID, dryRun.ID, 0)
607+
},
608+
// Don't show log output for the dry-run unless there's an error.
609+
Silent: true,
610+
})
611+
if err != nil {
612+
// TODO (Dean): reprompt for parameter values if we deem it to
613+
// be a validation error
614+
return nil, xerrors.Errorf("dry-run workspace: %w", err)
615+
}
613616

614-
resources, err := client.TemplateVersionDryRunResources(inv.Context(), templateVersion.ID, dryRun.ID)
615-
if err != nil {
616-
return nil, xerrors.Errorf("get workspace dry-run resources: %w", err)
617-
}
617+
resources, err := client.TemplateVersionDryRunResources(inv.Context(), templateVersion.ID, dryRun.ID)
618+
if err != nil {
619+
return nil, xerrors.Errorf("get workspace dry-run resources: %w", err)
620+
}
618621

619-
err = cliui.WorkspaceResources(inv.Stdout, resources, cliui.WorkspaceResourcesOptions{
620-
WorkspaceName: args.NewWorkspaceName,
621-
// Since agents haven't connected yet, hiding this makes more sense.
622-
HideAgentState: true,
623-
Title: "Workspace Preview",
624-
})
625-
if err != nil {
626-
return nil, xerrors.Errorf("get resources: %w", err)
622+
err = cliui.WorkspaceResources(inv.Stdout, resources, cliui.WorkspaceResourcesOptions{
623+
WorkspaceName: args.NewWorkspaceName,
624+
// Since agents haven't connected yet, hiding this makes more sense.
625+
HideAgentState: true,
626+
Title: "Workspace Preview",
627+
})
628+
if err != nil {
629+
return nil, xerrors.Errorf("get resources: %w", err)
630+
}
627631
}
628632

629633
return buildParameters, nil

0 commit comments

Comments
 (0)