Skip to content

Commit b41f2a4

Browse files
committed
pkg/cvo: Wait until ConfigMap informers have synced
Avoid startup-time "hey, I can't find those ConfigMaps!" complaints from pkg/risk/adminack by waiting until the informers have synchronized. The pkg/cvo Operator type has a cacheSynced that's populated in Operator.New (with the ClusterVersion, ClusterOperator, and FeatureGate informers). cacheSynced goes all the way back to 648d27c (metrics: Report a gauge for when cluster operator conditions change, 2019-01-31, #107). But when we added the initial ConfigMap informer as cmLister back in c147732 (metrics: add cluster_installer series, 2019-06-26, #213) we didn't add that informer to cacheSynced. And when I added cmManagedInformer in d18deee (pkg/cvo: Separate ConfigMap informer for openshift-config-managed, 2020-08-21, #441), I didn't add that informer to cacheSynced. None of these earlier commits explain why the ConfigMap informers were not included in cacheSynced, so I'm assuming that was oversight, and catching up now. There is a risk that the ConfigMap informers fail to synchronize quickly, but I'm having trouble imagining a situation where that would happen but the ClusterVersion and ClusterOperator informers wouldn't also fail to synchronize. And either way, we don't serve metrics while we wait on this cache synchronization, so the critical ClusterVersionOperatorDown alert will be firing within 10 minutes, and the responding system administrators can sort out whatever was giving the cache-sync logic trouble.
1 parent cba244e commit b41f2a4

2 files changed

Lines changed: 14 additions & 1 deletion

File tree

pkg/cvo/cvo.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,9 @@ func New(
296296

297297
optr.proxyLister = proxyInformer.Lister()
298298
optr.cmConfigLister = cmConfigInformer.Lister().ConfigMaps(internal.ConfigNamespace)
299+
optr.cacheSynced = append(optr.cacheSynced, cmConfigInformer.Informer().HasSynced)
299300
optr.cmConfigManagedLister = cmConfigManagedInformer.Lister().ConfigMaps(internal.ConfigManagedNamespace)
301+
optr.cacheSynced = append(optr.cacheSynced, cmConfigManagedInformer.Informer().HasSynced)
300302

301303
optr.featureGateLister = featureGateInformer.Lister()
302304
optr.cacheSynced = append(optr.cacheSynced, featureGateInformer.Informer().HasSynced)

pkg/risk/adminack/adminack.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,21 @@ type adminAck struct {
3737
currentVersion func() configv1.Release
3838
adminGatesLister listerscorev1.ConfigMapNamespaceLister
3939
adminAcksLister listerscorev1.ConfigMapNamespaceLister
40+
hasSynced func() bool
4041
lastSeen []configv1.ConditionalUpdateRisk
4142
}
4243

4344
// New returns a new update-risk source, tracking administrator acknowledgements.
4445
func New(name string, currentVersion func() configv1.Release, adminGatesInformer, adminAcksInformer informerscorev1.ConfigMapInformer, changeCallback func()) risk.Source {
4546
adminGatesLister := adminGatesInformer.Lister().ConfigMaps(internal.ConfigManagedNamespace)
4647
adminAcksLister := adminAcksInformer.Lister().ConfigMaps(internal.ConfigNamespace)
47-
source := &adminAck{name: name, currentVersion: currentVersion, adminGatesLister: adminGatesLister, adminAcksLister: adminAcksLister}
48+
source := &adminAck{
49+
name: name,
50+
currentVersion: currentVersion,
51+
adminGatesLister: adminGatesLister,
52+
adminAcksLister: adminAcksLister,
53+
hasSynced: func() bool { return adminGatesInformer.Informer().HasSynced() && adminAcksInformer.Informer().HasSynced() },
54+
}
4855
adminGatesInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
4956
AddFunc: func(_ interface{}) { source.eventHandler(changeCallback) },
5057
UpdateFunc: func(_, _ interface{}) { source.eventHandler(changeCallback) },
@@ -102,6 +109,10 @@ func (a *adminAck) risks() ([]configv1.ConditionalUpdateRisk, semver.Version, er
102109
}}, version, err
103110
}
104111

112+
if !a.hasSynced() {
113+
return nil, version, nil
114+
}
115+
105116
var gateCm *corev1.ConfigMap
106117
if gateCm, err = a.adminGatesLister.Get(internal.AdminGatesConfigMap); err != nil {
107118
return []configv1.ConditionalUpdateRisk{{

0 commit comments

Comments
 (0)