From 5b5db7db3df8c062b537111989c90d2b58178620 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Thu, 26 Aug 2021 17:57:11 -0700 Subject: [PATCH] pkg/cvo/upgradeable: Enable admin-ack logic 519b466793 (Bug 1978376: Add admin ack Upgradeable condition gate, 2021-07-27, #633) had these commented out, because 4.9 has no built-in acks. But with the code commented out, it's hard to verify that the logic works in 4.9 before backporting to 4.8 [1]. Enabling these checks should be a no-op outside of verification, because admins are unlikely to inject additional keys in the openshift-config-managed namespace's admin-gates ConfigMap. And it allows us to verify the logic in 4.9 and cook there with live code before approving the 4.8 backports. It's also one less thing we might forget before enabling new admin acks in future versions, like 4.10 or later. [1]: https://bugzilla.redhat.com/show_bug.cgi?id=1978376#c19 --- pkg/cvo/cvo_test.go | 6 ------ pkg/cvo/upgradeable.go | 17 +++++++++-------- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/pkg/cvo/cvo_test.go b/pkg/cvo/cvo_test.go index bacb2ceac3..f175188424 100644 --- a/pkg/cvo/cvo_test.go +++ b/pkg/cvo/cvo_test.go @@ -3756,12 +3756,6 @@ func TestOperator_upgradeableSync(t *testing.T) { optr.cmConfigLister = cmInformerLister.Lister().ConfigMaps("test") optr.upgradeableChecks = optr.defaultUpgradeableChecks() - - // This Upgradeable check must be added here since it is not active in 4.9 but present to allow - // back port to 4.8 where it will first become active. - optr.upgradeableChecks = append(optr.upgradeableChecks, - &clusterAdminAcksCompletedUpgradeable{optr.cmConfigManagedLister, optr.cmConfigLister, optr.cvLister, optr.name}) - optr.eventRecorder = record.NewFakeRecorder(100) if tt.gateCm != nil { diff --git a/pkg/cvo/upgradeable.go b/pkg/cvo/upgradeable.go index 9c893f32e2..6b46261e6d 100644 --- a/pkg/cvo/upgradeable.go +++ b/pkg/cvo/upgradeable.go @@ -380,10 +380,14 @@ func (check *clusterAdminAcksCompletedUpgradeable) Check() *configv1.ClusterOper return nil } -// Since there are no admin ack gates in this initial release the Upgradeable check -// clusterAdminAcksCompletedUpgradeable is not included. func (optr *Operator) defaultUpgradeableChecks() []upgradeableCheck { return []upgradeableCheck{ + &clusterAdminAcksCompletedUpgradeable{ + adminGatesLister: optr.cmConfigManagedLister, + adminAcksLister: optr.cmConfigLister, + cvLister: optr.cvLister, + cvoName: optr.name, + }, &clusterOperatorsUpgradeable{coLister: optr.coLister}, &clusterVersionOverridesUpgradeable{name: optr.name, cvLister: optr.cvLister}, &clusterManifestDeleteInProgressUpgradeable{}, @@ -394,8 +398,7 @@ func (optr *Operator) addFunc(obj interface{}) { cm := obj.(*corev1.ConfigMap) if cm.Name == internal.AdminGatesConfigMap || cm.Name == internal.AdminAcksConfigMap { klog.V(4).Infof("ConfigMap %s/%s added.", cm.Namespace, cm.Name) - // When clusterAdminAcksCompletedUpgradeable upgardeable check added we will call - // optr.setUpgradeableConditions() here + optr.setUpgradeableConditions() } } @@ -405,8 +408,7 @@ func (optr *Operator) updateFunc(oldObj, newObj interface{}) { oldCm := oldObj.(*corev1.ConfigMap) if !equality.Semantic.DeepEqual(cm, oldCm) { klog.V(4).Infof("ConfigMap %s/%s updated.", cm.Namespace, cm.Name) - // When clusterAdminAcksCompletedUpgradeable upgardeable check added we will call - // optr.setUpgradeableConditions() here + optr.setUpgradeableConditions() } } } @@ -415,8 +417,7 @@ func (optr *Operator) deleteFunc(obj interface{}) { cm := obj.(*corev1.ConfigMap) if cm.Name == internal.AdminGatesConfigMap || cm.Name == internal.AdminAcksConfigMap { klog.V(4).Infof("ConfigMap %s/%s deleted.", cm.Namespace, cm.Name) - // When clusterAdminAcksCompletedUpgradeable upgardeable check added we will call - // optr.setUpgradeableConditions() here + optr.setUpgradeableConditions() } }