MULTIARCH-4971: Set import mode in image config based on ClusterVersion desired Architecture

Prashanth684 · Prashanth684 · commit 37dcccc4bb1f · 2024-12-17T07:07:39.000-08:00
openshift/api#2024 introduces the architecture property in the desired status field which indicates the architecture of the cluster (or the architecture being reconciled to). Set import mode based on this status - if "Multi", set importmode to PreserveOriginal, else to Legacy. Backport of #1164
diff --git a/cmd/cluster-image-registry-operator/main.go b/cmd/cluster-image-registry-operator/main.go
@@ -11,6 +11,7 @@ import (
 
 	_ "k8s.io/client-go/plugin/pkg/client/auth/gcp"
 	"k8s.io/klog/v2"
+	"k8s.io/utils/clock"
 
 	configv1 "github.com/openshift/api/config/v1"
 	"github.com/openshift/library-go/pkg/controller/controllercmd"
@@ -67,6 +68,7 @@ func main() {
 					go metrics.RunServer(metricsPort)
 					return operator.RunOperator(ctx, cctx.KubeConfig)
 				},
+				clock.RealClock{},
 			).WithKubeConfigFile(
 				kubeconfig, nil,
 			).WithLeaderElection(
diff --git a/pkg/client/operatorclient.go b/pkg/client/operatorclient.go
@@ -13,6 +13,8 @@ import (
 	imageregistryv1client "github.com/openshift/client-go/imageregistry/clientset/versioned/typed/imageregistry/v1"
 	imageregistryv1informers "github.com/openshift/client-go/imageregistry/informers/externalversions/imageregistry/v1"
 	imageregistryv1listers "github.com/openshift/client-go/imageregistry/listers/imageregistry/v1"
+	applyoperatorv1 "github.com/openshift/client-go/operator/applyconfigurations/operator/v1"
+	"github.com/openshift/library-go/pkg/apiserver/jsonpatch"
 	"github.com/openshift/library-go/pkg/operator/v1helpers"
 )
 
@@ -95,3 +97,15 @@ func (c *ConfigOperatorClient) UpdateOperatorStatus(ctx context.Context, oldReso
 
 	return &updatedConfig.Status.OperatorStatus, nil
 }
+
+func (c *ConfigOperatorClient) ApplyOperatorSpec(ctx context.Context, fieldManager string, applyConfiguration *applyoperatorv1.OperatorSpecApplyConfiguration) (err error) {
+	return fmt.Errorf("not implemented")
+}
+
+func (c *ConfigOperatorClient) ApplyOperatorStatus(ctx context.Context, fieldManager string, applyConfiguration *applyoperatorv1.OperatorStatusApplyConfiguration) (err error) {
+	return fmt.Errorf("not implemented")
+}
+
+func (c *ConfigOperatorClient) PatchOperatorStatus(ctx context.Context, jsonPatch *jsonpatch.PatchSet) (err error) {
+	return fmt.Errorf("not implemented")
+}
diff --git a/pkg/operator/awstagcontroller.go b/pkg/operator/awstagcontroller.go
@@ -40,7 +40,7 @@ type AWSTagController struct {
 
 	event        events.Recorder
 	cachesToSync []cache.InformerSynced
-	queue        workqueue.RateLimitingInterface
+	queue        workqueue.TypedRateLimitingInterface[any]
 }
 
 // tagKeyRegex is used to check that the keys and values of a tag contain only valid characters.
@@ -75,7 +75,7 @@ func NewAWSTagController(
 		featureGateAccessor:       featureGateAccessor,
 		event:                     eventRecorder,
 		queue: workqueue.NewNamedRateLimitingQueue(
-			workqueue.DefaultControllerRateLimiter(),
+			workqueue.DefaultTypedControllerRateLimiter[any](),
 			"AWSTagController"),
 	}
 
diff --git a/pkg/operator/azurepathfixcontroller.go b/pkg/operator/azurepathfixcontroller.go
@@ -47,7 +47,7 @@ type AzurePathFixController struct {
 	kubeconfig                *restclient.Config
 
 	cachesToSync []cache.InformerSynced
-	queue        workqueue.RateLimitingInterface
+	queue        workqueue.TypedRateLimitingInterface[any]
 
 	featureGateAccessor featuregates.FeatureGateAccess
 }
@@ -76,7 +76,7 @@ func NewAzurePathFixController(
 		proxyLister:               proxyInformer.Lister(),
 		openshiftConfigLister:     openshiftConfigInformer.Lister().ConfigMaps(defaults.OpenShiftConfigNamespace),
 		kubeconfig:                kubeconfig,
-		queue:                     workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "AzurePathFixController"),
+		queue:                     workqueue.NewNamedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any](), "AzurePathFixController"),
 		featureGateAccessor:       featureGateAccessor,
 	}
 
diff --git a/pkg/operator/azurestackcloud.go b/pkg/operator/azurestackcloud.go
@@ -28,7 +28,7 @@ type AzureStackCloudController struct {
 	openshiftConfigLister corev1listers.ConfigMapNamespaceLister
 
 	cachesToSync []cache.InformerSynced
-	queue        workqueue.RateLimitingInterface
+	queue        workqueue.TypedRateLimitingInterface[any]
 }
 
 func NewAzureStackCloudController(
@@ -38,7 +38,7 @@ func NewAzureStackCloudController(
 	c := &AzureStackCloudController{
 		operatorClient:        operatorClient,
 		openshiftConfigLister: openshiftConfigInformer.Lister().ConfigMaps(defaults.OpenShiftConfigNamespace),
-		queue:                 workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "AzureStackCloudController"),
+		queue:                 workqueue.NewNamedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any](), "AzureStackCloudController"),
 	}
 
 	c.cachesToSync = append(c.cachesToSync, operatorClient.Informer().HasSynced)
diff --git a/pkg/operator/clusteroperator.go b/pkg/operator/clusteroperator.go
@@ -33,7 +33,7 @@ type ClusterOperatorStatusController struct {
 	deploymentLister          appsv1listers.DeploymentNamespaceLister
 
 	cachesToSync []cache.InformerSynced
-	queue        workqueue.RateLimitingInterface
+	queue        workqueue.TypedRateLimitingInterface[any]
 }
 
 func NewClusterOperatorStatusController(
@@ -51,7 +51,7 @@ func NewClusterOperatorStatusController(
 		imageRegistryConfigLister: imageRegistryConfigInformer.Lister(),
 		imagePrunerLister:         imagePrunerInformer.Lister(),
 		deploymentLister:          deploymentInformer.Lister().Deployments(defaults.ImageRegistryOperatorNamespace),
-		queue:                     workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ClusterOperatorStatusController"),
+		queue:                     workqueue.NewNamedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any](), "ClusterOperatorStatusController"),
 	}
 
 	if _, err := clusterOperatorInformer.Informer().AddEventHandler(c.eventHandler()); err != nil {
diff --git a/pkg/operator/controller.go b/pkg/operator/controller.go
@@ -86,7 +86,7 @@ func NewController(
 	c := &Controller{
 		kubeconfig: kubeconfig,
 		generator:  resource.NewGenerator(eventRecorder, kubeconfig, clients, listers, featureGateAccessor),
-		workqueue:  workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "Changes"),
+		workqueue:  workqueue.NewNamedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any](), "Changes"),
 		listers:    listers,
 		clients:    clients,
 	}
@@ -189,7 +189,7 @@ func NewController(
 type Controller struct {
 	kubeconfig   *restclient.Config
 	generator    *resource.Generator
-	workqueue    workqueue.RateLimitingInterface
+	workqueue    workqueue.TypedRateLimitingInterface[any]
 	listers      *regopclient.Listers
 	clients      *regopclient.Clients
 	cachesToSync []cache.InformerSynced
diff --git a/pkg/operator/controllerimagepruner.go b/pkg/operator/controllerimagepruner.go
@@ -57,7 +57,7 @@ func NewImagePrunerController(
 	clients := &regopclient.Clients{}
 	c := &ImagePrunerController{
 		generator: resource.NewImagePrunerGenerator(clients, listers),
-		workqueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), imagePrunerWorkQueueKey),
+		workqueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any](), imagePrunerWorkQueueKey),
 		listers:   listers,
 		clients:   clients,
 	}
@@ -131,7 +131,7 @@ func NewImagePrunerController(
 // ImagePrunerController keeps track of openshift image pruner components.
 type ImagePrunerController struct {
 	generator    *resource.ImagePrunerGenerator
-	workqueue    workqueue.RateLimitingInterface
+	workqueue    workqueue.TypedRateLimitingInterface[any]
 	listers      *regopclient.ImagePrunerControllerListers
 	clients      *regopclient.Clients
 	cachesToSync []cache.InformerSynced
diff --git a/pkg/operator/imageconfig.go b/pkg/operator/imageconfig.go
@@ -22,9 +22,11 @@ import (
 	"k8s.io/klog/v2"
 
 	configapi "github.com/openshift/api/config/v1"
+	configv1 "github.com/openshift/api/config/v1"
 	operatorv1 "github.com/openshift/api/operator/v1"
 	configset "github.com/openshift/client-go/config/clientset/versioned/typed/config/v1"
 	configv1informers "github.com/openshift/client-go/config/informers/externalversions/config/v1"
+	configlister "github.com/openshift/client-go/config/listers/config/v1"
 	routev1informers "github.com/openshift/client-go/route/informers/externalversions/route/v1"
 	routev1lister "github.com/openshift/client-go/route/listers/route/v1"
 	"github.com/openshift/library-go/pkg/operator/v1helpers"
@@ -42,8 +44,9 @@ type ImageConfigController struct {
 	operatorClient               v1helpers.OperatorClient
 	routeLister                  routev1lister.RouteNamespaceLister
 	serviceLister                corev1listers.ServiceNamespaceLister
+	clusterVersionLister         configlister.ClusterVersionLister
 	cachesToSync                 []cache.InformerSynced
-	queue                        workqueue.RateLimitingInterface
+	queue                        workqueue.TypedRateLimitingInterface[any]
 	imageStreamImportModeEnabled bool
 }
 
@@ -53,14 +56,16 @@ func NewImageConfigController(
 	routeInformer routev1informers.RouteInformer,
 	serviceInformer corev1informers.ServiceInformer,
 	imageConfigInformer configv1informers.ImageInformer,
+	clusterVersionInformer configv1informers.ClusterVersionInformer,
 	imageStreamImportModeEnabled bool,
 ) (*ImageConfigController, error) {
 	icc := &ImageConfigController{
 		configClient:                 configClient,
 		operatorClient:               operatorClient,
 		routeLister:                  routeInformer.Lister().Routes(defaults.ImageRegistryOperatorNamespace),
 		serviceLister:                serviceInformer.Lister().Services(defaults.ImageRegistryOperatorNamespace),
-		queue:                        workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ImageConfigController"),
+		clusterVersionLister:         clusterVersionInformer.Lister(),
+		queue:                        workqueue.NewNamedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any](), "ImageConfigController"),
 		imageStreamImportModeEnabled: imageStreamImportModeEnabled,
 	}
 
@@ -178,9 +183,17 @@ func (icc *ImageConfigController) syncImageStatus() error {
 		if cfg.Spec.ImageStreamImportMode != "" {
 			importmode = cfg.Spec.ImageStreamImportMode
 		} else {
-			// TODO: once clusterversion reports the type of payload in its status, use that to determine what the default will be
-			// i.e, multi payload => PreserveOriginal, single arch payload => Legacy
-			importmode = configapi.ImportModeLegacy
+			cv, err := icc.clusterVersionLister.Get("version")
+			if err != nil {
+				return err
+			}
+			// If the clusterversion reports that the desired architecture (existing or desired) of the
+			// cluster is "Multi", set import mode to PreserveOriginal. Else set it to Legacy
+			if cv.Status.Desired.Architecture == configv1.ClusterVersionArchitectureMulti {
+				importmode = configapi.ImportModePreserveOriginal
+			} else {
+				importmode = configapi.ImportModeLegacy
+			}
 		}
 		if cfg.Status.ImageStreamImportMode != importmode {
 			cfg.Status.ImageStreamImportMode = importmode
diff --git a/pkg/operator/imageregistrycertificates.go b/pkg/operator/imageregistrycertificates.go
@@ -41,7 +41,7 @@ type ImageRegistryCertificatesController struct {
 	storageListers            *client.StorageListers
 
 	cachesToSync []cache.InformerSynced
-	queue        workqueue.RateLimitingInterface
+	queue        workqueue.TypedRateLimitingInterface[any]
 
 	featureGateAccessor featuregates.FeatureGateAccess
 }
@@ -69,7 +69,7 @@ func NewImageRegistryCertificatesController(
 		imageConfigLister:         imageConfigInformer.Lister(),
 		openshiftConfigLister:     openshiftConfigInformer.Lister().ConfigMaps(defaults.OpenShiftConfigNamespace),
 		imageRegistryConfigLister: imageRegistryConfigInformer.Lister(),
-		queue:                     workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ImageRegistryCertificatesController"),
+		queue:                     workqueue.NewNamedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any](), "ImageRegistryCertificatesController"),
 	}
 
 	if _, err := configMapInformer.Informer().AddEventHandler(c.eventHandler()); err != nil {
diff --git a/pkg/operator/nodecadaemon.go b/pkg/operator/nodecadaemon.go
@@ -35,7 +35,7 @@ type NodeCADaemonController struct {
 	serviceLister   corev1listers.ServiceNamespaceLister
 
 	cachesToSync []cache.InformerSynced
-	queue        workqueue.RateLimitingInterface
+	queue        workqueue.TypedRateLimitingInterface[any]
 }
 
 func NewNodeCADaemonController(
@@ -51,7 +51,7 @@ func NewNodeCADaemonController(
 		operatorClient:  operatorClient,
 		daemonSetLister: daemonSetInformer.Lister().DaemonSets(defaults.ImageRegistryOperatorNamespace),
 		serviceLister:   serviceInformer.Lister().Services(defaults.ImageRegistryOperatorNamespace),
-		queue:           workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "NodeCADaemonController"),
+		queue:           workqueue.NewNamedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any](), "NodeCADaemonController"),
 	}
 
 	c.cachesToSync = append(c.cachesToSync, operatorClient.Informer().HasSynced)
diff --git a/pkg/operator/starter.go b/pkg/operator/starter.go
@@ -9,6 +9,7 @@ import (
 	kubeclient "k8s.io/client-go/kubernetes"
 	restclient "k8s.io/client-go/rest"
 	"k8s.io/klog/v2"
+	"k8s.io/utils/clock"
 
 	configv1 "github.com/openshift/api/config/v1"
 	"github.com/openshift/api/features"
@@ -71,7 +72,7 @@ func RunOperator(ctx context.Context, kubeconfig *restclient.Config) error {
 	if err != nil {
 		klog.Warningf("unable to get owner reference (falling back to namespace): %v", err)
 	}
-	eventRecorder := events.NewKubeRecorder(kubeClient.CoreV1().Events(defaults.ImageRegistryOperatorNamespace), "image-registry-operator", controllerRef)
+	eventRecorder := events.NewKubeRecorder(kubeClient.CoreV1().Events(defaults.ImageRegistryOperatorNamespace), "image-registry-operator", controllerRef, clock.RealClock{})
 
 	desiredVersion := status.VersionForOperatorFromEnv()
 	missingVersion := "0.0.1-snapshot"
@@ -123,6 +124,7 @@ func RunOperator(ctx context.Context, kubeconfig *restclient.Config) error {
 		routeInformers.Route().V1().Routes(),
 		kubeInformers.Core().V1().Services(),
 		configInformers.Config().V1().Images(),
+		configInformers.Config().V1().ClusterVersions(),
 		imageStreamImportModeEnabled,
 	)
 	if err != nil {
diff --git a/pkg/resource/nodecadaemon_test.go b/pkg/resource/nodecadaemon_test.go
@@ -10,6 +10,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kfake "k8s.io/client-go/kubernetes/fake"
+	"k8s.io/utils/clock"
 
 	imageregistryv1 "github.com/openshift/api/imageregistry/v1"
 	imageregistryfake "github.com/openshift/client-go/imageregistry/clientset/versioned/fake"
@@ -53,7 +54,7 @@ func TestNodeCADaemon(t *testing.T) {
 	imageregistryInformers.Start(ctx.Done())
 	imageregistryInformers.WaitForCacheSync(ctx.Done())
 
-	g := NewGeneratorNodeCADaemonSet(events.NewInMemoryRecorder("image-registry-operator"), nil, nil, clientset.AppsV1(), operatorClient)
+	g := NewGeneratorNodeCADaemonSet(events.NewInMemoryRecorder("image-registry-operator", clock.RealClock{}), nil, nil, clientset.AppsV1(), operatorClient)
 	obj, err := g.Create()
 	if err != nil {
 		t.Fatal(err)

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,7 @@ type AWSTagController struct {`
`40`	`40`
`41`	`41`	`event events.Recorder`
`42`	`42`	`cachesToSync []cache.InformerSynced`
`43`		`- queue workqueue.RateLimitingInterface`
	`43`	`+ queue workqueue.TypedRateLimitingInterface[any]`
`44`	`44`	`}`
`45`	`45`
`46`	`46`	`// tagKeyRegex is used to check that the keys and values of a tag contain only valid characters.`
`@@ -75,7 +75,7 @@ func NewAWSTagController(`
`75`	`75`	`featureGateAccessor: featureGateAccessor,`
`76`	`76`	`event: eventRecorder,`
`77`	`77`	`queue: workqueue.NewNamedRateLimitingQueue(`
`78`		`- workqueue.DefaultControllerRateLimiter(),`
	`78`	`+ workqueue.DefaultTypedControllerRateLimiter[any](),`
`79`	`79`	`"AWSTagController"),`
`80`	`80`	`}`
`81`	`81`
Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ type AzurePathFixController struct {`
`47`	`47`	`kubeconfig *restclient.Config`
`48`	`48`
`49`	`49`	`cachesToSync []cache.InformerSynced`
`50`		`- queue workqueue.RateLimitingInterface`
	`50`	`+ queue workqueue.TypedRateLimitingInterface[any]`
`51`	`51`
`52`	`52`	`featureGateAccessor featuregates.FeatureGateAccess`
`53`	`53`	`}`
`@@ -76,7 +76,7 @@ func NewAzurePathFixController(`
`76`	`76`	`proxyLister: proxyInformer.Lister(),`
`77`	`77`	`openshiftConfigLister: openshiftConfigInformer.Lister().ConfigMaps(defaults.OpenShiftConfigNamespace),`
`78`	`78`	`kubeconfig: kubeconfig,`
`79`		`- queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "AzurePathFixController"),`
	`79`	`+ queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any](), "AzurePathFixController"),`
`80`	`80`	`featureGateAccessor: featureGateAccessor,`
`81`	`81`	`}`
`82`	`82`
Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ type AzureStackCloudController struct {`
`28`	`28`	`openshiftConfigLister corev1listers.ConfigMapNamespaceLister`
`29`	`29`
`30`	`30`	`cachesToSync []cache.InformerSynced`
`31`		`- queue workqueue.RateLimitingInterface`
	`31`	`+ queue workqueue.TypedRateLimitingInterface[any]`
`32`	`32`	`}`
`33`	`33`
`34`	`34`	`func NewAzureStackCloudController(`
`@@ -38,7 +38,7 @@ func NewAzureStackCloudController(`
`38`	`38`	`c := &AzureStackCloudController{`
`39`	`39`	`operatorClient: operatorClient,`
`40`	`40`	`openshiftConfigLister: openshiftConfigInformer.Lister().ConfigMaps(defaults.OpenShiftConfigNamespace),`
`41`		`- queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "AzureStackCloudController"),`
	`41`	`+ queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any](), "AzureStackCloudController"),`
`42`	`42`	`}`
`43`	`43`
`44`	`44`	`c.cachesToSync = append(c.cachesToSync, operatorClient.Informer().HasSynced)`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ type ClusterOperatorStatusController struct {`
`33`	`33`	`deploymentLister appsv1listers.DeploymentNamespaceLister`
`34`	`34`
`35`	`35`	`cachesToSync []cache.InformerSynced`
`36`		`- queue workqueue.RateLimitingInterface`
	`36`	`+ queue workqueue.TypedRateLimitingInterface[any]`
`37`	`37`	`}`
`38`	`38`
`39`	`39`	`func NewClusterOperatorStatusController(`
`@@ -51,7 +51,7 @@ func NewClusterOperatorStatusController(`
`51`	`51`	`imageRegistryConfigLister: imageRegistryConfigInformer.Lister(),`
`52`	`52`	`imagePrunerLister: imagePrunerInformer.Lister(),`
`53`	`53`	`deploymentLister: deploymentInformer.Lister().Deployments(defaults.ImageRegistryOperatorNamespace),`
`54`		`- queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ClusterOperatorStatusController"),`
	`54`	`+ queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any](), "ClusterOperatorStatusController"),`
`55`	`55`	`}`
`56`	`56`
`57`	`57`	`if _, err := clusterOperatorInformer.Informer().AddEventHandler(c.eventHandler()); err != nil {`
Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ type ImageRegistryCertificatesController struct {`
`41`	`41`	`storageListers *client.StorageListers`
`42`	`42`
`43`	`43`	`cachesToSync []cache.InformerSynced`
`44`		`- queue workqueue.RateLimitingInterface`
	`44`	`+ queue workqueue.TypedRateLimitingInterface[any]`
`45`	`45`
`46`	`46`	`featureGateAccessor featuregates.FeatureGateAccess`
`47`	`47`	`}`
`@@ -69,7 +69,7 @@ func NewImageRegistryCertificatesController(`
`69`	`69`	`imageConfigLister: imageConfigInformer.Lister(),`
`70`	`70`	`openshiftConfigLister: openshiftConfigInformer.Lister().ConfigMaps(defaults.OpenShiftConfigNamespace),`
`71`	`71`	`imageRegistryConfigLister: imageRegistryConfigInformer.Lister(),`
`72`		`- queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "ImageRegistryCertificatesController"),`
	`72`	`+ queue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultTypedControllerRateLimiter[any](), "ImageRegistryCertificatesController"),`
`73`	`73`	`}`
`74`	`74`
`75`	`75`	`if _, err := configMapInformer.Informer().AddEventHandler(c.eventHandler()); err != nil {`