diff --git a/ci/integration/envs_test.go b/ci/integration/envs_test.go index 2f3c19c8..a4f75480 100644 --- a/ci/integration/envs_test.go +++ b/ci/integration/envs_test.go @@ -9,7 +9,7 @@ import ( ) // From Coder organization images -const ubuntuImgID = "5f443b16-30652892427b955601330fa5" +// const ubuntuImgID = "5f443b16-30652892427b955601330fa5" func TestEnvsCLI(t *testing.T) { t.Parallel() @@ -37,6 +37,18 @@ func TestEnvsCLI(t *testing.T) { tcli.StderrEmpty(), ) + // Image unset. + c.Run(ctx, "coder envs create test-env").Assert(t, + tcli.StderrMatches(regexp.QuoteMeta("fatal: required flag(s) \"image\" not set")), + tcli.Error(), + ) + + // Image not imported. + c.Run(ctx, "coder envs create test-env --image doesntmatter").Assert(t, + tcli.StderrMatches(regexp.QuoteMeta("fatal: image not found - did you forget to import this image?")), + tcli.Error(), + ) + // TODO(Faris) : uncomment this when we can safely purge the environments // the integrations tests would create in the sidecar // Successfully create environment. @@ -46,12 +58,6 @@ func TestEnvsCLI(t *testing.T) { // tcli.StderrMatches(regexp.QuoteMeta("Successfully created environment \"test-ubuntu\"")), // ) - // Invalid environment name should fail. - c.Run(ctx, "coder envs create --image "+ubuntuImgID+" this-IS-an-invalid-EnvironmentName").Assert(t, - tcli.Error(), - tcli.StderrMatches(regexp.QuoteMeta("environment name must conform to regex ^[a-z0-9]([a-z0-9-]+)?")), - ) - // TODO(Faris) : uncomment this when we can safely purge the environments // the integrations tests would create in the sidecar // Successfully provision environment with fractional resource amounts @@ -59,11 +65,5 @@ func TestEnvsCLI(t *testing.T) { // tcli.Success(), // tcli.StderrMatches(regexp.QuoteMeta("Successfully created environment \"non-whole-resource-amounts\"")), // ) - - // Image does not exist should fail. - c.Run(ctx, "coder envs create --image does-not-exist env-will-not-be-created").Assert(t, - tcli.Error(), - tcli.StderrMatches(regexp.QuoteMeta("does not exist")), - ) }) } diff --git a/coder-sdk/image.go b/coder-sdk/image.go index ba59e8bf..c9122cc9 100644 --- a/coder-sdk/image.go +++ b/coder-sdk/image.go @@ -13,7 +13,7 @@ type Image struct { Description string `json:"description"` URL string `json:"url"` // User-supplied URL for image. DefaultCPUCores float32 `json:"default_cpu_cores"` - DefaultMemoryGB int `json:"default_memory_gb"` + DefaultMemoryGB float32 `json:"default_memory_gb"` DefaultDiskGB int `json:"default_disk_gb"` Deprecated bool `json:"deprecated"` } @@ -47,3 +47,12 @@ func (c Client) ImportImage(ctx context.Context, orgID string, req ImportImageRe } return &img, nil } + +// OrganizationImages returns all of the images imported for orgID. +func (c Client) OrganizationImages(ctx context.Context, orgID string) ([]Image, error) { + var imgs []Image + if err := c.requestBody(ctx, http.MethodGet, "/api/orgs/"+orgID+"/images", nil, &imgs); err != nil { + return nil, err + } + return imgs, nil +} diff --git a/internal/clog/error.go b/internal/clog/error.go index 66d28ec1..d27a583a 100644 --- a/internal/clog/error.go +++ b/internal/clog/error.go @@ -68,6 +68,16 @@ func LogSuccess(header string, lines ...string) { }.String()) } +// LogWarn prints the given warn message to stderr. +func LogWarn(header string, lines ...string) { + fmt.Fprint(os.Stderr, CLIMessage{ + Level: "warning", + Color: color.FgYellow, + Header: header, + Lines: lines, + }.String()) +} + // Warn creates an error with the level "warning". func Warn(header string, lines ...string) CLIError { return CLIError{ diff --git a/internal/cmd/ceapi.go b/internal/cmd/ceapi.go index 5c6297ad..e453cbae 100644 --- a/internal/cmd/ceapi.go +++ b/internal/cmd/ceapi.go @@ -3,6 +3,7 @@ package cmd import ( "context" "fmt" + "strings" "cdr.dev/coder-cli/coder-sdk" "cdr.dev/coder-cli/internal/clog" @@ -81,3 +82,111 @@ func findEnv(ctx context.Context, client *coder.Client, envName, userEmail strin clog.Tipf("run \"coder envs ls\" to view your environments"), ) } + +type findImgConf struct { + client *coder.Client + email string + imgName string + orgName string +} + +func findImg(ctx context.Context, conf findImgConf) (*coder.Image, error) { + switch { + case conf.email == "": + return nil, xerrors.New("user email unset") + case conf.imgName == "": + return nil, xerrors.New("image name unset") + } + + imgs, err := getImgs(ctx, + getImgsConf{ + client: conf.client, + email: conf.email, + orgName: conf.orgName, + }, + ) + if err != nil { + return nil, err + } + + var possibleMatches []coder.Image + + // The user may provide an image thats not an exact match + // to one of their imported images but they may be close. + // We can assist the user by collecting images that contain + // the user provided image flag value as a substring. + for _, img := range imgs { + // If it's an exact match we can just return and exit. + if img.Repository == conf.imgName { + return &img, nil + } + if strings.Contains(img.Repository, conf.imgName) { + possibleMatches = append(possibleMatches, img) + } + } + + if len(possibleMatches) == 0 { + return nil, xerrors.New("image not found - did you forget to import this image?") + } + + lines := []string{clog.Tipf("Did you mean?")} + + for _, img := range possibleMatches { + lines = append(lines, img.Repository) + } + return nil, clog.Fatal( + fmt.Sprintf("Found %d possible matches for %q.", len(possibleMatches), conf.imgName), + lines..., + ) +} + +type getImgsConf struct { + client *coder.Client + email string + orgName string +} + +func getImgs(ctx context.Context, conf getImgsConf) ([]coder.Image, error) { + u, err := conf.client.UserByEmail(ctx, conf.email) + if err != nil { + return nil, err + } + + orgs, err := conf.client.Organizations(ctx) + if err != nil { + return nil, err + } + + orgs = lookupUserOrgs(u, orgs) + + for _, org := range orgs { + imgs, err := conf.client.OrganizationImages(ctx, org.ID) + if err != nil { + return nil, err + } + // If orgName is set we know the user is a multi-org member + // so we should only return the imported images that beong to the org they specified. + if conf.orgName != "" && conf.orgName == org.Name { + return imgs, nil + } + + if conf.orgName == "" { + // if orgName is unset we know the user is only part of one org. + return imgs, nil + } + } + return nil, xerrors.Errorf("org name %q not found", conf.orgName) +} + +func isMultiOrgMember(ctx context.Context, client *coder.Client, email string) (bool, error) { + u, err := client.UserByEmail(ctx, email) + if err != nil { + return false, xerrors.New("email not found") + } + + orgs, err := client.Organizations(ctx) + if err != nil { + return false, err + } + return len(lookupUserOrgs(u, orgs)) > 1, nil +} diff --git a/internal/cmd/envs.go b/internal/cmd/envs.go index c746d23f..60a52074 100644 --- a/internal/cmd/envs.go +++ b/internal/cmd/envs.go @@ -1,6 +1,7 @@ package cmd import ( + "context" "encoding/json" "fmt" "os" @@ -9,20 +10,14 @@ import ( "cdr.dev/coder-cli/coder-sdk" "cdr.dev/coder-cli/internal/clog" "cdr.dev/coder-cli/internal/x/xtabwriter" + "github.com/manifoldco/promptui" "github.com/spf13/cobra" "golang.org/x/sync/errgroup" "golang.org/x/xerrors" ) -const ( - defaultOrg = "default" - defaultImgTag = "latest" - defaultCPUCores float32 = 1 - defaultMemGB float32 = 1 - defaultDiskGB = 10 - defaultGPUs = 0 -) +const defaultImgTag = "latest" func envsCommand() *cobra.Command { var user string @@ -39,7 +34,7 @@ func envsCommand() *cobra.Command { rmEnvsCommand(&user), watchBuildLogCommand(), rebuildEnvCommand(), - createEnvCommand(), + createEnvCommand(&user), editEnvCommand(&user), ) return cmd @@ -147,7 +142,7 @@ coder envs --user charlie@coder.com ls -o json \ } } -func createEnvCommand() *cobra.Command { +func createEnvCommand(user *string) *cobra.Command { var ( org string img string @@ -169,27 +164,61 @@ coder envs create --image 5f443b16-30652892427b955601330fa5 my-env-name coder envs create --cpu 4 --disk 100 --memory 8 --image 5f443b16-30652892427b955601330fa5 my-env-name`, RunE: func(cmd *cobra.Command, args []string) error { if img == "" { - return xerrors.New("image id unset") + return xerrors.New("image unset") } + + client, err := newClient() + if err != nil { + return err + } + + multiOrgMember, err := isMultiOrgMember(cmd.Context(), client, *user) + if err != nil { + return err + } + + if multiOrgMember && org == "" { + return xerrors.New("org is required for multi-org members") + } + + importedImg, err := findImg(cmd.Context(), + findImgConf{ + client: client, + email: *user, + imgName: img, + orgName: org, + }, + ) + if err != nil { + return err + } + // ExactArgs(1) ensures our name value can't panic on an out of bounds. createReq := &coder.CreateEnvironmentRequest{ Name: args[0], - ImageID: img, + ImageID: importedImg.ID, ImageTag: tag, } - // We're explicitly ignoring errors for these because all of these flags - // have a non-zero-value default value set already. + // We're explicitly ignoring errors for these because all we + // need to now is if the numeric type is 0 or not. createReq.CPUCores, _ = cmd.Flags().GetFloat32("cpu") createReq.MemoryGB, _ = cmd.Flags().GetFloat32("memory") createReq.DiskGB, _ = cmd.Flags().GetInt("disk") createReq.GPUs, _ = cmd.Flags().GetInt("gpus") - client, err := newClient() - if err != nil { - return err + // if any of these defaulted to their zero value we provision + // the create request with the imported image defaults instead. + if createReq.CPUCores == 0 { + createReq.CPUCores = importedImg.DefaultCPUCores + } + if createReq.MemoryGB == 0 { + createReq.MemoryGB = importedImg.DefaultMemoryGB + } + if createReq.DiskGB == 0 { + createReq.DiskGB = importedImg.DefaultDiskGB } - env, err := client.CreateEnvironment(cmd.Context(), org, *createReq) + env, err := client.CreateEnvironment(cmd.Context(), importedImg.OrganizationID, *createReq) if err != nil { return xerrors.Errorf("create environment: %w", err) } @@ -209,13 +238,13 @@ coder envs create --cpu 4 --disk 100 --memory 8 --image 5f443b16-30652892427b955 return nil }, } - cmd.Flags().StringVarP(&org, "org", "o", defaultOrg, "ID of the organization the environment should be created under.") + cmd.Flags().StringVarP(&org, "org", "o", "", "ID of the organization the environment should be created under.") cmd.Flags().StringVarP(&tag, "tag", "t", defaultImgTag, "tag of the image the environment will be based off of.") - cmd.Flags().Float32P("cpu", "c", defaultCPUCores, "number of cpu cores the environment should be provisioned with.") - cmd.Flags().Float32P("memory", "m", defaultMemGB, "GB of RAM an environment should be provisioned with.") - cmd.Flags().IntP("disk", "d", defaultDiskGB, "GB of disk storage an environment should be provisioned with.") - cmd.Flags().IntP("gpus", "g", defaultGPUs, "number GPUs an environment should be provisioned with.") - cmd.Flags().StringVarP(&img, "image", "i", "", "ID of the image to base the environment off of.") + cmd.Flags().Float32P("cpu", "c", 0, "number of cpu cores the environment should be provisioned with.") + cmd.Flags().Float32P("memory", "m", 0, "GB of RAM an environment should be provisioned with.") + cmd.Flags().IntP("disk", "d", 0, "GB of disk storage an environment should be provisioned with.") + cmd.Flags().IntP("gpus", "g", 0, "number GPUs an environment should be provisioned with.") + cmd.Flags().StringVarP(&img, "image", "i", "", "name of the image to base the environment off of.") cmd.Flags().BoolVar(&follow, "follow", false, "follow buildlog after initiating rebuild") _ = cmd.MarkFlagRequired("image") return cmd @@ -223,6 +252,7 @@ coder envs create --cpu 4 --disk 100 --memory 8 --image 5f443b16-30652892427b955 func editEnvCommand(user *string) *cobra.Command { var ( + org string img string tag string cpuCores float32 @@ -242,13 +272,6 @@ func editEnvCommand(user *string) *cobra.Command { coder envs edit back-end-env --disk 20`, RunE: func(cmd *cobra.Command, args []string) error { - // We're explicitly ignoring these errors because if any of these - // fail we are left with the zero value for the corresponding numeric type. - cpuCores, _ = cmd.Flags().GetFloat32("cpu") - memGB, _ = cmd.Flags().GetFloat32("memory") - diskGB, _ = cmd.Flags().GetInt("disk") - gpus, _ = cmd.Flags().GetInt("gpus") - client, err := newClient() if err != nil { return err @@ -261,48 +284,41 @@ coder envs edit back-end-env --disk 20`, return err } - var updateReq coder.UpdateEnvironmentReq - - // If any of the flags have defaulted to zero-values, it implies the user does not wish to change that value. - // With that said, we can enforce this by reassigning the request field to the corresponding existing environment value. - if cpuCores == 0 { - updateReq.CPUCores = &env.CPUCores - } else { - updateReq.CPUCores = &cpuCores - } - - if memGB == 0 { - updateReq.MemoryGB = &env.MemoryGB - } else { - updateReq.MemoryGB = &memGB - } - - if diskGB == 0 { - updateReq.DiskGB = &env.DiskGB - } else { - updateReq.DiskGB = &diskGB + multiOrgMember, err := isMultiOrgMember(cmd.Context(), client, *user) + if err != nil { + return err } - if gpus == 0 { - updateReq.GPUs = &env.GPUs - } else { - updateReq.GPUs = &gpus + // if the user belongs to multiple organizations we need them to specify which one. + if multiOrgMember && org == "" { + return xerrors.New("org is required for multi-org members") } - if img == "" { - updateReq.ImageID = &env.ImageID - } else { - updateReq.ImageID = &img - } + cpuCores, _ = cmd.Flags().GetFloat32("cpu") + memGB, _ = cmd.Flags().GetFloat32("memory") + diskGB, _ = cmd.Flags().GetInt("disk") + gpus, _ = cmd.Flags().GetInt("gpus") - if tag == "" { - updateReq.ImageTag = &env.ImageTag - } else { - updateReq.ImageTag = &tag + req, err := buildUpdateReq(cmd.Context(), + updateConf{ + cpu: cpuCores, + memGB: memGB, + diskGB: diskGB, + gpus: gpus, + client: client, + environment: env, + user: user, + image: img, + imageTag: tag, + orgName: org, + }, + ) + if err != nil { + return err } - if err := client.EditEnvironment(cmd.Context(), env.ID, updateReq); err != nil { - return xerrors.Errorf("failed to apply changes to environment: '%s'", envName) + if err := client.EditEnvironment(cmd.Context(), env.ID, *req); err != nil { + return xerrors.Errorf("failed to apply changes to environment %q: %w", envName, err) } if follow { @@ -320,7 +336,8 @@ coder envs edit back-end-env --disk 20`, return nil }, } - cmd.Flags().StringVarP(&img, "image", "i", "", "image ID of the image you wan't the environment to be based off of.") + cmd.Flags().StringVarP(&org, "org", "o", "", "name of the organization the environment should be created under.") + cmd.Flags().StringVarP(&img, "image", "i", "", "name of the image you wan't the environment to be based off of.") cmd.Flags().StringVarP(&tag, "tag", "t", "latest", "image tag of the image you wan't to base the environment off of.") cmd.Flags().Float32P("cpu", "c", cpuCores, "The number of cpu cores the environment should be provisioned with.") cmd.Flags().Float32P("memory", "m", memGB, "The amount of RAM an environment should be provisioned with.") @@ -387,3 +404,110 @@ func rmEnvsCommand(user *string) *cobra.Command { cmd.Flags().BoolVarP(&force, "force", "f", false, "force remove the specified environments without prompting first") return cmd } + +type updateConf struct { + cpu float32 + memGB float32 + diskGB int + gpus int + client *coder.Client + environment *coder.Environment + user *string + image string + imageTag string + orgName string +} + +func buildUpdateReq(ctx context.Context, conf updateConf) (*coder.UpdateEnvironmentReq, error) { + var ( + updateReq coder.UpdateEnvironmentReq + defaultCPUCores float32 + defaultMemGB float32 + defaultDiskGB int + ) + + // If this is not empty it means the user is requesting to change the environment image. + if conf.image != "" { + importedImg, err := findImg(ctx, + findImgConf{ + client: conf.client, + email: *conf.user, + imgName: conf.image, + orgName: conf.orgName, + }, + ) + if err != nil { + return nil, err + } + + // If the user passes an image arg of the image that + // the environment is already using, it was most likely a mistake. + if conf.image != importedImg.Repository { + return nil, xerrors.Errorf("environment is already using image %q", conf.image) + } + + // Since the environment image is being changed, + // the resource amount defaults should be changed to + // reflect that of the default resource amounts of the new image. + defaultCPUCores = importedImg.DefaultCPUCores + defaultMemGB = importedImg.DefaultMemoryGB + defaultDiskGB = importedImg.DefaultDiskGB + updateReq.ImageID = &importedImg.ID + } else { + // if the environment image is not being changed, the default + // resource amounts should reflect the default resource amounts + // of the image the environment is already using. + defaultCPUCores = conf.environment.CPUCores + defaultMemGB = conf.environment.MemoryGB + defaultDiskGB = conf.environment.DiskGB + updateReq.ImageID = &conf.environment.ImageID + } + + // The following logic checks to see if the user specified + // any resource amounts for the environment that need to be changed. + // If they did not, then we will get the zero value back + // and should set the resource amount to the default. + + if conf.cpu == 0 { + updateReq.CPUCores = &defaultCPUCores + } else { + updateReq.CPUCores = &conf.cpu + } + + if conf.memGB == 0 { + updateReq.MemoryGB = &defaultMemGB + } else { + updateReq.MemoryGB = &conf.memGB + } + + if conf.diskGB == 0 { + updateReq.DiskGB = &defaultDiskGB + } else { + updateReq.DiskGB = &conf.diskGB + } + + // Environment disks can not be shrink so we have to overwrite this + // if the user accidentally requests it or if the default diskGB value for a + // newly requested image is smaller than the current amount the environment is using. + if *updateReq.DiskGB < conf.environment.DiskGB { + clog.LogWarn("disk can not be shrunk", + fmt.Sprintf("keeping environment disk at %d GB", conf.environment.DiskGB), + ) + updateReq.DiskGB = &conf.environment.DiskGB + } + + if conf.gpus != 0 { + updateReq.GPUs = &conf.gpus + } + + if conf.imageTag == "" { + // We're forced to make an alloc here because untyped string consts are not addressable. + // i.e. updateReq.ImageTag = &defaultImgTag results in : + // invalid operation: cannot take address of defaultImgTag (untyped string constant "latest") + imgTag := defaultImgTag + updateReq.ImageTag = &imgTag + } else { + updateReq.ImageTag = &conf.imageTag + } + return &updateReq, nil +}