diff --git a/install/0000_00_cluster-version-operator_01_adminack_configmap.yaml b/install/0000_00_cluster-version-operator_01_adminack_configmap.yaml new file mode 100644 index 000000000..b77c9676e --- /dev/null +++ b/install/0000_00_cluster-version-operator_01_adminack_configmap.yaml @@ -0,0 +1,10 @@ +apiVersion: v1 +kind: ConfigMap +metadata: + name: admin-acks + namespace: openshift-config + annotations: + include.release.openshift.io/ibm-cloud-managed: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" + release.openshift.io/create-only: "true" diff --git a/install/0000_00_cluster-version-operator_01_admingate_configmap.yaml b/install/0000_00_cluster-version-operator_01_admingate_configmap.yaml new file mode 100644 index 000000000..f86c5198c --- /dev/null +++ b/install/0000_00_cluster-version-operator_01_admingate_configmap.yaml @@ -0,0 +1,13 @@ +apiVersion: v1 +kind: ConfigMap +data: + ack-4.8-kube-1.22-api-removals-in-4.9: | + Kubernetes 1.22 and therefore OpenShift 4.9 remove several APIs which require admin consideration. Please see + the knowledge article https://access.redhat.com/articles/6329921 for details and instructions. +metadata: + name: admin-gates + namespace: openshift-config-managed + annotations: + include.release.openshift.io/ibm-cloud-managed: "true" + include.release.openshift.io/self-managed-high-availability: "true" + include.release.openshift.io/single-node-developer: "true" diff --git a/pkg/cvo/cvo.go b/pkg/cvo/cvo.go index d572bef8c..7ac116602 100644 --- a/pkg/cvo/cvo.go +++ b/pkg/cvo/cvo.go @@ -202,6 +202,8 @@ func New( } cvInformer.Informer().AddEventHandler(optr.eventHandler()) + cmConfigInformer.Informer().AddEventHandler(optr.adminAcksEventHandler()) + cmConfigManagedInformer.Informer().AddEventHandler(optr.adminGatesEventHandler()) optr.coLister = coInformer.Lister() optr.cacheSynced = append(optr.cacheSynced, coInformer.Informer().HasSynced) @@ -587,7 +589,7 @@ func (optr *Operator) upgradeableSync(ctx context.Context, key string) error { return nil } - return optr.syncUpgradeable(config) + return optr.syncUpgradeable() } // isOlderThanLastUpdate returns true if the cluster version is older than diff --git a/pkg/cvo/cvo_test.go b/pkg/cvo/cvo_test.go index 7e8e31b96..2c23b0441 100644 --- a/pkg/cvo/cvo_test.go +++ b/pkg/cvo/cvo_test.go @@ -26,11 +26,15 @@ import ( "k8s.io/apimachinery/pkg/runtime/schema" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/diff" + "k8s.io/apimachinery/pkg/util/wait" "k8s.io/apimachinery/pkg/watch" "k8s.io/client-go/discovery" + "k8s.io/client-go/informers" kfake "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/rest" + clienttesting "k8s.io/client-go/testing" ktesting "k8s.io/client-go/testing" + "k8s.io/client-go/tools/cache" "k8s.io/client-go/tools/record" "k8s.io/client-go/util/workqueue" "k8s.io/klog/v2" @@ -2792,19 +2796,40 @@ func TestOperator_availableUpdatesSync(t *testing.T) { } } +// waits for admin ack configmap changes +func waitForCm(t *testing.T, cmChan chan *corev1.ConfigMap) { + select { + case cm := <-cmChan: + t.Logf("Got configmap from channel: %s/%s", cm.Namespace, cm.Name) + case <-time.After(wait.ForeverTestTimeout): + t.Error("Informer did not get the configmap") + } +} + func TestOperator_upgradeableSync(t *testing.T) { id := uuid.Must(uuid.NewRandom()).String() + var defaultGateCm = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + } + var defaultAckCm = corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-acks", + Namespace: "test"}, + } tests := []struct { - name string - key string - optr Operator - wantErr func(*testing.T, error) - want *upgradeable + name string + key string + optr *Operator + cm corev1.ConfigMap + gateCm *corev1.ConfigMap + ackCm *corev1.ConfigMap + wantErr func(*testing.T, error) + expectedResult *upgradeable }{ { name: "when version is missing, do nothing (other loops should create it)", - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Version: "4.0.1", Image: "image/image:v4.0.1", @@ -2813,10 +2838,14 @@ func TestOperator_upgradeableSync(t *testing.T) { name: "default", client: fake.NewSimpleClientset(), }, + cm: corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + }, }, { name: "report error condition when overrides is set for version", - optr: Operator{ + optr: &Operator{ release: configv1.Release{ Image: "image/image:v4.0.1", }, @@ -2842,7 +2871,11 @@ func TestOperator_upgradeableSync(t *testing.T) { }, ), }, - want: &upgradeable{ + cm: corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + }, + expectedResult: &upgradeable{ Conditions: []configv1.ClusterOperatorStatusCondition{{ Type: configv1.OperatorUpgradeable, Status: configv1.ConditionFalse, @@ -2853,7 +2886,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "report error condition when the single clusteroperator is not upgradeable", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -2891,7 +2924,11 @@ func TestOperator_upgradeableSync(t *testing.T) { }, ), }, - want: &upgradeable{ + cm: corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + }, + expectedResult: &upgradeable{ Conditions: []configv1.ClusterOperatorStatusCondition{{ Type: configv1.OperatorUpgradeable, Status: configv1.ConditionFalse, @@ -2902,7 +2939,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "report error condition when single clusteroperator is not upgradeable and another has no conditions", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -2948,7 +2985,11 @@ func TestOperator_upgradeableSync(t *testing.T) { }, ), }, - want: &upgradeable{ + cm: corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + }, + expectedResult: &upgradeable{ Conditions: []configv1.ClusterOperatorStatusCondition{{ Type: configv1.OperatorUpgradeable, Status: configv1.ConditionFalse, @@ -2959,7 +3000,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "report error condition when single clusteroperator is not upgradeable and another is upgradeable", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -3008,7 +3049,11 @@ func TestOperator_upgradeableSync(t *testing.T) { }, ), }, - want: &upgradeable{ + cm: corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + }, + expectedResult: &upgradeable{ Conditions: []configv1.ClusterOperatorStatusCondition{{ Type: configv1.OperatorUpgradeable, Status: configv1.ConditionFalse, @@ -3019,7 +3064,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "report error condition when two clusteroperators are not upgradeable", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -3070,7 +3115,11 @@ func TestOperator_upgradeableSync(t *testing.T) { }, ), }, - want: &upgradeable{ + cm: corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + }, + expectedResult: &upgradeable{ Conditions: []configv1.ClusterOperatorStatusCondition{{ Type: configv1.OperatorUpgradeable, Status: configv1.ConditionFalse, @@ -3081,7 +3130,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "report error condition when clusteroperators and version are not upgradeable", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -3135,7 +3184,11 @@ func TestOperator_upgradeableSync(t *testing.T) { }, ), }, - want: &upgradeable{ + cm: corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + }, + expectedResult: &upgradeable{ Conditions: []configv1.ClusterOperatorStatusCondition{{ Type: configv1.OperatorUpgradeable, Status: configv1.ConditionFalse, @@ -3156,7 +3209,7 @@ func TestOperator_upgradeableSync(t *testing.T) { }, { name: "no error conditions", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -3182,11 +3235,15 @@ func TestOperator_upgradeableSync(t *testing.T) { }, ), }, - want: &upgradeable{}, + cm: corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + }, + expectedResult: &upgradeable{}, }, { name: "no error conditions", - optr: Operator{ + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ Version: "v4.0.0", @@ -3214,15 +3271,19 @@ func TestOperator_upgradeableSync(t *testing.T) { }, ), }, - want: &upgradeable{}, + cm: corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + }, + expectedResult: &upgradeable{}, }, { - name: "no error conditions", - optr: Operator{ + name: "no error conditions and admin ack gate does not apply to version", + optr: &Operator{ defaultUpstreamServer: "http://localhost:8080/graph", release: configv1.Release{ - Version: "v4.0.0", - Image: "image/image:v4.0.1", + Version: "v4.9.0", + Image: "image/image:v4.9.1", }, namespace: "test", name: "default", @@ -3240,7 +3301,8 @@ func TestOperator_upgradeableSync(t *testing.T) { }, Status: configv1.ClusterVersionStatus{ History: []configv1.UpdateHistory{ - {Image: "image/image:v4.0.1"}, + {Version: "4.9.0"}, + {Image: "image/image:v4.9.1"}, }, }, }, @@ -3257,42 +3319,691 @@ func TestOperator_upgradeableSync(t *testing.T) { }, ), }, - want: &upgradeable{}, + cm: corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + }, + expectedResult: &upgradeable{}, + }, + { + name: "admin-gates configmap gate does not have value", + optr: &Operator{ + defaultUpstreamServer: "http://localhost:8080/graph", + release: configv1.Release{ + Version: "v4.8.0", + Image: "image/image:v4.8.1", + }, + namespace: "test", + name: "default", + client: fake.NewSimpleClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(id), + Channel: "", + Overrides: []configv1.ComponentOverride{{ + Unmanaged: false, + }}, + }, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + {Version: "4.8.0"}, + }, + }, + }, + ), + }, + gateCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": ""}, + }, + ackCm: &defaultAckCm, + expectedResult: &upgradeable{ + Conditions: []configv1.ClusterOperatorStatusCondition{{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionFalse, + Reason: "AdminAckConfigMapGateValueError", + Message: "admin-gates configmap gate ack-4.8-kube-122-api-removals-in-4.9 must contain a non-empty value."}}, + }, + }, + { + name: "admin ack required", + optr: &Operator{ + defaultUpstreamServer: "http://localhost:8080/graph", + release: configv1.Release{ + Version: "v4.8.0", + Image: "image/image:v4.8.1", + }, + namespace: "test", + name: "default", + client: fake.NewSimpleClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(id), + Channel: "", + Overrides: []configv1.ComponentOverride{{ + Unmanaged: false, + }}, + }, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + {Version: "4.8.0"}, + }, + }, + }, + ), + }, + gateCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "Description."}, + }, + ackCm: &defaultAckCm, + expectedResult: &upgradeable{ + Conditions: []configv1.ClusterOperatorStatusCondition{{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionFalse, + Reason: "AdminAckRequired", + Message: "Description."}}, + }, + }, + { + name: "admin ack required and admin ack gate does not apply to version", + optr: &Operator{ + defaultUpstreamServer: "http://localhost:8080/graph", + release: configv1.Release{ + Version: "v4.8.0", + Image: "image/image:v4.8.1", + }, + namespace: "test", + name: "default", + client: fake.NewSimpleClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(id), + Channel: "", + Overrides: []configv1.ComponentOverride{{ + Unmanaged: false, + }}, + }, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + {Version: "4.8.0"}, + }, + }, + }, + ), + }, + gateCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "Description.", + "ack-4.9-kube-122-api-removals-in-4.9": "Description 2."}, + }, + ackCm: &defaultAckCm, expectedResult: &upgradeable{ + Conditions: []configv1.ClusterOperatorStatusCondition{{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionFalse, + Reason: "AdminAckRequired", + Message: "Description."}}, + }, + }, + { + name: "admin ack required and configmap gate does not have value", + optr: &Operator{ + defaultUpstreamServer: "http://localhost:8080/graph", + release: configv1.Release{ + Version: "v4.8.0", + Image: "image/image:v4.8.1", + }, + namespace: "test", + name: "default", + client: fake.NewSimpleClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(id), + Channel: "", + Overrides: []configv1.ComponentOverride{{ + Unmanaged: false, + }}, + }, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + {Version: "4.8.0"}, + }, + }, + }, + ), + }, + gateCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "Description.", + "ack-4.8-foo": ""}, + }, + ackCm: &defaultAckCm, expectedResult: &upgradeable{ + Conditions: []configv1.ClusterOperatorStatusCondition{{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionFalse, + Reason: "MultipleReasons", + Message: "Description. admin-gates configmap gate ack-4.8-foo must contain a non-empty value."}}, + }, + }, + { + name: "multiple admin acks required", + optr: &Operator{ + defaultUpstreamServer: "http://localhost:8080/graph", + release: configv1.Release{ + Version: "v4.8.0", + Image: "image/image:v4.8.1", + }, + namespace: "test", + name: "default", + client: fake.NewSimpleClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(id), + Channel: "", + Overrides: []configv1.ComponentOverride{{ + Unmanaged: false, + }}, + }, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + {Version: "4.8.0"}, + }, + }, + }, + ), + }, + gateCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "Description 2.", + "ack-4.8-foo": "Description 1.", + "ack-4.8-bar": "Description 3."}, + }, + ackCm: &defaultAckCm, expectedResult: &upgradeable{ + Conditions: []configv1.ClusterOperatorStatusCondition{{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionFalse, + Reason: "AdminAckRequired", + Message: "Description 1. Description 2. Description 3."}}, + }, + }, + { + name: "admin ack found", + optr: &Operator{ + defaultUpstreamServer: "http://localhost:8080/graph", + release: configv1.Release{ + Version: "v4.8.0", + Image: "image/image:v4.8.1", + }, + namespace: "test", + name: "default", + client: fake.NewSimpleClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(id), + Channel: "", + Overrides: []configv1.ComponentOverride{{ + Unmanaged: false, + }}, + }, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + {Version: "4.8.0"}, + }, + }, + }, + ), + }, + gateCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "Description."}, + }, + ackCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-acks", + Namespace: "test"}, Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "true"}, + }, + expectedResult: &upgradeable{}, + }, + { + name: "admin ack 2 of 3 found", + optr: &Operator{ + defaultUpstreamServer: "http://localhost:8080/graph", + release: configv1.Release{ + Version: "v4.8.0", + Image: "image/image:v4.8.1", + }, + namespace: "test", + name: "default", + client: fake.NewSimpleClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(id), + Channel: "", + Overrides: []configv1.ComponentOverride{{ + Unmanaged: false, + }}, + }, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + {Version: "4.8.0"}, + }, + }, + }, + ), + }, + gateCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "Description.", + "ack-4.8-foo": "Description foo.", + "ack-4.8-bar": "Description bar."}, + }, + ackCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-acks", + Namespace: "test"}, + Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "true", + "ack-4.8-bar": "true"}}, + expectedResult: &upgradeable{ + Conditions: []configv1.ClusterOperatorStatusCondition{{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionFalse, + Reason: "AdminAckRequired", + Message: "Description foo."}}, + }, }, + { + name: "multiple admin acks found", + optr: &Operator{ + defaultUpstreamServer: "http://localhost:8080/graph", + release: configv1.Release{ + Version: "v4.8.0", + Image: "image/image:v4.8.1", + }, + namespace: "test", + name: "default", + client: fake.NewSimpleClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(id), + Channel: "", + Overrides: []configv1.ComponentOverride{{ + Unmanaged: false, + }}, + }, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + {Version: "4.8.0"}, + }, + }, + }, + ), + }, + gateCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "Description.", + "ack-4.8-foo": "Description foo.", + "ack-4.8-bar": "Description bar."}, + }, + ackCm: &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: "admin-acks", + Namespace: "test"}, + Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "true", + "ack-4.8-bar": "true", + "ack-4.8-foo": "true"}}, + expectedResult: &upgradeable{}, + }, + // delete tests are last so we don't have to re-create the config map for other tests + { + name: "admin-acks configmap not found", + optr: &Operator{ + defaultUpstreamServer: "http://localhost:8080/graph", + release: configv1.Release{ + Version: "v4.8.0", + Image: "image/image:v4.8.1", + }, + namespace: "test", + name: "default", + client: fake.NewSimpleClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(id), + Channel: "", + Overrides: []configv1.ComponentOverride{{ + Unmanaged: false, + }}, + }, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + {Version: "4.8.0"}, + }, + }, + }, + ), + }, + gateCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "admin-gates", + Namespace: "test"}, + Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "Description."}, + }, + // Name triggers deletion of config map + ackCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "delete", + Namespace: "test"}, Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "true"}, + }, + expectedResult: &upgradeable{ + Conditions: []configv1.ClusterOperatorStatusCondition{{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionFalse, + Reason: "UnableToAccessAdminAcksConfigMap", + Message: "admin-acks configmap not found."}}, + }, + }, + // delete tests are last so we don't have to re-create the config map for other tests + { + name: "admin-gates configmap not found", + optr: &Operator{ + defaultUpstreamServer: "http://localhost:8080/graph", + release: configv1.Release{ + Version: "v4.8.0", + Image: "image/image:v4.8.1", + }, + namespace: "test", + name: "default", + client: fake.NewSimpleClientset( + &configv1.ClusterVersion{ + ObjectMeta: metav1.ObjectMeta{ + Name: "default", + }, + Spec: configv1.ClusterVersionSpec{ + ClusterID: configv1.ClusterID(id), + Channel: "", + Overrides: []configv1.ComponentOverride{{ + Unmanaged: false, + }}, + }, + Status: configv1.ClusterVersionStatus{ + History: []configv1.UpdateHistory{ + {Version: "4.8.0"}, + }, + }, + }, + ), + }, + // Name triggers deletion of config map + gateCm: &corev1.ConfigMap{ + ObjectMeta: metav1.ObjectMeta{Name: "delete", + Namespace: "test"}, + Data: map[string]string{"ack-4.8-kube-122-api-removals-in-4.9": "Description."}, + }, + ackCm: nil, + expectedResult: &upgradeable{ + Conditions: []configv1.ClusterOperatorStatusCondition{{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionFalse, + Reason: "UnableToAccessAdminGatesConfigMap", + Message: "admin-gates configmap not found."}}, + }, + }, + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + watcherStarted := make(chan struct{}) + f := kfake.NewSimpleClientset() + + // A catch-all watch reactor that allows us to inject the watcherStarted channel. + f.PrependWatchReactor("*", func(action clienttesting.Action) (handled bool, ret watch.Interface, err error) { + gvr := action.GetResource() + ns := action.GetNamespace() + watch, err := f.Tracker().Watch(gvr, ns) + if err != nil { + return false, nil, err + } + close(watcherStarted) + return true, watch, nil + }) + cms := make(chan *corev1.ConfigMap, 1) + configManagedInformer := informers.NewSharedInformerFactory(f, 0) + cmInformerLister := configManagedInformer.Core().V1().ConfigMaps() + cmInformer := configManagedInformer.Core().V1().ConfigMaps().Informer() + cmInformer.AddEventHandler(&cache.ResourceEventHandlerFuncs{ + AddFunc: func(obj interface{}) { + cm := obj.(*corev1.ConfigMap) + t.Logf("cm added: %s/%s", cm.Namespace, cm.Name) + cms <- cm + }, + DeleteFunc: func(obj interface{}) { + cm := obj.(*corev1.ConfigMap) + t.Logf("cm deleted: %s/%s", cm.Namespace, cm.Name) + cms <- cm + }, + UpdateFunc: func(oldObj, newObj interface{}) { + cm := newObj.(*corev1.ConfigMap) + t.Logf("cm updated: %s/%s", cm.Namespace, cm.Name) + cms <- cm + }, + }) + configManagedInformer.Start(ctx.Done()) + + _, err := f.CoreV1().ConfigMaps("test").Create(ctx, &defaultGateCm, metav1.CreateOptions{}) + if err != nil { + t.Errorf("error injecting admin-gates configmap: %v", err) + } + waitForCm(t, cms) + _, err = f.CoreV1().ConfigMaps("test").Create(ctx, &defaultAckCm, metav1.CreateOptions{}) + if err != nil { + t.Errorf("error injecting admin-acks configmap: %v", err) } + waitForCm(t, cms) + for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - optr := tt.optr - optr.queue = workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()) - optr.proxyLister = &clientProxyLister{client: optr.client} - optr.coLister = &clientCOLister{client: optr.client} - optr.cvLister = &clientCVLister{client: optr.client} - optr.upgradeableChecks = optr.defaultUpgradeableChecks() - optr.eventRecorder = record.NewFakeRecorder(100) + { + t.Run(tt.name, func(t *testing.T) { + optr := tt.optr + optr.queue = workqueue.NewRateLimitingQueue(workqueue.DefaultControllerRateLimiter()) + optr.proxyLister = &clientProxyLister{client: optr.client} + optr.coLister = &clientCOLister{client: optr.client} + optr.cvLister = &clientCVLister{client: optr.client} + optr.cmConfigManagedLister = cmInformerLister.Lister().ConfigMaps("test") + optr.cmConfigLister = cmInformerLister.Lister().ConfigMaps("test") - ctx := context.Background() - err := optr.upgradeableSync(ctx, optr.queueKey()) - if err != nil && tt.wantErr == nil { - t.Fatalf("Operator.sync() unexpected error: %v", err) - } - if err != nil { - return - } + optr.upgradeableChecks = optr.defaultUpgradeableChecks() + optr.eventRecorder = record.NewFakeRecorder(100) - if optr.upgradeable != nil { - optr.upgradeable.At = time.Time{} - for i := range optr.upgradeable.Conditions { - optr.upgradeable.Conditions[i].LastTransitionTime = metav1.Time{} + if tt.gateCm != nil { + if tt.gateCm.Name == "delete" { + err := f.CoreV1().ConfigMaps("test").Delete(ctx, "admin-gates", metav1.DeleteOptions{}) + if err != nil { + t.Errorf("error deleting configmap admin-gates: %v", err) + } + } else { + _, err = f.CoreV1().ConfigMaps("test").Update(ctx, tt.gateCm, metav1.UpdateOptions{}) + if err != nil { + t.Errorf("error updating configmap admin-gates: %v", err) + } + } + waitForCm(t, cms) + } + if tt.ackCm != nil { + if tt.ackCm.Name == "delete" { + err := f.CoreV1().ConfigMaps("test").Delete(ctx, "admin-acks", metav1.DeleteOptions{}) + if err != nil { + t.Errorf("error deleting configmap admin-acks: %v", err) + } + } else { + _, err = f.CoreV1().ConfigMaps("test").Update(ctx, tt.ackCm, metav1.UpdateOptions{}) + if err != nil { + t.Errorf("error updating configmap admin-acks: %v", err) + } + } + waitForCm(t, cms) } - } - if !reflect.DeepEqual(optr.upgradeable, tt.want) { - t.Fatalf("unexpected: %s", diff.ObjectReflectDiff(tt.want, optr.upgradeable)) - } - if (optr.queue.Len() > 0) != (optr.upgradeable != nil) { - t.Fatalf("unexpected queue") - } - }) + err = optr.upgradeableSync(ctx, optr.queueKey()) + if err != nil && tt.wantErr == nil { + t.Fatalf("Operator.sync() unexpected error: %v", err) + } + if err != nil { + return + } + + if optr.upgradeable != nil { + optr.upgradeable.At = time.Time{} + for i := range optr.upgradeable.Conditions { + optr.upgradeable.Conditions[i].LastTransitionTime = metav1.Time{} + } + } + + if !reflect.DeepEqual(optr.upgradeable, tt.expectedResult) { + t.Fatalf("unexpected: %s", diff.ObjectReflectDiff(tt.expectedResult, optr.upgradeable)) + } + if (optr.queue.Len() > 0) != (optr.upgradeable != nil) { + t.Fatalf("unexpected queue") + } + }) + } + } +} + +func Test_gateApplicableToCurrentVersion(t *testing.T) { + tests := []struct { + name string + gateName string + cv string + wantErr bool + expectedResult bool + }{ + { + name: "gate name invalid format no dot", + gateName: "ack-4>8-foo", + cv: "4.8.1", + wantErr: true, + expectedResult: false, + }, + { + name: "gate name invalid format 2 dots", + gateName: "ack-4..8-foo", + cv: "4.8.1", + wantErr: true, + expectedResult: false, + }, + { + name: "gate name invalid format does not start with ack", + gateName: "ck-4.8-foo", + cv: "4.8.1", + wantErr: true, + expectedResult: false, + }, + { + name: "gate name invalid format major version must be 4 or 5", + gateName: "ack-3.8-foo", + cv: "4.8.1", + wantErr: true, + expectedResult: false, + }, + { + name: "gate name invalid format minor version must be a number", + gateName: "ack-4.x-foo", + cv: "4.8.1", + wantErr: true, + expectedResult: false, + }, + { + name: "gate name invalid format no following dash", + gateName: "ack-4.8.1-foo", + cv: "4.8.1", + wantErr: true, + expectedResult: false, + }, + { + name: "gate name invalid format 2 following dashes", + gateName: "ack-4.x--foo", + cv: "4.8.1", + wantErr: true, + expectedResult: false, + }, + { + name: "gate name invalid format no description following dash", + gateName: "ack-4.x-", + cv: "4.8.1", + wantErr: true, + expectedResult: false, + }, + { + name: "gate name match", + gateName: "ack-4.8-foo", + cv: "4.8.1", + wantErr: false, + expectedResult: true, + }, + { + name: "gate name match big minor version", + gateName: "ack-4.123456-foo", + cv: "4.123456", + wantErr: false, + expectedResult: true, + }, + { + name: "gate name no match", + gateName: "ack-4.8-foo", + cv: "4.9.1", + wantErr: false, + expectedResult: false, + }, + { + name: "gate name no match multi digit minor", + gateName: "ack-4.8-foo", + cv: "4.80.1", + wantErr: false, + expectedResult: false, + }, + } + for _, tt := range tests { + { + t.Run(tt.name, func(t *testing.T) { + isApplicable, err := gateApplicableToCurrentVersion(tt.gateName, tt.cv) + if err != nil && !tt.wantErr { + t.Fatalf("gateApplicableToCurrentVersion() unexpected error: %v", err) + } + if err != nil { + return + } + if isApplicable && !tt.expectedResult { + t.Fatalf("gateApplicableToCurrentVersion() %s should not apply", tt.gateName) + } + }) + } } } diff --git a/pkg/cvo/upgradeable.go b/pkg/cvo/upgradeable.go index 253aed0ba..fa851ebf4 100644 --- a/pkg/cvo/upgradeable.go +++ b/pkg/cvo/upgradeable.go @@ -2,6 +2,7 @@ package cvo import ( "fmt" + "regexp" "sort" "strings" "time" @@ -9,26 +10,45 @@ import ( configv1 "github.com/openshift/api/config/v1" configlistersv1 "github.com/openshift/client-go/config/listers/config/v1" "github.com/pkg/errors" + corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/labels" + listerscorev1 "k8s.io/client-go/listers/core/v1" + "k8s.io/client-go/tools/cache" "k8s.io/klog/v2" "github.com/openshift/cluster-version-operator/lib/resourcemerge" + "github.com/openshift/cluster-version-operator/pkg/internal" + "github.com/openshift/cluster-version-operator/pkg/payload/precondition/clusterversion" ) +const ( + adminAckGateFmt string = "^ack-[4-5][.]([0-9]{1,})-[^-]" + upgradeableAdminAckRequired = configv1.ClusterStatusConditionType("UpgradeableAdminAckRequired") +) + +var adminAckGateRegexp = regexp.MustCompile(adminAckGateFmt) + // syncUpgradeable. The status is only checked if it has been more than // the minimumUpdateCheckInterval since the last check. -func (optr *Operator) syncUpgradeable(config *configv1.ClusterVersion) error { +func (optr *Operator) syncUpgradeable() error { // updates are only checked at most once per minimumUpdateCheckInterval or if the generation changes u := optr.getUpgradeable() if u != nil && u.RecentlyChanged(optr.minimumUpdateCheckInterval) { klog.V(4).Infof("Upgradeable conditions were recently checked, will try later.") return nil } + optr.setUpgradeableConditions() + + // requeue + optr.queue.Add(optr.queueKey()) + return nil +} +func (optr *Operator) setUpgradeableConditions() { now := metav1.Now() var conds []configv1.ClusterOperatorStatusCondition var reasons []string @@ -60,9 +80,6 @@ func (optr *Operator) syncUpgradeable(config *configv1.ClusterVersion) error { optr.setUpgradeable(&upgradeable{ Conditions: conds, }) - // requeue - optr.queue.Add(optr.queueKey()) - return nil } type upgradeable struct { @@ -215,9 +232,191 @@ func (check *clusterVersionOverridesUpgradeable) Check() *configv1.ClusterOperat return cond } +func gateApplicableToCurrentVersion(gateName string, currentVersion string) (bool, error) { + var applicable bool + if ackVersion := adminAckGateRegexp.FindString(gateName); ackVersion == "" { + return false, fmt.Errorf("%s configmap gate name %s has invalid format; must comply with %q.", + internal.AdminGatesConfigMap, gateName, adminAckGateFmt) + } else { + parts := strings.Split(ackVersion, "-") + ackMinor := clusterversion.GetEffectiveMinor(parts[1]) + cvMinor := clusterversion.GetEffectiveMinor(currentVersion) + if ackMinor == cvMinor { + applicable = true + } + } + return applicable, nil +} + +func checkAdminGate(gateName string, gateValue string, currentVersion string, + ackConfigmap *corev1.ConfigMap) (string, string) { + + if applies, err := gateApplicableToCurrentVersion(gateName, currentVersion); err == nil { + if !applies { + return "", "" + } + } else { + klog.Error(err) + return "AdminAckConfigMapGateNameError", err.Error() + } + if gateValue == "" { + message := fmt.Sprintf("%s configmap gate %s must contain a non-empty value.", internal.AdminGatesConfigMap, gateName) + klog.Error(message) + return "AdminAckConfigMapGateValueError", message + } + if val, ok := ackConfigmap.Data[gateName]; !ok || val != "true" { + return "AdminAckRequired", gateValue + } + return "", "" +} + +type clusterAdminAcksCompletedUpgradeable struct { + adminGatesLister listerscorev1.ConfigMapNamespaceLister + adminAcksLister listerscorev1.ConfigMapNamespaceLister + cvLister configlistersv1.ClusterVersionLister + cvoName string +} + +func (check *clusterAdminAcksCompletedUpgradeable) Check() *configv1.ClusterOperatorStatusCondition { + cv, err := check.cvLister.Get(check.cvoName) + if meta.IsNoMatchError(err) || apierrors.IsNotFound(err) { + message := fmt.Sprintf("Unable to get ClusterVersion, err=%v.", err) + klog.Error(message) + return &configv1.ClusterOperatorStatusCondition{ + Type: upgradeableAdminAckRequired, + Status: configv1.ConditionFalse, + Reason: "UnableToGetClusterVersion", + Message: message, + } + } + currentVersion := clusterversion.GetCurrentVersion(cv.Status.History) + + // This can occur in early start up when the configmap is first added and version history + // has not yet been populated. + if currentVersion == "" { + return nil + } + + var gateCm *corev1.ConfigMap + if gateCm, err = check.adminGatesLister.Get(internal.AdminGatesConfigMap); err != nil { + var message string + if apierrors.IsNotFound(err) { + message = fmt.Sprintf("%s configmap not found.", internal.AdminGatesConfigMap) + } else if err != nil { + message = fmt.Sprintf("Unable to access configmap %s, err=%v.", internal.AdminGatesConfigMap, err) + } + klog.Error(message) + return &configv1.ClusterOperatorStatusCondition{ + Type: upgradeableAdminAckRequired, + Status: configv1.ConditionFalse, + Reason: "UnableToAccessAdminGatesConfigMap", + Message: message, + } + } + var ackCm *corev1.ConfigMap + if ackCm, err = check.adminAcksLister.Get(internal.AdminAcksConfigMap); err != nil { + var message string + if apierrors.IsNotFound(err) { + message = fmt.Sprintf("%s configmap not found.", internal.AdminAcksConfigMap) + } else if err != nil { + message = fmt.Sprintf("Unable to access configmap %s, err=%v.", internal.AdminAcksConfigMap, err) + } + klog.Error(message) + return &configv1.ClusterOperatorStatusCondition{ + Type: upgradeableAdminAckRequired, + Status: configv1.ConditionFalse, + Reason: "UnableToAccessAdminAcksConfigMap", + Message: message, + } + } + reasons := make(map[string][]string) + for k, v := range gateCm.Data { + if reason, message := checkAdminGate(k, v, currentVersion, ackCm); reason != "" { + reasons[reason] = append(reasons[reason], message) + } + } + var reason string + var messages []string + for k, v := range reasons { + reason = k + sort.Strings(v) + messages = append(messages, strings.Join(v, " ")) + } + if len(reasons) == 1 { + return &configv1.ClusterOperatorStatusCondition{ + Type: upgradeableAdminAckRequired, + Status: configv1.ConditionFalse, + Reason: reason, + Message: messages[0], + } + } else if len(reasons) > 1 { + sort.Strings(messages) + return &configv1.ClusterOperatorStatusCondition{ + Type: upgradeableAdminAckRequired, + Status: configv1.ConditionFalse, + Reason: "MultipleReasons", + Message: strings.Join(messages, " "), + } + } + return nil +} + 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}, } } + +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) + optr.setUpgradeableConditions() + } +} + +func (optr *Operator) updateFunc(oldObj, newObj interface{}) { + cm := newObj.(*corev1.ConfigMap) + if cm.Name == internal.AdminGatesConfigMap || cm.Name == internal.AdminAcksConfigMap { + oldCm := oldObj.(*corev1.ConfigMap) + if !equality.Semantic.DeepEqual(cm, oldCm) { + klog.V(4).Infof("ConfigMap %s/%s updated.", cm.Namespace, cm.Name) + optr.setUpgradeableConditions() + } + } +} + +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) + optr.setUpgradeableConditions() + } +} + +// adminAcksEventHandler handles changes to the admin-acks configmap by re-assessing all +// Upgradeable conditions. +func (optr *Operator) adminAcksEventHandler() cache.ResourceEventHandler { + return cache.ResourceEventHandlerFuncs{ + AddFunc: optr.addFunc, + UpdateFunc: optr.updateFunc, + DeleteFunc: optr.deleteFunc, + } +} + +// adminGatesEventHandler handles changes to the admin-gates configmap by re-assessing all +// Upgradeable conditions. +func (optr *Operator) adminGatesEventHandler() cache.ResourceEventHandler { + return cache.ResourceEventHandlerFuncs{ + AddFunc: optr.addFunc, + UpdateFunc: optr.updateFunc, + DeleteFunc: optr.deleteFunc, + } +} diff --git a/pkg/internal/constants.go b/pkg/internal/constants.go index b651a52bb..6980082aa 100644 --- a/pkg/internal/constants.go +++ b/pkg/internal/constants.go @@ -3,6 +3,8 @@ package internal const ( ConfigNamespace = "openshift-config" ConfigManagedNamespace = "openshift-config-managed" + AdminGatesConfigMap = "admin-gates" + AdminAcksConfigMap = "admin-acks" InstallerConfigMap = "openshift-install" ManifestsConfigMap = "openshift-install-manifests" ) diff --git a/pkg/payload/precondition/clusterversion/upgradable_test.go b/pkg/payload/precondition/clusterversion/upgradable_test.go index 3fe8bd47f..25dc1896f 100644 --- a/pkg/payload/precondition/clusterversion/upgradable_test.go +++ b/pkg/payload/precondition/clusterversion/upgradable_test.go @@ -45,7 +45,7 @@ func TestGetEffectiveMinor(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - actual := getEffectiveMinor(tc.input) + actual := GetEffectiveMinor(tc.input) if tc.expected != actual { t.Error(actual) } diff --git a/pkg/payload/precondition/clusterversion/upgradeable.go b/pkg/payload/precondition/clusterversion/upgradeable.go index f796ff335..0e316d2ad 100644 --- a/pkg/payload/precondition/clusterversion/upgradeable.go +++ b/pkg/payload/precondition/clusterversion/upgradeable.go @@ -82,9 +82,9 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele return nil } - currentVersion := getCurrentVersion(cv.Status.History) - currentMinor := getEffectiveMinor(currentVersion) - desiredMinor := getEffectiveMinor(releaseContext.DesiredVersion) + currentVersion := GetCurrentVersion(cv.Status.History) + currentMinor := GetEffectiveMinor(currentVersion) + desiredMinor := GetEffectiveMinor(releaseContext.DesiredVersion) klog.V(5).Infof("currentMinor %s releaseContext.DesiredVersion %s desiredMinor %s", currentMinor, releaseContext.DesiredVersion, desiredMinor) // if there is no difference in the minor version (4.y.z where 4.y is the same for current and desired), then we can still upgrade @@ -169,9 +169,9 @@ func (pf *RecentEtcdBackup) Run(ctx context.Context, releaseContext precondition } } - currentVersion := getCurrentVersion(cv.Status.History) - currentMinor := getEffectiveMinor(currentVersion) - desiredMinor := getEffectiveMinor(releaseContext.DesiredVersion) + currentVersion := GetCurrentVersion(cv.Status.History) + currentMinor := GetEffectiveMinor(currentVersion) + desiredMinor := GetEffectiveMinor(releaseContext.DesiredVersion) if minorVersionUpgrade(currentMinor, desiredMinor) { reason, message := recentEtcdBackupCondition(pf.coLister) @@ -189,23 +189,27 @@ func (pf *RecentEtcdBackup) Run(ctx context.Context, releaseContext precondition // Name returns Name for the precondition. func (pf *RecentEtcdBackup) Name() string { return "EtcdRecentBackup" } -// getCurrentVersion determines and returns the cluster's current version by iterating through the +// GetCurrentVersion determines and returns the cluster's current version by iterating through the // provided update history until it finds the first version with update State of Completed. If a // Completed version is not found the version of the oldest history entry, which is the originally -// installed version, is returned. -func getCurrentVersion(history []configv1.UpdateHistory) string { +// installed version, is returned. If history is empty the empty string is returned. +func GetCurrentVersion(history []configv1.UpdateHistory) string { for _, h := range history { if h.State == configv1.CompletedUpdate { klog.V(5).Infof("Cluster current version=%s", h.Version) return h.Version } } - return history[len(history)-1].Version + // Empty history should only occur if method is called early in startup before history is populated. + if len(history) != 0 { + return history[len(history)-1].Version + } + return "" } -// getEffectiveMinor attempts to do a simple parse of the version provided. If it does not parse, the value is considered +// GetEffectiveMinor attempts to do a simple parse of the version provided. If it does not parse, the value is considered // empty string, which works for the comparison done here for equivalence. -func getEffectiveMinor(version string) string { +func GetEffectiveMinor(version string) string { splits := strings.Split(version, ".") if len(splits) < 2 { return ""