Skip to content

Commit 6756467

Browse files
njhalenjhale
committed
fix(bundles): execute opm from tooling container
Execute the opm command from the container it was packaged with to prevent dynamic linking issues caused by running in a "FROM scratch" bundle container.
1 parent 8775d5a commit 6756467

File tree

4 files changed

+137
-59
lines changed

4 files changed

+137
-59
lines changed

‎pkg/controller/bundle/bundle_unpacker.go

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,9 @@ func (c *ConfigMapUnpacker) job(cmRef *corev1.ObjectReference, bundlePath string
5454
RestartPolicy: corev1.RestartPolicyOnFailure,
5555
Containers: []corev1.Container{
5656
{
57-
Name: cmRef.Name,
58-
Image: bundlePath,
59-
Command: []string{"/injected/opm", "alpha", "bundle", "extract", "-n", cmRef.Namespace, "-c", cmRef.Name},
57+
Name: "extract",
58+
Image: c.opmImage,
59+
Command: []string{"opm", "alpha", "bundle", "extract", "-m", "/bundle/", "-n", cmRef.Namespace, "-c", cmRef.Name},
6060
Env: []corev1.EnvVar{
6161
{
6262
Name: configmap.EnvContainerImage,
@@ -65,28 +65,49 @@ func (c *ConfigMapUnpacker) job(cmRef *corev1.ObjectReference, bundlePath string
6565
},
6666
VolumeMounts: []corev1.VolumeMount{
6767
{
68-
Name: "copydir",
69-
MountPath: "/injected",
68+
Name: "bundle", // Expected bundle content mount
69+
MountPath: "/bundle",
7070
},
7171
},
7272
},
7373
},
7474
InitContainers: []corev1.Container{
7575
{
76-
Name: "copy-binary",
77-
Image: c.copyImage,
78-
Command: []string{"/bin/cp", "/bin/opm", "/copy-dest"},
76+
Name: "tools",
77+
Image: "busybox",
78+
Command: []string{"/bin/cp", "-Rv", "/bin/cp", "/tools/cp"}, // Copy tooling for the bundle container to use
7979
VolumeMounts: []corev1.VolumeMount{
8080
{
81-
Name: "copydir",
82-
MountPath: "/copy-dest",
81+
Name: "tools",
82+
MountPath: "/tools",
83+
},
84+
},
85+
},
86+
{
87+
Name: "pull",
88+
Image: bundlePath,
89+
Command: []string{"/tools/cp", "-Rv", "/manifests", "/metadata", "/bundle"}, // Copy bundle content to its mount
90+
VolumeMounts: []corev1.VolumeMount{
91+
{
92+
Name: "bundle",
93+
MountPath: "/bundle",
94+
},
95+
{
96+
Name: "tools",
97+
MountPath: "/tools",
8398
},
8499
},
85100
},
86101
},
87102
Volumes: []corev1.Volume{
88103
{
89-
Name: "copydir",
104+
Name: "bundle", // Used to share bundle content
105+
VolumeSource: corev1.VolumeSource{
106+
EmptyDir: &corev1.EmptyDirVolumeSource{},
107+
},
108+
},
109+
{
110+
Name: "tools", // Used to share tool
90111
VolumeSource: corev1.VolumeSource{
91112
EmptyDir: &corev1.EmptyDirVolumeSource{},
92113
},
@@ -110,7 +131,7 @@ type Unpacker interface {
110131
}
111132

112133
type ConfigMapUnpacker struct {
113-
copyImage string
134+
opmImage string
114135
client kubernetes.Interface
115136
csLister listersoperatorsv1alpha1.CatalogSourceLister
116137
cmLister listerscorev1.ConfigMapLister
@@ -136,9 +157,9 @@ func NewConfigmapUnpacker(options ...ConfigMapUnpackerOption) (*ConfigMapUnpacke
136157
return unpacker, nil
137158
}
138159

139-
func WithCopyImage(copyImage string) ConfigMapUnpackerOption {
160+
func WithOPMImage(opmImage string) ConfigMapUnpackerOption {
140161
return func(unpacker *ConfigMapUnpacker) {
141-
unpacker.copyImage = copyImage
162+
unpacker.opmImage = opmImage
142163
}
143164
}
144165

@@ -192,7 +213,7 @@ func (c *ConfigMapUnpacker) apply(options ...ConfigMapUnpackerOption) {
192213

193214
func (c *ConfigMapUnpacker) validate() (err error) {
194215
switch {
195-
case c.copyImage == "":
216+
case c.opmImage == "":
196217
err = fmt.Errorf("no copy image given")
197218
case c.client == nil:
198219
err = fmt.Errorf("client is nil")

‎pkg/controller/bundle/bundle_unpacker_test.go

Lines changed: 98 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ const (
2626
etcdBackup = "{\"apiVersion\":\"apiextensions.k8s.io/v1beta1\",\"kind\":\"CustomResourceDefinition\",\"metadata\":{\"name\":\"etcdbackups.etcd.database.coreos.com\"},\"spec\":{\"group\":\"etcd.database.coreos.com\",\"names\":{\"kind\":\"EtcdBackup\",\"listKind\":\"EtcdBackupList\",\"plural\":\"etcdbackups\",\"singular\":\"etcdbackup\"},\"scope\":\"Namespaced\",\"version\":\"v1beta2\"}}"
2727
etcdCluster = "{\"apiVersion\":\"apiextensions.k8s.io/v1beta1\",\"kind\":\"CustomResourceDefinition\",\"metadata\":{\"name\":\"etcdclusters.etcd.database.coreos.com\"},\"spec\":{\"group\":\"etcd.database.coreos.com\",\"names\":{\"kind\":\"EtcdCluster\",\"listKind\":\"EtcdClusterList\",\"plural\":\"etcdclusters\",\"shortNames\":[\"etcdclus\",\"etcd\"],\"singular\":\"etcdcluster\"},\"scope\":\"Namespaced\",\"version\":\"v1beta2\"}}"
2828
etcdRestore = "{\"apiVersion\":\"apiextensions.k8s.io/v1beta1\",\"kind\":\"CustomResourceDefinition\",\"metadata\":{\"name\":\"etcdrestores.etcd.database.coreos.com\"},\"spec\":{\"group\":\"etcd.database.coreos.com\",\"names\":{\"kind\":\"EtcdRestore\",\"listKind\":\"EtcdRestoreList\",\"plural\":\"etcdrestores\",\"singular\":\"etcdrestore\"},\"scope\":\"Namespaced\",\"version\":\"v1beta2\"}}"
29-
copyImage = "bundle-image"
29+
opmImage = "opm-image"
3030
bundlePath = "bundle-path"
3131
)
3232

@@ -179,9 +179,9 @@ func TestConfigMapUnpacker(t *testing.T) {
179179
RestartPolicy: corev1.RestartPolicyOnFailure,
180180
Containers: []corev1.Container{
181181
{
182-
Name: pathHash,
183-
Image: bundlePath,
184-
Command: []string{"/injected/opm", "alpha", "bundle", "extract", "-n", "ns-a", "-c", pathHash},
182+
Name: "extract",
183+
Image: opmImage,
184+
Command: []string{"opm", "alpha", "bundle", "extract", "-m", "/bundle/", "-n", "ns-a", "-c", pathHash},
185185
Env: []corev1.EnvVar{
186186
{
187187
Name: configmap.EnvContainerImage,
@@ -190,28 +190,49 @@ func TestConfigMapUnpacker(t *testing.T) {
190190
},
191191
VolumeMounts: []corev1.VolumeMount{
192192
{
193-
Name: "copydir",
194-
MountPath: "/injected",
193+
Name: "bundle",
194+
MountPath: "/bundle",
195195
},
196196
},
197197
},
198198
},
199199
InitContainers: []corev1.Container{
200200
{
201-
Name: "copy-binary",
202-
Image: copyImage,
203-
Command: []string{"/bin/cp", "/bin/opm", "/copy-dest"},
201+
Name: "tools",
202+
Image: "busybox",
203+
Command: []string{"/bin/cp", "-Rv", "/bin/cp", "/tools/cp"},
204+
VolumeMounts: []corev1.VolumeMount{
205+
{
206+
Name: "tools",
207+
MountPath: "/tools",
208+
},
209+
},
210+
},
211+
{
212+
Name: "pull",
213+
Image: bundlePath,
214+
Command: []string{"/tools/cp", "-Rv", "/manifests", "/metadata", "/bundle"},
204215
VolumeMounts: []corev1.VolumeMount{
205216
{
206-
Name: "copydir",
207-
MountPath: "/copy-dest",
217+
Name: "bundle",
218+
MountPath: "/bundle",
219+
},
220+
{
221+
Name: "tools",
222+
MountPath: "/tools",
208223
},
209224
},
210225
},
211226
},
212227
Volumes: []corev1.Volume{
213228
{
214-
Name: "copydir",
229+
Name: "bundle",
230+
VolumeSource: corev1.VolumeSource{
231+
EmptyDir: &corev1.EmptyDirVolumeSource{},
232+
},
233+
},
234+
{
235+
Name: "tools",
215236
VolumeSource: corev1.VolumeSource{
216237
EmptyDir: &corev1.EmptyDirVolumeSource{},
217238
},
@@ -314,9 +335,9 @@ func TestConfigMapUnpacker(t *testing.T) {
314335
RestartPolicy: corev1.RestartPolicyOnFailure,
315336
Containers: []corev1.Container{
316337
{
317-
Name: pathHash,
318-
Image: bundlePath,
319-
Command: []string{"/injected/opm", "alpha", "bundle", "extract", "-n", "ns-a", "-c", pathHash},
338+
Name: "extract",
339+
Image: opmImage,
340+
Command: []string{"opm", "alpha", "bundle", "extract", "-m", "/bundle/", "-n", "ns-a", "-c", pathHash},
320341
Env: []corev1.EnvVar{
321342
{
322343
Name: configmap.EnvContainerImage,
@@ -325,28 +346,49 @@ func TestConfigMapUnpacker(t *testing.T) {
325346
},
326347
VolumeMounts: []corev1.VolumeMount{
327348
{
328-
Name: "copydir",
329-
MountPath: "/injected",
349+
Name: "bundle",
350+
MountPath: "/bundle",
330351
},
331352
},
332353
},
333354
},
334355
InitContainers: []corev1.Container{
335356
{
336-
Name: "copy-binary",
337-
Image: copyImage,
338-
Command: []string{"/bin/cp", "/bin/opm", "/copy-dest"},
357+
Name: "tools",
358+
Image: "busybox",
359+
Command: []string{"/bin/cp", "-Rv", "/bin/cp", "/tools/cp"},
339360
VolumeMounts: []corev1.VolumeMount{
340361
{
341-
Name: "copydir",
342-
MountPath: "/copy-dest",
362+
Name: "tools",
363+
MountPath: "/tools",
364+
},
365+
},
366+
},
367+
{
368+
Name: "pull",
369+
Image: bundlePath,
370+
Command: []string{"/tools/cp", "-Rv", "/manifests", "/metadata", "/bundle"},
371+
VolumeMounts: []corev1.VolumeMount{
372+
{
373+
Name: "bundle",
374+
MountPath: "/bundle",
375+
},
376+
{
377+
Name: "tools",
378+
MountPath: "/tools",
343379
},
344380
},
345381
},
346382
},
347383
Volumes: []corev1.Volume{
348384
{
349-
Name: "copydir",
385+
Name: "bundle",
386+
VolumeSource: corev1.VolumeSource{
387+
EmptyDir: &corev1.EmptyDirVolumeSource{},
388+
},
389+
},
390+
{
391+
Name: "tools",
350392
VolumeSource: corev1.VolumeSource{
351393
EmptyDir: &corev1.EmptyDirVolumeSource{},
352394
},
@@ -488,9 +530,9 @@ func TestConfigMapUnpacker(t *testing.T) {
488530
RestartPolicy: corev1.RestartPolicyOnFailure,
489531
Containers: []corev1.Container{
490532
{
491-
Name: pathHash,
492-
Image: bundlePath,
493-
Command: []string{"/injected/opm", "alpha", "bundle", "extract", "-n", "ns-a", "-c", pathHash},
533+
Name: "extract",
534+
Image: opmImage,
535+
Command: []string{"opm", "alpha", "bundle", "extract", "-m", "/bundle/", "-n", "ns-a", "-c", pathHash},
494536
Env: []corev1.EnvVar{
495537
{
496538
Name: configmap.EnvContainerImage,
@@ -499,28 +541,49 @@ func TestConfigMapUnpacker(t *testing.T) {
499541
},
500542
VolumeMounts: []corev1.VolumeMount{
501543
{
502-
Name: "copydir",
503-
MountPath: "/injected",
544+
Name: "bundle",
545+
MountPath: "/bundle",
504546
},
505547
},
506548
},
507549
},
508550
InitContainers: []corev1.Container{
509551
{
510-
Name: "copy-binary",
511-
Image: copyImage,
512-
Command: []string{"/bin/cp", "/bin/opm", "/copy-dest"},
552+
Name: "tools",
553+
Image: "busybox",
554+
Command: []string{"/bin/cp", "-Rv", "/bin/cp", "/tools/cp"},
555+
VolumeMounts: []corev1.VolumeMount{
556+
{
557+
Name: "tools",
558+
MountPath: "/tools",
559+
},
560+
},
561+
},
562+
{
563+
Name: "pull",
564+
Image: bundlePath,
565+
Command: []string{"/tools/cp", "-Rv", "/manifests", "/metadata", "/bundle"},
513566
VolumeMounts: []corev1.VolumeMount{
514567
{
515-
Name: "copydir",
516-
MountPath: "/copy-dest",
568+
Name: "bundle",
569+
MountPath: "/bundle",
570+
},
571+
{
572+
Name: "tools",
573+
MountPath: "/tools",
517574
},
518575
},
519576
},
520577
},
521578
Volumes: []corev1.Volume{
522579
{
523-
Name: "copydir",
580+
Name: "bundle",
581+
VolumeSource: corev1.VolumeSource{
582+
EmptyDir: &corev1.EmptyDirVolumeSource{},
583+
},
584+
},
585+
{
586+
Name: "tools",
524587
VolumeSource: corev1.VolumeSource{
525588
EmptyDir: &corev1.EmptyDirVolumeSource{},
526589
},
@@ -641,7 +704,7 @@ func TestConfigMapUnpacker(t *testing.T) {
641704
WithJobLister(jobLister),
642705
WithRoleLister(roleLister),
643706
WithRoleBindingLister(rbLister),
644-
WithCopyImage(copyImage),
707+
WithOPMImage(opmImage),
645708
WithNow(now),
646709
)
647710
require.NoError(t, err)

‎pkg/controller/operators/catalog/operator.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
313313
bundle.WithJobLister(jobInformer.Lister()),
314314
bundle.WithRoleLister(roleInformer.Lister()),
315315
bundle.WithRoleBindingLister(roleBindingInformer.Lister()),
316-
bundle.WithCopyImage(op.bundleUnpackerImage),
316+
bundle.WithOPMImage(op.bundleUnpackerImage),
317317
bundle.WithNow(op.now),
318318
)
319319
if err != nil {

‎test/e2e/installplan_e2e_test.go

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2574,7 +2574,6 @@ func TestInstallPlanCRDValidation(t *testing.T) {
25742574
}
25752575

25762576
func TestInstallPlanUnpacksBundleImage(t *testing.T) {
2577-
defer cleaner.NotifyTestComplete(t, true)
25782577
c := newKubeClient(t)
25792578
crc := newCRClient(t)
25802579

@@ -2597,25 +2596,20 @@ func TestInstallPlanUnpacksBundleImage(t *testing.T) {
25972596
Labels: map[string]string{"olm.catalogSource": "kaili-catalog"},
25982597
},
25992598
Spec: v1alpha1.CatalogSourceSpec{
2600-
Image: "quay.io/olmtest/installplan_e2e-registry-image:latest",
2599+
Image: "quay.io/olmtest/single-bundle-index:1.0.0",
26012600
SourceType: v1alpha1.SourceTypeGrpc,
26022601
},
26032602
}
26042603
catsrc, err = crc.OperatorsV1alpha1().CatalogSources(catsrc.GetNamespace()).Create(catsrc)
26052604
require.NoError(t, err)
26062605

2607-
defer func() {
2608-
require.NoError(t, crc.OperatorsV1alpha1().CatalogSources(catsrc.GetNamespace()).Delete(catsrc.GetName(), deleteOpts))
2609-
}()
2610-
26112606
// Wait for the CatalogSource to be ready
26122607
catsrc, err = fetchCatalogSource(t, crc, catsrc.GetName(), catsrc.GetNamespace(), catalogSourceRegistryPodSynced)
26132608
require.NoError(t, err)
26142609

26152610
// Generate a Subscription
26162611
subName := genName("kiali-")
2617-
cleanupSub := createSubscriptionForCatalog(t, crc, catsrc.GetNamespace(), subName, catsrc.GetName(), "kiali", stableChannel, "", v1alpha1.ApprovalAutomatic)
2618-
defer cleanupSub()
2612+
createSubscriptionForCatalog(t, crc, catsrc.GetNamespace(), subName, catsrc.GetName(), "kiali", stableChannel, "", v1alpha1.ApprovalAutomatic)
26192613

26202614
sub, err := fetchSubscription(t, crc, catsrc.GetNamespace(), subName, subscriptionHasInstallPlanChecker)
26212615
require.NoError(t, err)

0 commit comments

Comments
 (0)