Skip to content

Commit

Permalink
controller/registry: use static copy-content binary for copy
Browse files Browse the repository at this point in the history
Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
  • Loading branch information
stevekuznetsov committed Sep 21, 2023
1 parent 44935f1 commit 662ff51
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 35 deletions.
1 change: 1 addition & 0 deletions cmd/copy-content/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion e2e.Dockerfile
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/operators/catalog/operator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/operators/catalog/operator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/registry/reconciler/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 6 additions & 4 deletions pkg/controller/registry/reconciler/grpc.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ type grpcCatalogSourceDecorator struct {
*v1alpha1.CatalogSource
createPodAsUser int64
opmImage string
utilImage string
}

type UpdateNotReadyErr struct {
Expand Down Expand Up @@ -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
}
Expand All @@ -141,6 +142,7 @@ type GrpcRegistryReconciler struct {
SSAClient *controllerclient.ServerSideApplier
createPodAsUser int64
opmImage string
utilImage string
}

var _ RegistryReconciler = &GrpcRegistryReconciler{}
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
Expand Down Expand Up @@ -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 ||
Expand Down
29 changes: 16 additions & 13 deletions pkg/controller/registry/reconciler/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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{
Expand All @@ -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,
Expand All @@ -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)
Expand Down Expand Up @@ -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},
})

Expand Down
31 changes: 18 additions & 13 deletions pkg/controller/registry/reconciler/reconciler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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{
Expand All @@ -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"},
Expand Down Expand Up @@ -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)
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down

0 comments on commit 662ff51

Please sign in to comment.