Commit 9bfbc57
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 3de459f commit 9bfbc57
2 files changed
Lines changed: 11 additions & 20 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
135 | 135 | | |
136 | 136 | | |
137 | 137 | | |
138 | | - | |
139 | 138 | | |
140 | 139 | | |
141 | 140 | | |
| |||
238 | 237 | | |
239 | 238 | | |
240 | 239 | | |
241 | | - | |
242 | | - | |
| 240 | + | |
243 | 241 | | |
244 | 242 | | |
245 | 243 | | |
| |||
317 | 315 | | |
318 | 316 | | |
319 | 317 | | |
320 | | - | |
321 | | - | |
322 | | - | |
323 | 318 | | |
324 | 319 | | |
325 | 320 | | |
| |||
373 | 368 | | |
374 | 369 | | |
375 | 370 | | |
376 | | - | |
377 | | - | |
378 | | - | |
379 | | - | |
380 | | - | |
| 371 | + | |
381 | 372 | | |
382 | 373 | | |
383 | 374 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
207 | 207 | | |
208 | 208 | | |
209 | 209 | | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
210 | 213 | | |
211 | 214 | | |
212 | 215 | | |
| |||
357 | 360 | | |
358 | 361 | | |
359 | 362 | | |
360 | | - | |
361 | | - | |
362 | | - | |
363 | | - | |
364 | | - | |
365 | | - | |
366 | | - | |
367 | 363 | | |
368 | 364 | | |
369 | 365 | | |
| |||
647 | 643 | | |
648 | 644 | | |
649 | 645 | | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
650 | 651 | | |
651 | 652 | | |
652 | 653 | | |
| |||
660 | 661 | | |
661 | 662 | | |
662 | 663 | | |
663 | | - | |
664 | | - | |
| 664 | + | |
665 | 665 | | |
666 | 666 | | |
667 | 667 | | |
| |||
0 commit comments