From 3273a3ab8700aa29b356f2a62b419154bf2808ed Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Wed, 12 Jun 2024 16:32:58 -0500 Subject: [PATCH] two-flag approach (with cleanup of old dockerfile) Signed-off-by: Jordan Keister --- alpha/action/generate_dockerfile.go | 53 +++++----------- alpha/action/generate_dockerfile_test.go | 76 ++++++++++++++--------- cmd/opm/generate/cmd.go | 36 ++++++++--- ohio.Dockerfile | 6 -- pkg/containertools/dockerfilegenerator.go | 9 +-- 5 files changed, 91 insertions(+), 89 deletions(-) delete mode 100644 ohio.Dockerfile diff --git a/alpha/action/generate_dockerfile.go b/alpha/action/generate_dockerfile.go index 7c33a87c3f..421a2749b5 100644 --- a/alpha/action/generate_dockerfile.go +++ b/alpha/action/generate_dockerfile.go @@ -9,11 +9,11 @@ import ( ) type GenerateDockerfile struct { - BaseImage string - IndexDir string - ExtraLabels map[string]string - Writer io.Writer - Lite bool + BaseImage string + BuilderImage string + IndexDir string + ExtraLabels map[string]string + Writer io.Writer } func (i GenerateDockerfile) Run() error { @@ -21,14 +21,7 @@ func (i GenerateDockerfile) Run() error { return err } - var dockerfileTemplate string - if i.Lite { - dockerfileTemplate = binlessDockerfileTmpl - } else { - dockerfileTemplate = dockerfileTmpl - } - - t, err := template.New("dockerfile").Parse(dockerfileTemplate) + t, err := template.New("dockerfile").Parse(dockerfileTmpl) if err != nil { // The template is hardcoded in the binary, so if // there is a parse error, it was a programmer error. @@ -47,44 +40,28 @@ func (i GenerateDockerfile) validate() error { return nil } -const binlessDockerfileTmpl = `# The builder image is expected to contain +const dockerfileTmpl = `# The builder image is expected to contain # /bin/opm (with serve subcommand) -FROM {{.BaseImage}} as builder +FROM {{.BuilderImage}} as builder # Copy FBC root into image at /configs and pre-populate serve cache ADD {{.IndexDir}} /configs RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"] -FROM scratch - -COPY --from=builder /configs /configs -COPY --from=builder /tmp/cache /tmp/cache - -# Set FBC-specific label for the location of the FBC root directory -# in the image -LABEL ` + containertools.ConfigsLocationLabel + `=/configs -{{- if .ExtraLabels }} - -# Set other custom labels -{{- range $key, $value := .ExtraLabels }} -LABEL "{{ $key }}"="{{ $value }}" -{{- end }} -{{- end }} -` - -const dockerfileTmpl = `# The base image is expected to contain -# /bin/opm (with a serve subcommand) and /bin/grpc_health_probe FROM {{.BaseImage}} +{{- if ne .BaseImage "scratch" }} +# The base image is expected to contain +# /bin/opm (with serve subcommand) # Configure the entrypoint and command ENTRYPOINT ["/bin/opm"] CMD ["serve", "/configs", "--cache-dir=/tmp/cache"] +{{- end }} -# Copy declarative config root into image at /configs and pre-populate serve cache -ADD {{.IndexDir}} /configs -RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"] +COPY --from=builder /configs /configs +COPY --from=builder /tmp/cache /tmp/cache -# Set DC-specific label for the location of the DC root directory +# Set FBC-specific label for the location of the FBC root directory # in the image LABEL ` + containertools.ConfigsLocationLabel + `=/configs {{- if .ExtraLabels }} diff --git a/alpha/action/generate_dockerfile_test.go b/alpha/action/generate_dockerfile_test.go index 54d805f4a7..cf87dbcb4f 100644 --- a/alpha/action/generate_dockerfile_test.go +++ b/alpha/action/generate_dockerfile_test.go @@ -41,22 +41,29 @@ func TestGenerateDockerfile(t *testing.T) { { name: "Success/WithoutExtraLabels", gen: GenerateDockerfile{ - BaseImage: "foo", - IndexDir: "bar", + BuilderImage: "foo", + BaseImage: "foo", + IndexDir: "bar", }, - expectedDockerfile: `# The base image is expected to contain -# /bin/opm (with a serve subcommand) and /bin/grpc_health_probe -FROM foo + expectedDockerfile: `# The builder image is expected to contain +# /bin/opm (with serve subcommand) +FROM foo as builder + +# Copy FBC root into image at /configs and pre-populate serve cache +ADD bar /configs +RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"] +FROM foo +# The base image is expected to contain +# /bin/opm (with serve subcommand) # Configure the entrypoint and command ENTRYPOINT ["/bin/opm"] CMD ["serve", "/configs", "--cache-dir=/tmp/cache"] -# Copy declarative config root into image at /configs and pre-populate serve cache -ADD bar /configs -RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"] +COPY --from=builder /configs /configs +COPY --from=builder /tmp/cache /tmp/cache -# Set DC-specific label for the location of the DC root directory +# Set FBC-specific label for the location of the FBC root directory # in the image LABEL operators.operatorframework.io.index.configs.v1=/configs `, @@ -64,26 +71,33 @@ LABEL operators.operatorframework.io.index.configs.v1=/configs { name: "Success/WithExtraLabels", gen: GenerateDockerfile{ - BaseImage: "foo", - IndexDir: "bar", + BuilderImage: "foo", + BaseImage: "foo", + IndexDir: "bar", ExtraLabels: map[string]string{ "key1": "value1", "key2": "value2", }, }, - expectedDockerfile: `# The base image is expected to contain -# /bin/opm (with a serve subcommand) and /bin/grpc_health_probe -FROM foo + expectedDockerfile: `# The builder image is expected to contain +# /bin/opm (with serve subcommand) +FROM foo as builder +# Copy FBC root into image at /configs and pre-populate serve cache +ADD bar /configs +RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"] + +FROM foo +# The base image is expected to contain +# /bin/opm (with serve subcommand) # Configure the entrypoint and command ENTRYPOINT ["/bin/opm"] CMD ["serve", "/configs", "--cache-dir=/tmp/cache"] -# Copy declarative config root into image at /configs and pre-populate serve cache -ADD bar /configs -RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"] +COPY --from=builder /configs /configs +COPY --from=builder /tmp/cache /tmp/cache -# Set DC-specific label for the location of the DC root directory +# Set FBC-specific label for the location of the FBC root directory # in the image LABEL operators.operatorframework.io.index.configs.v1=/configs @@ -94,35 +108,35 @@ LABEL "key2"="value2" }, { - name: "Lite/Fail/EmptyBaseImage", + name: "Scratch/Fail/EmptyBaseImage", gen: GenerateDockerfile{ - IndexDir: "bar", + BuilderImage: "foo", + IndexDir: "bar", ExtraLabels: map[string]string{ "key1": "value1", "key2": "value2", }, - Lite: true, }, expectedErr: "base image is unset", }, { - name: "Lite/Fail/EmptyFromDir", + name: "Scratch/Fail/EmptyFromDir", gen: GenerateDockerfile{ - BaseImage: "foo", + BuilderImage: "foo", + BaseImage: "scratch", ExtraLabels: map[string]string{ "key1": "value1", "key2": "value2", }, - Lite: true, }, expectedErr: "index directory is unset", }, { - name: "Lite/Success/WithoutExtraLabels", + name: "Scratch/Success/WithoutExtraLabels", gen: GenerateDockerfile{ - BaseImage: "foo", - IndexDir: "bar", - Lite: true, + BuilderImage: "foo", + BaseImage: "scratch", + IndexDir: "bar", }, expectedDockerfile: `# The builder image is expected to contain # /bin/opm (with serve subcommand) @@ -145,13 +159,13 @@ LABEL operators.operatorframework.io.index.configs.v1=/configs { name: "Lite/Success/WithExtraLabels", gen: GenerateDockerfile{ - BaseImage: "foo", - IndexDir: "bar", + BuilderImage: "foo", + BaseImage: "scratch", + IndexDir: "bar", ExtraLabels: map[string]string{ "key1": "value1", "key2": "value2", }, - Lite: true, }, expectedDockerfile: `# The builder image is expected to contain # /bin/opm (with serve subcommand) diff --git a/cmd/opm/generate/cmd.go b/cmd/opm/generate/cmd.go index 0e3c80df40..7db8f8673f 100644 --- a/cmd/opm/generate/cmd.go +++ b/cmd/opm/generate/cmd.go @@ -27,9 +27,9 @@ func NewCmd() *cobra.Command { func newDockerfileCmd() *cobra.Command { var ( - baseImage string + binaryImage string + builderImage string extraLabelStrs []string - lite bool ) cmd := &cobra.Command{ Use: "dockerfile ", @@ -43,10 +43,26 @@ Dockerfile with the same name already exists, this command will fail. When specifying extra labels, note that if duplicate keys exist, only the last value of each duplicate key will be added to the generated Dockerfile. + +If --builder-image is set, this will generate a multi-stage Dockerfile which + will NOT include a serving binary. + If --builder-image is set, then --binary-image must not be set to anything other than "scratch". +Note: '--builder-image' and '--binary-image' are mutually exclusive. `, - RunE: func(_ *cobra.Command, args []string) error { + RunE: func(inCmd *cobra.Command, args []string) error { fromDir := filepath.Clean(args[0]) + switch { + // user specified both flags ... reject unless they set binary-image to scratch + case inCmd.Flags().Changed("binary-image") && inCmd.Flags().Changed("builder-image"): + if binaryImage != "scratch" { + return fmt.Errorf("invalid flag combination: if specifying --builder-image then --binary-image can only be \"scratch\" or default") + } + // user specifed only the builder image, set binary image to scratch + case inCmd.Flags().Changed("builder-image"): + binaryImage = "scratch" + } + extraLabels, err := parseLabels(extraLabelStrs) if err != nil { return err @@ -72,11 +88,11 @@ value of each duplicate key will be added to the generated Dockerfile. defer f.Close() gen := action.GenerateDockerfile{ - BaseImage: baseImage, - IndexDir: indexName, - ExtraLabels: extraLabels, - Writer: f, - Lite: lite, + BaseImage: binaryImage, + BuilderImage: builderImage, + IndexDir: indexName, + ExtraLabels: extraLabels, + Writer: f, } if err := gen.Run(); err != nil { log.Fatal(err) @@ -84,8 +100,8 @@ value of each duplicate key will be added to the generated Dockerfile. return nil }, } - cmd.Flags().StringVarP(&baseImage, "binary-image", "i", containertools.DefaultBinarySourceImage, "Image in which to build catalog.") - cmd.Flags().BoolVarP(&lite, "lite", "t", false, "Generate a smaller, binary-less Dockerfile.") + cmd.Flags().StringVarP(&binaryImage, "binary-image", "i", containertools.DefaultBinarySourceImage, "Image in which to build catalog.") + cmd.Flags().StringVarP(&builderImage, "builder-image", "b", containertools.DefaultBinarySourceImage, "Image to use as a build stage.") cmd.Flags().StringSliceVarP(&extraLabelStrs, "extra-labels", "l", []string{}, "Extra labels to include in the generated Dockerfile. Labels should be of the form 'key=value'.") return cmd } diff --git a/ohio.Dockerfile b/ohio.Dockerfile deleted file mode 100644 index 9c8680fc89..0000000000 --- a/ohio.Dockerfile +++ /dev/null @@ -1,6 +0,0 @@ -FROM quay.io/operatorhubio/catalog:latest - -COPY opm /bin/opm - -ENTRYPOINT ["/bin/opm"] -CMD ["serve", "/configs", "--cache-dir=/tmp/cache"] diff --git a/pkg/containertools/dockerfilegenerator.go b/pkg/containertools/dockerfilegenerator.go index 79059b9ee7..b9ebc476a9 100644 --- a/pkg/containertools/dockerfilegenerator.go +++ b/pkg/containertools/dockerfilegenerator.go @@ -8,10 +8,11 @@ import ( ) const ( - DefaultBinarySourceImage = "quay.io/operator-framework/opm:latest" - DefaultDbLocation = "/database/index.db" - DbLocationLabel = "operators.operatorframework.io.index.database.v1" - ConfigsLocationLabel = "operators.operatorframework.io.index.configs.v1" + DefaultScratchSourceImage = "scratch" + DefaultBinarySourceImage = "quay.io/operator-framework/opm:latest" + DefaultDbLocation = "/database/index.db" + DbLocationLabel = "operators.operatorframework.io.index.database.v1" + ConfigsLocationLabel = "operators.operatorframework.io.index.configs.v1" ) // DockerfileGenerator defines functions to generate index dockerfiles