Skip to content

Commit

Permalink
Reduce e2e test failure rate
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
awgreene committed Dec 22, 2020
1 parent e2b51e2 commit 1da7d66
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 38 deletions.
34 changes: 24 additions & 10 deletions test/e2e/csv_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 25 additions & 28 deletions test/e2e/installplan_e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -2076,7 +2077,7 @@ var _ = Describe("Install Plan", func() {
})

// This It spec creates an InstallPlan with a CSV containing a set of permissions to be resolved.
It("creation with permissions", func() {
FIt("creation with permissions", func() {

packageName := genName("nginx")
stableChannel := "stable"
Expand Down Expand Up @@ -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()},
},
},
},
}
Expand Down Expand Up @@ -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) {
Expand All @@ -2217,6 +2225,7 @@ var _ = Describe("Install Plan", func() {
}
return true, nil
})
require.NoError(GinkgoT(), err)
}
}

Expand All @@ -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)
Expand All @@ -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
}
Expand All @@ -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
}
Expand All @@ -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() {
Expand Down

0 comments on commit 1da7d66

Please sign in to comment.