From 3de459fb797a9067681a0686443a306d4d8368f3 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Mon, 1 Jun 2026 15:42:38 -0400 Subject: [PATCH 1/2] Revert "tls: Defer profile manager initialization until informer sync" This reverts commit 97a5bd23f73a649f1ca8d0364eca57318f3ed274. --- pkg/cvo/cvo.go | 21 +++++---------------- pkg/start/start.go | 5 ----- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/pkg/cvo/cvo.go b/pkg/cvo/cvo.go index 4a8aee411..74bbe180e 100644 --- a/pkg/cvo/cvo.go +++ b/pkg/cvo/cvo.go @@ -138,9 +138,7 @@ type Operator struct { apiServerLister configlistersv1.APIServerLister cacheSynced []cache.InformerSynced - apiServerInformer configinformersv1.APIServerInformer - tlsOverrides *cvotls.Settings - profileMgr *cvotls.ProfileManager + profileMgr *cvotls.ProfileManager // queue tracks applying updates to a cluster. queue workqueue.TypedRateLimitingInterface[any] @@ -322,10 +320,6 @@ func New( optr.apiServerLister = apiServerInformer.Lister() optr.cacheSynced = append(optr.cacheSynced, apiServerInformer.Informer().HasSynced) - // Store for deferred TLS profile manager initialization (after informer sync) - optr.apiServerInformer = apiServerInformer - optr.tlsOverrides = overrides - // make sure this is initialized after all the listers are initialized riskSourceCallback := func() { optr.availableUpdatesQueue.Add(optr.queueKey()) } @@ -379,18 +373,13 @@ func New( }, ) - return optr, nil -} - -// InitializeProfileManager initializes the TLS profile manager. -// Must be called after informers are started and synced. -func (optr *Operator) InitializeProfileManager() error { - profileMgr, err := cvotls.NewProfileManager(optr.apiServerInformer, optr.tlsOverrides) + profileMgr, err := cvotls.NewProfileManager(apiServerInformer, overrides) if err != nil { - return fmt.Errorf("failed to initialize TLS profile manager: %w", err) + return nil, fmt.Errorf("failed to initialize TLS profile manager: %w", err) } optr.profileMgr = profileMgr - return nil + + return optr, nil } // LoadInitialPayload waits until a ClusterVersion object exists. It then retrieves the payload contents, verifies the diff --git a/pkg/start/start.go b/pkg/start/start.go index b1a6291e3..b3e766b6f 100644 --- a/pkg/start/start.go +++ b/pkg/start/start.go @@ -364,11 +364,6 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock resource } } - // Initialize TLS profile manager after informers are synced - if err := controllerCtx.CVO.InitializeProfileManager(); err != nil { - klog.Fatalf("Failed to initialize TLS profile manager: %v", err) - } - resultChannelCount++ go func() { defer utilruntime.HandleCrash() From a040ae49acf0ad64dc925c967e0641ec041f6174 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Mon, 1 Jun 2026 16:03:43 -0400 Subject: [PATCH 2/2] Fix empty lister for API server Follow up the issue from https://github.com/openshift/cluster-version-operator/pull/1338#issuecomment-4593792803: The right way to call things out for `InformerFactory`: e.g., [processInitialFeatureGate](https://github.com/openshift/cluster-version-operator/blob/c1dc3e8f0b7c366d45177af391c4f888059b446d/pkg/start/start.go#L269-L280), * `Lister()` such as `configInformerFactory.Config().V1().FeatureGates().Lister()` * `Start()` such as `configInformerFactory.Start(ctx.Done())` * `WaitForCacheSync*()` such as `configInformerFactory.WaitForCacheSync(ctx.Done())` The lister is then ready to retrieve instance, e.g., via `Get()`. ```console $ git --no-pager log --pretty=oneline -1 6a8c6072b2f4e45fe5ab9269b4efa96d4608b0d0 (HEAD -> pr1338-e2e-6a8c6072b2f4e45fe5ab9269b4efa96d4608b0d0) make update ``` The issue with the commit above: ``` start.go: Line 190 func (o *Options) Run(ctx context.Context) error * Line 216 o.NewControllerContext() * Line 650 cvo, err := cvo.New() where Lister() and cvotls.NewProfileManager(apiServerInformer, tlsOverrides) are called where apiServerInformer.Lister().Get(tlsprofile.APIServerName) is called * Line 221 o.run() * start and sync ``` The issue is that `Get()` is called in the wrong order: `Lister()`, `Get()`, `Start()` and `Sync()`. It explains `apiserver.config.openshift.io "cluster" not found` from https://github.com/openshift/cluster-version-operator/pull/1338#issuecomment-4593792803. In short, we need the informer factory is ready before initializing the TLS profile manager. The fix is moving `NewProfileManager()` from `cvo.go` to `start.go` and the instance passes to `cvo.go`. Note that the same `configInformerFactory` instance is started and synced in `o.processInitialFeatureGate`. The only thing missing is invoking its `List()` ahead of it. Once it is done, we do not need to do those again in `cvo.go`. I prefer the current fix over the reverted one because the code is simpler this way. `cvo` does not need to have the fields such as the informer and the overrides that actually belong to the TLS profile manager. --- pkg/cvo/cvo.go | 24 +++++++----------------- pkg/start/start.go | 20 ++++++++++---------- 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/pkg/cvo/cvo.go b/pkg/cvo/cvo.go index 74bbe180e..7e4c2dec2 100644 --- a/pkg/cvo/cvo.go +++ b/pkg/cvo/cvo.go @@ -65,7 +65,6 @@ import ( overridesrisk "github.com/openshift/cluster-version-operator/pkg/risk/overrides" updatingrisk "github.com/openshift/cluster-version-operator/pkg/risk/updating" upgradeablerisk "github.com/openshift/cluster-version-operator/pkg/risk/upgradeable" - cvotls "github.com/openshift/cluster-version-operator/pkg/tls" ) const ( @@ -135,10 +134,9 @@ type Operator struct { cmConfigManagedLister listerscorev1.ConfigMapNamespaceLister proxyLister configlistersv1.ProxyLister featureGateLister configlistersv1.FeatureGateLister - apiServerLister configlistersv1.APIServerLister cacheSynced []cache.InformerSynced - profileMgr *cvotls.ProfileManager + applyTLSSettings func(config *tls.Config) // queue tracks applying updates to a cluster. queue workqueue.TypedRateLimitingInterface[any] @@ -238,8 +236,7 @@ func New( proxyInformer configinformersv1.ProxyInformer, operatorInformerFactory operatorexternalversions.SharedInformerFactory, featureGateInformer configinformersv1.FeatureGateInformer, - apiServerInformer configinformersv1.APIServerInformer, - overrides *cvotls.Settings, + applyTLSSettings func(config *tls.Config), client clientset.Interface, kubeClient kubernetes.Interface, operatorClient operatorclientset.Interface, @@ -290,6 +287,8 @@ func New( enabledManifestFeatureGates: startingEnabledManifestFeatureGates, alwaysEnableCapabilities: alwaysEnableCapabilities, + + applyTLSSettings: applyTLSSettings, } if _, err := cvInformer.Informer().AddEventHandler(optr.clusterVersionEventHandler()); err != nil { @@ -317,9 +316,6 @@ func New( optr.featureGateLister = featureGateInformer.Lister() optr.cacheSynced = append(optr.cacheSynced, featureGateInformer.Informer().HasSynced) - optr.apiServerLister = apiServerInformer.Lister() - optr.cacheSynced = append(optr.cacheSynced, apiServerInformer.Informer().HasSynced) - // make sure this is initialized after all the listers are initialized riskSourceCallback := func() { optr.availableUpdatesQueue.Add(optr.queueKey()) } @@ -373,12 +369,6 @@ func New( }, ) - profileMgr, err := cvotls.NewProfileManager(apiServerInformer, overrides) - if err != nil { - return nil, fmt.Errorf("failed to initialize TLS profile manager: %w", err) - } - optr.profileMgr = profileMgr - return optr, nil } @@ -1234,7 +1224,7 @@ func (optr *Operator) shouldEnableProposalController() bool { return optr.requiredFeatureSet == configv1.TechPreviewNoUpgrade } -// ApplySettings returns the ApplySettings function of the TLS profile manager -func (optr *Operator) ApplySettings() func(config *tls.Config) { - return optr.profileMgr.ApplySettings +// ApplyTLSSettings returns the function that applies TLS settings to the TLS config +func (optr *Operator) ApplyTLSSettings() func(config *tls.Config) { + return optr.applyTLSSettings } diff --git a/pkg/start/start.go b/pkg/start/start.go index b3e766b6f..bee0115b1 100644 --- a/pkg/start/start.go +++ b/pkg/start/start.go @@ -207,6 +207,9 @@ func (o *Options) Run(ctx context.Context) error { } clusterVersionConfigInformerFactory, configInformerFactory := o.prepareConfigInformerFactories(cb) + // This is to ensure that APIServers get loaded when configInformerFactory is started and synced in o.processInitialFeatureGate(). + // It is important when creating TLS profile manager later. + configInformerFactory.Config().V1().APIServers().Lister() startingFeatureSet, startingCvoGates, startingEnabledManifestFeatureGates, err := o.processInitialFeatureGate(ctx, configInformerFactory) if err != nil { return fmt.Errorf("error processing feature gates: %w", err) @@ -357,13 +360,6 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock resource } } - configSynced := controllerCtx.ConfigInformerFactory.WaitForCacheSync(informersDone) - for _, synced := range configSynced { - if !synced { - klog.Fatalf("Caches never synchronized: %v", postMainContext.Err()) - } - } - resultChannelCount++ go func() { defer utilruntime.HandleCrash() @@ -381,7 +377,7 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock resource resultChannelCount++ go func() { defer utilruntime.HandleCrash() - err := cvo.RunMetrics(postMainContext, shutdownContext, restConfig, controllerCtx.CVO.ApplySettings(), o.MetricsOptions) + err := cvo.RunMetrics(postMainContext, shutdownContext, restConfig, controllerCtx.CVO.ApplyTLSSettings(), o.MetricsOptions) resultChannel <- asyncResult{name: "metrics server", error: err} }() } @@ -647,6 +643,11 @@ func (o *Options) NewControllerContext( } rtClient := cb.RuntimeControllerClientOrDie("runtime-controller-client") + tlsProfileMgr, err := tls.NewProfileManager(configInformerFactory.Config().V1().APIServers(), o.TLSOptions.GetOverrides()) + if err != nil { + return nil, fmt.Errorf("failed to initialize TLS profile manager: %w", err) + } + cvo, err := cvo.New( o.NodeName, o.Namespace, o.Name, @@ -660,8 +661,7 @@ func (o *Options) NewControllerContext( configInformerFactory.Config().V1().Proxies(), operatorInformerFactory, configInformerFactory.Config().V1().FeatureGates(), - configInformerFactory.Config().V1().APIServers(), - o.TLSOptions.GetOverrides(), + tlsProfileMgr.ApplySettings, cb.ClientOrDie(o.Namespace), cvoKubeClient, operatorClient,