From ea86970bff7d7c2065689c7bbb4f32db76158a49 Mon Sep 17 00:00:00 2001 From: David Hurta Date: Tue, 23 Sep 2025 15:55:21 +0200 Subject: [PATCH 1/6] config: Extract apply log level logic to function --- pkg/cvo/configuration/configuration.go | 28 +---------------- pkg/cvo/configuration/loglevel.go | 42 ++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 27 deletions(-) create mode 100644 pkg/cvo/configuration/loglevel.go diff --git a/pkg/cvo/configuration/configuration.go b/pkg/cvo/configuration/configuration.go index 66782421e..67be8ad59 100644 --- a/pkg/cvo/configuration/configuration.go +++ b/pkg/cvo/configuration/configuration.go @@ -148,31 +148,5 @@ func (config *ClusterVersionOperatorConfiguration) sync(ctx context.Context, des config.desiredLogLevel = operatorv1.Normal } - currentLogLevel, notFound := loglevel.GetLogLevel() - if notFound { - klog.Warningf("The current log level could not be found; an attempt to set the log level to the desired level will be made") - } - - if !notFound && currentLogLevel == config.desiredLogLevel { - klog.V(i.Debug).Infof("No need to update the current CVO log level '%s'; it is already set to the desired value", currentLogLevel) - } else { - if err := loglevel.SetLogLevel(config.desiredLogLevel); err != nil { - return fmt.Errorf("failed to set the log level to %q: %w", config.desiredLogLevel, err) - } - - // E2E testing will be checking for existence or absence of these logs - switch config.desiredLogLevel { - case operatorv1.Normal: - klog.V(i.Normal).Infof("Successfully updated the log level from '%s' to 'Normal'", currentLogLevel) - case operatorv1.Debug: - klog.V(i.Debug).Infof("Successfully updated the log level from '%s' to 'Debug'", currentLogLevel) - case operatorv1.Trace: - klog.V(i.Trace).Infof("Successfully updated the log level from '%s' to 'Trace'", currentLogLevel) - case operatorv1.TraceAll: - klog.V(i.TraceAll).Infof("Successfully updated the log level from '%s' to 'TraceAll'", currentLogLevel) - default: - klog.Errorf("The CVO logging level has unexpected value '%s'", config.desiredLogLevel) - } - } - return nil + return applyLogLevel(config.desiredLogLevel) } diff --git a/pkg/cvo/configuration/loglevel.go b/pkg/cvo/configuration/loglevel.go new file mode 100644 index 000000000..0e87f47dc --- /dev/null +++ b/pkg/cvo/configuration/loglevel.go @@ -0,0 +1,42 @@ +package configuration + +import ( + "fmt" + + "k8s.io/klog/v2" + + operatorv1 "github.com/openshift/api/operator/v1" + "github.com/openshift/library-go/pkg/operator/loglevel" + + i "github.com/openshift/cluster-version-operator/pkg/internal" +) + +func applyLogLevel(level operatorv1.LogLevel) error { + currentLogLevel, notFound := loglevel.GetLogLevel() + if notFound { + klog.Warningf("The current log level could not be found; an attempt to set the log level to the desired level will be made") + } + + if !notFound && currentLogLevel == level { + klog.V(i.Debug).Infof("No need to update the current CVO log level '%s'; it is already set to the desired value", currentLogLevel) + } else { + if err := loglevel.SetLogLevel(level); err != nil { + return fmt.Errorf("failed to set the log level to %q: %w", level, err) + } + + // E2E testing will be checking for existence or absence of these logs + switch level { + case operatorv1.Normal: + klog.V(i.Normal).Infof("Successfully updated the log level from '%s' to 'Normal'", currentLogLevel) + case operatorv1.Debug: + klog.V(i.Debug).Infof("Successfully updated the log level from '%s' to 'Debug'", currentLogLevel) + case operatorv1.Trace: + klog.V(i.Trace).Infof("Successfully updated the log level from '%s' to 'Trace'", currentLogLevel) + case operatorv1.TraceAll: + klog.V(i.TraceAll).Infof("Successfully updated the log level from '%s' to 'TraceAll'", currentLogLevel) + default: + klog.Errorf("The CVO logging level has unexpected value '%s'", level) + } + } + return nil +} From 1b5d7c8e48e2eb4926d184d24cf321a4b8fc812f Mon Sep 17 00:00:00 2001 From: David Hurta Date: Tue, 23 Sep 2025 17:05:56 +0200 Subject: [PATCH 2/6] config: Refactor configuration representation Utilize a new structure, which holds the relevant information regarding the desired CVO configuration and its status to encapsulate this information to a simple data structure. --- pkg/cvo/configuration/configuration.go | 24 ++++++----- pkg/cvo/configuration/configuration_test.go | 45 +++++++++++++-------- 2 files changed, 42 insertions(+), 27 deletions(-) diff --git a/pkg/cvo/configuration/configuration.go b/pkg/cvo/configuration/configuration.go index 67be8ad59..3d2490b48 100644 --- a/pkg/cvo/configuration/configuration.go +++ b/pkg/cvo/configuration/configuration.go @@ -24,6 +24,11 @@ import ( const ClusterVersionOperatorConfigurationName = "cluster" +type configuration struct { + lastObservedGeneration int64 + desiredLogLevel operatorv1.LogLevel +} + type ClusterVersionOperatorConfiguration struct { queueKey string // queue tracks checking for the CVO configuration. @@ -37,8 +42,7 @@ type ClusterVersionOperatorConfiguration struct { started bool - desiredLogLevel operatorv1.LogLevel - lastObservedGeneration int64 + configuration configuration } func (config *ClusterVersionOperatorConfiguration) Queue() workqueue.TypedRateLimitingInterface[any] { @@ -80,9 +84,9 @@ func NewClusterVersionOperatorConfiguration(client operatorclientset.Interface, queue: workqueue.NewTypedRateLimitingQueueWithConfig[any]( workqueue.DefaultTypedControllerRateLimiter[any](), workqueue.TypedRateLimitingQueueConfig[any]{Name: "configuration"}), - client: client.OperatorV1alpha1().ClusterVersionOperators(), - factory: factory, - desiredLogLevel: desiredLogLevel, + client: client.OperatorV1alpha1().ClusterVersionOperators(), + factory: factory, + configuration: configuration{desiredLogLevel: desiredLogLevel}, } } @@ -142,11 +146,11 @@ func (config *ClusterVersionOperatorConfiguration) sync(ctx context.Context, des } } - config.lastObservedGeneration = desiredConfig.Generation - config.desiredLogLevel = desiredConfig.Spec.OperatorLogLevel - if config.desiredLogLevel == "" { - config.desiredLogLevel = operatorv1.Normal + config.configuration.lastObservedGeneration = desiredConfig.Generation + config.configuration.desiredLogLevel = desiredConfig.Spec.OperatorLogLevel + if config.configuration.desiredLogLevel == "" { + config.configuration.desiredLogLevel = operatorv1.Normal } - return applyLogLevel(config.desiredLogLevel) + return applyLogLevel(config.configuration.desiredLogLevel) } diff --git a/pkg/cvo/configuration/configuration_test.go b/pkg/cvo/configuration/configuration_test.go index 8ed7602b5..583cf8b62 100644 --- a/pkg/cvo/configuration/configuration_test.go +++ b/pkg/cvo/configuration/configuration_test.go @@ -22,8 +22,8 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { name string config operatorv1alpha1.ClusterVersionOperator expectedConfig operatorv1alpha1.ClusterVersionOperator - internalConfig ClusterVersionOperatorConfiguration - expectedInternalConfig ClusterVersionOperatorConfiguration + internalConfig configuration + expectedInternalConfig configuration }{ { name: "first sync run correctly updates the status", @@ -46,11 +46,11 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { ObservedGeneration: 1, }, }, - internalConfig: ClusterVersionOperatorConfiguration{ + internalConfig: configuration{ desiredLogLevel: operatorv1.Normal, lastObservedGeneration: 0, }, - expectedInternalConfig: ClusterVersionOperatorConfiguration{ + expectedInternalConfig: configuration{ desiredLogLevel: operatorv1.Normal, lastObservedGeneration: 1, }, @@ -79,11 +79,11 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { ObservedGeneration: 3, }, }, - internalConfig: ClusterVersionOperatorConfiguration{ + internalConfig: configuration{ desiredLogLevel: operatorv1.Normal, lastObservedGeneration: 2, }, - expectedInternalConfig: ClusterVersionOperatorConfiguration{ + expectedInternalConfig: configuration{ desiredLogLevel: operatorv1.Normal, lastObservedGeneration: 3, }, @@ -112,11 +112,11 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { ObservedGeneration: 4, }, }, - internalConfig: ClusterVersionOperatorConfiguration{ + internalConfig: configuration{ desiredLogLevel: operatorv1.Normal, lastObservedGeneration: 3, }, - expectedInternalConfig: ClusterVersionOperatorConfiguration{ + expectedInternalConfig: configuration{ desiredLogLevel: operatorv1.Trace, lastObservedGeneration: 4, }, @@ -145,11 +145,11 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { ObservedGeneration: 40, }, }, - internalConfig: ClusterVersionOperatorConfiguration{ + internalConfig: configuration{ desiredLogLevel: operatorv1.Normal, lastObservedGeneration: 3, }, - expectedInternalConfig: ClusterVersionOperatorConfiguration{ + expectedInternalConfig: configuration{ desiredLogLevel: operatorv1.TraceAll, lastObservedGeneration: 40, }, @@ -158,24 +158,35 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Initialize testing logic + tt.config.Name = ClusterVersionOperatorConfigurationName + tt.expectedConfig.Name = ClusterVersionOperatorConfigurationName + client := operatorclientsetfake.NewClientset(&tt.config) - tt.internalConfig.client = client.OperatorV1alpha1().ClusterVersionOperators() + factory := operatorexternalversions.NewSharedInformerFactoryWithOptions(client, time.Minute) + + configController := NewClusterVersionOperatorConfiguration(client, factory) + ctx, cancelFunc := context.WithDeadline(context.Background(), time.Now().Add(time.Minute)) + if err := configController.Start(ctx); err != nil { + t.Errorf("unexpected error %v", err) + } + configController.configuration = tt.internalConfig + // Run tested functionality - if err := tt.internalConfig.sync(ctx, &tt.config); err != nil { + if err := configController.Sync(ctx, "key"); err != nil { t.Errorf("unexpected error %v", err) } // Verify results - if tt.internalConfig.lastObservedGeneration != tt.expectedInternalConfig.lastObservedGeneration { - t.Errorf("unexpected 'lastObservedGeneration' value; wanted=%v, got=%v", tt.expectedInternalConfig.lastObservedGeneration, tt.internalConfig.lastObservedGeneration) + if configController.configuration.lastObservedGeneration != tt.expectedInternalConfig.lastObservedGeneration { + t.Errorf("unexpected 'lastObservedGeneration' value; wanted=%v, got=%v", tt.expectedInternalConfig.lastObservedGeneration, configController.configuration.lastObservedGeneration) } - if tt.internalConfig.desiredLogLevel != tt.expectedInternalConfig.desiredLogLevel { - t.Errorf("unexpected 'desiredLogLevel' value; wanted=%v, got=%v", tt.expectedInternalConfig.desiredLogLevel, tt.internalConfig.desiredLogLevel) + if configController.configuration.desiredLogLevel != tt.expectedInternalConfig.desiredLogLevel { + t.Errorf("unexpected 'desiredLogLevel' value; wanted=%v, got=%v", tt.expectedInternalConfig.desiredLogLevel, configController.configuration.desiredLogLevel) } - config, err := client.OperatorV1alpha1().ClusterVersionOperators().Get(ctx, "", metav1.GetOptions{}) + config, err := client.OperatorV1alpha1().ClusterVersionOperators().Get(ctx, ClusterVersionOperatorConfigurationName, metav1.GetOptions{}) if err != nil { t.Errorf("unexpected error %v", err) } From 6bbb6fc875ebf68aaa69eb84c7d5a393ea163926 Mon Sep 17 00:00:00 2001 From: David Hurta Date: Tue, 23 Sep 2025 17:21:51 +0200 Subject: [PATCH 3/6] config: Add handler field to CVO configuration Using the field we can avoid changing the actual logging during testing by assigning a different function to the field if necessary. We can now also verify that the handler field is being executed. --- pkg/cvo/configuration/configuration.go | 14 +++++++++++++- pkg/cvo/configuration/configuration_test.go | 15 +++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/pkg/cvo/configuration/configuration.go b/pkg/cvo/configuration/configuration.go index 3d2490b48..947284bae 100644 --- a/pkg/cvo/configuration/configuration.go +++ b/pkg/cvo/configuration/configuration.go @@ -43,6 +43,14 @@ type ClusterVersionOperatorConfiguration struct { started bool configuration configuration + + // Function to handle an update to the internal configuration. + // + // In the future, the desired configuration may be consumed via other controllers. + // This will require some sort of synchronization upon a configuration change. + // For the moment, the log level is the sole consumer of the configuration. + // Apply the log level directly here for simplicity for the time being. + handler func(configuration) error } func (config *ClusterVersionOperatorConfiguration) Queue() workqueue.TypedRateLimitingInterface[any] { @@ -87,6 +95,7 @@ func NewClusterVersionOperatorConfiguration(client operatorclientset.Interface, client: client.OperatorV1alpha1().ClusterVersionOperators(), factory: factory, configuration: configuration{desiredLogLevel: desiredLogLevel}, + handler: func(c configuration) error { return applyLogLevel(c.desiredLogLevel) }, } } @@ -152,5 +161,8 @@ func (config *ClusterVersionOperatorConfiguration) sync(ctx context.Context, des config.configuration.desiredLogLevel = operatorv1.Normal } - return applyLogLevel(config.configuration.desiredLogLevel) + if config.handler != nil { + return config.handler(config.configuration) + } + return nil } diff --git a/pkg/cvo/configuration/configuration_test.go b/pkg/cvo/configuration/configuration_test.go index 583cf8b62..e212718b9 100644 --- a/pkg/cvo/configuration/configuration_test.go +++ b/pkg/cvo/configuration/configuration_test.go @@ -24,6 +24,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { expectedConfig operatorv1alpha1.ClusterVersionOperator internalConfig configuration expectedInternalConfig configuration + handlerFunctionCalled bool }{ { name: "first sync run correctly updates the status", @@ -54,6 +55,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { desiredLogLevel: operatorv1.Normal, lastObservedGeneration: 1, }, + handlerFunctionCalled: true, }, { name: "sync updates observed generation correctly", @@ -87,6 +89,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { desiredLogLevel: operatorv1.Normal, lastObservedGeneration: 3, }, + handlerFunctionCalled: true, }, { name: "sync updates desired log level correctly", @@ -120,6 +123,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { desiredLogLevel: operatorv1.Trace, lastObservedGeneration: 4, }, + handlerFunctionCalled: true, }, { name: "number of not observed generations does not impact sync", @@ -153,6 +157,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { desiredLogLevel: operatorv1.TraceAll, lastObservedGeneration: 40, }, + handlerFunctionCalled: true, }, } for _, tt := range tests { @@ -166,6 +171,12 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { configController := NewClusterVersionOperatorConfiguration(client, factory) + called := false + configController.handler = func(_ configuration) error { + called = true + return nil + } + ctx, cancelFunc := context.WithDeadline(context.Background(), time.Now().Add(time.Minute)) if err := configController.Start(ctx); err != nil { @@ -194,6 +205,10 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { t.Errorf("unexpected config (-want, +got) = %v", diff) } + if tt.handlerFunctionCalled != called { + t.Errorf("unexpected handler function execution; wanted=%v, got=%v", tt.handlerFunctionCalled, called) + } + // Shutdown created resources cancelFunc() }) From 64ec2c1208f370239e75d80465c5858e04b89a1b Mon Sep 17 00:00:00 2001 From: David Hurta Date: Tue, 23 Sep 2025 17:33:38 +0200 Subject: [PATCH 4/6] config: Conditionally initialize configuration controller The information regarding enabled feature gates is now available sooner in the code. Utilize this information to conditionally initialize the configuration controller at a one specific place. If no configuration controller is needed, this information is clearly stated by the noninitialized configuration field. The subsequent code can then easily check for existence of the configuration using a nil. --- pkg/cvo/cvo.go | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/pkg/cvo/cvo.go b/pkg/cvo/cvo.go index 6f186479f..844c3b7ed 100644 --- a/pkg/cvo/cvo.go +++ b/pkg/cvo/cvo.go @@ -290,7 +290,9 @@ func New( // make sure this is initialized after all the listers are initialized optr.upgradeableChecks = optr.defaultUpgradeableChecks() - optr.configuration = configuration.NewClusterVersionOperatorConfiguration(operatorClient, operatorInformerFactory) + if shouldReconcileCVOConfiguration(cvoGates, optr.hypershift) { + optr.configuration = configuration.NewClusterVersionOperatorConfiguration(operatorClient, operatorInformerFactory) + } return optr, nil } @@ -444,7 +446,9 @@ func (optr *Operator) Run(runContext context.Context, shutdownContext context.Co defer optr.queue.ShutDown() defer optr.availableUpdatesQueue.ShutDown() defer optr.upgradeableQueue.ShutDown() - defer optr.configuration.Queue().ShutDown() + if optr.configuration != nil { + defer optr.configuration.Queue().ShutDown() + } stopCh := runContext.Done() klog.Infof("Starting ClusterVersionOperator with minimum reconcile period %s", optr.minimumUpdateCheckInterval) @@ -483,7 +487,7 @@ func (optr *Operator) Run(runContext context.Context, shutdownContext context.Co resultChannel <- asyncResult{name: "available updates"} }() - if optr.shouldReconcileCVOConfiguration() { + if optr.configuration != nil { resultChannelCount++ go func() { defer utilruntime.HandleCrash() @@ -569,7 +573,9 @@ func (optr *Operator) Run(runContext context.Context, shutdownContext context.Co optr.queue.ShutDown() optr.availableUpdatesQueue.ShutDown() optr.upgradeableQueue.ShutDown() - optr.configuration.Queue().ShutDown() + if optr.configuration != nil { + optr.configuration.Queue().ShutDown() + } } } @@ -1070,7 +1076,7 @@ func (optr *Operator) HTTPClient() (*http.Client, error) { // shouldReconcileCVOConfiguration returns whether the CVO should reconcile its configuration using the API server. // // enabledFeatureGates must be initialized before the function is called. -func (optr *Operator) shouldReconcileCVOConfiguration() bool { +func shouldReconcileCVOConfiguration(enabledFeatureGates featuregates.CvoGateChecker, isHypershift bool) bool { // The relevant CRD and CR are not applied in HyperShift, which configures the CVO via a configuration file - return optr.enabledFeatureGates.CVOConfiguration() && !optr.hypershift + return enabledFeatureGates.CVOConfiguration() && !isHypershift } From 012816b68cf13104bd36ec2e79b8f97b2bd6af45 Mon Sep 17 00:00:00 2001 From: David Hurta Date: Wed, 24 Sep 2025 17:24:51 +0200 Subject: [PATCH 5/6] config: Shutdown CVO if configuration controller fails to start An error in the start logic will now result in the immediate shutdown of the CVO, rather than in the CVO catching the failed controller and continuing to work by ignoring the failed start. --- pkg/cvo/cvo.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/pkg/cvo/cvo.go b/pkg/cvo/cvo.go index 844c3b7ed..441239125 100644 --- a/pkg/cvo/cvo.go +++ b/pkg/cvo/cvo.go @@ -461,6 +461,12 @@ func (optr *Operator) Run(runContext context.Context, shutdownContext context.Co return fmt.Errorf("caches never synchronized: %w", runContext.Err()) } + if optr.configuration != nil { + if err := optr.configuration.Start(runContext); err != nil { + return fmt.Errorf("unable to initialize the CVO configuration controller: %v", err) + } + } + // trigger the first cluster version reconcile always optr.queue.Add(optr.queueKey()) @@ -491,13 +497,9 @@ func (optr *Operator) Run(runContext context.Context, shutdownContext context.Co resultChannelCount++ go func() { defer utilruntime.HandleCrash() - if err := optr.configuration.Start(runContext); err != nil { - utilruntime.HandleError(fmt.Errorf("unable to initialize the CVO configuration sync: %v", err)) - } else { - wait.UntilWithContext(runContext, func(runContext context.Context) { - optr.worker(runContext, optr.configuration.Queue(), optr.configuration.Sync) - }, time.Second) - } + wait.UntilWithContext(runContext, func(runContext context.Context) { + optr.worker(runContext, optr.configuration.Queue(), optr.configuration.Sync) + }, time.Second) resultChannel <- asyncResult{name: "cvo configuration"} }() } else { From 439176caedabf12dfe73e1a8e45efdcad3c537bf Mon Sep 17 00:00:00 2001 From: David Hurta Date: Tue, 23 Sep 2025 23:17:54 +0200 Subject: [PATCH 6/6] config: test: Merge test functions together Due to recent changes, the two test functions can now be safely merged together. The second function verified the overall synchronization of the configuration, which is now also being exercised in the first function. The tests cases were migrated over to the first function or were already present. --- pkg/cvo/configuration/configuration_test.go | 142 ++++++-------------- 1 file changed, 39 insertions(+), 103 deletions(-) diff --git a/pkg/cvo/configuration/configuration_test.go b/pkg/cvo/configuration/configuration_test.go index e212718b9..eb5517097 100644 --- a/pkg/cvo/configuration/configuration_test.go +++ b/pkg/cvo/configuration/configuration_test.go @@ -17,18 +17,33 @@ import ( operatorexternalversions "github.com/openshift/client-go/operator/informers/externalversions" ) -func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { +func TestClusterVersionOperatorConfiguration_APIServerSync(t *testing.T) { tests := []struct { name string - config operatorv1alpha1.ClusterVersionOperator - expectedConfig operatorv1alpha1.ClusterVersionOperator + config *operatorv1alpha1.ClusterVersionOperator + expectedConfig *operatorv1alpha1.ClusterVersionOperator internalConfig configuration expectedInternalConfig configuration handlerFunctionCalled bool }{ + { + name: "the configuration resource does not exist in the cluster -> default configuration", + config: nil, + expectedConfig: nil, + internalConfig: configuration{ + lastObservedGeneration: 3, + desiredLogLevel: "Trace", + }, + expectedInternalConfig: configuration{ + lastObservedGeneration: 3, // TODO: Default to 0 + desiredLogLevel: "Trace", // TODO: Default to Normal + }, + // TODO: Apply the log level when the defaulting is implemented + handlerFunctionCalled: false, + }, { name: "first sync run correctly updates the status", - config: operatorv1alpha1.ClusterVersionOperator{ + config: &operatorv1alpha1.ClusterVersionOperator{ ObjectMeta: metav1.ObjectMeta{ Generation: 1, }, @@ -36,7 +51,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { OperatorLogLevel: operatorv1.Normal, }, }, - expectedConfig: operatorv1alpha1.ClusterVersionOperator{ + expectedConfig: &operatorv1alpha1.ClusterVersionOperator{ ObjectMeta: metav1.ObjectMeta{ Generation: 1, }, @@ -59,7 +74,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { }, { name: "sync updates observed generation correctly", - config: operatorv1alpha1.ClusterVersionOperator{ + config: &operatorv1alpha1.ClusterVersionOperator{ ObjectMeta: metav1.ObjectMeta{ Generation: 3, }, @@ -70,7 +85,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { ObservedGeneration: 2, }, }, - expectedConfig: operatorv1alpha1.ClusterVersionOperator{ + expectedConfig: &operatorv1alpha1.ClusterVersionOperator{ ObjectMeta: metav1.ObjectMeta{ Generation: 3, }, @@ -93,7 +108,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { }, { name: "sync updates desired log level correctly", - config: operatorv1alpha1.ClusterVersionOperator{ + config: &operatorv1alpha1.ClusterVersionOperator{ ObjectMeta: metav1.ObjectMeta{ Generation: 4, }, @@ -104,7 +119,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { ObservedGeneration: 3, }, }, - expectedConfig: operatorv1alpha1.ClusterVersionOperator{ + expectedConfig: &operatorv1alpha1.ClusterVersionOperator{ ObjectMeta: metav1.ObjectMeta{ Generation: 4, }, @@ -127,7 +142,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { }, { name: "number of not observed generations does not impact sync", - config: operatorv1alpha1.ClusterVersionOperator{ + config: &operatorv1alpha1.ClusterVersionOperator{ ObjectMeta: metav1.ObjectMeta{ Generation: 40, }, @@ -138,7 +153,7 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { ObservedGeneration: 3, }, }, - expectedConfig: operatorv1alpha1.ClusterVersionOperator{ + expectedConfig: &operatorv1alpha1.ClusterVersionOperator{ ObjectMeta: metav1.ObjectMeta{ Generation: 40, }, @@ -163,12 +178,13 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // Initialize testing logic - tt.config.Name = ClusterVersionOperatorConfigurationName - tt.expectedConfig.Name = ClusterVersionOperatorConfigurationName - - client := operatorclientsetfake.NewClientset(&tt.config) + client := operatorclientsetfake.NewClientset() + if tt.config != nil { + tt.config.Name = ClusterVersionOperatorConfigurationName + tt.expectedConfig.Name = ClusterVersionOperatorConfigurationName + client = operatorclientsetfake.NewClientset(tt.config) + } factory := operatorexternalversions.NewSharedInformerFactoryWithOptions(client, time.Minute) - configController := NewClusterVersionOperatorConfiguration(client, factory) called := false @@ -198,10 +214,15 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { } config, err := client.OperatorV1alpha1().ClusterVersionOperators().Get(ctx, ClusterVersionOperatorConfigurationName, metav1.GetOptions{}) - if err != nil { + if err != nil && !apierrors.IsNotFound(err) { t.Errorf("unexpected error %v", err) } - if diff := cmp.Diff(tt.expectedConfig, *config, cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ManagedFields")); diff != "" { + + // Set nil to differentiate between nonexisting configurations + if apierrors.IsNotFound(err) { + config = nil + } + if diff := cmp.Diff(tt.expectedConfig, config, cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ManagedFields")); diff != "" { t.Errorf("unexpected config (-want, +got) = %v", diff) } @@ -214,88 +235,3 @@ func TestClusterVersionOperatorConfiguration_sync(t *testing.T) { }) } } - -func TestClusterVersionOperatorConfiguration_Sync(t *testing.T) { - tests := []struct { - name string - config *operatorv1alpha1.ClusterVersionOperator - expectedConfig *operatorv1alpha1.ClusterVersionOperator - }{ - { - name: "the configuration resource does not exist in the cluster -> ignore", - config: nil, - expectedConfig: nil, - }, - { - name: "Sync updates the ClusterVersionOperator resource", - config: &operatorv1alpha1.ClusterVersionOperator{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster", - Generation: 4, - }, - Spec: operatorv1alpha1.ClusterVersionOperatorSpec{ - OperatorLogLevel: operatorv1.Trace, - }, - Status: operatorv1alpha1.ClusterVersionOperatorStatus{ - ObservedGeneration: 3, - }, - }, - expectedConfig: &operatorv1alpha1.ClusterVersionOperator{ - ObjectMeta: metav1.ObjectMeta{ - Name: "cluster", - Generation: 4, - }, - Spec: operatorv1alpha1.ClusterVersionOperatorSpec{ - OperatorLogLevel: operatorv1.Trace, - }, - Status: operatorv1alpha1.ClusterVersionOperatorStatus{ - ObservedGeneration: 4, - }, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Initialize testing logic - var client *operatorclientsetfake.Clientset - if tt.config != nil { - client = operatorclientsetfake.NewClientset(tt.config) - } else { - client = operatorclientsetfake.NewClientset() - } - - factory := operatorexternalversions.NewSharedInformerFactoryWithOptions(client, time.Minute) - cvoConfiguration := NewClusterVersionOperatorConfiguration(client, factory) - defer cvoConfiguration.queue.ShutDown() - - ctx, cancelFunc := context.WithDeadline(context.Background(), time.Now().Add(time.Minute)) - defer cancelFunc() - - err := cvoConfiguration.Start(ctx) - if err != nil { - t.Errorf("unexpected error %v", err) - } - - // Run tested functionality - err = cvoConfiguration.Sync(ctx, "ClusterVersionOperator/cluster") - if err != nil { - t.Errorf("unexpected error %v", err) - } - - // Verify results - config, err := client.OperatorV1alpha1().ClusterVersionOperators().Get(ctx, "cluster", metav1.GetOptions{}) - if err != nil && !apierrors.IsNotFound(err) { - t.Errorf("unexpected error %v", err) - } - - switch { - case apierrors.IsNotFound(err) && tt.expectedConfig != nil: - t.Errorf("expected config to be '%v', got NotFound", *tt.expectedConfig) - case err == nil: - if diff := cmp.Diff(*tt.expectedConfig, *config, cmpopts.IgnoreFields(metav1.ObjectMeta{}, "ManagedFields")); diff != "" { - t.Errorf("unexpected config (-want, +got) = %v", diff) - } - } - }) - } -}