Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

generate binary-less dockerfiles #1338

Merged
merged 2 commits into from
Jun 24, 2024
Merged

generate binary-less dockerfiles #1338

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4a89a44
generate binary-less dockerfiles
grokspawn May 31, 2024
ca611d8
two-flag approach (with cleanup of old dockerfile)
grokspawn Jun 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 28 additions & 10 deletions alpha/action/generate_dockerfile.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ import (
)

type GenerateDockerfile struct {
BaseImage string
IndexDir string
ExtraLabels map[string]string
Writer io.Writer
BaseImage string
BuilderImage string
IndexDir string
ExtraLabels map[string]string
Writer io.Writer
}

func (i GenerateDockerfile) Run() error {
Expand All @@ -39,19 +40,36 @@ func (i GenerateDockerfile) validate() error {
return nil
}

const dockerfileTmpl = `# The base image is expected to contain
# /bin/opm (with a serve subcommand) and /bin/grpc_health_probe
const dockerfileTmpl = `# The builder image is expected to contain
# /bin/opm (with serve subcommand)
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 {{.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 }}
grokspawn marked this conversation as resolved.
Show resolved Hide resolved

# 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 }}
Expand Down
139 changes: 125 additions & 14 deletions alpha/action/generate_dockerfile_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,49 +41,160 @@ 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
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 operators.operatorframework.io.index.configs.v1=/configs

# Set other custom labels
LABEL "key1"="value1"
LABEL "key2"="value2"
`,
},

{
name: "Scratch/Fail/EmptyBaseImage",
gen: GenerateDockerfile{
BuilderImage: "foo",
IndexDir: "bar",
ExtraLabels: map[string]string{
"key1": "value1",
"key2": "value2",
},
},
expectedErr: "base image is unset",
},
{
name: "Scratch/Fail/EmptyFromDir",
gen: GenerateDockerfile{
BuilderImage: "foo",
BaseImage: "scratch",
ExtraLabels: map[string]string{
"key1": "value1",
"key2": "value2",
},
},
expectedErr: "index directory is unset",
},
{
name: "Scratch/Success/WithoutExtraLabels",
gen: GenerateDockerfile{
BuilderImage: "foo",
BaseImage: "scratch",
IndexDir: "bar",
},
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"]

# Set DC-specific label for the location of the DC root directory
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

# Set FBC-specific label for the location of the FBC root directory
# in the image
LABEL operators.operatorframework.io.index.configs.v1=/configs
`,
},
{
name: "Scratch/Success/WithExtraLabels",
gen: GenerateDockerfile{
BuilderImage: "foo",
BaseImage: "scratch",
IndexDir: "bar",
ExtraLabels: map[string]string{
"key1": "value1",
"key2": "value2",
},
},
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 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

# Set FBC-specific label for the location of the FBC root directory
# in the image
LABEL operators.operatorframework.io.index.configs.v1=/configs

Expand Down
41 changes: 29 additions & 12 deletions cmd/opm/generate/cmd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -28,24 +28,36 @@ func NewCmd() *cobra.Command {
func newDockerfileCmd() *cobra.Command {
var (
baseImage string
builderImage string
extraLabelStrs []string
)
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
Expand All @@ -71,19 +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,
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().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")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did we land on adding this?

cmd.MarkFlagsMutuallyExclusive("binary-image", "builder-image")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we said that we'd allow mixing them, while complaining.
So someone gives us binary-file and we go "that's deprecated", but we also use it appropriately.
The only special handling is if they use binary-image and nothing else, where we use that value for builder AND base.
But I'm not strongly opinionated on this, so let me know if you are.

return cmd
}

Expand Down
6 changes: 0 additions & 6 deletions ohio.Dockerfile
View file
Open in desktop

This file was deleted.

Loading