Merge pull request #603 from jpeeler/operator-group-robust

ecordell · web-flow · commit 8429cb39b98e · 2018-12-06T16:30:25.000-05:00
fix(e2e): make operator group test more robust
diff --git a/pkg/controller/operators/olm/operator_test.go b/pkg/controller/operators/olm/operator_test.go
@@ -2522,9 +2522,12 @@ func TestSyncOperatorGroups(t *testing.T) {
 
 			stopCh := make(chan struct{})
 			defer func() { stopCh <- struct{}{} }()
-			op, _, err := NewFakeOperator(tt.initial.clientObjs, tt.initial.k8sObjs, tt.initial.crds, tt.initial.apis, &install.StrategyResolver{}, namespaces, stopCh)
+			op, hasSyncedFns, err := NewFakeOperator(tt.initial.clientObjs, tt.initial.k8sObjs, tt.initial.crds, tt.initial.apis, &install.StrategyResolver{}, namespaces, stopCh)
 			require.NoError(t, err)
 
+			ok := cache.WaitForCacheSync(stopCh, hasSyncedFns...)
+			require.True(t, ok, "wait for cache sync failed")
+
 			err = op.syncOperatorGroups(tt.initial.operatorGroup)
 			require.NoError(t, err)
 
diff --git a/pkg/controller/operators/olm/operatorgroup.go b/pkg/controller/operators/olm/operatorgroup.go
@@ -251,16 +251,16 @@ func (a *Operator) copyCsvToTargetNamespace(csv *v1alpha1.ClusterServiceVersion,
 			newCSV := csv.DeepCopy()
 			newCSV.SetNamespace(ns)
 			newCSV.SetResourceVersion("")
-			newCSV.Status.Reason = v1alpha1.CSVReasonCopied
-			newCSV.Status.Message = fmt.Sprintf("The operator is running in %s but is managing this namespace", csv.GetNamespace())
-			newCSV.Status.LastUpdateTime = timeNow()
 
 			log.Debugf("Copying CSV %v to namespace %v", csv.GetName(), ns)
 			createdCSV, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(ns).Create(newCSV)
 			if err != nil {
 				log.Errorf("Create for new CSV failed: %v", err)
 				return err
 			}
+			createdCSV.Status.Reason = v1alpha1.CSVReasonCopied
+			createdCSV.Status.Message = fmt.Sprintf("The operator is running in %s but is managing this namespace", csv.GetNamespace())
+			createdCSV.Status.LastUpdateTime = timeNow()
 			if _, err := a.client.OperatorsV1alpha1().ClusterServiceVersions(ns).UpdateStatus(createdCSV); err != nil {
 				log.Errorf("Status update for CSV failed: %v", err)
 				return err
@@ -388,6 +388,7 @@ func (a *Operator) ensureClusterRoles(op *v1alpha2.OperatorGroup) error {
 			},
 			Rules: apiEditPolicyRules,
 		}
+		//ownerutil.AddNonBlockingOwner(operatorGroupEditClusterRole, csv)
 		_, err = a.OpClient.KubernetesInterface().RbacV1().ClusterRoles().Create(operatorGroupEditClusterRole)
 		if k8serrors.IsAlreadyExists(err) {
 			if _, err = a.OpClient.UpdateClusterRole(operatorGroupEditClusterRole); err != nil {
@@ -404,6 +405,7 @@ func (a *Operator) ensureClusterRoles(op *v1alpha2.OperatorGroup) error {
 			},
 			Rules: apiViewPolicyRules,
 		}
+		//ownerutil.AddNonBlockingOwner(operatorGroupViewClusterRole, csv)
 		_, err = a.OpClient.KubernetesInterface().RbacV1().ClusterRoles().Create(operatorGroupViewClusterRole)
 		if k8serrors.IsAlreadyExists(err) {
 			if _, err = a.OpClient.UpdateClusterRole(operatorGroupViewClusterRole); err != nil {
diff --git a/test/e2e/operator_groups_e2e_test.go b/test/e2e/operator_groups_e2e_test.go
@@ -116,10 +116,11 @@ func TestOperatorGroup(t *testing.T) {
 	require.NoError(t, err)
 
 	t.Log("Creating CRD")
-	mainCRDPlural := genName("ins")
-	apiGroup := "cluster.com"
+	mainCRDPlural := genName("opgroup")
+	apiGroup := "opcluster.com"
 	mainCRDName := mainCRDPlural + "." + apiGroup
 	mainCRD := newCRD(mainCRDName, mainCRDPlural)
+	mainCRD.Spec.Group = apiGroup
 	cleanupCRD, err := createCRD(c, mainCRD)
 	require.NoError(t, err)
 	defer cleanupCRD()
@@ -129,6 +130,10 @@ func TestOperatorGroup(t *testing.T) {
 	createdCSV, err := crc.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(&aCSV)
 	require.NoError(t, err)
 
+	t.Log("wait for CSV to succeed")
+	_, err = fetchCSV(t, crc, createdCSV.GetName(), csvSucceededChecker)
+	require.NoError(t, err)
+
 	t.Log("Creating operator group")
 	operatorGroup := v1alpha2.OperatorGroup{
 		ObjectMeta: metav1.ObjectMeta{
@@ -145,7 +150,7 @@ func TestOperatorGroup(t *testing.T) {
 	_, err = crc.OperatorsV1alpha2().OperatorGroups(testNamespace).Create(&operatorGroup)
 	require.NoError(t, err)
 	expectedOperatorGroupStatus := v1alpha2.OperatorGroupStatus{
-		Namespaces: []string{createdOtherNamespace.GetName()},
+		Namespaces: []string{createdOtherNamespace.GetName(), testNamespace},
 	}
 
 	t.Log("Waiting on operator group to have correct status")
@@ -161,30 +166,51 @@ func TestOperatorGroup(t *testing.T) {
 		return false, nil
 	})
 
-	t.Log("Checking for proper generated operator-group RBAC roles")
-	roleList, err := c.KubernetesInterface().RbacV1().ClusterRoles().List(metav1.ListOptions{})
-	for _, item := range roleList.Items {
-		role, err := c.GetClusterRole(item.GetName())
-		require.NoError(t, err)
-		switch roleName := item.GetName(); roleName {
-		case "owned-crd-manager-another-csv":
-			managerPolicyRules := []rbacv1.PolicyRule{
-				{Verbs: []string{"*"}, APIGroups: []string{apiGroup}, Resources: []string{mainCRDPlural}},
-			}
-			require.Equal(t, managerPolicyRules, role.Rules)
-		case "e2e-operator-group-edit":
-			editPolicyRules := []rbacv1.PolicyRule{
-				{Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{apiGroup}, Resources: []string{mainCRDPlural}},
+	t.Log("Waiting for expected operator group test APIGroup from CSV")
+	err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {
+		// (view role is the last role created, so the rest should exist as well by this point)
+		viewRole, fetchErr := c.KubernetesInterface().RbacV1().ClusterRoles().Get("e2e-operator-group-view", metav1.GetOptions{})
+		if fetchErr != nil {
+			if errors.IsNotFound(fetchErr) {
+				return false, nil
 			}
-			t.Log(role)
-			require.Equal(t, editPolicyRules, role.Rules)
-		case "e2e-operator-group-view":
-			viewPolicyRules := []rbacv1.PolicyRule{
-				{Verbs: []string{"get", "list", "watch"}, APIGroups: []string{apiGroup}, Resources: []string{mainCRDPlural}},
+			t.Logf("Unable to fetch view role: %v", fetchErr.Error())
+			return false, fetchErr
+		}
+		for _, rule := range viewRole.Rules {
+			for _, group := range rule.APIGroups {
+				if group == apiGroup {
+					return true, nil
+				}
 			}
-			require.Equal(t, viewPolicyRules, role.Rules)
 		}
+		return false, nil
+	})
+
+	t.Log("Checking for proper generated operator group RBAC roles")
+	editRole, err := c.KubernetesInterface().RbacV1().ClusterRoles().Get("e2e-operator-group-edit", metav1.GetOptions{})
+	require.NoError(t, err)
+	editPolicyRules := []rbacv1.PolicyRule{
+		{Verbs: []string{"create", "update", "patch", "delete"}, APIGroups: []string{apiGroup}, Resources: []string{mainCRDPlural}},
+	}
+	t.Log(editRole)
+	require.Equal(t, editPolicyRules, editRole.Rules)
+
+	viewRole, err := c.KubernetesInterface().RbacV1().ClusterRoles().Get("e2e-operator-group-view", metav1.GetOptions{})
+	require.NoError(t, err)
+	viewPolicyRules := []rbacv1.PolicyRule{
+		{Verbs: []string{"get", "list", "watch"}, APIGroups: []string{apiGroup}, Resources: []string{mainCRDPlural}},
+	}
+	t.Log(viewRole)
+	require.Equal(t, viewPolicyRules, viewRole.Rules)
+
+	managerRole, err := c.KubernetesInterface().RbacV1().ClusterRoles().Get("owned-crd-manager-another-csv", metav1.GetOptions{})
+	require.NoError(t, err)
+	managerPolicyRules := []rbacv1.PolicyRule{
+		{Verbs: []string{"*"}, APIGroups: []string{apiGroup}, Resources: []string{mainCRDPlural}},
 	}
+	t.Log(managerRole)
+	require.Equal(t, managerPolicyRules, managerRole.Rules)
 
 	t.Log("Waiting for operator namespace csv to have annotations")
 	err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {
@@ -203,7 +229,10 @@ func TestOperatorGroup(t *testing.T) {
 	err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {
 		fetchedCSV, fetchErr := crc.OperatorsV1alpha1().ClusterServiceVersions(otherNamespaceName).Get(csvName, metav1.GetOptions{})
 		if fetchErr != nil {
-			t.Log(fetchErr.Error())
+			if errors.IsNotFound(fetchErr) {
+				return false, nil
+			}
+			t.Logf("Error (in %v): %v", otherNamespaceName, fetchErr.Error())
 			return false, fetchErr
 		}
 		if checkOperatorGroupAnnotations(fetchedCSV, &operatorGroup, bothNamespaceNames) == nil {
@@ -214,12 +243,21 @@ func TestOperatorGroup(t *testing.T) {
 	})
 	// since annotations are set along with status, no reason to poll for this check as done above
 	t.Log("Checking status on csv in target namespace")
-	fetchedCSV, err := crc.OperatorsV1alpha1().ClusterServiceVersions(otherNamespaceName).Get(csvName, metav1.GetOptions{})
+	err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {
+		fetchedCSV, fetchErr := crc.OperatorsV1alpha1().ClusterServiceVersions(otherNamespaceName).Get(csvName, metav1.GetOptions{})
+		if fetchErr != nil {
+			if errors.IsNotFound(fetchErr) {
+				return false, nil
+			}
+			t.Logf("Error (in %v): %v", otherNamespaceName, fetchErr.Error())
+			return false, fetchErr
+		}
+		if fetchedCSV.Status.Reason == v1alpha1.CSVReasonCopied {
+			return true, nil
+		}
+		return false, nil
+	})
 	require.NoError(t, err)
-	require.EqualValues(t, v1alpha1.CSVReasonCopied, fetchedCSV.Status.Reason)
-	// also check name and spec
-	require.EqualValues(t, createdCSV.Name, fetchedCSV.Name)
-	require.EqualValues(t, createdCSV.Spec, fetchedCSV.Spec)
 
 	t.Log("Waiting on deployment to have correct annotations")
 	err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {
