Reduce e2e test failure rate

This commit introduces three changes to the e2e test suite:
1. It reduces the failure rate of the CSV e2e test by allowing the
    operator to reach a succeeded install before searching for resources.
2. An installplan e2e test was failing a diff check against two
    installplans because the operator controller was applying a component
label to one of the installplans. An update was made to still compare
the spec and status of the two installplans but ignores other fields.
3. A test that ensures OLM elevates scoped  permissions to cluster levels
    when the CSV is installed in a global OperatorGroup was not elevating
them  because all of the permissions defined in the role were already
provided by a clusterrole defined in the csv. The test was sped up
significantly by making the rules defined in the CSV's spec.Permissions
field different from those defined in the spec.ClusterPermissions field.

test/e2e/csv_e2e_test.go

@@ -1306,28 +1306,42 @@ var _ = Describe("ClusterServiceVersion", func() {
 		Expect(err).ShouldNot(HaveOccurred(), "error getting expected APIService")
 
 		// Should create Service
-		_, err = c.GetService(testNamespace, serviceName)
-		Expect(err).ShouldNot(HaveOccurred(), "error getting expected Service")
+		Eventually(func() error {
+			_, err := c.GetService(testNamespace, serviceName)
+			return err
+		}, timeout, interval).ShouldNot(HaveOccurred())
 
 		// Should create certificate Secret
 		secretName = fmt.Sprintf("%s-cert", serviceName)
-		_, err = c.GetSecret(testNamespace, secretName)
-		Expect(err).ShouldNot(HaveOccurred(), "error getting expected Secret")
+		Eventually(func() error {
+			_, err = c.GetSecret(testNamespace, secretName)
+			return err
+		}, timeout, interval).ShouldNot(HaveOccurred())
 
 		// Should create a Role for the Secret
 		_, err = c.GetRole(testNamespace, secretName)
-		Expect(err).ShouldNot(HaveOccurred(), "error getting expected Secret Role")
+		Eventually(func() error {
+			_, err = c.GetRole(testNamespace, secretName)
+			return err
+		}, timeout, interval).ShouldNot(HaveOccurred())
 
 		// Should create a RoleBinding for the Secret
-		_, err = c.GetRoleBinding(testNamespace, secretName)
-		Expect(err).ShouldNot(HaveOccurred(), "error getting exptected Secret RoleBinding")
+		Eventually(func() error {
+			_, err = c.GetRoleBinding(testNamespace, secretName)
+			return err
+		}, timeout, interval).ShouldNot(HaveOccurred())
 
 		// Should create a system:auth-delegator Cluster RoleBinding
-		_, err = c.GetClusterRoleBinding(fmt.Sprintf("%s-system:auth-delegator", serviceName))
-		Expect(err).ShouldNot(HaveOccurred(), "error getting expected system:auth-delegator ClusterRoleBinding")
+		Eventually(func() error {
+			_, err = c.GetClusterRoleBinding(fmt.Sprintf("%s-system:auth-delegator", serviceName))
+			return err
+		}, timeout, interval).ShouldNot(HaveOccurred())
 
 		// Should create an extension-apiserver-authentication-reader RoleBinding in kube-system
-		_, err = c.GetRoleBinding("kube-system", fmt.Sprintf("%s-auth-reader", serviceName))
+		Eventually(func() error {
+			_, err = c.GetRoleBinding("kube-system", fmt.Sprintf("%s-auth-reader", serviceName))
+			return err
+		}, timeout, interval).ShouldNot(HaveOccurred())
 		Expect(err).ShouldNot(HaveOccurred(), "error getting expected extension-apiserver-authentication-reader RoleBinding")
 
 		// Should eventually GC the CSV

test/e2e/installplan_e2e_test.go

@@ -222,11 +222,12 @@ var _ = Describe("Install Plan", func() {
 
 		// Fetch installplan again to check for unnecessary control loops
 		fetchedInstallPlan, err = fetchInstallPlan(GinkgoT(), crc, fetchedInstallPlan.GetName(), func(fip *operatorsv1alpha1.InstallPlan) bool {
-			compareResources(GinkgoT(), fetchedInstallPlan, fip)
+			// Don't compare object meta as labels can be applied by the operator controller.
+			compareResources(GinkgoT(), fetchedInstallPlan.Spec, fip.Spec)
+			compareResources(GinkgoT(), fetchedInstallPlan.Status, fip.Status)
 			return true
 		})
 		require.NoError(GinkgoT(), err)
-
 		require.Equal(GinkgoT(), len(expectedStepSources), len(fetchedInstallPlan.Status.Plan), "Number of resolved steps matches the number of expected steps")
 
 		// Ensure resolved step resources originate from the correct catalog sources
@@ -2111,6 +2112,12 @@ var _ = Describe("Install Plan", func() {
 						APIGroups: []string{"cluster.com"},
 						Resources: []string{crdPlural},
 					},
+					// Permissions must be different than ClusterPermissions defined below if OLM is going to lift role/rolebindings to cluster level.
+					{
+						Verbs:     []string{rbac.VerbAll},
+						APIGroups: []string{corev1.GroupName},
+						Resources: []string{corev1.ResourceConfigMaps.String()},
+					},
 				},
 			},
 		}
@@ -2205,6 +2212,7 @@ var _ = Describe("Install Plan", func() {
 					}
 					return true, nil
 				})
+				require.NoError(GinkgoT(), err)
 			}
 			if step.Resource.Kind == "RoleBinding" {
 				err = wait.Poll(pollInterval, pollDuration, func() (bool, error) {
@@ -2217,6 +2225,7 @@ var _ = Describe("Install Plan", func() {
 					}
 					return true, nil
 				})
+				require.NoError(GinkgoT(), err)
 			}
 		}
 
@@ -2238,16 +2247,10 @@ var _ = Describe("Install Plan", func() {
 			GinkgoT().Logf("Monitoring cluster role binding %v", binding.GetName())
 		}
 
-		// can't query by owner reference, so just use the name we know is in the install plan
-		createdServiceAccountNames := map[string]struct{}{serviceAccountName: {}}
-		GinkgoT().Logf("Monitoring service account %v", serviceAccountName)
-
 		crWatcher, err := c.KubernetesInterface().RbacV1().ClusterRoles().Watch(context.TODO(), metav1.ListOptions{LabelSelector: fmt.Sprintf("%v=%v", ownerutil.OwnerKey, stableCSVName)})
 		require.NoError(GinkgoT(), err)
 		crbWatcher, err := c.KubernetesInterface().RbacV1().ClusterRoleBindings().Watch(context.TODO(), metav1.ListOptions{LabelSelector: fmt.Sprintf("%v=%v", ownerutil.OwnerKey, stableCSVName)})
 		require.NoError(GinkgoT(), err)
-		saWatcher, err := c.KubernetesInterface().CoreV1().ServiceAccounts(testNamespace).Watch(context.TODO(), metav1.ListOptions{})
-		require.NoError(GinkgoT(), err)
 
 		done := make(chan struct{})
 		errExit := make(chan error)
@@ -2266,7 +2269,7 @@ var _ = Describe("Install Plan", func() {
 							continue
 						}
 						delete(createdClusterRoleNames, cr.GetName())
-						if len(createdClusterRoleNames) == 0 && len(createdClusterRoleBindingNames) == 0 && len(createdServiceAccountNames) == 0 {
+						if len(createdClusterRoleNames) == 0 && len(createdClusterRoleBindingNames) == 0 {
 							done <- struct{}{}
 							return
 						}
@@ -2282,23 +2285,7 @@ var _ = Describe("Install Plan", func() {
 							continue
 						}
 						delete(createdClusterRoleBindingNames, crb.GetName())
-						if len(createdClusterRoleNames) == 0 && len(createdClusterRoleBindingNames) == 0 && len(createdServiceAccountNames) == 0 {
-							done <- struct{}{}
-							return
-						}
-					}
-				case evt, ok := <-saWatcher.ResultChan():
-					if !ok {
-						errExit <- errors.New("sa watch channel closed unexpectedly")
-						return
-					}
-					if evt.Type == watch.Deleted {
-						sa, ok := evt.Object.(*corev1.ServiceAccount)
-						if !ok {
-							continue
-						}
-						delete(createdServiceAccountNames, sa.GetName())
-						if len(createdClusterRoleNames) == 0 && len(createdClusterRoleBindingNames) == 0 && len(createdServiceAccountNames) == 0 {
+						if len(createdClusterRoleNames) == 0 && len(createdClusterRoleBindingNames) == 0 {
 							done <- struct{}{}
 							return
 						}
@@ -2320,7 +2307,17 @@ var _ = Describe("Install Plan", func() {
 
 		require.Emptyf(GinkgoT(), createdClusterRoleNames, "unexpected cluster role remain: %v", createdClusterRoleNames)
 		require.Emptyf(GinkgoT(), createdClusterRoleBindingNames, "unexpected cluster role binding remain: %v", createdClusterRoleBindingNames)
-		require.Emptyf(GinkgoT(), createdServiceAccountNames, "unexpected service account remain: %v", createdServiceAccountNames)
+
+		Eventually(func() error {
+			_, err := c.GetServiceAccount(testNamespace, serviceAccountName)
+			if err == nil {
+				return fmt.Errorf("The %v/%v ServiceAccount should have been deleted", testNamespace, serviceAccountName)
+			}
+			if !k8serrors.IsNotFound(err) {
+				return err
+			}
+			return nil
+		}, timeout, interval).Should(BeNil())
 	})
 
 	It("CRD validation", func() {