Skip to content

Commit

Permalink
feat(cli): add image flag to job init
Browse files Browse the repository at this point in the history
  • Loading branch information
iamhopaul123 committed Oct 7, 2020
1 parent ee187f3 commit 9ef9011
Show file tree
Hide file tree
Showing 10 changed files with 113 additions and 18 deletions.
18 changes: 14 additions & 4 deletions internal/pkg/cli/job_init.go
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 at the same time", dockerFileFlag, imageFlag)
}
if o.dockerfilePath != "" {
if _, err := o.fs.Stat(o.dockerfilePath); err != nil {
return err
Expand Down Expand Up @@ -209,14 +213,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 @@ -252,7 +261,7 @@ func (o *initJobOpts) askJobName() error {
}

func (o *initJobOpts) askDockerfile() error {
if o.dockerfilePath != "" {
if o.dockerfilePath != "" || o.image != "" {
return nil
}
df, err := o.sel.Dockerfile(
Expand Down Expand Up @@ -339,6 +348,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
71 changes: 69 additions & 2 deletions internal/pkg/cli/job_init_test.go
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 at the same time"),
},
}

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,19 @@ 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,
},
"prompt for existing dockerfile": {
inJobType: wantedJobType,
inJobName: wantedJobName,
Expand Down Expand Up @@ -274,6 +296,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 +320,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 +336,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 +382,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 +527,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
7 changes: 7 additions & 0 deletions internal/pkg/cli/svc_init_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ func TestSvcInitOpts_Ask(t *testing.T) {
wantedSvcName = "frontend"
wantedDockerfilePath = "frontend/Dockerfile"
wantedSvcPort = 80
wantedImage = "mockImage"
)
testCases := map[string]struct {
inSvcType string
Expand Down Expand Up @@ -333,6 +334,12 @@ func TestSvcInitOpts_Ask(t *testing.T) {
require.NoError(t, err)
require.Equal(t, wantedSvcType, opts.serviceType)
require.Equal(t, wantedSvcName, opts.name)
if opts.dockerfilePath != "" {
require.Equal(t, wantedDockerfilePath, opts.dockerfilePath)
}
if opts.image != "" {
require.Equal(t, wantedImage, opts.image)
}
}
})
}
Expand Down
13 changes: 9 additions & 4 deletions internal/pkg/manifest/job.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ type ScheduledJob struct {

// ScheduledJobConfig holds the configuration for a scheduled job
type ScheduledJobConfig struct {
ImageConfig Image `yaml:",flow"`
ImageConfig ServiceImageWithPort `yaml:",flow"`
TaskConfig `yaml:",inline"`
*Logging `yaml:"logging,flow"`
Sidecar `yaml:",inline"`
Expand Down Expand Up @@ -73,7 +73,7 @@ func newDefaultScheduledJob() *ScheduledJob {
Type: aws.String(ScheduledJobType),
},
ScheduledJobConfig: ScheduledJobConfig{
ImageConfig: Image{},
ImageConfig: ServiceImageWithPort{},
TaskConfig: TaskConfig{
CPU: aws.Int(256),
Memory: aws.Int(512),
Expand All @@ -90,7 +90,12 @@ func NewScheduledJob(props *ScheduledJobProps) *ScheduledJob {
job := newDefaultScheduledJob()
// Apply overrides.
job.Name = aws.String(props.Name)
job.ScheduledJobConfig.ImageConfig.Build.BuildArgs.Dockerfile = aws.String(props.Dockerfile)
if props.Dockerfile != "" {
job.ScheduledJobConfig.ImageConfig.Build.BuildArgs.Dockerfile = aws.String(props.Dockerfile)
}
if props.Image != "" {
job.ScheduledJobConfig.ImageConfig.Location = aws.String(props.Image)
}
job.Schedule = props.Schedule
job.Retries = props.Retries
job.Timeout = props.Timeout
Expand Down Expand Up @@ -133,7 +138,7 @@ func (j *ScheduledJob) BuildArgs(wsRoot string) *DockerBuildArgs {

// BuildRequired returns if the service requires building from the local Dockerfile.
func (j *ScheduledJob) BuildRequired() (bool, error) {
return requiresBuild(j.ImageConfig)
return requiresBuild(j.ImageConfig.Image)
}

// JobDockerfileBuildRequired returns if the job container image should be built from local Dockerfile.
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/manifest/job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ func TestScheduledJob_MarshalBinary(t *testing.T) {
"without timeout or retries": {
inProps: ScheduledJobProps{
WorkloadProps: &WorkloadProps{
Name: "cuteness-aggregator",
Dockerfile: "./cuteness-aggregator/Dockerfile",
Name: "cuteness-aggregator",
Image: "copilot/cuteness-aggregator",
},
Schedule: "@weekly",
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ name: cuteness-aggregator
type: Scheduled Job

image:
# Docker build arguments. You can specify additional overrides here. Supported: dockerfile, context, args
# Docker build arguments. You can specify additional overrides here. Supported: dockerfile, context, args.
build: ./cuteness-aggregator/Dockerfile

# Number of CPU units for the task.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ name: cuteness-aggregator
type: Scheduled Job

image:
# Docker build arguments. You can specify additional overrides here. Supported: dockerfile, context, args
# Docker build arguments. You can specify additional overrides here. Supported: dockerfile, context, args.
build: ./cuteness-aggregator/Dockerfile

# Number of CPU units for the task.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ name: cuteness-aggregator
type: Scheduled Job

image:
# Docker build arguments. You can specify additional overrides here. Supported: dockerfile, context, args
build: ./cuteness-aggregator/Dockerfile
# The Docker image used for your service.
location: copilot/cuteness-aggregator

# Number of CPU units for the task.
cpu: 256
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ name: cuteness-aggregator
type: Scheduled Job

image:
# Docker build arguments. You can specify additional overrides here. Supported: dockerfile, context, args
# Docker build arguments. You can specify additional overrides here. Supported: dockerfile, context, args.
build: ./cuteness-aggregator/Dockerfile

# Number of CPU units for the task.
Expand Down
8 changes: 7 additions & 1 deletion templates/workloads/jobs/scheduled-job/manifest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,14 @@ name: {{.Name}}
type: {{.Type}}

image:
# Docker build arguments. You can specify additional overrides here. Supported: dockerfile, context, args
{{- if .ImageConfig.Build.BuildArgs.Dockerfile}}
# Docker build arguments. You can specify additional overrides here. Supported: dockerfile, context, args.
build: {{.ImageConfig.Build.BuildArgs.Dockerfile}}
{{- end}}
{{- if .ImageConfig.Image.Location}}
# The Docker image used for your service.
location: {{.ImageConfig.Image.Location}}
{{- end}}

# Number of CPU units for the task.
cpu: {{.CPU}}
Expand Down

0 comments on commit 9ef9011

Please sign in to comment.