two-flag approach (with cleanup of old dockerfile)

Signed-off-by: Jordan Keister <[email protected]>

Author: grokspawn

alpha/action/generate_dockerfile.go

@@ -9,26 +9,19 @@ 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 {
 	if err := i.validate(); err != nil {
 		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,36 @@ 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) and /bin/grpc_health_probe
+
 # Configure the entrypoint and command
 ENTRYPOINT ["/bin/opm"]
 CMD ["serve", "/configs", "--cache-dir=/tmp/cache"]
+{{- else }}
+# OLMv0 CatalogSources that use binary-less images must set:
+# spec:
+#   grpcPodConfig:
+#     extractContent:
+#       catalogDir: /configs
+#       cacheDir: /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 }}

alpha/action/generate_dockerfile_test.go

@@ -41,49 +41,65 @@ 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
+			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) and /bin/grpc_health_probe
 
 # 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
 `,
 		},
 		{
 			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
+			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) and /bin/grpc_health_probe
 
 # 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 +110,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)
@@ -133,6 +149,12 @@ ADD bar /configs
 RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]
 
 FROM scratch
+# OLMv0 CatalogSources that use binary-less images must set:
+# spec:
+#   grpcPodConfig:
+#     extractContent:
+#       catalogDir: /configs
+#       cacheDir: /tmp/cache
 
 COPY --from=builder /configs /configs
 COPY --from=builder /tmp/cache /tmp/cache
@@ -143,15 +165,15 @@ LABEL operators.operatorframework.io.index.configs.v1=/configs
 `,
 		},
 		{
-			name: "Lite/Success/WithExtraLabels",
+			name: "Scratch/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)
@@ -162,6 +184,12 @@ ADD bar /configs
 RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]
 
 FROM scratch
+# OLMv0 CatalogSources that use binary-less images must set:
+# spec:
+#   grpcPodConfig:
+#     extractContent:
+#       catalogDir: /configs
+#       cacheDir: /tmp/cache
 
 COPY --from=builder /configs /configs
 COPY --from=builder /tmp/cache /tmp/cache

cmd/opm/generate/cmd.go

@@ -17,7 +17,7 @@ import (
 func NewCmd() *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "generate",
-		Short: "Generate various artifacts for declarative config indexes",
+		Short: "Generate various artifacts for file-based catalogs",
 	}
 	cmd.AddCommand(
 		newDockerfileCmd(),
@@ -28,25 +28,36 @@ func NewCmd() *cobra.Command {
 func newDockerfileCmd() *cobra.Command {
 	var (
 		baseImage      string
+		builderImage   string
 		extraLabelStrs []string
-		lite           bool
 	)
 	cmd := &cobra.Command{
-		Use:   "dockerfile <dcRootDir>",
+		Use:   "dockerfile <fbcRootDir>",
 		Args:  cobra.ExactArgs(1),
-		Short: "Generate a Dockerfile for a declarative config index",
-		Long: `Generate a Dockerfile for a declarative config index.
+		Short: "Generate a Dockerfile for a file-based catalog",
+		Long: `Generate a Dockerfile for a file-based catalog.
 
-This command creates a Dockerfile in the same directory as the <dcRootDir>
-(named <dcDirName>.Dockerfile) that can be used to build the index. If a
+This command creates a Dockerfile in the same directory as the <fbcRootDir>
+(named <fbcRootDir>.Dockerfile) that can be used to build the index. If a
 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.
+
+A separate builder and base image can be specified. The builder image may not be "scratch".
 `,
-		RunE: func(_ *cobra.Command, args []string) error {
+		RunE: func(inCmd *cobra.Command, args []string) error {
 			fromDir := filepath.Clean(args[0])
 
+			if builderImage == "scratch" {
+				return fmt.Errorf("invalid builder image: %q", builderImage)
+			}
+
+			// preserving old behavior, if binary-image is set but not builder-image, set builder-image to binary-image
+			if inCmd.Flags().Changed("binary-image") && !inCmd.Flags().Changed("builder-image") {
+				builderImage = baseImage
+			}
+
 			extraLabels, err := parseLabels(extraLabelStrs)
 			if err != nil {
 				return err
@@ -72,21 +83,24 @@ 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:    baseImage,
+				BuilderImage: builderImage,
+				IndexDir:     indexName,
+				ExtraLabels:  extraLabels,
+				Writer:       f,
 			}
 			if err := gen.Run(); err != nil {
 				log.Fatal(err)
 			}
 			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().StringVar(&baseImage, "binary-image", containertools.DefaultBinarySourceImage, "Image in which to build catalog.")
+	cmd.Flags().StringVarP(&baseImage, "base-image", "i", containertools.DefaultBinarySourceImage, "Image base to use 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'.")
+	cmd.Flags().MarkDeprecated("binary-image", "use --base-image instead")
+	cmd.MarkFlagsMutuallyExclusive("binary-image", "base-image")
 	return cmd
 }