Skip to content

Commit

Permalink
feat(cli): add image flag to svc/job init (aws#1473)
Browse files Browse the repository at this point in the history 
<!-- Provide summary of changes -->
Add image flag to `svc init` and `job init` so that users can opt-in using existing docker image instead of building from local Dockerfile, and write to the manifest.
<!-- Issue number, if available. E.g. "Fixes aws#31", "Addresses aws#42, 77" -->

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
  • Loading branch information
@iamhopaul123 @thrau
iamhopaul123 authored and thrau committed Dec 9, 2022
1 parent cc6689c commit 7142c43
Show file tree
Hide file tree
Showing 23 changed files with 440 additions and 121 deletions.
9 changes: 6 additions & 3 deletions internal/pkg/cli/flag.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ const (
)

// Short flag names.
// A short flag only exists if the flag is mandatory by the command.
// A short flag only exists if the flag or flag set is mandatory by the command.
const (
nameFlagShort = "n"
appFlagShort = "a"
Expand All @@ -100,6 +100,7 @@ const (
jobTypeFlagShort = "t"

dockerFileFlagShort = "d"
imageFlagShort = "i"
githubURLFlagShort = "u"
githubAccessTokenFlagShort = "t"
gitBranchFlagShort = "b"
Expand All @@ -112,6 +113,10 @@ const (
var (
svcTypeFlagDescription = fmt.Sprintf(`Type of service to create. Must be one of:
%s`, strings.Join(template.QuoteSliceFunc(manifest.ServiceTypes), ", "))
imageFlagDescription = fmt.Sprintf(`The location of an existing Docker image.
Mutually exclusive with -%s, --%s`, dockerFileFlagShort, dockerFileFlag)
dockerFileFlagDescription = fmt.Sprintf(`Path to the Dockerfile.
Mutually exclusive with -%s, --%s`, imageFlagShort, imageFlag)
storageTypeFlagDescription = fmt.Sprintf(`Type of storage to add. Must be one of:
%s`, strings.Join(template.QuoteSliceFunc(storageTypes), ", "))
jobTypeFlagDescription = fmt.Sprintf(`Type of job to create. Must be one of:
Expand Down Expand Up @@ -139,7 +144,6 @@ const (
yesFlagDescription = "Skips confirmation prompt."
jsonFlagDescription = "Optional. Outputs in JSON format."

dockerFileFlagDescription = "Path to the Dockerfile."
imageTagFlagDescription = `Optional. The container image tag.`
resourceTagsFlagDescription = `Optional. Labels with a key and value separated with commas.
Allows you to categorize resources.`
Expand Down Expand Up @@ -185,7 +189,6 @@ Must be of the format '<keyName>:<dataType>'.`
countFlagDescription = "Optional. The number of tasks to set up."
cpuFlagDescription = "Optional. The number of CPU units to reserve for each task."
memoryFlagDescription = "Optional. The amount of memory to reserve in MiB for each task."
imageFlagDescription = "Optional. The image to run instead of building a Dockerfile."
taskRoleFlagDescription = "Optional. The ARN of the role for the task to use."
executionRoleFlagDescription = "Optional. The ARN of the role that grants the container agent permission to make AWS API calls."
envVarsFlagDescription = "Optional. Environment variables specified by key=value separated with commas."
Expand Down
49 changes: 41 additions & 8 deletions internal/pkg/cli/job_init.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ type initJobVars struct {
appName string
name string
dockerfilePath string
image string
timeout string
retries int
schedule string
Expand Down Expand Up @@ -109,6 +110,9 @@ func (o *initJobOpts) Validate() error {
return err
}
}
if o.dockerfilePath != "" && o.image != "" {
return fmt.Errorf("--%s and --%s cannot be specified together", dockerFileFlag, imageFlag)
}
if o.dockerfilePath != "" {
if _, err := o.fs.Stat(o.dockerfilePath); err != nil {
return err
Expand Down Expand Up @@ -138,9 +142,15 @@ func (o *initJobOpts) Ask() error {
if err := o.askJobName(); err != nil {
return err
}
if err := o.askDockerfile(); err != nil {
dfSelected, err := o.askDockerfile()
if err != nil {
return err
}
if !dfSelected {
if err := o.askImage(); err != nil {
return err
}
}
if err := o.askSchedule(); err != nil {
return err
}
Expand Down Expand Up @@ -209,14 +219,19 @@ func (o *initJobOpts) createManifest() (string, error) {
}

func (o *initJobOpts) newJobManifest() (*manifest.ScheduledJob, error) {
dfPath, err := relativeDockerfilePath(o.ws, o.dockerfilePath)
if err != nil {
return nil, err
var dfPath string
if o.dockerfilePath != "" {
path, err := relativeDockerfilePath(o.ws, o.dockerfilePath)
if err != nil {
return nil, err
}
dfPath = path
}
return manifest.NewScheduledJob(&manifest.ScheduledJobProps{
WorkloadProps: &manifest.WorkloadProps{
Name: o.name,
Dockerfile: dfPath,
Image: o.image,
},
Schedule: o.schedule,
Timeout: o.timeout,
Expand Down Expand Up @@ -251,10 +266,24 @@ func (o *initJobOpts) askJobName() error {
return nil
}

func (o *initJobOpts) askDockerfile() error {
if o.dockerfilePath != "" {
func (o *initJobOpts) askImage() error {
if o.image != "" {
return nil
}
image, err := o.prompt.Get(wkldInitImagePrompt, wkldInitImagePromptHelp, nil,
prompt.WithFinalMessage("Image:"))
if err != nil {
return fmt.Errorf("get image location: %w", err)
}
o.image = image
return nil
}

// isDfSelected indicates if any Dockerfile is in use.
func (o *initJobOpts) askDockerfile() (isDfSelected bool, err error) {
if o.dockerfilePath != "" || o.image != "" {
return true, nil
}
df, err := o.sel.Dockerfile(
fmt.Sprintf(fmtWkldInitDockerfilePrompt, color.HighlightUserInput(o.name)),
fmt.Sprintf(fmtWkldInitDockerfilePathPrompt, color.HighlightUserInput(o.name)),
Expand All @@ -265,10 +294,13 @@ func (o *initJobOpts) askDockerfile() error {
},
)
if err != nil {
return fmt.Errorf("select Dockerfile: %w", err)
return false, fmt.Errorf("select Dockerfile: %w", err)
}
if df == selector.DockerfilePromptUseImage {
return false, nil
}
o.dockerfilePath = df
return nil
return true, nil
}

func (o *initJobOpts) askSchedule() error {
Expand Down Expand Up @@ -339,6 +371,7 @@ func buildJobInitCmd() *cobra.Command {
cmd.Flags().StringVarP(&vars.schedule, scheduleFlag, scheduleFlagShort, "", scheduleFlagDescription)
cmd.Flags().StringVar(&vars.timeout, timeoutFlag, "", timeoutFlagDescription)
cmd.Flags().IntVar(&vars.retries, retriesFlag, 0, retriesFlagDescription)
cmd.Flags().StringVarP(&vars.image, imageFlag, imageFlagShort, "", imageFlagDescription)

cmd.Annotations = map[string]string{
"group": group.Develop,
Expand Down
114 changes: 112 additions & 2 deletions internal/pkg/cli/job_init_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ func TestJobInitOpts_Validate(t *testing.T) {
testCases := map[string]struct {
inJobName string
inDockerfilePath string
inImage string
inTimeout string
inRetries int
inSchedule string
Expand Down Expand Up @@ -102,13 +103,19 @@ func TestJobInitOpts_Validate(t *testing.T) {
inRetries: -3,
wantedErr: errors.New("number of retries must be non-negative"),
},
"fail if both image and dockerfile are set": {
inDockerfilePath: "mockDockerfile",
inImage: "mockImage",
wantedErr: fmt.Errorf("--dockerfile and --image cannot be specified together"),
},
}

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
opts := initJobOpts{
initJobVars: initJobVars{
name: tc.inJobName,
image: tc.inImage,
dockerfilePath: tc.inDockerfilePath,
timeout: tc.inTimeout,
retries: tc.inRetries,
Expand Down Expand Up @@ -137,11 +144,13 @@ func TestJobInitOpts_Ask(t *testing.T) {
wantedJobType = manifest.ScheduledJobType
wantedJobName = "cuteness-aggregator"
wantedDockerfilePath = "cuteness-aggregator/Dockerfile"
wantedImage = "mockImage"
wantedCronSchedule = "0 9-17 * * MON-FRI"
)
testCases := map[string]struct {
inJobType string
inJobName string
inImage string
inDockerfilePath string
inJobSchedule string

Expand Down Expand Up @@ -185,6 +194,62 @@ func TestJobInitOpts_Ask(t *testing.T) {
mockSel: func(m *mocks.MockinitJobSelector) {},
wantedErr: fmt.Errorf("get job name: some error"),
},
"skip selecting Dockerfile if image flag is set": {
inJobType: wantedJobType,
inJobName: wantedJobName,
inImage: "mockImage",
inDockerfilePath: "",
inJobSchedule: wantedCronSchedule,

mockPrompt: func(m *mocks.Mockprompter) {},
mockSel: func(m *mocks.MockinitJobSelector) {},
mockFileSystem: func(mockFS afero.Fs) {},
wantedErr: nil,
wantedSchedule: wantedCronSchedule,
},
"returns an error if fail to get image location": {
inJobType: wantedJobType,
inJobName: wantedJobName,
inDockerfilePath: "",

mockPrompt: func(m *mocks.Mockprompter) {
m.EXPECT().Get(wkldInitImagePrompt, wkldInitImagePromptHelp, nil, gomock.Any()).
Return("", mockError)
},
mockSel: func(m *mocks.MockinitJobSelector) {
m.EXPECT().Dockerfile(
gomock.Eq(fmt.Sprintf(fmtWkldInitDockerfilePrompt, wantedJobName)),
gomock.Eq(fmt.Sprintf(fmtWkldInitDockerfilePathPrompt, wantedJobName)),
gomock.Eq(wkldInitDockerfileHelpPrompt),
gomock.Eq(wkldInitDockerfilePathHelpPrompt),
gomock.Any(),
).Return("Use an existing image instead", nil)
},
mockFileSystem: func(mockFS afero.Fs) {},
wantedErr: fmt.Errorf("get image location: mock error"),
},
"using existing image": {
inJobType: wantedJobType,
inJobName: wantedJobName,
inJobSchedule: wantedCronSchedule,
inDockerfilePath: "",

mockPrompt: func(m *mocks.Mockprompter) {
m.EXPECT().Get(wkldInitImagePrompt, wkldInitImagePromptHelp, nil, gomock.Any()).
Return("mockImage", nil)
},
mockSel: func(m *mocks.MockinitJobSelector) {
m.EXPECT().Dockerfile(
gomock.Eq(fmt.Sprintf(fmtWkldInitDockerfilePrompt, wantedJobName)),
gomock.Eq(fmt.Sprintf(fmtWkldInitDockerfilePathPrompt, wantedJobName)),
gomock.Eq(wkldInitDockerfileHelpPrompt),
gomock.Eq(wkldInitDockerfilePathHelpPrompt),
gomock.Any(),
).Return("Use an existing image instead", nil)
},
mockFileSystem: func(mockFS afero.Fs) {},
wantedSchedule: wantedCronSchedule,
},
"prompt for existing dockerfile": {
inJobType: wantedJobType,
inJobName: wantedJobName,
Expand Down Expand Up @@ -274,6 +339,7 @@ func TestJobInitOpts_Ask(t *testing.T) {
initJobVars: initJobVars{
jobType: tc.inJobType,
name: tc.inJobName,
image: tc.inImage,
dockerfilePath: tc.inDockerfilePath,
schedule: tc.inJobSchedule,
},
Expand All @@ -297,9 +363,13 @@ func TestJobInitOpts_Ask(t *testing.T) {
require.NoError(t, err)
require.Equal(t, wantedJobType, opts.jobType)
require.Equal(t, wantedJobName, opts.name)
require.Equal(t, wantedDockerfilePath, opts.dockerfilePath)
if opts.dockerfilePath != "" {
require.Equal(t, wantedDockerfilePath, opts.dockerfilePath)
}
if opts.image != "" {
require.Equal(t, wantedImage, opts.image)
}
require.Equal(t, tc.wantedSchedule, opts.schedule)

})
}
}
Expand All @@ -309,6 +379,7 @@ func TestJobInitOpts_Execute(t *testing.T) {
inJobType string
inJobName string
inDockerfilePath string
inImage string
inAppName string
mockWriter func(m *mocks.MockjobDirManifestWriter)
mockstore func(m *mocks.Mockstore)
Expand Down Expand Up @@ -354,6 +425,44 @@ func TestJobInitOpts_Execute(t *testing.T) {
m.EXPECT().Stop(log.Ssuccessf(fmtAddJobToAppComplete, "resizer"))
},
},
"using existing image": {
inJobType: manifest.ScheduledJobType,
inAppName: "app",
inJobName: "resizer",
inImage: "mockImage",

mockWriter: func(m *mocks.MockjobDirManifestWriter) {
m.EXPECT().WriteJobManifest(gomock.Any(), "resizer").Do(func(m *manifest.ScheduledJob, _ string) {
require.Equal(t, *m.Workload.Type, manifest.ScheduledJobType)
require.Equal(t, *m.ImageConfig.Location, "mockImage")
}).Return("/resizer/manifest.yml", nil)
},
mockstore: func(m *mocks.Mockstore) {
m.EXPECT().CreateJob(gomock.Any()).
Do(func(app *config.Workload) {
require.Equal(t, &config.Workload{
Name: "resizer",
App: "app",
Type: manifest.ScheduledJobType,
}, app)
}).
Return(nil)
m.EXPECT().GetApplication("app").Return(&config.Application{
Name: "app",
AccountID: "1234",
}, nil)
},
mockappDeployer: func(m *mocks.MockappDeployer) {
m.EXPECT().AddJobToApp(&config.Application{
Name: "app",
AccountID: "1234",
}, "resizer")
},
mockProg: func(m *mocks.Mockprogress) {
m.EXPECT().Start(fmt.Sprintf(fmtAddJobToAppStart, "resizer"))
m.EXPECT().Stop(log.Ssuccessf(fmtAddJobToAppComplete, "resizer"))
},
},
"write manifest error": {
inJobType: manifest.ScheduledJobType,
inAppName: "app",
Expand Down Expand Up @@ -461,6 +570,7 @@ func TestJobInitOpts_Execute(t *testing.T) {
appName: tc.inAppName,
name: tc.inJobName,
dockerfilePath: tc.inDockerfilePath,
image: tc.inImage,
jobType: tc.inJobType,
},
ws: mockWriter,
Expand Down
Loading

0 comments on commit 7142c43

Please sign in to comment.