Skip to content

Commit a040ae4

Browse files
committed
Fix empty lister for API server
Follow up the issue from #1338 (comment): 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 6a8c607 (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 #1338 (comment). 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.
1 parent 3de459f commit a040ae4

2 files changed

Lines changed: 17 additions & 27 deletions

File tree

pkg/cvo/cvo.go

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ import (
6565
overridesrisk "github.com/openshift/cluster-version-operator/pkg/risk/overrides"
6666
updatingrisk "github.com/openshift/cluster-version-operator/pkg/risk/updating"
6767
upgradeablerisk "github.com/openshift/cluster-version-operator/pkg/risk/upgradeable"
68-
cvotls "github.com/openshift/cluster-version-operator/pkg/tls"
6968
)
7069

7170
const (
@@ -135,10 +134,9 @@ type Operator struct {
135134
cmConfigManagedLister listerscorev1.ConfigMapNamespaceLister
136135
proxyLister configlistersv1.ProxyLister
137136
featureGateLister configlistersv1.FeatureGateLister
138-
apiServerLister configlistersv1.APIServerLister
139137
cacheSynced []cache.InformerSynced
140138

141-
profileMgr *cvotls.ProfileManager
139+
applyTLSSettings func(config *tls.Config)
142140

143141
// queue tracks applying updates to a cluster.
144142
queue workqueue.TypedRateLimitingInterface[any]
@@ -238,8 +236,7 @@ func New(
238236
proxyInformer configinformersv1.ProxyInformer,
239237
operatorInformerFactory operatorexternalversions.SharedInformerFactory,
240238
featureGateInformer configinformersv1.FeatureGateInformer,
241-
apiServerInformer configinformersv1.APIServerInformer,
242-
overrides *cvotls.Settings,
239+
applyTLSSettings func(config *tls.Config),
243240
client clientset.Interface,
244241
kubeClient kubernetes.Interface,
245242
operatorClient operatorclientset.Interface,
@@ -290,6 +287,8 @@ func New(
290287
enabledManifestFeatureGates: startingEnabledManifestFeatureGates,
291288

292289
alwaysEnableCapabilities: alwaysEnableCapabilities,
290+
291+
applyTLSSettings: applyTLSSettings,
293292
}
294293

295294
if _, err := cvInformer.Informer().AddEventHandler(optr.clusterVersionEventHandler()); err != nil {
@@ -317,9 +316,6 @@ func New(
317316
optr.featureGateLister = featureGateInformer.Lister()
318317
optr.cacheSynced = append(optr.cacheSynced, featureGateInformer.Informer().HasSynced)
319318

320-
optr.apiServerLister = apiServerInformer.Lister()
321-
optr.cacheSynced = append(optr.cacheSynced, apiServerInformer.Informer().HasSynced)
322-
323319
// make sure this is initialized after all the listers are initialized
324320
riskSourceCallback := func() { optr.availableUpdatesQueue.Add(optr.queueKey()) }
325321

@@ -373,12 +369,6 @@ func New(
373369
},
374370
)
375371

376-
profileMgr, err := cvotls.NewProfileManager(apiServerInformer, overrides)
377-
if err != nil {
378-
return nil, fmt.Errorf("failed to initialize TLS profile manager: %w", err)
379-
}
380-
optr.profileMgr = profileMgr
381-
382372
return optr, nil
383373
}
384374

@@ -1234,7 +1224,7 @@ func (optr *Operator) shouldEnableProposalController() bool {
12341224
return optr.requiredFeatureSet == configv1.TechPreviewNoUpgrade
12351225
}
12361226

1237-
// ApplySettings returns the ApplySettings function of the TLS profile manager
1238-
func (optr *Operator) ApplySettings() func(config *tls.Config) {
1239-
return optr.profileMgr.ApplySettings
1227+
// ApplyTLSSettings returns the function that applies TLS settings to the TLS config
1228+
func (optr *Operator) ApplyTLSSettings() func(config *tls.Config) {
1229+
return optr.applyTLSSettings
12401230
}

pkg/start/start.go

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,9 @@ func (o *Options) Run(ctx context.Context) error {
207207
}
208208

209209
clusterVersionConfigInformerFactory, configInformerFactory := o.prepareConfigInformerFactories(cb)
210+
// This is to ensure that APIServers get loaded when configInformerFactory is started and synced in o.processInitialFeatureGate().
211+
// It is important when creating TLS profile manager later.
212+
configInformerFactory.Config().V1().APIServers().Lister()
210213
startingFeatureSet, startingCvoGates, startingEnabledManifestFeatureGates, err := o.processInitialFeatureGate(ctx, configInformerFactory)
211214
if err != nil {
212215
return fmt.Errorf("error processing feature gates: %w", err)
@@ -357,13 +360,6 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock resource
357360
}
358361
}
359362

360-
configSynced := controllerCtx.ConfigInformerFactory.WaitForCacheSync(informersDone)
361-
for _, synced := range configSynced {
362-
if !synced {
363-
klog.Fatalf("Caches never synchronized: %v", postMainContext.Err())
364-
}
365-
}
366-
367363
resultChannelCount++
368364
go func() {
369365
defer utilruntime.HandleCrash()
@@ -381,7 +377,7 @@ func (o *Options) run(ctx context.Context, controllerCtx *Context, lock resource
381377
resultChannelCount++
382378
go func() {
383379
defer utilruntime.HandleCrash()
384-
err := cvo.RunMetrics(postMainContext, shutdownContext, restConfig, controllerCtx.CVO.ApplySettings(), o.MetricsOptions)
380+
err := cvo.RunMetrics(postMainContext, shutdownContext, restConfig, controllerCtx.CVO.ApplyTLSSettings(), o.MetricsOptions)
385381
resultChannel <- asyncResult{name: "metrics server", error: err}
386382
}()
387383
}
@@ -647,6 +643,11 @@ func (o *Options) NewControllerContext(
647643
}
648644
rtClient := cb.RuntimeControllerClientOrDie("runtime-controller-client")
649645

646+
tlsProfileMgr, err := tls.NewProfileManager(configInformerFactory.Config().V1().APIServers(), o.TLSOptions.GetOverrides())
647+
if err != nil {
648+
return nil, fmt.Errorf("failed to initialize TLS profile manager: %w", err)
649+
}
650+
650651
cvo, err := cvo.New(
651652
o.NodeName,
652653
o.Namespace, o.Name,
@@ -660,8 +661,7 @@ func (o *Options) NewControllerContext(
660661
configInformerFactory.Config().V1().Proxies(),
661662
operatorInformerFactory,
662663
configInformerFactory.Config().V1().FeatureGates(),
663-
configInformerFactory.Config().V1().APIServers(),
664-
o.TLSOptions.GetOverrides(),
664+
tlsProfileMgr.ApplySettings,
665665
cb.ClientOrDie(o.Namespace),
666666
cvoKubeClient,
667667
operatorClient,

0 commit comments

Comments
 (0)