- Notifications
You must be signed in to change notification settings - Fork948
feat: Unify cli behavior for templates create and update#1385
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes fromall commits
File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -27,7 +27,8 @@ func templateCreate() *cobra.Command { | ||
) | ||
cmd := &cobra.Command{ | ||
Use: "create [name]", | ||
Short: "Create a template from the current directory or as specified by flag", | ||
Args: cobra.MaximumNArgs(1), | ||
RunE: func(cmd *cobra.Command, args []string) error { | ||
client, err := createClient(cmd) | ||
if err != nil { | ||
@@ -51,19 +52,10 @@ func templateCreate() *cobra.Command { | ||
return xerrors.Errorf("A template already exists named %q!", templateName) | ||
} | ||
// Confirm upload of the directory. | ||
prettyDir := prettyDirectoryPath(directory) | ||
_, err = cliui.Prompt(cmd, cliui.PromptOptions{ | ||
Text: fmt.Sprintf("Create and upload %q?",prettyDir), | ||
IsConfirm: true, | ||
Default: "yes", | ||
}) | ||
@@ -73,7 +65,7 @@ func templateCreate() *cobra.Command { | ||
spin := spinner.New(spinner.CharSets[5], 100*time.Millisecond) | ||
spin.Writer = cmd.OutOrStdout() | ||
spin.Suffix = cliui.Styles.Keyword.Render(" Uploading directory...") | ||
spin.Start() | ||
defer spin.Stop() | ||
archive, err := provisionersdk.Tar(directory, provisionersdk.TemplateArchiveLimit) | ||
@@ -123,9 +115,9 @@ func templateCreate() *cobra.Command { | ||
} | ||
currentDirectory, _ := os.Getwd() | ||
cmd.Flags().StringVarP(&directory, "directory", "d", currentDirectory, "Specify the directory to create from") | ||
cmd.Flags().StringVarP(&provisioner, "test.provisioner", "", "terraform", "Customize the provisioner backend") | ||
// This is for testing! | ||
err := cmd.Flags().MarkHidden("test.provisioner") | ||
if err != nil { | ||
panic(err) | ||
} | ||
@@ -228,3 +220,20 @@ func createValidTemplateVersion(cmd *cobra.Command, client *codersdk.Client, org | ||
return &version, parameters, nil | ||
} | ||
// prettyDirectoryPath returns a prettified path when inside the users | ||
// home directory. Falls back to dir if the users home directory cannot | ||
// discerned. This function calls filepath.Clean on the result. | ||
func prettyDirectoryPath(dir string) string { | ||
dir = filepath.Clean(dir) | ||
homeDir, err := os.UserHomeDir() | ||
if err != nil { | ||
return dir | ||
} | ||
pretty := dir | ||
if strings.HasPrefix(pretty, homeDir) { | ||
mafredri marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
pretty = strings.TrimPrefix(pretty, homeDir) | ||
pretty = "~" + pretty | ||
} | ||
return pretty | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -3,22 +3,28 @@ package cli | ||
import ( | ||
"fmt" | ||
"os" | ||
"time" | ||
"github.com/briandowns/spinner" | ||
"github.com/spf13/cobra" | ||
"golang.org/x/xerrors" | ||
"github.com/coder/coder/cli/cliui" | ||
"github.com/coder/coder/coderd/database" | ||
"github.com/coder/coder/codersdk" | ||
"github.com/coder/coder/provisionersdk" | ||
) | ||
func templateUpdate() *cobra.Command { | ||
var ( | ||
directory string | ||
provisioner string | ||
) | ||
cmd := &cobra.Command{ | ||
Use: "update <template>", | ||
Args: cobra.ExactArgs(1), | ||
Short: "Update the source-code of a template from the current directory or as specified by flag", | ||
RunE: func(cmd *cobra.Command, args []string) error { | ||
client, err := createClient(cmd) | ||
if err != nil { | ||
@@ -33,16 +39,22 @@ func templateUpdate() *cobra.Command { | ||
return err | ||
} | ||
// Confirm upload of the directory. | ||
prettyDir := prettyDirectoryPath(directory) | ||
_, err = cliui.Prompt(cmd, cliui.PromptOptions{ | ||
Text: fmt.Sprintf("Upload %q?", prettyDir), | ||
IsConfirm: true, | ||
Default: "yes", | ||
}) | ||
if err != nil { | ||
return err | ||
} | ||
spin := spinner.New(spinner.CharSets[5], 100*time.Millisecond) | ||
spin.Writer = cmd.OutOrStdout() | ||
spin.Suffix = cliui.Styles.Keyword.Render(" Uploading directory...") | ||
spin.Start() | ||
defer spin.Stop() | ||
content, err := provisionersdk.Tar(directory, provisionersdk.TemplateArchiveLimit) | ||
if err != nil { | ||
return err | ||
@@ -51,13 +63,14 @@ func templateUpdate() *cobra.Command { | ||
if err != nil { | ||
return err | ||
} | ||
spin.Stop() | ||
before := time.Now() | ||
templateVersion, err := client.CreateTemplateVersion(cmd.Context(), organization.ID, codersdk.CreateTemplateVersionRequest{ | ||
TemplateID: template.ID, | ||
StorageMethod: database.ProvisionerStorageMethodFile, | ||
StorageSource: resp.Hash, | ||
Provisioner: database.ProvisionerType(provisioner), | ||
}) | ||
if err != nil { | ||
return err | ||
@@ -71,7 +84,7 @@ func templateUpdate() *cobra.Command { | ||
if !ok { | ||
break | ||
} | ||
_, _ = fmt.Printf("%s (%s): %s\n", provisioner, log.Level, log.Output) | ||
} | ||
templateVersion, err = client.TemplateVersion(cmd.Context(), templateVersion.ID) | ||
if err != nil { | ||
@@ -92,4 +105,15 @@ func templateUpdate() *cobra.Command { | ||
return nil | ||
}, | ||
} | ||
currentDirectory, _ := os.Getwd() | ||
cmd.Flags().StringVarP(&directory, "directory", "d", currentDirectory, "Specify the directory to create from") | ||
cmd.Flags().StringVarP(&provisioner, "test.provisioner", "", "terraform", "Customize the provisioner backend") | ||
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 really like this! 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. Why is 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. Also, is it possible / advisable to allow people to switch from one provisioner to another for a template that already exists? 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. It's because this is not a flag that should be exposed to users (which is why it's hidden on the lines below). Naming it 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 mean, I get that terraform is the only "real" provisioner at the moment, but that may not be true long term, and so you'd use that flag outside of testing purposes. 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 think at that point we'd figure out how to officially support it, and not re-use this flag. | ||
// This is for testing! | ||
err := cmd.Flags().MarkHidden("test.provisioner") | ||
if err != nil { | ||
panic(err) | ||
} | ||
return cmd | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,64 @@ | ||
package cli_test | ||
import ( | ||
"context" | ||
"testing" | ||
"github.com/stretchr/testify/assert" | ||
"github.com/stretchr/testify/require" | ||
"github.com/coder/coder/cli/clitest" | ||
"github.com/coder/coder/coderd/coderdtest" | ||
"github.com/coder/coder/coderd/database" | ||
"github.com/coder/coder/codersdk" | ||
"github.com/coder/coder/provisioner/echo" | ||
"github.com/coder/coder/pty/ptytest" | ||
) | ||
func TestTemplateUpdate(t *testing.T) { | ||
t.Parallel() | ||
client := coderdtest.New(t, nil) | ||
user := coderdtest.CreateFirstUser(t, client) | ||
_ = coderdtest.NewProvisionerDaemon(t, client) | ||
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil) | ||
_ = coderdtest.AwaitTemplateVersionJob(t, client, version.ID) | ||
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) | ||
// Test the cli command. | ||
source := clitest.CreateTemplateVersionSource(t, &echo.Responses{ | ||
Parse: echo.ParseComplete, | ||
Provision: echo.ProvisionComplete, | ||
}) | ||
cmd, root := clitest.New(t, "templates", "update", template.Name, "--directory", source, "--test.provisioner", string(database.ProvisionerTypeEcho)) | ||
clitest.SetupConfig(t, client, root) | ||
pty := ptytest.New(t) | ||
cmd.SetIn(pty.Input()) | ||
cmd.SetOut(pty.Output()) | ||
execDone := make(chan error) | ||
go func() { | ||
execDone <- cmd.Execute() | ||
}() | ||
matches := []struct { | ||
match string | ||
write string | ||
}{ | ||
{match: "Upload", write: "yes"}, | ||
} | ||
for _, m := range matches { | ||
pty.ExpectMatch(m.match) | ||
pty.WriteLine(m.write) | ||
} | ||
require.NoError(t, <-execDone) | ||
// Assert that the template version changed. | ||
templateVersions, err := client.TemplateVersionsByTemplate(context.Background(), codersdk.TemplateVersionsByTemplateRequest{ | ||
TemplateID: template.ID, | ||
}) | ||
require.NoError(t, err) | ||
assert.Len(t, templateVersions, 2) | ||
mafredri marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading.Please reload this page. | ||
assert.NotEqual(t, template.ActiveVersionID, templateVersions[1].ID) | ||
} |