-
Notifications
You must be signed in to change notification settings - Fork 949
feat(cli): add CLI support for creating a workspace with preset #18912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
982031a
ae16155
3d7b40e
7159de2
b8a1c14
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -21,10 +21,15 @@ import ( | |||||||||||||||||||||||||||
"github.com/coder/serpent" | ||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
// DefaultPresetName is used when a user runs `create --preset default`. | ||||||||||||||||||||||||||||
// It instructs the CLI to use the default preset defined for the template version, if one exists. | ||||||||||||||||||||||||||||
const DefaultPresetName = "default" | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
func (r *RootCmd) create() *serpent.Command { | ||||||||||||||||||||||||||||
var ( | ||||||||||||||||||||||||||||
templateName string | ||||||||||||||||||||||||||||
templateVersion string | ||||||||||||||||||||||||||||
presetName string | ||||||||||||||||||||||||||||
startAt string | ||||||||||||||||||||||||||||
stopAfter time.Duration | ||||||||||||||||||||||||||||
workspaceName string | ||||||||||||||||||||||||||||
|
@@ -263,11 +268,55 @@ func (r *RootCmd) create() *serpent.Command { | |||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
// If a preset name is provided, resolve the preset to use. | ||||||||||||||||||||||||||||
var preset *codersdk.Preset | ||||||||||||||||||||||||||||
var presetParameters []codersdk.WorkspaceBuildParameter | ||||||||||||||||||||||||||||
isDefaultPreset := false | ||||||||||||||||||||||||||||
if len(presetName) > 0 { | ||||||||||||||||||||||||||||
tvPresets, err := client.TemplateVersionPresets(inv.Context(), templateVersionID) | ||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||
return xerrors.Errorf("failed to get presets: %w", err) | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
for _, tvPreset := range tvPresets { | ||||||||||||||||||||||||||||
// If the preset name is the special "default" keyword, | ||||||||||||||||||||||||||||
// fetch the template version's default preset (if any). | ||||||||||||||||||||||||||||
if presetName == DefaultPresetName && tvPreset.Default { | ||||||||||||||||||||||||||||
preset = &tvPreset | ||||||||||||||||||||||||||||
isDefaultPreset = true | ||||||||||||||||||||||||||||
break | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
if tvPreset.Name == presetName { | ||||||||||||||||||||||||||||
preset = &tvPreset | ||||||||||||||||||||||||||||
break | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
if preset == nil { | ||||||||||||||||||||||||||||
return xerrors.Errorf("preset %q not found", presetName) | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
// Convert preset parameters into workspace build parameters. | ||||||||||||||||||||||||||||
presetBuildParameters := presetParameterAsWorkspaceBuildParameters(preset.Parameters) | ||||||||||||||||||||||||||||
presetParameters = append(presetParameters, presetBuildParameters...) | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
// Inform the user which preset was applied and its parameters. | ||||||||||||||||||||||||||||
presetLabel := fmt.Sprintf("Preset '%s'", preset.Name) | ||||||||||||||||||||||||||||
if isDefaultPreset { | ||||||||||||||||||||||||||||
presetLabel += " (default)" | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
_, _ = fmt.Fprintf(inv.Stdout, "%s applied:", cliui.Bold(presetLabel)) | ||||||||||||||||||||||||||||
for _, p := range presetParameters { | ||||||||||||||||||||||||||||
_, _ = fmt.Fprintf(inv.Stdout, " %s: '%s'\n", cliui.Bold(p.Name), p.Value) | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
richParameters, err := prepWorkspaceBuild(inv, client, prepWorkspaceBuildArgs{ | ||||||||||||||||||||||||||||
Action: WorkspaceCreate, | ||||||||||||||||||||||||||||
TemplateVersionID: templateVersionID, | ||||||||||||||||||||||||||||
NewWorkspaceName: workspaceName, | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
PresetParameters: presetParameters, | ||||||||||||||||||||||||||||
RichParameterFile: parameterFlags.richParameterFile, | ||||||||||||||||||||||||||||
RichParameters: cliBuildParameters, | ||||||||||||||||||||||||||||
RichParameterDefaults: cliBuildParameterDefaults, | ||||||||||||||||||||||||||||
|
@@ -291,14 +340,21 @@ func (r *RootCmd) create() *serpent.Command { | |||||||||||||||||||||||||||
ttlMillis = ptr.Ref(stopAfter.Milliseconds()) | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
workspace, err := client.CreateUserWorkspace(inv.Context(), workspaceOwner, codersdk.CreateWorkspaceRequest{ | ||||||||||||||||||||||||||||
req := codersdk.CreateWorkspaceRequest{ | ||||||||||||||||||||||||||||
TemplateVersionID: templateVersionID, | ||||||||||||||||||||||||||||
Name: workspaceName, | ||||||||||||||||||||||||||||
AutostartSchedule: schedSpec, | ||||||||||||||||||||||||||||
TTLMillis: ttlMillis, | ||||||||||||||||||||||||||||
RichParameterValues: richParameters, | ||||||||||||||||||||||||||||
AutomaticUpdates: codersdk.AutomaticUpdates(autoUpdates), | ||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
// If a preset exists, update the create workspace request's preset ID | ||||||||||||||||||||||||||||
if preset != nil { | ||||||||||||||||||||||||||||
req.TemplateVersionPresetID = preset.ID | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
workspace, err := client.CreateUserWorkspace(inv.Context(), workspaceOwner, req) | ||||||||||||||||||||||||||||
if err != nil { | ||||||||||||||||||||||||||||
return xerrors.Errorf("create workspace: %w", err) | ||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||
|
@@ -333,6 +389,12 @@ func (r *RootCmd) create() *serpent.Command { | |||||||||||||||||||||||||||
Description: "Specify a template version name.", | ||||||||||||||||||||||||||||
Value: serpent.StringOf(&templateVersion), | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
serpent.Option{ | ||||||||||||||||||||||||||||
Flag: "preset", | ||||||||||||||||||||||||||||
Env: "CODER_PRESET_NAME", | ||||||||||||||||||||||||||||
Description: "Specify a template version preset name. Use 'default' to apply the default preset defined in the template version, if available.", | ||||||||||||||||||||||||||||
Value: serpent.StringOf(&presetName), | ||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||
serpent.Option{ | ||||||||||||||||||||||||||||
Flag: "start-at", | ||||||||||||||||||||||||||||
Env: "CODER_WORKSPACE_START_AT", | ||||||||||||||||||||||||||||
|
@@ -377,6 +439,7 @@ type prepWorkspaceBuildArgs struct { | |||||||||||||||||||||||||||
PromptEphemeralParameters bool | ||||||||||||||||||||||||||||
EphemeralParameters []codersdk.WorkspaceBuildParameter | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
PresetParameters []codersdk.WorkspaceBuildParameter | ||||||||||||||||||||||||||||
PromptRichParameters bool | ||||||||||||||||||||||||||||
RichParameters []codersdk.WorkspaceBuildParameter | ||||||||||||||||||||||||||||
RichParameterFile string | ||||||||||||||||||||||||||||
|
@@ -411,6 +474,7 @@ func prepWorkspaceBuild(inv *serpent.Invocation, client *codersdk.Client, args p | |||||||||||||||||||||||||||
WithSourceWorkspaceParameters(args.SourceWorkspaceParameters). | ||||||||||||||||||||||||||||
WithPromptEphemeralParameters(args.PromptEphemeralParameters). | ||||||||||||||||||||||||||||
WithEphemeralParameters(args.EphemeralParameters). | ||||||||||||||||||||||||||||
WithPresetParameters(args.PresetParameters). | ||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this not be last in the list? Otherwise we allow preset parameters to be overwritten. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This step does not overwrite the preset parameters. It simply adds the preset parameters to the ParameterResolver: coder/cli/parameterresolver.go Lines 49 to 52 in ae16155
The actual overwrite logic happens later in the coder/cli/parameterresolver.go Lines 95 to 103 in ae16155
After that, the code verifies if there are any constraints and prompts the user for any missing parameters. |
||||||||||||||||||||||||||||
WithPromptRichParameters(args.PromptRichParameters). | ||||||||||||||||||||||||||||
WithRichParameters(args.RichParameters). | ||||||||||||||||||||||||||||
WithRichParametersFile(parameterFile). | ||||||||||||||||||||||||||||
|
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To preserve current behavior, if no
--preset
is provided, the CLI does not apply any preset. I added support for a specialdefault
value to allow users to explicitly use the default preset (if one exists). However, this approach might not be ideal, as it introduces ambiguity, e.g., what if a preset is literally named "default"?Another approach I considered was assigning a default value (like
default
) to the--preset
flag itself. But that could lead to unexpected behavior: the user may not realize a preset is being applied at all.A potentially better UX would be:
--preset
, prompt the user to select one.none
option to explicitly skip using a preset (as in the UI). This means that instead of having a default value for the preset, we can have a none value that explicitly indicates no preset should be used.However, the downside is that this would break existing automation/scripts that rely on creating workspaces silently when presets exist. Those scripts would now hang, waiting for input.
Let me know what you think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've summarised the trade-offs well. My assessment of them is that without a
--preset
flag specified, it should use the default as defined in the template. This use case is what we allow a default to be defined for.I see you've already added a line to the command output to inform the user which preset was chosen, so that should help make it more visible.
Like with the frontend, we can allow for a "None" preset. In fact it might be a good idea to move that from the frontend to the API so that we have it here without any additional trouble.