diff --git a/internal/pkg/cli/deploy.go b/internal/pkg/cli/deploy.go index 8224ce92d9c..4b21cc3fe9d 100644 --- a/internal/pkg/cli/deploy.go +++ b/internal/pkg/cli/deploy.go @@ -6,11 +6,17 @@ package cli import ( "errors" "fmt" + "github.com/aws/copilot-cli/internal/pkg/queue" + "math" + "slices" + "strconv" + "strings" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ssm" + "github.com/dustin/go-humanize/english" "github.com/spf13/afero" "github.com/spf13/cobra" - "slices" "github.com/aws/copilot-cli/cmd/copilot/template" "github.com/aws/copilot-cli/internal/pkg/aws/identity" @@ -36,14 +42,62 @@ const ( jobWkldType = "job" ) +// filterItemsByStrings is a generic function to return the subset of wantedStrings that exists in possibleItems. +// stringFunc is a method to convert the generic item type (T) to a string; for example, one can convert a struct of type +// *config.Workload to a string by passing +// +// func(w *config.Workload) string { return w.Name }. +// +// Likewise, filterItemsByStrings can work on a list of strings by returning the unmodified item: +// +// filterItemsByStrings(wantedStrings, stringSlice2, func(s string) string { return s }) +// +// It returns a list of strings (items whose stringFunc() exists in the list of wantedStrings). +func filterItemsByStrings[T any](wantedStrings []string, possibleItems []T, stringFunc func(T) string) []string { + m := make(map[string]bool) + for _, item := range wantedStrings { + m[item] = true + } + res := make([]string, 0, len(wantedStrings)) + for _, item := range possibleItems { + if m[stringFunc(item)] { + res = append(res, stringFunc(item)) + } + } + return res +} + +// filterOutItems is a generic function to return the subset of allItems which does not include the items specified in +// unwantedItems. stringFunc is a function which maps the unwantedItem type T to a string value. For example, one can +// convert a struct of type *config.Workload to a string by passing +// +// func(w *config.Workload) string { return w.Name } +// +// as the stringFunc parameter. +func filterOutItems[T any](allItems []string, unwantedItems []T, stringFunc func(T) string) []string { + isUnwanted := make(map[string]bool) + for _, item := range unwantedItems { + isUnwanted[stringFunc(item)] = true + } + var filtered []string + for _, str := range allItems { + if isUnwanted[str] { + continue + } + filtered = append(filtered, str) + } + return filtered +} + type deployVars struct { deployWkldVars - workloadNames []string + workloadNames []string + deployAllWorkloads bool + yesInitWkld bool - yesInitWkld *bool - deployEnv *bool - yesInitEnv *bool + deployEnv *bool + yesInitEnv *bool region string tempCreds tempCredsVars @@ -72,7 +126,10 @@ type deployOpts struct { envExistsInWs bool // Cached variables - wsEnvironments []string + wsEnvironments []string + wsWorkloads []string + storeWorkloads []*config.Workload + initializedWsWorkloads []string } func newDeployOpts(vars deployVars) (*deployOpts, error) { @@ -150,7 +207,7 @@ func newDeployOpts(vars deployVars) (*deployOpts, error) { } opts.name = workloadName return opts, nil - case slices.Contains(manifestinfo.JobTypes(), workloadType): + case slices.Contains(manifestinfo.ServiceTypes(), workloadType): opts := &deploySvcOpts{ deployWkldVars: o.deployWkldVars, @@ -169,6 +226,11 @@ func newDeployOpts(vars deployVars) (*deployOpts, error) { return newSvcDeployer(opts) } opts.name = workloadName + // Multi-deployments can have flags specified which are not compatible with all service types. + // Currently only forceNewUpdate is incompatible with Static Site types. + if workloadType == manifestinfo.StaticSiteType { + opts.forceNewUpdate = false + } return opts, nil } return nil, fmt.Errorf("unrecognized workload type %s", workloadType) @@ -176,19 +238,20 @@ func newDeployOpts(vars deployVars) (*deployOpts, error) { }, nil } +// maybeInitWkld decides whether a workload needs to be initialized before deployment. +// We do not prompt for workload initialization; specifying the workload by name suffices +// to convey the customer intention. When the customer specifies --all and --init-wkld, +// we will add all un-initialized local workloads to the list to be deployed. +// When the customer does not specify --init-wkld with --all, we will only deploy initialized workloads. func (o *deployOpts) maybeInitWkld(name string) error { // Confirm that the workload needs to be initialized after asking for the name. - initializedWorkloads, err := o.store.ListWorkloads(o.appName) + initializedWorkloads, err := o.listInitializedLocalWorkloads() if err != nil { - return fmt.Errorf("retrieve workloads: %w", err) - } - wlNames := make([]string, len(initializedWorkloads)) - for i := range initializedWorkloads { - wlNames[i] = initializedWorkloads[i].Name + return err } // Workload is already initialized. Return early. - if slices.Contains(wlNames, name) { + if slices.Contains(initializedWorkloads, name) { return nil } @@ -206,27 +269,185 @@ func (o *deployOpts) maybeInitWkld(name string) error { return fmt.Errorf("unrecognized workload type %q in manifest for workload %s", workloadType, name) } - if o.yesInitWkld == nil { - confirmInitWkld, err := o.prompt.Confirm(fmt.Sprintf("Found manifest for uninitialized %s %q. Initialize it?", workloadType, name), "This workload will be initialized, then deployed.", prompt.WithFinalMessage(fmt.Sprintf("Initialize %s:", workloadType))) - if err != nil { - return fmt.Errorf("confirm initialize workload: %w", err) + wkldAdder := o.newWorkloadAdder() + if err = wkldAdder.AddWorkloadToApp(o.appName, name, workloadType); err != nil { + return fmt.Errorf("add workload to app: %w", err) + } + return nil +} + +type workloadPriority struct { + priority int + workloads []string +} + +func (w workloadPriority) LessThan(a workloadPriority) bool { + return w.priority < a.priority +} + +// parseDeploymentOrderTags takes a list of workload names, optionally tagged with a priority. Lower priorities will be +// deployed first. +// +// []string{"fe/1", "be/1", "worker/2"} +// +// It returns a map from the workload name to its priority, and errors out if the order tag is incorrectly formatted. +// +// map[string]int{"fe": 1, "be": 1, "worker": 2} +// +// It is possible to have multiple workloads of the same priority. We don't guarantee that workloads within priority +// groups will have identical deployment orders each time. +func parseDeploymentOrderTags(namesWithOptionalOrder []string) (map[string]int, error) { + prioritiesMap := make(map[string]int) + // First pass through flags to identify priority groups + for _, wkldName := range namesWithOptionalOrder { + parts := strings.Split(wkldName, "/") + // If there's no priority tag, deploy this service after everything else. Signify this with math.MaxInt priority. + // If there's a valid tag, add it to the map of priorities. + if len(parts) == 1 { + prioritiesMap[parts[0]] = math.MaxInt + } else if len(parts) == 2 { + order, err := strconv.Atoi(parts[1]) + if err != nil { + return nil, fmt.Errorf("parse deployment order for workloads: %w", err) + } + prioritiesMap[parts[0]] = order + } else { + return nil, fmt.Errorf("invalid deployment order for workload %s", wkldName) } - o.yesInitWkld = aws.Bool(confirmInitWkld) } + return prioritiesMap, nil +} - if !aws.BoolValue(o.yesInitWkld) { - return fmt.Errorf("workload %s is uninitialized but --%s=false was specified", name, yesInitWorkloadFlag) +// getDeploymentOrder parses names and tags from the --name flag, respecting "all". +// It returns an ordered list of which workloads should be deployed and when. +// For example, when a customer specifies "fe/2,be/1,worker,job" and the --all flag +// in a workspace where there also exists a `db` service, this function will return +// +// [][]string{ {"be"}, {"fe"}, {"worker", "job", "db"} }. +// +// TODO: when there's a dependsOn field in the manifest, we should modify this function to respect it. +func (o *deployOpts) getDeploymentOrder() ([][]string, error) { + // Get a map from workload name to deployment priority + prioritiesMap, err := parseDeploymentOrderTags(o.workloadNames) + if err != nil { + return nil, err } - wkldAdder := o.newWorkloadAdder() - if err = wkldAdder.AddWorkloadToApp(o.appName, name, workloadType); err != nil { - return fmt.Errorf("add workload to app: %w", err) + // Iterate over priority map to invert it and get groups of workloads with the same priority. + groupsMap := make(map[int][]string) + for k, v := range prioritiesMap { + groupsMap[v] = append(groupsMap[v], k) } - return nil + if len(groupsMap) == 0 { + return nil, errors.New("generate deployment groups: no workloads were specified") + } + + // If --all is specified, we need to add the remainder of workloads with math.MaxInt priority according to whether or not --init-wkld is specified. + if o.deployAllWorkloads { + specifiedWorkloadList := make([]string, 0, len(prioritiesMap)) + for k := range prioritiesMap { + specifiedWorkloadList = append(specifiedWorkloadList, k) + } + + if o.yesInitWkld { + // Add all unspecified local workloads to the list of workloads to be deployed. + localWorkloads, err := o.listLocalWorkloads() + if err != nil { + return nil, err + } + workloadsToAppend := filterOutItems(localWorkloads, specifiedWorkloadList, func(s string) string { return s }) + if len(workloadsToAppend) != 0 { + groupsMap[math.MaxInt] = append(groupsMap[math.MaxInt], workloadsToAppend...) + } + } else { + // Otherwise (--init-wkld is false): get only get initialized local workloads. + initializedWorkloads, err := o.listInitializedLocalWorkloads() + if err != nil { + return nil, err + } + workloadsToAppend := filterOutItems(initializedWorkloads, specifiedWorkloadList, func(s string) string { return s }) + if len(workloadsToAppend) != 0 { + groupsMap[math.MaxInt] = append(groupsMap[math.MaxInt], workloadsToAppend...) + } + } + } + + deploymentGroups := queue.NewPriorityQueue[workloadPriority]() + for k, v := range groupsMap { + deploymentGroups.Push(workloadPriority{ + priority: k, + workloads: v, + }) + } + + res := make([][]string, 0, deploymentGroups.Len()) + for deploymentGroups.Len() > 0 { + v, ok := deploymentGroups.Pop() + if !ok || v == nil { + continue + } + res = append(res, v.workloads) + } + return res, nil +} + +func (o *deployOpts) listStoreWorkloads() ([]*config.Workload, error) { + if o.storeWorkloads != nil { + return o.storeWorkloads, nil + } + wls, err := o.store.ListWorkloads(o.appName) + if err != nil { + return nil, fmt.Errorf("retrieve store workloads: %w", err) + } + o.storeWorkloads = wls + return o.storeWorkloads, nil +} + +func (o *deployOpts) listLocalWorkloads() ([]string, error) { + if o.wsWorkloads != nil { + return o.wsWorkloads, nil + } + localWorkloads, err := o.ws.ListWorkloads() + if err != nil { + return nil, fmt.Errorf("retrieve workspace workloads: %w", err) + } + o.wsWorkloads = localWorkloads + + return o.wsWorkloads, nil +} + +func (o *deployOpts) listInitializedLocalWorkloads() ([]string, error) { + if o.initializedWsWorkloads != nil { + return o.initializedWsWorkloads, nil + } + storeWls, err := o.listStoreWorkloads() + if err != nil { + return nil, err + } + localWorkloads, err := o.listLocalWorkloads() + if err != nil { + return nil, err + } + + o.initializedWsWorkloads = filterItemsByStrings(localWorkloads, storeWls, func(workload *config.Workload) string { return workload.Name }) + return o.initializedWsWorkloads, nil +} + +type workloadCommand struct { + actionCommand + name string +} + +func getTotalNumberOfWorkloads(deploymentGroups [][]workloadCommand) int { + var count int + for i := 0; i < len(deploymentGroups); i++ { + count += len(deploymentGroups[i]) + } + return count } func (o *deployOpts) Run() error { - if err := o.askName(); err != nil { + if err := o.askNames(); err != nil { return err } @@ -246,40 +467,103 @@ func (o *deployOpts) Run() error { return err } - for _, workload := range o.workloadNames { - if err := o.maybeInitWkld(workload); err != nil { - return err - } - deployCmd, err := o.loadWkldCmd(workload) - if err != nil { - return err - } - if err := deployCmd.Ask(); err != nil { - return fmt.Errorf("ask %s deploy: %w", o.wlType, err) - } - if err := deployCmd.Validate(); err != nil { - return fmt.Errorf("validate %s deploy: %w", o.wlType, err) - } - if err := deployCmd.Execute(); err != nil { - return fmt.Errorf("execute %s deploy: %w", o.wlType, err) + deploymentOrderGroups, err := o.getDeploymentOrder() + if err != nil { + return err + } + + cmds := make([][]workloadCommand, len(deploymentOrderGroups)) + // Do all our asking before executing deploy commands. + // Also initialize workloads that need initialization (if they're specified by name and un-initialized, + // it means the customer probably wants to init them). + log.Infoln("Checking for all required information. We may ask you some questions.") + for order, deploymentGroup := range deploymentOrderGroups { + for _, workload := range deploymentGroup { + // 1. Decide whether the current workload needs initialization. + if err := o.maybeInitWkld(workload); err != nil { + return err + } + // 2. Set up workload command. + deployCmd, err := o.loadWkldCmd(workload) + if err != nil { + return err + } + + cmds[order] = append(cmds[order], workloadCommand{ + name: workload, + actionCommand: deployCmd, + }) + // 3. Ask() and Validate() for required info. + if err := deployCmd.Ask(); err != nil { + return fmt.Errorf("ask %s deploy: %w", o.wlType, err) + } + if err := deployCmd.Validate(); err != nil { + return fmt.Errorf("validate %s deploy: %w", o.wlType, err) + } } - if err := deployCmd.RecommendActions(); err != nil { - return err + } + + if count := getTotalNumberOfWorkloads(cmds); count > 1 { + logDeploymentOrderInfo(cmds, count) + } + + for g, deploymentGroup := range cmds { + // TODO parallelize this. Steps involve: + // 1. Modify the cmd to optionally disable progress tracker + // 2. Modify labeledSyncBuffer so it can display a spinner. + // 3. Wrap Execute() in a goroutine with ErrorGroup and context + for i, cmd := range deploymentGroup { + if err := cmd.Execute(); err != nil { + var errNoInfraChanges *errNoInfrastructureChanges + if !errors.As(err, &errNoInfraChanges) { + return fmt.Errorf("execute deployment %d of %d in group %d: %w", i+1, len(deploymentGroup), g+1, err) + } + // Don't run recommended actions if there is an infra error. It's possible for RecommendActions to panic + // when it returns an error (the actionRecommender return is nil in error cases and the struct member + // is never set. + continue + } + if err := cmd.RecommendActions(); err != nil { + return err + } } } return nil } -func (o *deployOpts) askName() error { +func logDeploymentOrderInfo(cmds [][]workloadCommand, totalCount int) { + log.Infof("Will deploy %d %s in the following order.\n", totalCount, english.PluralWord(totalCount, "workload", "")) + for i := 0; i < len(cmds); i++ { + names := make([]string, 0, len(cmds)) + for _, cmd := range cmds[i] { + names = append(names, cmd.name) + } + log.Infof("%d. %s\n", i+1, strings.Join(names, ", ")) + } +} + +func (o *deployOpts) askNames() error { if o.workloadNames != nil || len(o.workloadNames) != 0 { return nil } - name, err := o.sel.Workload("Select a service or job in your workspace", "") + + if o.deployAllWorkloads { + // --all and --init-wkld=true, means we should use ALL local workloads as our list of names. + if o.yesInitWkld { + o.workloadNames = o.wsWorkloads + return nil + } + // --all and --init-wkld=false means we should only use the initialized local workloads. + o.workloadNames = o.initializedWsWorkloads + return nil + } + + names, err := o.sel.Workloads("Select one or more services or jobs in your workspace.", "") if err != nil { return fmt.Errorf("select service or job: %w", err) } - o.workloadNames = []string{name} + o.workloadNames = names return nil } @@ -445,24 +729,28 @@ func (o *deployOpts) loadWkldCmd(name string) (actionCommand, error) { // BuildDeployCmd is the deploy command. func BuildDeployCmd() *cobra.Command { vars := deployVars{} - var initWorkload bool var initEnvironment bool var deployEnvironment bool - var name string cmd := &cobra.Command{ Use: "deploy", - Short: "Deploy a Copilot job or service.", - Long: "Deploy a Copilot job or service.", + Short: "Deploy one or more Copilot jobs or services.", + Long: "Deploy one or more Copilot jobs or services.", Example: ` Deploys a service named "frontend" to a "test" environment. - /code $ copilot deploy --name frontend --env test --deploy-env=false + /code $ copilot deploy --name frontend --env test Deploys a job named "mailer" with additional resource tags to a "prod" environment. /code $ copilot deploy -n mailer -e prod --resource-tags source/revision=bb133e7,deployment/initiator=manual --deploy-env=false Initializes and deploys an environment named "test" in us-west-2 under the "default" profile with local manifest, then deploys a service named "api" /code $ copilot deploy --init-env --deploy-env --env test --name api --profile default --region us-west-2 Initializes and deploys a service named "backend" to a "prod" environment. - /code $ copilot deploy --init-wkld --deploy-env=false --env prod --name backend`, + /code $ copilot deploy --init-wkld --env prod --name backend + Deploys all local, initialized workloads in no particular order. + /code $ copilot deploy --all --env prod --name backend + Deploys multiple workloads in a prescribed order (fe and worker, then be). + /code $ copilot deploy -n fe/1 -n be/2 -n worker/1 + Initializes and deploys all local workloads after deploying environment changes. + /code $ copilot deploy --all --init-wkld --deploy-env -e prod`, RunE: runCmdE(func(cmd *cobra.Command, args []string) error { opts, err := newDeployOpts(vars) @@ -470,13 +758,6 @@ func BuildDeployCmd() *cobra.Command { return err } - if cmd.Flags().Changed(yesInitWorkloadFlag) { - opts.yesInitWkld = aws.Bool(false) - if initWorkload { - opts.yesInitWkld = aws.Bool(true) - } - } - if cmd.Flags().Changed(yesInitEnvFlag) { opts.yesInitEnv = aws.Bool(false) if initEnvironment { @@ -491,10 +772,6 @@ func BuildDeployCmd() *cobra.Command { } } - if cmd.Flags().Changed(nameFlag) { - opts.workloadNames = []string{name} - } - if err := opts.Run(); err != nil { return err } @@ -502,7 +779,7 @@ func BuildDeployCmd() *cobra.Command { }), } cmd.Flags().StringVarP(&vars.appName, appFlag, appFlagShort, tryReadingAppName(), appFlagDescription) - cmd.Flags().StringVarP(&name, nameFlag, nameFlagShort, "", workloadFlagDescription) + cmd.Flags().StringSliceVarP(&vars.workloadNames, nameFlag, nameFlagShort, nil, workloadsFlagDescription) cmd.Flags().StringVarP(&vars.envName, envFlag, envFlagShort, "", envFlagDescription) cmd.Flags().StringVar(&vars.imageTag, imageTagFlag, "", imageTagFlagDescription) cmd.Flags().StringToStringVar(&vars.resourceTags, resourceTagsFlag, nil, resourceTagsFlagDescription) @@ -513,7 +790,8 @@ func BuildDeployCmd() *cobra.Command { cmd.Flags().BoolVar(&deployEnvironment, deployEnvFlag, false, deployEnvFlagDescription) cmd.Flags().BoolVar(&initEnvironment, yesInitEnvFlag, false, yesInitEnvFlagDescription) - cmd.Flags().BoolVar(&initWorkload, yesInitWorkloadFlag, false, yesInitWorkloadFlagDescription) + cmd.Flags().BoolVar(&vars.yesInitWkld, yesInitWorkloadFlag, false, yesInitWorkloadFlagDescription) + cmd.Flags().BoolVar(&vars.deployAllWorkloads, allFlag, false, allWorkloadsFlagDescription) cmd.Flags().StringVar(&vars.profile, profileFlag, "", profileFlagDescription) cmd.Flags().StringVar(&vars.tempCreds.AccessKeyID, accessKeyIDFlag, "", accessKeyIDFlagDescription) diff --git a/internal/pkg/cli/deploy_test.go b/internal/pkg/cli/deploy_test.go index 5ccd7dae01b..aeaa18d7278 100644 --- a/internal/pkg/cli/deploy_test.go +++ b/internal/pkg/cli/deploy_test.go @@ -36,11 +36,15 @@ func TestDeployOpts_Run(t *testing.T) { mockManifest := workspace.WorkloadManifest(` name: fe type: Load Balanced Web Service`) + mockWorkerManifest := workspace.WorkloadManifest(` +name: worker +type: Worker Service`) testCases := map[string]struct { inAppName string inNames []string + inDeployAll bool inEnvName string - inShouldInit *bool + inShouldInit bool inDeployEnv *bool inInitEnv *bool @@ -87,6 +91,7 @@ type: Load Balanced Web Service`) }, mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) + m.EXPECT().ListWorkloads().Return([]string{"fe"}, nil) }, mockInit: func(m *mocks.MockwkldInitializerWithoutManifest) { @@ -98,7 +103,7 @@ type: Load Balanced Web Service`) inInitEnv: aws.Bool(false), inDeployEnv: aws.Bool(false), mockSel: func(m *mocks.MockwsSelector) { - m.EXPECT().Workload("Select a service or job in your workspace", "").Return("fe", nil) + m.EXPECT().Workloads("Select one or more services or jobs in your workspace.", "").Return([]string{"fe"}, nil) }, mockActionCommand: func(m *mocks.MockactionCommand) { m.EXPECT().Ask() @@ -117,53 +122,22 @@ type: Load Balanced Web Service`) mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ReadWorkloadManifest("fe").Times(0) m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) + m.EXPECT().ListWorkloads().Return([]string{"fe"}, nil) }, mockPrompt: func(m *mocks.Mockprompter) {}, mockInit: func(m *mocks.MockwkldInitializerWithoutManifest) {}, }, - "prompts for initializing workload": { - inAppName: "app", - inNames: []string{"fe"}, - inEnvName: "test", - inInitEnv: aws.Bool(false), - inDeployEnv: aws.Bool(false), - inShouldInit: nil, - mockWs: func(m *mocks.MockwsWlDirReader) { - m.EXPECT().ReadWorkloadManifest("fe").Return(mockManifest, nil) - m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) - }, - mockPrompt: func(m *mocks.Mockprompter) { - m.EXPECT().Confirm(gomock.Any(), gomock.Any(), gomock.Any()).Return(true, nil) - }, - mockStore: func(m *mocks.Mockstore) { - m.EXPECT().GetEnvironment("app", "test").Return(&mockEnv, nil) - m.EXPECT().ListWorkloads("app").Return(nil, nil) - m.EXPECT().GetWorkload("app", "fe").Return(&mockWl, nil) - }, - mockActionCommand: func(m *mocks.MockactionCommand) { - m.EXPECT().Ask() - m.EXPECT().Validate() - m.EXPECT().Execute() - m.EXPECT().RecommendActions() - }, - mockCmd: func(m *mocks.Mockcmd) { - - }, - mockSel: func(m *mocks.MockwsSelector) {}, - mockInit: func(m *mocks.MockwkldInitializerWithoutManifest) { - m.EXPECT().AddWorkloadToApp("app", "fe", manifestinfo.LoadBalancedWebServiceType).Return(nil) - }, - }, "initializes workload with flag specified": { inAppName: "app", inNames: []string{"fe"}, inEnvName: "test", inInitEnv: aws.Bool(false), inDeployEnv: aws.Bool(false), - inShouldInit: aws.Bool(true), + inShouldInit: true, mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ReadWorkloadManifest("fe").Return(mockManifest, nil) m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) + m.EXPECT().ListWorkloads().Return([]string{"fe"}, nil) }, mockPrompt: func(m *mocks.Mockprompter) {}, mockStore: func(m *mocks.Mockstore) { @@ -185,37 +159,6 @@ type: Load Balanced Web Service`) m.EXPECT().AddWorkloadToApp("app", "fe", manifestinfo.LoadBalancedWebServiceType).Return(nil) }, }, - "errors if noInit specified": { - inAppName: "app", - inNames: []string{"fe"}, - inEnvName: "test", - inInitEnv: aws.Bool(false), - inDeployEnv: aws.Bool(false), - inShouldInit: aws.Bool(false), - mockWs: func(m *mocks.MockwsWlDirReader) { - m.EXPECT().ReadWorkloadManifest("fe").Return(mockManifest, nil) - m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) - }, - mockPrompt: func(m *mocks.Mockprompter) {}, - mockStore: func(m *mocks.Mockstore) { - m.EXPECT().GetEnvironment("app", "test").Return(&mockEnv, nil) - m.EXPECT().ListWorkloads("app").Return(nil, nil) - }, - mockActionCommand: func(m *mocks.MockactionCommand) { - m.EXPECT().Ask().Times(0) - m.EXPECT().Validate().Times(0) - m.EXPECT().Execute().Times(0) - m.EXPECT().RecommendActions().Times(0) - }, - mockCmd: func(m *mocks.Mockcmd) { - - }, - mockSel: func(m *mocks.MockwsSelector) {}, - mockInit: func(m *mocks.MockwkldInitializerWithoutManifest) { - m.EXPECT().AddWorkloadToApp(gomock.Any(), gomock.Any(), gomock.Any()).Times(0) - }, - wantedErr: "workload fe is uninitialized but --init-wkld=false was specified", - }, "errors reading manifest": { inAppName: "app", inNames: []string{"fe"}, @@ -225,6 +168,7 @@ type: Load Balanced Web Service`) mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) m.EXPECT().ReadWorkloadManifest("fe").Return(nil, errors.New("some error")) + m.EXPECT().ListWorkloads().Return([]string{"fe"}, nil) }, mockPrompt: func(m *mocks.Mockprompter) {}, mockStore: func(m *mocks.Mockstore) { @@ -253,6 +197,7 @@ type: Load Balanced Web Service`) mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) m.EXPECT().ReadWorkloadManifest("fe").Return(workspace.WorkloadManifest(`type: nothing here`), nil) + m.EXPECT().ListWorkloads().Return([]string{"fe"}, nil) }, mockPrompt: func(m *mocks.Mockprompter) {}, mockStore: func(m *mocks.Mockstore) { @@ -298,9 +243,9 @@ type: Load Balanced Web Service`) m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) m.EXPECT().ReadWorkloadManifest("fe").Times(0) }, - wantedErr: "retrieve workloads: some error", + wantedErr: "retrieve store workloads: some error", }, - "initializes and deploys local manifest with prompts": { + "initializes and deploys local manifest": { inAppName: "app", inEnvName: "test", inInitEnv: aws.Bool(false), @@ -308,10 +253,9 @@ type: Load Balanced Web Service`) mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) m.EXPECT().ReadWorkloadManifest("fe").Return(mockManifest, nil) + m.EXPECT().ListWorkloads().Return([]string{"fe"}, nil) }, - mockPrompt: func(m *mocks.Mockprompter) { - m.EXPECT().Confirm(gomock.Any(), gomock.Any(), gomock.Any()).Return(true, nil) - }, + mockPrompt: func(m *mocks.Mockprompter) {}, mockStore: func(m *mocks.Mockstore) { m.EXPECT().GetEnvironment("app", "test").Return(&mockEnv, nil) m.EXPECT().ListWorkloads("app").Return(nil, nil) @@ -327,7 +271,7 @@ type: Load Balanced Web Service`) }, mockSel: func(m *mocks.MockwsSelector) { - m.EXPECT().Workload(gomock.Any(), gomock.Any()).Return("fe", nil) + m.EXPECT().Workloads(gomock.Any(), gomock.Any()).Return([]string{"fe"}, nil) }, mockInit: func(m *mocks.MockwkldInitializerWithoutManifest) { m.EXPECT().AddWorkloadToApp("app", "fe", manifestinfo.LoadBalancedWebServiceType).Return(nil) @@ -340,7 +284,7 @@ type: Load Balanced Web Service`) inDeployEnv: aws.Bool(false), wantedErr: "ask job deploy: some error", mockSel: func(m *mocks.MockwsSelector) { - m.EXPECT().Workload("Select a service or job in your workspace", "").Return("mailer", nil) + m.EXPECT().Workloads("Select one or more services or jobs in your workspace.", "").Return([]string{"mailer"}, nil) }, mockActionCommand: func(m *mocks.MockactionCommand) { m.EXPECT().Ask().Return(errors.New("some error")) @@ -360,6 +304,7 @@ type: Load Balanced Web Service`) mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) m.EXPECT().ReadWorkloadManifest("mailer").Times(0) + m.EXPECT().ListWorkloads().Return([]string{"mailer"}, nil) }, }, "doesn't prompt if name is specified": { @@ -390,6 +335,7 @@ type: Load Balanced Web Service`) mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) m.EXPECT().ReadWorkloadManifest("fe").Times(0) + m.EXPECT().ListWorkloads().Return([]string{"fe"}, nil) }, }, "get name error": { @@ -399,7 +345,7 @@ type: Load Balanced Web Service`) inDeployEnv: aws.Bool(false), wantedErr: "select service or job: some error", mockSel: func(m *mocks.MockwsSelector) { - m.EXPECT().Workload(gomock.Any(), gomock.Any()).Return("", errors.New("some error")) + m.EXPECT().Workloads(gomock.Any(), gomock.Any()).Return(nil, errors.New("some error")) }, mockActionCommand: func(m *mocks.MockactionCommand) {}, mockCmd: func(m *mocks.Mockcmd) {}, @@ -438,6 +384,7 @@ type: Load Balanced Web Service`) mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) m.EXPECT().ReadWorkloadManifest("fe").Times(0) + m.EXPECT().ListWorkloads().Return([]string{"fe"}, nil) }, }, "validate error": { @@ -467,6 +414,7 @@ type: Load Balanced Web Service`) mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) m.EXPECT().ReadWorkloadManifest("fe").Times(0) + m.EXPECT().ListWorkloads().Return([]string{"fe"}, nil) }, }, "execute error": { @@ -475,7 +423,7 @@ type: Load Balanced Web Service`) inEnvName: "test", inInitEnv: aws.Bool(false), inDeployEnv: aws.Bool(false), - wantedErr: "execute svc deploy: some error", + wantedErr: "execute deployment 1 of 1 in group 1: some error", mockSel: func(m *mocks.MockwsSelector) {}, mockActionCommand: func(m *mocks.MockactionCommand) { @@ -488,7 +436,6 @@ type: Load Balanced Web Service`) m.EXPECT().GetEnvironment("app", "test").Return(&mockEnv, nil) m.EXPECT().GetWorkload("app", "fe").Return(&mockWl, nil) m.EXPECT().ListWorkloads("app").Return([]*config.Workload{&mockWl}, nil) - }, mockPrompt: func(m *mocks.Mockprompter) {}, mockInit: func(m *mocks.MockwkldInitializerWithoutManifest) { @@ -497,6 +444,7 @@ type: Load Balanced Web Service`) mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) m.EXPECT().ReadWorkloadManifest("fe").Times(0) + m.EXPECT().ListWorkloads().Return([]string{"fe"}, nil) }, }, "workload init error": { @@ -505,7 +453,7 @@ type: Load Balanced Web Service`) inEnvName: "test", wantedErr: "add workload to app: some error", inDeployEnv: aws.Bool(false), - inShouldInit: aws.Bool(true), + inShouldInit: true, mockSel: func(m *mocks.MockwsSelector) {}, mockPrompt: func(m *mocks.Mockprompter) {}, mockActionCommand: func(m *mocks.MockactionCommand) {}, @@ -520,41 +468,18 @@ type: Load Balanced Web Service`) mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) m.EXPECT().ReadWorkloadManifest("fe").Return(mockManifest, nil) + m.EXPECT().ListWorkloads().Return([]string{"fe"}, nil) }, mockInit: func(m *mocks.MockwkldInitializerWithoutManifest) { m.EXPECT().AddWorkloadToApp("app", "fe", "Load Balanced Web Service").Return(errors.New("some error")) }, }, - "confirm workload init error": { - inAppName: "app", - inNames: []string{"fe"}, - inEnvName: "test", - wantedErr: "confirm initialize workload: some error", - inDeployEnv: aws.Bool(false), - mockSel: func(m *mocks.MockwsSelector) {}, - mockPrompt: func(m *mocks.Mockprompter) { - m.EXPECT().Confirm("Found manifest for uninitialized Load Balanced Web Service \"fe\". Initialize it?", gomock.Any(), gomock.Any()).Return(false, errors.New("some error")) - }, - mockActionCommand: func(m *mocks.MockactionCommand) {}, - mockCmd: func(m *mocks.Mockcmd) {}, - mockStore: func(m *mocks.Mockstore) { - m.EXPECT().GetEnvironment("app", "test").Return(&mockEnv, nil) - // After env init/deploy - m.EXPECT().ListWorkloads("app").Return(nil, nil) - // After wkld init - m.EXPECT().GetWorkload("app", "fe").Times(0) - }, - mockWs: func(m *mocks.MockwsWlDirReader) { - m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) - m.EXPECT().ReadWorkloadManifest("fe").Return(mockManifest, nil) - }, - mockInit: func(m *mocks.MockwkldInitializerWithoutManifest) {}, - }, "both uninitialized and initialized environments and workloads": { inAppName: "app", wantedErr: "", mockSel: func(m *mocks.MockwsSelector) { - m.EXPECT().Workload("Select a service or job in your workspace", "").Return("fe", nil) + + m.EXPECT().Workloads("Select one or more services or jobs in your workspace.", "").Return([]string{"fe"}, nil) m.EXPECT().Environment("Select an environment to deploy to", "", "app", prompt.Option{Value: "prod", Hint: "uninitialized"}).Return("prod", nil) }, mockPrompt: func(m *mocks.Mockprompter) { @@ -586,6 +511,7 @@ type: Load Balanced Web Service`) }, mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ListEnvironments().Return([]string{"test", "prod"}, nil) + m.EXPECT().ListWorkloads().Return([]string{"fe"}, nil) }, mockInit: func(m *mocks.MockwkldInitializerWithoutManifest) { @@ -650,17 +576,117 @@ type: Load Balanced Web Service`) Type: "Backend Service", } m.EXPECT().GetEnvironment("app", "test").Return(&mockEnv, nil) - m.EXPECT().ListWorkloads("app").Return([]*config.Workload{&mockWl, &mockBeWl}, nil).Times(2) + m.EXPECT().ListWorkloads("app").Return([]*config.Workload{&mockWl, &mockBeWl}, nil) m.EXPECT().GetWorkload("app", "fe").Return(&mockWl, nil) m.EXPECT().GetWorkload("app", "be").Return(&mockBeWl, nil) }, mockWs: func(m *mocks.MockwsWlDirReader) { m.EXPECT().ReadWorkloadManifest("fe").Times(0) m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) + m.EXPECT().ListWorkloads().Return([]string{"fe", "be"}, nil) }, mockPrompt: func(m *mocks.Mockprompter) {}, mockInit: func(m *mocks.MockwkldInitializerWithoutManifest) {}, }, + "deploys multiple workloads with specified order": { + inAppName: "app", + inEnvName: "test", + inDeployAll: true, + inShouldInit: true, + inNames: []string{"be/1", "worker/2"}, + inInitEnv: aws.Bool(false), + inDeployEnv: aws.Bool(false), + + mockSel: func(m *mocks.MockwsSelector) { + }, + mockActionCommand: func(m *mocks.MockactionCommand) { + m.EXPECT().Ask().Times(3) + m.EXPECT().Validate().Times(3) + m.EXPECT().Execute().Times(3) + m.EXPECT().RecommendActions().Times(3) + }, + mockCmd: func(m *mocks.Mockcmd) { + + }, + mockStore: func(m *mocks.Mockstore) { + mockBeWl := config.Workload{ + App: "app", + Name: "be", + Type: "Backend Service", + } + mockWorkerWl := config.Workload{ + App: "app", + Name: "worker", + Type: "Worker Service", + } + m.EXPECT().GetEnvironment("app", "test").Return(&mockEnv, nil) + m.EXPECT().ListWorkloads("app").Return([]*config.Workload{&mockWl, &mockBeWl}, nil) + m.EXPECT().GetWorkload("app", "fe").Return(&mockWl, nil) + m.EXPECT().GetWorkload("app", "be").Return(&mockBeWl, nil) + m.EXPECT().GetWorkload("app", "worker").Return(&mockWorkerWl, nil) + }, + mockWs: func(m *mocks.MockwsWlDirReader) { + m.EXPECT().ReadWorkloadManifest("fe").Times(0) + m.EXPECT().ReadWorkloadManifest("worker").Return(mockWorkerManifest, nil) + m.EXPECT().ListWorkloads().Return([]string{"fe", "be", "worker"}, nil) + m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) + }, + mockPrompt: func(m *mocks.Mockprompter) {}, + mockInit: func(m *mocks.MockwkldInitializerWithoutManifest) { + m.EXPECT().AddWorkloadToApp("app", "worker", manifestinfo.WorkerServiceType).Return(nil) + }, + }, + "skips deploying workload with no changes": { + inAppName: "app", + inEnvName: "test", + inDeployAll: true, + inShouldInit: true, + inNames: []string{"be/1", "worker/2"}, + inInitEnv: aws.Bool(false), + inDeployEnv: aws.Bool(false), + + mockSel: func(m *mocks.MockwsSelector) { + }, + mockActionCommand: func(m *mocks.MockactionCommand) { + m.EXPECT().Ask().Times(3) + m.EXPECT().Validate().Times(3) + m.EXPECT().Execute().Times(2) + m.EXPECT().Execute().Return(&errNoInfrastructureChanges{ + parentErr: errors.New("some error"), + }) + m.EXPECT().RecommendActions().Times(2) + }, + mockCmd: func(m *mocks.Mockcmd) { + + }, + mockStore: func(m *mocks.Mockstore) { + mockBeWl := config.Workload{ + App: "app", + Name: "be", + Type: "Backend Service", + } + mockWorkerWl := config.Workload{ + App: "app", + Name: "worker", + Type: "Worker Service", + } + m.EXPECT().GetEnvironment("app", "test").Return(&mockEnv, nil) + m.EXPECT().ListWorkloads("app").Return([]*config.Workload{&mockWl, &mockBeWl}, nil) + m.EXPECT().GetWorkload("app", "fe").Return(&mockWl, nil) + m.EXPECT().GetWorkload("app", "be").Return(&mockBeWl, nil) + m.EXPECT().GetWorkload("app", "worker").Return(&mockWorkerWl, nil) + }, + mockWs: func(m *mocks.MockwsWlDirReader) { + m.EXPECT().ReadWorkloadManifest("fe").Times(0) + m.EXPECT().ReadWorkloadManifest("worker").Return(mockWorkerManifest, nil) + m.EXPECT().ListWorkloads().Return([]string{"fe", "be", "worker"}, nil) + m.EXPECT().ListEnvironments().Return([]string{"test"}, nil) + }, + mockPrompt: func(m *mocks.Mockprompter) {}, + mockInit: func(m *mocks.MockwkldInitializerWithoutManifest) { + m.EXPECT().AddWorkloadToApp("app", "worker", manifestinfo.WorkerServiceType).Return(nil) + }, + }, } for name, tc := range testCases { t.Run(name, func(t *testing.T) { @@ -690,9 +716,10 @@ type: Load Balanced Web Service`) appName: tc.inAppName, envName: tc.inEnvName, }, - yesInitWkld: tc.inShouldInit, - deployEnv: tc.inDeployEnv, - yesInitEnv: tc.inInitEnv, + yesInitWkld: tc.inShouldInit, + deployEnv: tc.inDeployEnv, + yesInitEnv: tc.inInitEnv, + deployAllWorkloads: tc.inDeployAll, }, newInitEnvCmd: func(o *deployOpts) (cmd, error) { return mockNoActionCmd, nil }, newDeployEnvCmd: func(o *deployOpts) (cmd, error) { return mockNoActionCmd, nil }, @@ -1041,3 +1068,186 @@ func Test_deployOpts_maybeDeployEnv(t *testing.T) { }) } } + +func Test_deployOpts_getDeploymentOrder(t *testing.T) { + mockFe := &config.Workload{ + App: "app", + Name: "fe", + Type: "Load Balanced Web Service", + } + mockBe := &config.Workload{ + App: "app", + Name: "be", + Type: "Backend Service", + } + mockDb := &config.Workload{ + App: "app", + Name: "db", + Type: "Backend Service", + } + mockWorker := &config.Workload{ + App: "app", + Name: "worker", + Type: "Worker Service", + } + tests := map[string]struct { + inWorkloadNames []string + inInitWkld bool + inDeployAll bool + want [][]string + mockWs func(m *mocks.MockwsWlDirReader) + mockStore func(m *mocks.Mockstore) + wantErr string + }{ + "no order tags, --all, initWkld unspecified": { + inWorkloadNames: []string{"fe", "be", "db"}, + inDeployAll: true, + want: [][]string{{"be", "db", "fe"}}, + mockWs: func(m *mocks.MockwsWlDirReader) { + m.EXPECT().ListWorkloads().Return([]string{"be", "db", "fe"}, nil) + }, + mockStore: func(m *mocks.Mockstore) { + m.EXPECT().ListWorkloads("app").Return([]*config.Workload{ + mockFe, + mockBe, + mockDb, + }, nil) + }, + }, + "with order tags and groups, --all, initWkld unspecified": { + inWorkloadNames: []string{"fe/1", "be/2", "db/2", "worker/5"}, + inDeployAll: true, + want: [][]string{{"fe"}, {"be", "db"}, {"worker"}}, + mockWs: func(m *mocks.MockwsWlDirReader) { + m.EXPECT().ListWorkloads().Return([]string{"be", "db", "fe", "worker"}, nil) + }, + mockStore: func(m *mocks.Mockstore) { + m.EXPECT().ListWorkloads("app").Return([]*config.Workload{ + mockFe, + mockBe, + mockDb, + mockWorker, + }, nil) + }, + }, + "with all order tags, --all, initWkld unspecified": { + inWorkloadNames: []string{"fe/1", "be/2", "db/3"}, + inDeployAll: true, + want: [][]string{{"fe"}, {"be"}, {"db"}, {"worker"}}, + mockWs: func(m *mocks.MockwsWlDirReader) { + m.EXPECT().ListWorkloads().Return([]string{"be", "db", "fe", "worker"}, nil) + }, + mockStore: func(m *mocks.Mockstore) { + m.EXPECT().ListWorkloads("app").Return([]*config.Workload{ + mockFe, + mockBe, + mockDb, + mockWorker, + }, nil) + }, + }, + "with some order tags --all, intWkld unspecified": { + inWorkloadNames: []string{"be/2", "db/3"}, + inDeployAll: true, + want: [][]string{{"be"}, {"db"}, {"fe", "worker"}}, + mockWs: func(m *mocks.MockwsWlDirReader) { + m.EXPECT().ListWorkloads().Return([]string{"be", "db", "fe", "worker"}, nil) + }, + mockStore: func(m *mocks.Mockstore) { + m.EXPECT().ListWorkloads("app").Return([]*config.Workload{ + mockFe, + mockBe, + mockDb, + mockWorker, + }, nil) + }, + }, + "with some order tags, some workloads uninitialized, initWkld unspecified": { + inWorkloadNames: []string{"be/2", "db/3"}, + inDeployAll: true, + want: [][]string{{"be"}, {"db"}, {"fe"}}, + mockWs: func(m *mocks.MockwsWlDirReader) { + m.EXPECT().ListWorkloads().Return([]string{"be", "db", "fe", "worker"}, nil) + }, + mockStore: func(m *mocks.Mockstore) { + m.EXPECT().ListWorkloads("app").Return([]*config.Workload{ + mockFe, + mockBe, + mockDb, + }, nil) + }, + }, + "with some order tags, --all false, intWkld unspecified": { + inWorkloadNames: []string{"be/2", "db/3"}, + inDeployAll: false, + want: [][]string{{"be"}, {"db"}}, + mockWs: func(m *mocks.MockwsWlDirReader) {}, + mockStore: func(m *mocks.Mockstore) {}, + }, + "order tags, --all true, initWkld false": { + inWorkloadNames: []string{"fe/2", "be/1"}, + inInitWkld: false, + inDeployAll: true, + want: [][]string{{"be"}, {"fe"}, {"db"}}, + mockWs: func(m *mocks.MockwsWlDirReader) { + m.EXPECT().ListWorkloads().Return([]string{"be", "db", "fe", "worker"}, nil) + }, + mockStore: func(m *mocks.Mockstore) { + m.EXPECT().ListWorkloads("app").Return([]*config.Workload{ + { + App: "app", + Name: "fe", + Type: "Load Balanced Web Service", + }, + { + App: "app", + Name: "be", + Type: "Backend Service", + }, + { + App: "app", + Name: "db", + Type: "Backend Service", + }, + }, nil) + }, + }, + "no workloads, empty groupsMap": { + inWorkloadNames: []string{}, + wantErr: "generate deployment groups: no workloads were specified", + mockWs: func(m *mocks.MockwsWlDirReader) {}, + mockStore: func(m *mocks.Mockstore) {}, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctrl := gomock.NewController(t) + mockStore := mocks.NewMockstore(ctrl) + mockWs := mocks.NewMockwsWlDirReader(ctrl) + tt.mockWs(mockWs) + tt.mockStore(mockStore) + o := &deployOpts{ + deployVars: deployVars{ + deployWkldVars: deployWkldVars{ + appName: "app", + }, + yesInitWkld: tt.inInitWkld, + deployAllWorkloads: tt.inDeployAll, + workloadNames: tt.inWorkloadNames, + }, + + store: mockStore, + ws: mockWs, + } + got, err := o.getDeploymentOrder() + if tt.wantErr != "" { + require.EqualError(t, err, tt.wantErr) + } else { + require.NoError(t, err) + for i := range got { + require.ElementsMatch(t, tt.want[i], got[i], "list 1: %v, list2: %v", tt.want, got) + } + } + }) + } +} diff --git a/internal/pkg/cli/flag.go b/internal/pkg/cli/flag.go index 123212ed2da..36d05d5f725 100644 --- a/internal/pkg/cli/flag.go +++ b/internal/pkg/cli/flag.go @@ -262,6 +262,7 @@ const ( svcFlagDescription = "Name of the service." jobFlagDescription = "Name of the job." workloadFlagDescription = "Name of the service or job." + workloadsFlagDescription = "Names of the service or jobs to deploy, with an optional priority tag." nameFlagDescription = "Name of the service, job, or task group." yesFlagDescription = "Skips confirmation prompt." resourceTagsFlagDescription = `Optional. Labels with a key and value separated by commas. @@ -280,8 +281,8 @@ rollback in case of deployment failure. We do not recommend using this flag for a production environment.` forceEnvDeployFlagDescription = "Optional. Force update the environment stack template." - yesInitWorkloadFlagDescription = "Optional. Initialize a workload before deploying it." - allWorkloadsFlagDescription = "Optional. Deploy all workloads with manifests in the current Copilot workspace." + yesInitWorkloadFlagDescription = "Optional. When specified with --all, initialize all local workloads before deployment." + allWorkloadsFlagDescription = "Optional. Deploy all workloads with manifests in the current Copilot workspace." detachFlagDescription = "Optional. Skip displaying CloudFormation deployment progress." // Operational. diff --git a/internal/pkg/cli/interfaces.go b/internal/pkg/cli/interfaces.go index c14dc34cd04..d147a6178dd 100644 --- a/internal/pkg/cli/interfaces.go +++ b/internal/pkg/cli/interfaces.go @@ -554,6 +554,7 @@ type wsSelector interface { Service(prompt, help string) (string, error) Job(prompt, help string) (string, error) Workload(msg, help string) (string, error) + Workloads(msg, help string) ([]string, error) } type staticSourceSelector interface { diff --git a/internal/pkg/cli/mocks/mock_interfaces.go b/internal/pkg/cli/mocks/mock_interfaces.go index 32dfff16721..ebb17d8e5fc 100644 --- a/internal/pkg/cli/mocks/mock_interfaces.go +++ b/internal/pkg/cli/mocks/mock_interfaces.go @@ -6181,6 +6181,21 @@ func (mr *MockwsSelectorMockRecorder) Workload(msg, help interface{}) *gomock.Ca return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Workload", reflect.TypeOf((*MockwsSelector)(nil).Workload), msg, help) } +// Workloads mocks base method. +func (m *MockwsSelector) Workloads(msg, help string) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Workloads", msg, help) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Workloads indicates an expected call of Workloads. +func (mr *MockwsSelectorMockRecorder) Workloads(msg, help interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Workloads", reflect.TypeOf((*MockwsSelector)(nil).Workloads), msg, help) +} + // MockstaticSourceSelector is a mock of staticSourceSelector interface. type MockstaticSourceSelector struct { ctrl *gomock.Controller diff --git a/internal/pkg/queue/queue.go b/internal/pkg/queue/queue.go new file mode 100644 index 00000000000..5d37c12c22a --- /dev/null +++ b/internal/pkg/queue/queue.go @@ -0,0 +1,86 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// Package queue provides a generic priority queue. +package queue + +import "container/heap" + +// Lesser is an interface to enable generic structs to be elements of a priority queue. +// Any struct can become a priority queue element by defining the LessThan method +// and initializing a new PriorityQueue. +// +// (s myStruct) LessThan(other myStruct) bool +// q := NewPriorityQueue[myStruct]() +type Lesser[T any] interface { + LessThan(T) bool +} + +// PriorityQueue implements a priority queue. +type PriorityQueue[T Lesser[T]] struct { + pq pq[T] +} + +// Push adds a new element to the queue and puts it in the correct place. +func (p *PriorityQueue[T]) Push(x T) { + heap.Push(&p.pq, x) +} + +// Pop removes the top element of the queue and restructures it in log(n) time. +func (p *PriorityQueue[T]) Pop() (*T, bool) { + if p.pq.Len() == 0 { + return nil, false + } + v := heap.Pop(&p.pq).(T) + return &v, true +} + +// Len returns the length of the queue. +func (p *PriorityQueue[T]) Len() int { + return p.pq.Len() +} + +type pq[T Lesser[T]] []T + +// NewPriorityQueue returns an empty priority queue. +func NewPriorityQueue[T Lesser[T]]() *PriorityQueue[T] { + var arr pq[T] = make([]T, 0) + heap.Init(&arr) + return &PriorityQueue[T]{ + pq: arr, + } +} + +// Len returns the length of the data structure. +func (p *pq[T]) Len() int { return len(*p) } + +// Less returns whether element i is less than element j, using the generic type's LessThan function. +func (p *pq[T]) Less(i, j int) bool { return (*p)[i].LessThan((*p)[j]) } + +// Swap swaps the positions of two elements in the priority queue. +func (p *pq[T]) Swap(i, j int) { + (*p)[i], (*p)[j] = (*p)[j], (*p)[i] +} + +// Push appends a new element to the back of the underlying array. +func (p *pq[T]) Push(x any) { + *p = append(*p, x.(T)) +} + +// Pop removes the last element from the array. +func (p *pq[T]) Pop() any { + old := *p + n := len(old) + res := old[n-1] + *p = old[:n-1] + return res +} + +// Compile-time check that the PriorityQueue type works on a generic type. +type comparableInt int + +func (c comparableInt) LessThan(a comparableInt) bool { + return c < a +} + +var _ heap.Interface = (*pq[comparableInt])(nil) diff --git a/internal/pkg/queue/queue_test.go b/internal/pkg/queue/queue_test.go new file mode 100644 index 00000000000..eb836dfb266 --- /dev/null +++ b/internal/pkg/queue/queue_test.go @@ -0,0 +1,61 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 +package queue + +import ( + "github.com/stretchr/testify/require" + "testing" +) + +func TestPriorityQueue(t *testing.T) { + t.Run("sorts numbers using comparableInt", func(t *testing.T) { + nums := []comparableInt{1, 6, 2, 10, 1000, -5} + want := []comparableInt{-5, 1, 2, 6, 10, 1000} + pq := NewPriorityQueue[comparableInt]() + for _, n := range nums { + pq.Push(n) + } + require.Equal(t, len(nums), pq.Len()) + out := make([]comparableInt, 0, pq.Len()) + for pq.Len() > 0 { + v, ok := pq.Pop() + require.Equal(t, true, ok) + out = append(out, *v) + } + require.Equal(t, want, out) + }) + + t.Run("sorts arbitrary structs by length of member", func(t *testing.T) { + items := []arbitraryStruct{ + {"aaaaaa"}, {"x"}, {"aaa"}, + } + want := []arbitraryStruct{ + {"x"}, {"aaa"}, {"aaaaaa"}, + } + pq := NewPriorityQueue[arbitraryStruct]() + for _, n := range items { + pq.Push(n) + } + require.Equal(t, len(items), pq.Len()) + out := make([]arbitraryStruct, 0, pq.Len()) + for pq.Len() > 0 { + v, ok := pq.Pop() + require.Equal(t, true, ok) + out = append(out, *v) + } + require.Equal(t, want, out) + }) + + t.Run("can't pop from empty priority queue", func(t *testing.T) { + pq := NewPriorityQueue[arbitraryStruct]() + v, ok := pq.Pop() + require.Nil(t, v) + require.Equal(t, false, ok) + }) +} + +type arbitraryStruct struct { + fieldA string +} + +func (a arbitraryStruct) LessThan(b arbitraryStruct) bool { return len(a.fieldA) < len(b.fieldA) } diff --git a/internal/pkg/term/selector/selector.go b/internal/pkg/term/selector/selector.go index 9d462bbc03c..c06f2540e6b 100644 --- a/internal/pkg/term/selector/selector.go +++ b/internal/pkg/term/selector/selector.go @@ -824,7 +824,7 @@ func (s *LocalWorkloadSelector) getWorkloadSelectOptions(workloadType string) ([ return nil, fmt.Errorf("no %s found in workspace", pluralNounString) } // Get the list of un-initialized workloads that are present in the workspace and add them to options. - unInitializedLocalWorkloads := filterOutStrings(wsWlNames, initializedLocalWorkloads) + unInitializedLocalWorkloads := filterOutItems(wsWlNames, initializedLocalWorkloads, func(a string) string { return a }) for _, wl := range unInitializedLocalWorkloads { options = append(options, prompt.Option{ Value: wl, @@ -885,32 +885,24 @@ func (s *LocalWorkloadSelector) Workload(msg, help string) (wl string, err error return selectedWlName, nil } -func filterStrings(allStrings []string, wantedStrings []string) []string { - isWanted := make(map[string]bool) - for _, name := range wantedStrings { - isWanted[name] = true +// filterOutItems is a generic function to return the subset of allItems which does not include the items specified in +// unwantedItems. stringFunc is a function which maps the unwantedItem type T to a string value. For example, one can +// convert a struct of type *config.Workload to a string by passing +// +// func(w *config.Workload) string { return w.Name } +// +// as the stringFunc parameter. +func filterOutItems[T any](allItems []string, unwantedItems []T, stringFunc func(T) string) []string { + isUnwanted := make(map[string]bool) + for _, item := range unwantedItems { + isUnwanted[stringFunc(item)] = true } var filtered []string - for _, wl := range allStrings { - if _, ok := isWanted[wl]; !ok { + for _, str := range allItems { + if isUnwanted[str] { continue } - filtered = append(filtered, wl) - } - return filtered -} - -func filterOutStrings(allStrings []string, unwantedStrings []string) []string { - isUnWanted := make(map[string]bool) - for _, name := range unwantedStrings { - isUnWanted[name] = true - } - var filtered []string - for _, wl := range allStrings { - if isUnWanted[wl] { - continue - } - filtered = append(filtered, wl) + filtered = append(filtered, str) } return filtered } @@ -945,27 +937,36 @@ func (s *LocalEnvironmentSelector) LocalEnvironment(msg, help string) (string, e } func filterEnvsByName(envs []*config.Environment, wantedNames []string) []string { - // TODO: refactor this and `filterWlsByName` when generic supports using common struct fields: https://github.com/golang/go/issues/48522 - isWanted := make(map[string]bool) - for _, name := range wantedNames { - isWanted[name] = true - } - var filtered []string - for _, wl := range envs { - if _, ok := isWanted[wl.Name]; !ok { - continue - } - filtered = append(filtered, wl.Name) - } - return filtered + return filterItemsByStrings(wantedNames, envs, func(e *config.Environment) string { return e.Name }) } func filterWlsByName(wls []*config.Workload, wantedNames []string) []string { - stringWls := make([]string, len(wls)) - for i := range wls { - stringWls[i] = wls[i].Name + return filterItemsByStrings(wantedNames, wls, func(w *config.Workload) string { return w.Name }) +} + +// filterItemsByStrings is a generic function to return the subset of wantedStrings that exists in possibleItems. +// stringFunc is a method to convert the generic item type (T) to a string; for example, one can convert a struct of type +// *config.Workload to a string by passing +// +// func(w *config.Workload) string { return w.Name }. +// +// Likewise, filterItemsByStrings can work on a list of strings by returning the unmodified item: +// +// filterItemsByStrings(wantedStrings, stringSlice2, func(s string) string { return s }) +// +// It returns a list of strings (items whose stringFunc() exists in the list of wantedStrings). +func filterItemsByStrings[T any](wantedStrings []string, possibleItems []T, stringFunc func(T) string) []string { + m := make(map[string]bool) + for _, item := range wantedStrings { + m[item] = true + } + res := make([]string, 0, len(wantedStrings)) + for _, item := range possibleItems { + if m[stringFunc(item)] { + res = append(res, stringFunc(item)) + } } - return filterStrings(stringWls, wantedNames) + return res } // WsPipeline fetches all the pipelines in a workspace and prompts the user to select one.