mirror of
https://github.com/coder/coder.git
synced 2026-06-02 20:48:20 +00:00
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..." ✅
This commit is contained in:
+48
-44
@@ -577,53 +577,57 @@ func prepWorkspaceBuild(inv *serpent.Invocation, client *codersdk.Client, args p
|
||||
return nil, xerrors.Errorf("template version git auth: %w", err)
|
||||
}
|
||||
|
||||
// Run a dry-run with the given parameters to check correctness
|
||||
dryRun, err := client.CreateTemplateVersionDryRun(inv.Context(), templateVersion.ID, codersdk.CreateTemplateVersionDryRunRequest{
|
||||
WorkspaceName: args.NewWorkspaceName,
|
||||
RichParameterValues: buildParameters,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("begin workspace dry-run: %w", err)
|
||||
}
|
||||
// Only perform dry-run for workspace creation and updates
|
||||
// Skip for start and restart to avoid unnecessary delays
|
||||
if args.Action == WorkspaceCreate || args.Action == WorkspaceUpdate {
|
||||
// Run a dry-run with the given parameters to check correctness
|
||||
dryRun, err := client.CreateTemplateVersionDryRun(inv.Context(), templateVersion.ID, codersdk.CreateTemplateVersionDryRunRequest{
|
||||
WorkspaceName: args.NewWorkspaceName,
|
||||
RichParameterValues: buildParameters,
|
||||
})
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("begin workspace dry-run: %w", err)
|
||||
}
|
||||
|
||||
matchedProvisioners, err := client.TemplateVersionDryRunMatchedProvisioners(inv.Context(), templateVersion.ID, dryRun.ID)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("get matched provisioners: %w", err)
|
||||
}
|
||||
cliutil.WarnMatchedProvisioners(inv.Stdout, &matchedProvisioners, dryRun)
|
||||
_, _ = fmt.Fprintln(inv.Stdout, "Planning workspace...")
|
||||
err = cliui.ProvisionerJob(inv.Context(), inv.Stdout, cliui.ProvisionerJobOptions{
|
||||
Fetch: func() (codersdk.ProvisionerJob, error) {
|
||||
return client.TemplateVersionDryRun(inv.Context(), templateVersion.ID, dryRun.ID)
|
||||
},
|
||||
Cancel: func() error {
|
||||
return client.CancelTemplateVersionDryRun(inv.Context(), templateVersion.ID, dryRun.ID)
|
||||
},
|
||||
Logs: func() (<-chan codersdk.ProvisionerJobLog, io.Closer, error) {
|
||||
return client.TemplateVersionDryRunLogsAfter(inv.Context(), templateVersion.ID, dryRun.ID, 0)
|
||||
},
|
||||
// Don't show log output for the dry-run unless there's an error.
|
||||
Silent: true,
|
||||
})
|
||||
if err != nil {
|
||||
// TODO (Dean): reprompt for parameter values if we deem it to
|
||||
// be a validation error
|
||||
return nil, xerrors.Errorf("dry-run workspace: %w", err)
|
||||
}
|
||||
matchedProvisioners, err := client.TemplateVersionDryRunMatchedProvisioners(inv.Context(), templateVersion.ID, dryRun.ID)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("get matched provisioners: %w", err)
|
||||
}
|
||||
cliutil.WarnMatchedProvisioners(inv.Stdout, &matchedProvisioners, dryRun)
|
||||
_, _ = fmt.Fprintln(inv.Stdout, "Planning workspace...")
|
||||
err = cliui.ProvisionerJob(inv.Context(), inv.Stdout, cliui.ProvisionerJobOptions{
|
||||
Fetch: func() (codersdk.ProvisionerJob, error) {
|
||||
return client.TemplateVersionDryRun(inv.Context(), templateVersion.ID, dryRun.ID)
|
||||
},
|
||||
Cancel: func() error {
|
||||
return client.CancelTemplateVersionDryRun(inv.Context(), templateVersion.ID, dryRun.ID)
|
||||
},
|
||||
Logs: func() (<-chan codersdk.ProvisionerJobLog, io.Closer, error) {
|
||||
return client.TemplateVersionDryRunLogsAfter(inv.Context(), templateVersion.ID, dryRun.ID, 0)
|
||||
},
|
||||
// Don't show log output for the dry-run unless there's an error.
|
||||
Silent: true,
|
||||
})
|
||||
if err != nil {
|
||||
// TODO (Dean): reprompt for parameter values if we deem it to
|
||||
// be a validation error
|
||||
return nil, xerrors.Errorf("dry-run workspace: %w", err)
|
||||
}
|
||||
|
||||
resources, err := client.TemplateVersionDryRunResources(inv.Context(), templateVersion.ID, dryRun.ID)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("get workspace dry-run resources: %w", err)
|
||||
}
|
||||
resources, err := client.TemplateVersionDryRunResources(inv.Context(), templateVersion.ID, dryRun.ID)
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("get workspace dry-run resources: %w", err)
|
||||
}
|
||||
|
||||
err = cliui.WorkspaceResources(inv.Stdout, resources, cliui.WorkspaceResourcesOptions{
|
||||
WorkspaceName: args.NewWorkspaceName,
|
||||
// Since agents haven't connected yet, hiding this makes more sense.
|
||||
HideAgentState: true,
|
||||
Title: "Workspace Preview",
|
||||
})
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("get resources: %w", err)
|
||||
err = cliui.WorkspaceResources(inv.Stdout, resources, cliui.WorkspaceResourcesOptions{
|
||||
WorkspaceName: args.NewWorkspaceName,
|
||||
// Since agents haven't connected yet, hiding this makes more sense.
|
||||
HideAgentState: true,
|
||||
Title: "Workspace Preview",
|
||||
})
|
||||
if err != nil {
|
||||
return nil, xerrors.Errorf("get resources: %w", err)
|
||||
}
|
||||
}
|
||||
|
||||
return buildParameters, nil
|
||||
|
||||
Reference in New Issue
Block a user