Skip to content

feat(cli): add CLI support for listing presets #18910

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 137 additions & 0 deletions cli/templateversionpresets.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package cli

import (
"fmt"
"strconv"
"strings"

"golang.org/x/xerrors"

"github.com/coder/coder/v2/cli/cliui"
"github.com/coder/coder/v2/codersdk"
"github.com/coder/serpent"
)

func (r *RootCmd) templateVersionPresets() *serpent.Command {
cmd := &serpent.Command{
Use: "presets",
Short: "Manage presets of the specified template version",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: we're not managing anything right now, and probably never will from this command.

Aliases: []string{"preset"},
Long: FormatExamples(
Example{
Description: "List presets of a specific template version",
Command: "coder templates versions presets list my-template my-template-version",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, the coder templates versions presets list command requires both a template and a template version as positional arguments. However, it might be more user-friendly to only require the template name, and by default use the active template version.

We could also introduce a --template-version flag for cases where the user wants to explicitly list the presets of a specific template version.

Example usage:

# Lists presets for the active version of the template
coder templates versions presets list my-template

# Lists presets for a specific template version
coder templates versions presets list my-template --template-version my-template-version

Let me know what you think

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

coder templates versions presets list is too verbose. Since we're requiring the template name / version as args/flags, there's no need to mention them in the command IMHO. coder presets should suffice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like coder presets. I recall seeing the term preset used in unrelated work in the frontend a few months ago. Perhaps just check that we won't have any namespacing conflicts if we shorten the command but otherwise I think its a good idea.

I think template version flag should be optional with the active version as a default.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an aside, I now see the logic of going for templates versions presets list, since it doesn't give presets any special treatment (or more special than versions which they exist inside of). I think the reasoning should've been spelled out in the description.

I'll just keep quiet about the naming now 😆

},
),
Handler: func(inv *serpent.Invocation) error {
return inv.Command.HelpHandler(inv)
},
Children: []*serpent.Command{
r.templateVersionPresetsList(),
},
}

return cmd
}

func (r *RootCmd) templateVersionPresetsList() *serpent.Command {
defaultColumns := []string{
"name",
"parameters",
"default",
"prebuilds",
}
formatter := cliui.NewOutputFormatter(
cliui.TableFormat([]templateVersionPresetRow{}, defaultColumns),
cliui.JSONFormat(),
)
client := new(codersdk.Client)
orgContext := NewOrganizationContext()

cmd := &serpent.Command{
Use: "list <template> <version>",
Middleware: serpent.Chain(
serpent.RequireNArgs(2),
r.InitClient(client),
),
Short: "List all the presets of the specified template version",
Options: serpent.OptionSet{},
Handler: func(inv *serpent.Invocation) error {
organization, err := orgContext.Selected(inv, client)
if err != nil {
return xerrors.Errorf("get current organization: %w", err)
}

template, err := client.TemplateByName(inv.Context(), organization.ID, inv.Args[0])
if err != nil {
return xerrors.Errorf("get template by name: %w", err)
}

version, err := client.TemplateVersionByName(inv.Context(), template.ID, inv.Args[1])
if err != nil {
return xerrors.Errorf("get template version by name: %w", err)
}

presets, err := client.TemplateVersionPresets(inv.Context(), version.ID)
if err != nil {
return xerrors.Errorf("get template versions presets by template version: %w", err)
}

if len(presets) == 0 {
return xerrors.Errorf("no presets found for template %q and template-version %q", template.Name, version.Name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be an error? Or just an empty list?

}

rows := templateVersionPresetsToRows(presets...)
out, err := formatter.Format(inv.Context(), rows)
if err != nil {
return xerrors.Errorf("render table: %w", err)
}

_, err = fmt.Fprintln(inv.Stdout, out)
return err
},
}

orgContext.AttachOptions(cmd)
formatter.AttachOptions(&cmd.Options)
return cmd
}

type templateVersionPresetRow struct {
// For json format:
TemplateVersionPreset codersdk.Preset `table:"-"`

// For table format:
Name string `json:"-" table:"name,default_sort"`
Parameters string `json:"-" table:"parameters"`
Default bool `json:"-" table:"default"`
Prebuilds string `json:"-" table:"prebuilds"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$ ./scripts/coder-dev.sh template versions presets list docker objective_elgamal6
NAME               PARAMETERS              DEFAULT  PREBUILDS
I Like GoLand      cpu=2,jetbrains_ide=GO  false    1
Some Like PyCharm  jetbrains_ide=PY        true     0

PREBUILDS is confusing here (maybe irrelevant?); we could rename it to DESIRED PREBUILT INSTANCES or similar.

}

func formatPresetParameters(params []codersdk.PresetParameter) string {
var paramsStr []string
for _, p := range params {
paramsStr = append(paramsStr, fmt.Sprintf("%s=%s", p.Name, p.Value))
}
return strings.Join(paramsStr, ",")
}

// templateVersionPresetsToRows converts a list of presets to a list of rows
// for outputting.
func templateVersionPresetsToRows(presets ...codersdk.Preset) []templateVersionPresetRow {
rows := make([]templateVersionPresetRow, len(presets))
for i, preset := range presets {
prebuilds := "-"
if preset.Prebuilds != nil {
prebuilds = strconv.Itoa(*preset.Prebuilds)
}
rows[i] = templateVersionPresetRow{
Name: preset.Name,
Parameters: formatPresetParameters(preset.Parameters),
Default: preset.Default,
Prebuilds: prebuilds,
}
}

return rows
}
137 changes: 137 additions & 0 deletions cli/templateversionpresets_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
package cli_test

import (
"fmt"
"testing"

"github.com/coder/coder/v2/provisioner/echo"
"github.com/coder/coder/v2/provisionersdk/proto"

"github.com/stretchr/testify/require"

"github.com/coder/coder/v2/cli/clitest"
"github.com/coder/coder/v2/coderd/coderdtest"
"github.com/coder/coder/v2/pty/ptytest"
)

func TestTemplateVersionPresets(t *testing.T) {
t.Parallel()

t.Run("ListPresets", func(t *testing.T) {
t.Parallel()

client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
owner := coderdtest.CreateFirstUser(t, client)
member, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)

// Given: a template version that includes presets
presets := []*proto.Preset{
{
Name: "preset-multiple-params",
Parameters: []*proto.PresetParameter{
{
Name: "k1",
Value: "v1",
}, {
Name: "k2",
Value: "v2",
},
},
},
{
Name: "preset-default",
Default: true,
Parameters: []*proto.PresetParameter{
{
Name: "k1",
Value: "v2",
},
},
Prebuild: &proto.Prebuild{
Instances: 0,
},
},
{
Name: "preset-prebuilds",
Parameters: []*proto.PresetParameter{},
Prebuild: &proto.Prebuild{
Instances: 2,
},
},
}
version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, templateWithPresets(presets))
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID)

// When: listing presets for that template and template version
inv, root := clitest.New(t, "templates", "versions", "presets", "list", template.Name, version.Name)
clitest.SetupConfig(t, member, root)

pty := ptytest.New(t).Attach(inv)
doneChan := make(chan struct{})
var runErr error
go func() {
defer close(doneChan)
runErr = inv.Run()
}()

<-doneChan
require.NoError(t, runErr)

// Should: return the presets sorted by name
pty.ExpectRegexMatch(`preset-default\s+k1=v2\s+true\s+0`)
// The parameter order is not guaranteed in the output, so we match both possible orders
pty.ExpectRegexMatch(`preset-multiple-params\s+(k1=v1,k2=v2)|(k2=v2,k1=v1)\s+false\s+-`)
pty.ExpectRegexMatch(`preset-prebuilds\s+\s+false\s+2`)
})

t.Run("NoPresets", func(t *testing.T) {
t.Parallel()

client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
owner := coderdtest.CreateFirstUser(t, client)
member, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)

// Given: a template version without presets
version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, templateWithPresets([]*proto.Preset{}))
_ = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
template := coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID)

// When: listing presets for that template and template version
inv, root := clitest.New(t, "templates", "versions", "presets", "list", template.Name, version.Name)
clitest.SetupConfig(t, member, root)

ptytest.New(t).Attach(inv)
doneChan := make(chan struct{})
var runErr error
go func() {
defer close(doneChan)
runErr = inv.Run()
}()
<-doneChan

// Should return an error when no presets are found for the given template and version.
require.Error(t, runErr)
expectedErr := fmt.Sprintf(
"no presets found for template %q and template-version %q",
template.Name,
version.Name,
)
require.Contains(t, runErr.Error(), expectedErr)
})
}

func templateWithPresets(presets []*proto.Preset) *echo.Responses {
return &echo.Responses{
Parse: echo.ParseComplete,
ProvisionPlan: []*proto.Response{
{
Type: &proto.Response_Plan{
Plan: &proto.PlanComplete{
Presets: presets,
},
},
},
},
}
}
1 change: 1 addition & 0 deletions cli/templateversions.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func (r *RootCmd) templateVersions() *serpent.Command {
r.archiveTemplateVersion(),
r.unarchiveTemplateVersion(),
r.templateVersionsPromote(),
r.templateVersionPresets(),
},
}

Expand Down
1 change: 1 addition & 0 deletions cli/testdata/coder_templates_versions_--help.golden
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ USAGE:
SUBCOMMANDS:
archive Archive a template version(s).
list List all the versions of the specified template
presets Manage presets of the specified template version
promote Promote a template version to active.
unarchive Unarchive a template version(s).

Expand Down
18 changes: 18 additions & 0 deletions cli/testdata/coder_templates_versions_presets_--help.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
coder v0.0.0-devel

USAGE:
coder templates versions presets

Manage presets of the specified template version

Aliases: preset

- List presets of a specific template version:

$ coder templates versions presets list my-template my-template-version

SUBCOMMANDS:
list List all the presets of the specified template version

———
Run `coder --help` for a list of global options.
18 changes: 18 additions & 0 deletions cli/testdata/coder_templates_versions_presets_--help_--help.golden
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
coder v0.0.0-devel

USAGE:
coder templates versions presets

Manage presets of the specified template version

Aliases: preset

- List presets of a specific template version:

$ coder templates versions presets list my-template my-template-version

SUBCOMMANDS:
list List all the presets of the specified template version

———
Run `coder --help` for a list of global options.
3 changes: 3 additions & 0 deletions coderd/apidoc/docs.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions coderd/apidoc/swagger.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 13 additions & 3 deletions coderd/presets.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package coderd

import (
"database/sql"
"net/http"

"github.com/coder/coder/v2/coderd/httpapi"
Expand Down Expand Up @@ -38,12 +39,21 @@ func (api *API) templateVersionPresets(rw http.ResponseWriter, r *http.Request)
return
}

getPrebuildInstances := func(desiredInstances sql.NullInt32) *int {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this name implies fetching from somewhere, but it only does a type conversion. perhaps consider a more descriptive name?

if desiredInstances.Valid {
value := int(desiredInstances.Int32)
return &value
}
return nil
}

var res []codersdk.Preset
for _, preset := range presets {
sdkPreset := codersdk.Preset{
ID: preset.ID,
Name: preset.Name,
Default: preset.IsDefault,
ID: preset.ID,
Name: preset.Name,
Default: preset.IsDefault,
Prebuilds: getPrebuildInstances(preset.DesiredInstances),
}
for _, presetParam := range presetParams {
if presetParam.TemplateVersionPresetID != preset.ID {
Expand Down
1 change: 1 addition & 0 deletions codersdk/presets.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ type Preset struct {
Name string
Parameters []PresetParameter
Default bool
Prebuilds *int
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider clarifying the naming to communicate whether this is the desired number of prebuilds or the eligible number of prebuilds. I can see from elsewhere that its the desired number, but we've spoken in the past about showing how many prebuilds are eligible in the frontend before. Would be good to leave room for both.

}

type PresetParameter struct {
Expand Down
Loading
Loading