Skip to content

Commit 27f3514

Browse files
committed
two-flag approach (with cleanup of old dockerfile)
Signed-off-by: Jordan Keister <[email protected]>
1 parent 4a89a44 commit 27f3514

File tree

5 files changed

+101
-87
lines changed

5 files changed

+101
-87
lines changed

Diff for: alpha/action/generate_dockerfile.go

+21-36
Original file line numberDiff line numberDiff line change
@@ -9,24 +9,25 @@ import (
99
)
1010

1111
type GenerateDockerfile struct {
12-
BaseImage string
13-
IndexDir string
14-
ExtraLabels map[string]string
15-
Writer io.Writer
16-
Lite bool
12+
BaseImage string
13+
BuilderImage string
14+
IndexDir string
15+
ExtraLabels map[string]string
16+
Writer io.Writer
1717
}
1818

1919
func (i GenerateDockerfile) Run() error {
2020
if err := i.validate(); err != nil {
2121
return err
2222
}
2323

24-
var dockerfileTemplate string
25-
if i.Lite {
26-
dockerfileTemplate = binlessDockerfileTmpl
24+
var entrypoint string
25+
if i.BaseImage == containertools.DefaultScratchSourceImage {
26+
entrypoint = ""
2727
} else {
28-
dockerfileTemplate = dockerfileTmpl
28+
entrypoint = entrypointStanza
2929
}
30+
dockerfileTemplate := fmt.Sprintf(dockerfileStanza, entrypoint)
3031

3132
t, err := template.New("dockerfile").Parse(dockerfileTemplate)
3233
if err != nil {
@@ -47,15 +48,23 @@ func (i GenerateDockerfile) validate() error {
4748
return nil
4849
}
4950

50-
const binlessDockerfileTmpl = `# The builder image is expected to contain
51+
const entrypointStanza = `# The base image is expected to contain
5152
# /bin/opm (with serve subcommand)
52-
FROM {{.BaseImage}} as builder
53+
# Configure the entrypoint and command
54+
ENTRYPOINT ["/bin/opm"]
55+
CMD ["serve", "/configs", "--cache-dir=/tmp/cache"]
56+
`
57+
58+
const dockerfileStanza = `# The builder image is expected to contain
59+
# /bin/opm (with serve subcommand)
60+
FROM {{.BuilderImage}} as builder
5361
5462
# Copy FBC root into image at /configs and pre-populate serve cache
5563
ADD {{.IndexDir}} /configs
5664
RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]
5765
58-
FROM scratch
66+
FROM {{.BaseImage}}
67+
%s
5968
6069
COPY --from=builder /configs /configs
6170
COPY --from=builder /tmp/cache /tmp/cache
@@ -71,27 +80,3 @@ LABEL "{{ $key }}"="{{ $value }}"
7180
{{- end }}
7281
{{- end }}
7382
`
74-
75-
const dockerfileTmpl = `# The base image is expected to contain
76-
# /bin/opm (with a serve subcommand) and /bin/grpc_health_probe
77-
FROM {{.BaseImage}}
78-
79-
# Configure the entrypoint and command
80-
ENTRYPOINT ["/bin/opm"]
81-
CMD ["serve", "/configs", "--cache-dir=/tmp/cache"]
82-
83-
# Copy declarative config root into image at /configs and pre-populate serve cache
84-
ADD {{.IndexDir}} /configs
85-
RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]
86-
87-
# Set DC-specific label for the location of the DC root directory
88-
# in the image
89-
LABEL ` + containertools.ConfigsLocationLabel + `=/configs
90-
{{- if .ExtraLabels }}
91-
92-
# Set other custom labels
93-
{{- range $key, $value := .ExtraLabels }}
94-
LABEL "{{ $key }}"="{{ $value }}"
95-
{{- end }}
96-
{{- end }}
97-
`

Diff for: alpha/action/generate_dockerfile_test.go

+49-31
Original file line numberDiff line numberDiff line change
@@ -41,49 +41,65 @@ func TestGenerateDockerfile(t *testing.T) {
4141
{
4242
name: "Success/WithoutExtraLabels",
4343
gen: GenerateDockerfile{
44-
BaseImage: "foo",
45-
IndexDir: "bar",
44+
BuilderImage: "foo",
45+
BaseImage: "foo",
46+
IndexDir: "bar",
4647
},
47-
expectedDockerfile: `# The base image is expected to contain
48-
# /bin/opm (with a serve subcommand) and /bin/grpc_health_probe
49-
FROM foo
48+
expectedDockerfile: `# The builder image is expected to contain
49+
# /bin/opm (with serve subcommand)
50+
FROM foo as builder
5051
52+
# Copy FBC root into image at /configs and pre-populate serve cache
53+
ADD bar /configs
54+
RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]
55+
56+
FROM foo
57+
# The base image is expected to contain
58+
# /bin/opm (with serve subcommand)
5159
# Configure the entrypoint and command
5260
ENTRYPOINT ["/bin/opm"]
5361
CMD ["serve", "/configs", "--cache-dir=/tmp/cache"]
5462
55-
# Copy declarative config root into image at /configs and pre-populate serve cache
56-
ADD bar /configs
57-
RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]
5863
59-
# Set DC-specific label for the location of the DC root directory
64+
COPY --from=builder /configs /configs
65+
COPY --from=builder /tmp/cache /tmp/cache
66+
67+
# Set FBC-specific label for the location of the FBC root directory
6068
# in the image
6169
LABEL operators.operatorframework.io.index.configs.v1=/configs
6270
`,
6371
},
6472
{
6573
name: "Success/WithExtraLabels",
6674
gen: GenerateDockerfile{
67-
BaseImage: "foo",
68-
IndexDir: "bar",
75+
BuilderImage: "foo",
76+
BaseImage: "foo",
77+
IndexDir: "bar",
6978
ExtraLabels: map[string]string{
7079
"key1": "value1",
7180
"key2": "value2",
7281
},
7382
},
74-
expectedDockerfile: `# The base image is expected to contain
75-
# /bin/opm (with a serve subcommand) and /bin/grpc_health_probe
76-
FROM foo
83+
expectedDockerfile: `# The builder image is expected to contain
84+
# /bin/opm (with serve subcommand)
85+
FROM foo as builder
86+
87+
# Copy FBC root into image at /configs and pre-populate serve cache
88+
ADD bar /configs
89+
RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]
7790
91+
FROM foo
92+
# The base image is expected to contain
93+
# /bin/opm (with serve subcommand)
7894
# Configure the entrypoint and command
7995
ENTRYPOINT ["/bin/opm"]
8096
CMD ["serve", "/configs", "--cache-dir=/tmp/cache"]
8197
82-
# Copy declarative config root into image at /configs and pre-populate serve cache
83-
ADD bar /configs
84-
RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]
8598
86-
# Set DC-specific label for the location of the DC root directory
99+
COPY --from=builder /configs /configs
100+
COPY --from=builder /tmp/cache /tmp/cache
101+
102+
# Set FBC-specific label for the location of the FBC root directory
87103
# in the image
88104
LABEL operators.operatorframework.io.index.configs.v1=/configs
89105
@@ -94,35 +110,35 @@ LABEL "key2"="value2"
94110
},
95111

96112
{
97-
name: "Lite/Fail/EmptyBaseImage",
113+
name: "Scratch/Fail/EmptyBaseImage",
98114
gen: GenerateDockerfile{
99-
IndexDir: "bar",
115+
BuilderImage: "foo",
116+
IndexDir: "bar",
100117
ExtraLabels: map[string]string{
101118
"key1": "value1",
102119
"key2": "value2",
103120
},
104-
Lite: true,
105121
},
106122
expectedErr: "base image is unset",
107123
},
108124
{
109-
name: "Lite/Fail/EmptyFromDir",
125+
name: "Scratch/Fail/EmptyFromDir",
110126
gen: GenerateDockerfile{
111-
BaseImage: "foo",
127+
BuilderImage: "foo",
128+
BaseImage: "scratch",
112129
ExtraLabels: map[string]string{
113130
"key1": "value1",
114131
"key2": "value2",
115132
},
116-
Lite: true,
117133
},
118134
expectedErr: "index directory is unset",
119135
},
120136
{
121-
name: "Lite/Success/WithoutExtraLabels",
137+
name: "Scratch/Success/WithoutExtraLabels",
122138
gen: GenerateDockerfile{
123-
BaseImage: "foo",
124-
IndexDir: "bar",
125-
Lite: true,
139+
BuilderImage: "foo",
140+
BaseImage: "scratch",
141+
IndexDir: "bar",
126142
},
127143
expectedDockerfile: `# The builder image is expected to contain
128144
# /bin/opm (with serve subcommand)
@@ -134,6 +150,7 @@ RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]
134150
135151
FROM scratch
136152
153+
137154
COPY --from=builder /configs /configs
138155
COPY --from=builder /tmp/cache /tmp/cache
139156
@@ -145,13 +162,13 @@ LABEL operators.operatorframework.io.index.configs.v1=/configs
145162
{
146163
name: "Lite/Success/WithExtraLabels",
147164
gen: GenerateDockerfile{
148-
BaseImage: "foo",
149-
IndexDir: "bar",
165+
BuilderImage: "foo",
166+
BaseImage: "scratch",
167+
IndexDir: "bar",
150168
ExtraLabels: map[string]string{
151169
"key1": "value1",
152170
"key2": "value2",
153171
},
154-
Lite: true,
155172
},
156173
expectedDockerfile: `# The builder image is expected to contain
157174
# /bin/opm (with serve subcommand)
@@ -163,6 +180,7 @@ RUN ["/bin/opm", "serve", "/configs", "--cache-dir=/tmp/cache", "--cache-only"]
163180
164181
FROM scratch
165182
183+
166184
COPY --from=builder /configs /configs
167185
COPY --from=builder /tmp/cache /tmp/cache
168186

Diff for: cmd/opm/generate/cmd.go

+26-10
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ func NewCmd() *cobra.Command {
2727

2828
func newDockerfileCmd() *cobra.Command {
2929
var (
30-
baseImage string
30+
binaryImage string
31+
builderImage string
3132
extraLabelStrs []string
32-
lite bool
3333
)
3434
cmd := &cobra.Command{
3535
Use: "dockerfile <dcRootDir>",
@@ -43,10 +43,26 @@ Dockerfile with the same name already exists, this command will fail.
4343
4444
When specifying extra labels, note that if duplicate keys exist, only the last
4545
value of each duplicate key will be added to the generated Dockerfile.
46+
47+
If --builder-image is set, this will generate a multi-stage Dockerfile which
48+
will NOT include a serving binary.
49+
If --builder-image is set, then --binary-image must not be set to anything other than "scratch".
50+
Note: '--builder-image' and '--binary-image' are mutually exclusive.
4651
`,
47-
RunE: func(_ *cobra.Command, args []string) error {
52+
RunE: func(inCmd *cobra.Command, args []string) error {
4853
fromDir := filepath.Clean(args[0])
4954

55+
switch {
56+
// user specified both flags ... reject unless they set binary-image to scratch
57+
case inCmd.Flags().Changed("binary-image") && inCmd.Flags().Changed("builder-image"):
58+
if binaryImage != "scratch" {
59+
return fmt.Errorf("invalid flag combination: if specifying --builder-image then --binary-image can only be \"scratch\" or default")
60+
}
61+
// user specifed only the builder image, set binary image to scratch
62+
case inCmd.Flags().Changed("builder-image"):
63+
binaryImage = "scratch"
64+
}
65+
5066
extraLabels, err := parseLabels(extraLabelStrs)
5167
if err != nil {
5268
return err
@@ -72,20 +88,20 @@ value of each duplicate key will be added to the generated Dockerfile.
7288
defer f.Close()
7389

7490
gen := action.GenerateDockerfile{
75-
BaseImage: baseImage,
76-
IndexDir: indexName,
77-
ExtraLabels: extraLabels,
78-
Writer: f,
79-
Lite: lite,
91+
BaseImage: binaryImage,
92+
BuilderImage: builderImage,
93+
IndexDir: indexName,
94+
ExtraLabels: extraLabels,
95+
Writer: f,
8096
}
8197
if err := gen.Run(); err != nil {
8298
log.Fatal(err)
8399
}
84100
return nil
85101
},
86102
}
87-
cmd.Flags().StringVarP(&baseImage, "binary-image", "i", containertools.DefaultBinarySourceImage, "Image in which to build catalog.")
88-
cmd.Flags().BoolVarP(&lite, "lite", "t", false, "Generate a smaller, binary-less Dockerfile.")
103+
cmd.Flags().StringVarP(&binaryImage, "binary-image", "i", containertools.DefaultBinarySourceImage, "Image in which to build catalog.")
104+
cmd.Flags().StringVarP(&builderImage, "builder-image", "b", containertools.DefaultBinarySourceImage, "Image to use as a build stage.")
89105
cmd.Flags().StringSliceVarP(&extraLabelStrs, "extra-labels", "l", []string{}, "Extra labels to include in the generated Dockerfile. Labels should be of the form 'key=value'.")
90106
return cmd
91107
}

Diff for: ohio.Dockerfile

-6
This file was deleted.

Diff for: pkg/containertools/dockerfilegenerator.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,11 @@ import (
88
)
99

1010
const (
11-
DefaultBinarySourceImage = "quay.io/operator-framework/opm:latest"
12-
DefaultDbLocation = "/database/index.db"
13-
DbLocationLabel = "operators.operatorframework.io.index.database.v1"
14-
ConfigsLocationLabel = "operators.operatorframework.io.index.configs.v1"
11+
DefaultScratchSourceImage = "scratch"
12+
DefaultBinarySourceImage = "quay.io/operator-framework/opm:latest"
13+
DefaultDbLocation = "/database/index.db"
14+
DbLocationLabel = "operators.operatorframework.io.index.database.v1"
15+
ConfigsLocationLabel = "operators.operatorframework.io.index.configs.v1"
1516
)
1617

1718
// DockerfileGenerator defines functions to generate index dockerfiles

0 commit comments

Comments
 (0)