Update OperatorCondition Role and e2e test case

1. Operator can create/update/patch OperatorCondition but not its
status.
2. Update the e2e test case to update OperatorCondition's spec
directly instead of its status

Upstream-commit: ff554972b5170a2757b4a09083503e7011bee382
Upstream-repository: operator-lifecycle-manager

Signed-off-by: Vu Dinh <[email protected]>

Author: dinhxuanvu

staging/operator-lifecycle-manager/pkg/controller/operators/olm/operatorconditions.go

@@ -9,6 +9,7 @@ import (
 
 	operatorsv1 "github.com/operator-framework/api/pkg/operators/v1"
 	"github.com/operator-framework/api/pkg/operators/v1alpha1"
+	"github.com/sirupsen/logrus"
 )
 
 func (a *Operator) isOperatorUpgradeable(csv *v1alpha1.ClusterServiceVersion) (bool, error) {
@@ -24,19 +25,27 @@ func (a *Operator) isOperatorUpgradeable(csv *v1alpha1.ClusterServiceVersion) (b
 		return false, err
 	}
 
+	logger := a.logger.WithFields(logrus.Fields{
+		"name":      csv.GetName(),
+		"namespace": csv.GetNamespace(),
+	})
+
 	// Check condition overrides
 	for _, override := range cond.Spec.Overrides {
 		if override.Type == operatorsv1.Upgradeable {
 			if override.Status == metav1.ConditionTrue {
+				logger.Infof("Upgradeable condition is overridden to true: %s", override.Message)
 				return true, nil
 			}
+			logger.Infof("Upgradeable condition is overridden to false: %s", override.Message)
 			return false, fmt.Errorf("The operator is not upgradeable: %s", override.Message)
 		}
 	}
 
 	// Check for OperatorUpgradeable condition status
 	if c := meta.FindStatusCondition(cond.Status.Conditions, operatorsv1.Upgradeable); c != nil {
 		if c.ObservedGeneration < cond.ObjectMeta.Generation {
+			logger.Debugf("Upgradeable condition's generation doesn't match: %d/%d", c.ObservedGeneration, cond.ObjectMeta.Generation)
 			return false, fmt.Errorf("The operatorcondition status %q=%q is outdated", c.Type, c.Status)
 		}
 		if c.Status == metav1.ConditionFalse {

staging/operator-lifecycle-manager/pkg/controller/operators/operatorcondition_controller.go

@@ -137,16 +137,10 @@ func (r *OperatorConditionReconciler) ensureOperatorConditionRole(operatorCondit
 			Namespace: operatorCondition.GetNamespace(),
 		},
 		Rules: []rbacv1.PolicyRule{
-			{
-				Verbs:         []string{"get"},
-				APIGroups:     []string{"operators.coreos.com"},
-				Resources:     []string{"operatorconditions"},
-				ResourceNames: []string{operatorCondition.GetName()},
-			},
 			{
 				Verbs:         []string{"get", "update", "patch"},
 				APIGroups:     []string{"operators.coreos.com"},
-				Resources:     []string{"operatorconditions/status"},
+				Resources:     []string{"operatorconditions"},
 				ResourceNames: []string{operatorCondition.GetName()},
 			},
 		},

staging/operator-lifecycle-manager/pkg/controller/operators/operatorcondition_controller_test.go

@@ -99,16 +99,10 @@ var _ = Describe("OperatorCondition", func() {
 					BlockOwnerDeletion: &falseBool,
 				}))
 				Expect(role.Rules).Should(Equal([]rbacv1.PolicyRule{
-					{
-						Verbs:         []string{"get"},
-						APIGroups:     []string{"operators.coreos.com"},
-						Resources:     []string{"operatorconditions"},
-						ResourceNames: []string{namespacedName.Name},
-					},
 					{
 						Verbs:         []string{"get", "update", "patch"},
 						APIGroups:     []string{"operators.coreos.com"},
-						Resources:     []string{"operatorconditions/status"},
+						Resources:     []string{"operatorconditions"},
 						ResourceNames: []string{namespacedName.Name},
 					},
 				}))

staging/operator-lifecycle-manager/test/e2e/operator_condition_e2e_test.go

@@ -26,7 +26,7 @@ var _ = Describe("Operator Condition", func() {
 		By("This test proves that an operator can upgrade successfully when" +
 			"Upgrade condition type is set in OperatorCondition CR. Plus, an operator" +
 			"chooses not to use OperatorCondition, the upgrade process will proceed as" +
-			" asexpected. The overrides specification in OperatorCondition can be used to" +
+			" as expected. The overrides specification in OperatorCondition can be used to" +
 			" override the status condition. The overrides spec will remain in place until" +
 			"they are unset.")
 		c := newKubeClient()
@@ -99,14 +99,17 @@ var _ = Describe("Operator Condition", func() {
 			Message:            "test",
 			LastTransitionTime: metav1.Now(),
 		}
+
+		var currentGen int64
 		Eventually(func() error {
 			cond, err = crc.OperatorsV1().OperatorConditions(testNamespace).Get(context.TODO(), csvA.GetName(), metav1.GetOptions{})
 			if err != nil {
 				return err
 			}
-
-			meta.SetStatusCondition(&cond.Status.Conditions, upgradeableFalseCondition)
-			_, err = crc.OperatorsV1().OperatorConditions(testNamespace).UpdateStatus(context.TODO(), cond, metav1.UpdateOptions{})
+			currentGen = cond.ObjectMeta.GetGeneration()
+			upgradeableFalseCondition.ObservedGeneration = currentGen
+			meta.SetStatusCondition(&cond.Spec.Conditions, upgradeableFalseCondition)
+			_, err = crc.OperatorsV1().OperatorConditions(testNamespace).Update(context.TODO(), cond, metav1.UpdateOptions{})
 			return err
 
 		}, pollInterval, pollDuration).Should(Succeed())
@@ -142,11 +145,13 @@ var _ = Describe("Operator Condition", func() {
 		}
 		Eventually(func() error {
 			cond, err = crc.OperatorsV1().OperatorConditions(testNamespace).Get(context.TODO(), csvA.GetName(), metav1.GetOptions{})
-			if err != nil {
+			if err != nil || currentGen == cond.ObjectMeta.GetGeneration() {
 				return err
 			}
-			meta.SetStatusCondition(&cond.Status.Conditions, upgradeableTrueCondition)
-			_, err = crc.OperatorsV1().OperatorConditions(testNamespace).UpdateStatus(context.TODO(), cond, metav1.UpdateOptions{})
+			currentGen = cond.ObjectMeta.GetGeneration()
+			upgradeableTrueCondition.ObservedGeneration = cond.ObjectMeta.GetGeneration()
+			meta.SetStatusCondition(&cond.Spec.Conditions, upgradeableTrueCondition)
+			_, err = crc.OperatorsV1().OperatorConditions(testNamespace).Update(context.TODO(), cond, metav1.UpdateOptions{})
 			return err
 		}, pollInterval, pollDuration).Should(Succeed())
 
@@ -157,11 +162,13 @@ var _ = Describe("Operator Condition", func() {
 		// Get the OperatorCondition for csvB and report that it is not upgradeable
 		Eventually(func() error {
 			cond, err = crc.OperatorsV1().OperatorConditions(testNamespace).Get(context.TODO(), csvB.GetName(), metav1.GetOptions{})
-			if err != nil {
+			if err != nil || currentGen == cond.ObjectMeta.GetGeneration() {
 				return err
 			}
-			meta.SetStatusCondition(&cond.Status.Conditions, upgradeableFalseCondition)
-			_, err = crc.OperatorsV1().OperatorConditions(testNamespace).UpdateStatus(context.TODO(), cond, metav1.UpdateOptions{})
+			currentGen = cond.ObjectMeta.GetGeneration()
+			upgradeableFalseCondition.ObservedGeneration = currentGen
+			meta.SetStatusCondition(&cond.Spec.Conditions, upgradeableFalseCondition)
+			_, err = crc.OperatorsV1().OperatorConditions(testNamespace).Update(context.TODO(), cond, metav1.UpdateOptions{})
 			return err
 		}, pollInterval, pollDuration).Should(Succeed())
 
@@ -189,7 +196,7 @@ var _ = Describe("Operator Condition", func() {
 		// Get the OperatorCondition for csvB and override the upgradeable false condition
 		Eventually(func() error {
 			cond, err = crc.OperatorsV1().OperatorConditions(testNamespace).Get(context.TODO(), csvB.GetName(), metav1.GetOptions{})
-			if err != nil {
+			if err != nil || currentGen == cond.ObjectMeta.GetGeneration() {
 				return err
 			}
 			// Set Condition overrides to True