-
Notifications
You must be signed in to change notification settings - Fork 887
feat: Updating workspace prompts new parameters #2598
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
Changes from all commits
99c8c9c
191c006
0bedbe3
856293d
bb4432e
a6a81e8
7f10c06
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 |
---|---|---|
|
@@ -6,11 +6,17 @@ import ( | |
|
||
"github.com/spf13/cobra" | ||
|
||
"github.com/coder/coder/cli/cliflag" | ||
"github.com/coder/coder/codersdk" | ||
) | ||
|
||
func update() *cobra.Command { | ||
return &cobra.Command{ | ||
var ( | ||
parameterFile string | ||
alwaysPrompt bool | ||
) | ||
|
||
cmd := &cobra.Command{ | ||
Annotations: workspaceCommand, | ||
Use: "update", | ||
Short: "Update a workspace to the latest template version", | ||
|
@@ -23,18 +29,38 @@ func update() *cobra.Command { | |
if err != nil { | ||
return err | ||
} | ||
if !workspace.Outdated { | ||
if !workspace.Outdated && !alwaysPrompt { | ||
_, _ = fmt.Printf("Workspace isn't outdated!\n") | ||
return nil | ||
} | ||
template, err := client.Template(cmd.Context(), workspace.TemplateID) | ||
if err != nil { | ||
return nil | ||
} | ||
|
||
var existingParams []codersdk.Parameter | ||
if !alwaysPrompt { | ||
existingParams, err = client.Parameters(cmd.Context(), codersdk.ParameterWorkspace, workspace.ID) | ||
if err != nil { | ||
return nil | ||
} | ||
} | ||
|
||
parameters, err := prepWorkspaceBuild(cmd, client, prepWorkspaceBuildArgs{ | ||
Template: template, | ||
ExistingParams: existingParams, | ||
ParameterFile: parameterFile, | ||
NewWorkspaceName: workspace.Name, | ||
}) | ||
if err != nil { | ||
return nil | ||
} | ||
|
||
before := time.Now() | ||
build, err := client.CreateWorkspaceBuild(cmd.Context(), workspace.ID, codersdk.CreateWorkspaceBuildRequest{ | ||
TemplateVersionID: template.ActiveVersionID, | ||
Transition: workspace.LatestBuild.Transition, | ||
ParameterValues: parameters, | ||
}) | ||
if err != nil { | ||
return err | ||
|
@@ -53,4 +79,8 @@ func update() *cobra.Command { | |
return nil | ||
}, | ||
} | ||
|
||
cmd.Flags().BoolVar(&alwaysPrompt, "always-prompt", false, "Always prompt all parameters. Does not pull parameter values from existing workspace") | ||
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.
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. Use I am not sold on 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. I meant 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. Ahh. I was trying to keep the flow quick. I think we need to invest time into thinking about our UX in general. Idk if we should do 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. I can add this prompt though if 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. Our |
||
cliflag.StringVarP(cmd.Flags(), ¶meterFile, "parameter-file", "", "CODER_PARAMETER_FILE", "", "Specify a file path with parameter values.") | ||
return cmd | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,10 @@ type CreateWorkspaceBuildRequest struct { | |
Transition WorkspaceTransition `json:"transition" validate:"oneof=create start stop delete,required"` | ||
DryRun bool `json:"dry_run,omitempty"` | ||
ProvisionerState []byte `json:"state,omitempty"` | ||
// ParameterValues are optional. It will write params to the 'workspace' scope. | ||
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. You should explain the behavior that it doesn't delete the old values, but any present values will overwrite any existing values with the same name. 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 is true... I wonder if I should delete the unused params 🤔. I can add this in another PR and test it more |
||
// This will overwrite any existing parameters with the same name. | ||
// This will not delete old params not included in this list. | ||
ParameterValues []CreateParameterRequest `json:"parameter_values,omitempty"` | ||
} | ||
|
||
type WorkspaceOptions struct { | ||
|
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.
It would be nice if this always ran and had an argument to prompt for missing params or not instead of only running if they prompted for params. We'd be able to always dry-run this way
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 mean instead of passing "ExistingParams", detect that inside the
prepWorkspaceBuild
? Cause if so, I do want to do this. Templates use similar logic, so I want to refactor the create/update of both workspaces/templates to use the same code.I was going to take a look in another PR.