Repository: operator-framework/operator-lifecycle-manager
Pull request #3041 (2023-09-21 18:28:41)

controller/registry: use static copy-content binary for copy
https://github.com/operator-framework/operator-lifecycle-manager/issues/3019

openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/operator-framework/operator-lifecycle-manager/pull/3041#" title="Author self-approved">stevekuznetsov</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign [njhale](https://github.com/njhale) for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=operator-framework%2Foperator-lifecycle-manager).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["njhale"]} -->
joelanford: /approve
openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/operator-framework/operator-lifecycle-manager/pull/3041#issuecomment-1730246730" title="Approved">joelanford</a>*, *<a href="https://github.com/operator-framework/operator-lifecycle-manager/pull/3041#" title="Author self-approved">stevekuznetsov</a>*

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=operator-framework%2Foperator-lifecycle-manager).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

<details >
Needs approval from an approver in each of these files:

- ~~[OWNERS](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/OWNERS)~~ [joelanford]

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":[]} -->

Patch:
diff --git a/cmd/copy-content/main.go b/cmd/copy-content/main.go
index dc70a7dd5..94c4046ef 100644
--- a/cmd/copy-content/main.go
+++ b/cmd/copy-content/main.go
@@ -13,6 +13,7 @@ func main() {
 	catalogDestination := flag.String("catalog.to", "", "Path to where catalog contents should be copied.")
 	cacheSource := flag.String("cache.from", "", "Path to cache contents to copy.")
 	cacheDestination := flag.String("cache.to", "", "Path to where cache contents should be copied.")
+	flag.Parse()
 
 	for flagName, value := range map[string]*string{
 		"catalog.from": catalogSource,
diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go
index 3a9e8d82e..615f2267e 100644
--- a/pkg/controller/operators/catalog/operator.go
+++ b/pkg/controller/operators/catalog/operator.go
@@ -215,7 +215,7 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
 	op.sources = grpc.NewSourceStore(logger, 10*time.Second, 10*time.Minute, op.syncSourceState)
 	op.sourceInvalidator = resolver.SourceProviderFromRegistryClientProvider(op.sources, logger)
 	resolverSourceProvider := NewOperatorGroupToggleSourceProvider(op.sourceInvalidator, logger, op.lister.OperatorsV1().OperatorGroupLister())
-	op.reconciler = reconciler.NewRegistryReconcilerFactory(lister, opClient, configmapRegistryImage, op.now, ssaClient, workloadUserID, opmImage)
+	op.reconciler = reconciler.NewRegistryReconcilerFactory(lister, opClient, configmapRegistryImage, op.now, ssaClient, workloadUserID, opmImage, utilImage)
 	res := resolver.NewOperatorStepResolver(lister, crClient, operatorNamespace, resolverSourceProvider, logger)
 	op.resolver = resolver.NewInstrumentedResolver(res, metrics.RegisterDependencyResolutionSuccess, metrics.RegisterDependencyResolutionFailure)
 
diff --git a/pkg/controller/registry/reconciler/configmap.go b/pkg/controller/registry/reconciler/configmap.go
index 68c405c57..1b6a97b07 100644
--- a/pkg/controller/registry/reconciler/configmap.go
+++ b/pkg/controller/registry/reconciler/configmap.go
@@ -106,7 +106,7 @@ func (s *configMapCatalogSourceDecorator) Service() *corev1.Service {
 }
 
 func (s *configMapCatalogSourceDecorator) Pod(image string) *corev1.Pod {
-	pod := Pod(s.CatalogSource, "configmap-registry-server", "", image, "", s.Labels(), s.Annotations(), 5, 5, s.runAsUser)
+	pod := Pod(s.CatalogSource, "configmap-registry-server", "", "", image, "", s.Labels(), s.Annotations(), 5, 5, s.runAsUser)
 	pod.Spec.ServiceAccountName = s.GetName() + ConfigMapServerPostfix
 	pod.Spec.Containers[0].Command = []string{"configmap-server", "-c", s.Spec.ConfigMap, "-n", s.GetNamespace()}
 	ownerutil.AddOwner(pod, s.CatalogSource, false, true)
diff --git a/pkg/controller/registry/reconciler/grpc.go b/pkg/controller/registry/reconciler/grpc.go
index e8d539a5b..b92658f5a 100644
--- a/pkg/controller/registry/reconciler/grpc.go
+++ b/pkg/controller/registry/reconciler/grpc.go
@@ -36,6 +36,7 @@ type grpcCatalogSourceDecorator struct {
 	*v1alpha1.CatalogSource
 	createPodAsUser int64
 	opmImage        string
+	utilImage       string
 }
 
 type UpdateNotReadyErr struct {
@@ -129,7 +130,7 @@ func (s *grpcCatalogSourceDecorator) ServiceAccount() *corev1.ServiceAccount {
 }
 
 func (s *grpcCatalogSourceDecorator) Pod(saName string) *corev1.Pod {
-	pod := Pod(s.CatalogSource, "registry-server", s.opmImage, s.Spec.Image, saName, s.Labels(), s.Annotations(), 5, 10, s.createPodAsUser)
+	pod := Pod(s.CatalogSource, "registry-server", s.opmImage, s.utilImage, s.Spec.Image, saName, s.Labels(), s.Annotations(), 5, 10, s.createPodAsUser)
 	ownerutil.AddOwner(pod, s.CatalogSource, false, true)
 	return pod
 }
@@ -141,6 +142,7 @@ type GrpcRegistryReconciler struct {
 	SSAClient       *controllerclient.ServerSideApplier
 	createPodAsUser int64
 	opmImage        string
+	utilImage       string
 }
 
 var _ RegistryReconciler = &GrpcRegistryReconciler{}
@@ -207,7 +209,7 @@ func (c *GrpcRegistryReconciler) currentPodsWithCorrectImageAndSpec(source grpcC
 
 func correctImages(source grpcCatalogSourceDecorator, pod *corev1.Pod) bool {
 	if source.CatalogSource.Spec.GrpcPodConfig != nil && source.CatalogSource.Spec.GrpcPodConfig.ExtractContent != nil {
-		return pod.Spec.InitContainers[0].Image == source.opmImage &&
+		return pod.Spec.InitContainers[0].Image == source.utilImage &&
 			pod.Spec.InitContainers[1].Image == source.CatalogSource.Spec.Image &&
 			pod.Spec.Containers[0].Image == source.opmImage
 	}
@@ -216,7 +218,7 @@ func correctImages(source grpcCatalogSourceDecorator, pod *corev1.Pod) bool {
 
 // EnsureRegistryServer ensures that all components of registry server are up to date.
 func (c *GrpcRegistryReconciler) EnsureRegistryServer(catalogSource *v1alpha1.CatalogSource) error {
-	source := grpcCatalogSourceDecorator{CatalogSource: catalogSource, createPodAsUser: c.createPodAsUser, opmImage: c.opmImage}
+	source := grpcCatalogSourceDecorator{CatalogSource: catalogSource, createPodAsUser: c.createPodAsUser, opmImage: c.opmImage, utilImage: c.utilImage}
 
 	// if service status is nil, we force create every object to ensure they're created the first time
 	overwrite := source.Status.RegistryServiceStatus == nil || !isRegistryServiceStatusValid(&source)
@@ -465,7 +467,7 @@ func (c *GrpcRegistryReconciler) removePods(pods []*corev1.Pod, namespace string
 
 // CheckRegistryServer returns true if the given CatalogSource is considered healthy; false otherwise.
 func (c *GrpcRegistryReconciler) CheckRegistryServer(catalogSource *v1alpha1.CatalogSource) (healthy bool, err error) {
-	source := grpcCatalogSourceDecorator{CatalogSource: catalogSource, createPodAsUser: c.createPodAsUser, opmImage: c.opmImage}
+	source := grpcCatalogSourceDecorator{CatalogSource: catalogSource, createPodAsUser: c.createPodAsUser, opmImage: c.opmImage, utilImage: c.utilImage}
 	// Check on registry resources
 	// TODO: add gRPC health check
 	if len(c.currentPodsWithCorrectImageAndSpec(source, source.ServiceAccount().GetName())) < 1 ||
diff --git a/pkg/controller/registry/reconciler/reconciler.go b/pkg/controller/registry/reconciler/reconciler.go
index de8098de2..d2f120de2 100644
--- a/pkg/controller/registry/reconciler/reconciler.go
+++ b/pkg/controller/registry/reconciler/reconciler.go
@@ -66,6 +66,7 @@ type registryReconcilerFactory struct {
 	SSAClient            *controllerclient.ServerSideApplier
 	createPodAsUser      int64
 	opmImage             string
+	utilImage            string
 }
 
 // ReconcilerForSource returns a RegistryReconciler based on the configuration of the given CatalogSource.
@@ -89,6 +90,7 @@ func (r *registryReconcilerFactory) ReconcilerForSource(source *operatorsv1alpha
 				SSAClient:       r.SSAClient,
 				createPodAsUser: r.createPodAsUser,
 				opmImage:        r.opmImage,
+				utilImage:       r.utilImage,
 			}
 		} else if source.Spec.Address != "" {
 			return &GrpcAddressRegistryReconciler{
@@ -100,7 +102,7 @@ func (r *registryReconcilerFactory) ReconcilerForSource(source *operatorsv1alpha
 }
 
 // NewRegistryReconcilerFactory returns an initialized RegistryReconcilerFactory.
-func NewRegistryReconcilerFactory(lister operatorlister.OperatorLister, opClient operatorclient.ClientInterface, configMapServerImage string, now nowFunc, ssaClient *controllerclient.ServerSideApplier, createPodAsUser int64, opmImage string) RegistryReconcilerFactory {
+func NewRegistryReconcilerFactory(lister operatorlister.OperatorLister, opClient operatorclient.ClientInterface, configMapServerImage string, now nowFunc, ssaClient *controllerclient.ServerSideApplier, createPodAsUser int64, opmImage, utilImage string) RegistryReconcilerFactory {
 	return &registryReconcilerFactory{
 		now:                  now,
 		Lister:               lister,
@@ -109,10 +111,11 @@ func NewRegistryReconcilerFactory(lister operatorlister.OperatorLister, opClient
 		SSAClient:            ssaClient,
 		createPodAsUser:      createPodAsUser,
 		opmImage:             opmImage,
+		utilImage:            utilImage,
 	}
 }
 
-func Pod(source *operatorsv1alpha1.CatalogSource, name, opmImg, img, saName string, labels, annotations map[string]string, readinessDelay, livenessDelay int32, runAsUser int64) *corev1.Pod {
+func Pod(source *operatorsv1alpha1.CatalogSource, name, opmImg, utilImage, img, saName string, labels, annotations map[string]string, readinessDelay, livenessDelay int32, runAsUser int64) *corev1.Pod {
 	// make a copy of the labels and annotations to avoid mutating the input parameters
 	podLabels := make(map[string]string)
 	podAnnotations := make(map[string]string)
@@ -265,21 +268,21 @@ func Pod(source *operatorsv1alpha1.CatalogSource, name, opmImg, img, saName stri
 				MountPath: catalogPath,
 			}
 			pod.Spec.InitContainers = append(pod.Spec.InitContainers, corev1.Container{
-				Name:    "extract-utilities",
-				Image:   opmImg,
-				Command: []string{"sh", "-c"},
-				Args: []string{fmt.Sprintf("cp $( command -v sh ) %s/sh && cp $( command -v cp ) %s/cp",
-					utilitiesPath, utilitiesPath,
-				)},
+				Name:         "extract-utilities",
+				Image:        utilImage,
+				Command:      []string{"cp"},
+				Args:         []string{"/bin/copy-content", fmt.Sprintf("%s/copy-content", utilitiesPath)},
 				VolumeMounts: []corev1.VolumeMount{utilitiesVolumeMount},
 			}, corev1.Container{
 				Name:    "extract-content",
 				Image:   img,
-				Command: []string{utilitiesPath + "/sh", "-c"},
-				Args: []string{fmt.Sprintf("%s/cp -r %s %s/catalog && %s/cp -r %s %s/cache",
-					utilitiesPath, grpcPodConfig.ExtractContent.CatalogDir, catalogPath,
-					utilitiesPath, grpcPodConfig.ExtractContent.CacheDir, catalogPath,
-				)},
+				Command: []string{utilitiesPath + "/copy-content"},
+				Args: []string{
+					"--catalog.from=" + grpcPodConfig.ExtractContent.CatalogDir,
+					"--catalog.to=" + fmt.Sprintf("%s/catalog", catalogPath),
+					"--cache.from=" + grpcPodConfig.ExtractContent.CacheDir,
+					"--cache.to=" + fmt.Sprintf("%s/cache", catalogPath),
+				},
 				VolumeMounts: []corev1.VolumeMount{utilitiesVolumeMount, contentVolumeMount},
 			})
 


Test patch:
diff --git a/e2e.Dockerfile b/e2e.Dockerfile
index 4f450f2d9..2cce485ab 100644
--- a/e2e.Dockerfile
+++ b/e2e.Dockerfile
@@ -1,6 +1,6 @@
 # ./pkg/controller/bundle/bundle_unpacker.go requires "/bin/cp"
 FROM busybox
-COPY olm catalog package-server wait cpb /bin/
+COPY olm catalog package-server wait cpb copy-content /bin/
 EXPOSE 8080
 EXPOSE 5443
 USER 1001
diff --git a/pkg/controller/operators/catalog/operator_test.go b/pkg/controller/operators/catalog/operator_test.go
index 4ad9803ba..79f70d2fb 100644
--- a/pkg/controller/operators/catalog/operator_test.go
+++ b/pkg/controller/operators/catalog/operator_test.go
@@ -1773,7 +1773,7 @@ func NewFakeOperator(ctx context.Context, namespace string, namespaces []string,
 		}
 		applier := controllerclient.NewFakeApplier(s, "testowner")
 
-		op.reconciler = reconciler.NewRegistryReconcilerFactory(lister, op.opClient, "test:pod", op.now, applier, 1001, "")
+		op.reconciler = reconciler.NewRegistryReconcilerFactory(lister, op.opClient, "test:pod", op.now, applier, 1001, "", "")
 	}
 
 	op.RunInformers(ctx)
@@ -1929,7 +1929,7 @@ func toManifest(t *testing.T, obj runtime.Object) string {
 }
 
 func pod(s v1alpha1.CatalogSource) *corev1.Pod {
-	pod := reconciler.Pod(&s, "registry-server", "central-opm", s.Spec.Image, s.GetName(), s.GetLabels(), s.GetAnnotations(), 5, 10, 1001)
+	pod := reconciler.Pod(&s, "registry-server", "central-opm", "central-util", s.Spec.Image, s.GetName(), s.GetLabels(), s.GetAnnotations(), 5, 10, 1001)
 	ownerutil.AddOwner(pod, &s, false, true)
 	return pod
 }
diff --git a/pkg/controller/registry/reconciler/reconciler_test.go b/pkg/controller/registry/reconciler/reconciler_test.go
index 2de431de1..4055b6312 100644
--- a/pkg/controller/registry/reconciler/reconciler_test.go
+++ b/pkg/controller/registry/reconciler/reconciler_test.go
@@ -167,7 +167,7 @@ func TestPodMemoryTarget(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
-		pod := Pod(testCase.input, "name", "opmImage", "image", "service-account", map[string]string{}, map[string]string{}, int32(0), int32(0), int64(workloadUserID))
+		pod := Pod(testCase.input, "name", "opmImage", "utilImage", "image", "service-account", map[string]string{}, map[string]string{}, int32(0), int32(0), int64(workloadUserID))
 		if diff := cmp.Diff(pod, testCase.expected); diff != "" {
 			t.Errorf("got incorrect pod: %v", diff)
 		}
@@ -267,7 +267,7 @@ func TestPodExtractContent(t *testing.T) {
 				ObjectMeta: metav1.ObjectMeta{
 					GenerateName: "test-",
 					Namespace:    "testns",
-					Labels:       map[string]string{"olm.pod-spec-hash": "c748655d4", "olm.managed": "true"},
+					Labels:       map[string]string{"olm.pod-spec-hash": "667f4b9769", "olm.managed": "true"},
 					Annotations:  map[string]string{"cluster-autoscaler.kubernetes.io/safe-to-evict": "true"},
 				},
 				Spec: corev1.PodSpec{
@@ -284,16 +284,21 @@ func TestPodExtractContent(t *testing.T) {
 					InitContainers: []corev1.Container{
 						{
 							Name:         "extract-utilities",
-							Image:        "opmImage",
-							Command:      []string{"sh", "-c"},
-							Args:         []string{"cp $( command -v sh ) /utilities/sh && cp $( command -v cp ) /utilities/cp"},
+							Image:        "utilImage",
+							Command:      []string{"cp"},
+							Args:         []string{"/bin/copy-content", "/utilities/copy-content"},
 							VolumeMounts: []corev1.VolumeMount{{Name: "utilities", MountPath: "/utilities"}},
 						},
 						{
 							Name:    "extract-content",
 							Image:   "image",
-							Command: []string{"/utilities/sh", "-c"},
-							Args:    []string{"/utilities/cp -r /catalog /extracted-catalog/catalog && /utilities/cp -r /tmp/cache /extracted-catalog/cache"},
+							Command: []string{"/utilities/copy-content"},
+							Args: []string{
+								"--catalog.from=/catalog",
+								"--catalog.to=/extracted-catalog/catalog",
+								"--cache.from=/tmp/cache",
+								"--cache.to=/extracted-catalog/cache",
+							},
 							VolumeMounts: []corev1.VolumeMount{
 								{Name: "utilities", MountPath: "/utilities"},
 								{Name: "catalog-content", MountPath: "/extracted-catalog"},
@@ -357,7 +362,7 @@ func TestPodExtractContent(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
-		pod := Pod(testCase.input, "name", "opmImage", "image", "service-account", map[string]string{}, map[string]string{}, int32(0), int32(0), int64(workloadUserID))
+		pod := Pod(testCase.input, "name", "opmImage", "utilImage", "image", "service-account", map[string]string{}, map[string]string{}, int32(0), int32(0), int64(workloadUserID))
 		if diff := cmp.Diff(pod, testCase.expected); diff != "" {
 			t.Errorf("got incorrect pod: %v", diff)
 		}
@@ -375,7 +380,7 @@ func TestPodNodeSelector(t *testing.T) {
 	key := "kubernetes.io/os"
 	value := "linux"
 
-	gotCatSrcPod := Pod(catsrc, "hello", "opmImage", "busybox", "", map[string]string{}, map[string]string{}, int32(0), int32(0), int64(workloadUserID))
+	gotCatSrcPod := Pod(catsrc, "hello", "utilImage", "opmImage", "busybox", "", map[string]string{}, map[string]string{}, int32(0), int32(0), int64(workloadUserID))
 	gotCatSrcPodSelector := gotCatSrcPod.Spec.NodeSelector
 
 	if gotCatSrcPodSelector[key] != value {
@@ -423,7 +428,7 @@ func TestPullPolicy(t *testing.T) {
 	}
 
 	for _, tt := range table {
-		p := Pod(source, "catalog", "opmImage", tt.image, "", nil, nil, int32(0), int32(0), int64(workloadUserID))
+		p := Pod(source, "catalog", "opmImage", "utilImage", tt.image, "", nil, nil, int32(0), int32(0), int64(workloadUserID))
 		policy := p.Spec.Containers[0].ImagePullPolicy
 		if policy != tt.policy {
 			t.Fatalf("expected pull policy %s for image  %s", tt.policy, tt.image)
@@ -535,7 +540,7 @@ func TestPodContainerSecurityContext(t *testing.T) {
 		},
 	}
 	for _, testcase := range testcases {
-		outputPod := Pod(testcase.inputCatsrc, "hello", "opmImage", "busybox", "", map[string]string{}, map[string]string{}, int32(0), int32(0), int64(workloadUserID))
+		outputPod := Pod(testcase.inputCatsrc, "hello", "utilImage", "opmImage", "busybox", "", map[string]string{}, map[string]string{}, int32(0), int32(0), int64(workloadUserID))
 		if testcase.expectedSecurityContext != nil {
 			require.Equal(t, testcase.expectedSecurityContext, outputPod.Spec.SecurityContext)
 		}
@@ -565,7 +570,7 @@ func TestPodAvoidsConcurrentWrite(t *testing.T) {
 		"annotation": "somethingelse",
 	}
 
-	gotPod := Pod(catsrc, "hello", "opmImage", "busybox", "", labels, annotations, int32(0), int32(0), int64(workloadUserID))
+	gotPod := Pod(catsrc, "hello", "opmImage", "utilImage", "busybox", "", labels, annotations, int32(0), int32(0), int64(workloadUserID))
 
 	// check labels and annotations point to different addresses between parameters and what's in the pod
 	require.NotEqual(t, &labels, &gotPod.Labels)
@@ -794,7 +799,7 @@ func TestPodSchedulingOverrides(t *testing.T) {
 	}
 
 	for _, testCase := range testCases {
-		pod := Pod(testCase.catalogSource, "hello", "opmImage", "busybox", "", map[string]string{}, testCase.annotations, int32(0), int32(0), int64(workloadUserID))
+		pod := Pod(testCase.catalogSource, "hello", "opmImage", "utilImage", "busybox", "", map[string]string{}, testCase.annotations, int32(0), int32(0), int64(workloadUserID))
 		require.Equal(t, testCase.expectedNodeSelectors, pod.Spec.NodeSelector)
 		require.Equal(t, testCase.expectedPriorityClassName, pod.Spec.PriorityClassName)
 		require.Equal(t, testCase.expectedTolerations, pod.Spec.Tolerations)