Skip to content

Commit f89bd0c

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 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 4e02efd commit f89bd0c

2 files changed

Lines changed: 11 additions & 20 deletions

File tree

pkg/cvo/cvo.go

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,6 @@ type Operator struct {
135135
cmConfigManagedLister listerscorev1.ConfigMapNamespaceLister
136136
proxyLister configlistersv1.ProxyLister
137137
featureGateLister configlistersv1.FeatureGateLister
138-
apiServerLister configlistersv1.APIServerLister
139138
cacheSynced []cache.InformerSynced
140139

141140
profileMgr *cvotls.ProfileManager
@@ -238,8 +237,7 @@ func New(
238237
proxyInformer configinformersv1.ProxyInformer,
239238
operatorInformerFactory operatorexternalversions.SharedInformerFactory,
240239
featureGateInformer configinformersv1.FeatureGateInformer,
241-
apiServerInformer configinformersv1.APIServerInformer,
242-
overrides *cvotls.Settings,
240+
tlsProfileMgr *cvotls.ProfileManager,
243241
client clientset.Interface,
244242
kubeClient kubernetes.Interface,
245243
operatorClient operatorclientset.Interface,
@@ -317,9 +315,6 @@ func New(
317315
optr.featureGateLister = featureGateInformer.Lister()
318316
optr.cacheSynced = append(optr.cacheSynced, featureGateInformer.Informer().HasSynced)
319317

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

@@ -373,11 +368,7 @@ func New(
373368
},
374369
)
375370

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
371+
optr.profileMgr = tlsProfileMgr
381372

382373
return optr, nil
383374
}

pkg/start/start.go

Lines changed: 9 additions & 9 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()
@@ -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,
665665
cb.ClientOrDie(o.Namespace),
666666
cvoKubeClient,
667667
operatorClient,

0 commit comments

Comments
 (0)