Skip to content

Commit

Permalink
test/e2e: Wrap flake-prone garbage collection tests in eventually ass…
Browse files Browse the repository at this point in the history 
…ertions

Update the test/e2e/gc_e2e_test.go and wrap any error-prone test blocks
in eventually assertions to reduce the number of flakes that occur when
attempting to create/update/delete/etc. Kubernetes resources during
individual CI runs on these garbage collection specs.

Signed-off-by: timflannagan <timflannagan@gmail.com>
  • Loading branch information
@timflannagan
timflannagan committed Oct 6, 2021
1 parent 07dd3a0 commit 9163ac6
Showing 1 changed file with 118 additions and 68 deletions.
186 changes: 118 additions & 68 deletions test/e2e/gc_e2e_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,7 @@ var _ = Describe("Garbage collection for dependent resources", func() {
BeforeEach(func() {
group := fmt.Sprintf("%s.com", rand.String(16))

// Create a CustomResourceDefinition
var err error
crd, err = kubeClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Create(context.TODO(), &apiextensionsv1.CustomResourceDefinition{
crd := &apiextensionsv1.CustomResourceDefinition{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("plural.%s", group),
},
Expand All @@ -73,22 +71,32 @@ var _ = Describe("Garbage collection for dependent resources", func() {
ListKind: "KindList",
},
},
}, metav1.CreateOptions{})
Expect(err).NotTo(HaveOccurred())
}

// Create a ClusterRole for the crd
cr, err = kubeClient.CreateClusterRole(&rbacv1.ClusterRole{
// Create a CustomResourceDefinition
var err error
Eventually(func() error {
crd, err = kubeClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Create(context.TODO(), crd, metav1.CreateOptions{})
return err
}).Should(Succeed())

cr := &rbacv1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "clusterrole-",
OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(crd)},
},
})
Expect(err).NotTo(HaveOccurred())
}

// Create a ClusterRole for the crd
Eventually(func() error {
cr, err = kubeClient.CreateClusterRole(cr)
return err
}).Should(Succeed())
})

AfterEach(func() {

// TODO(tflannag): Hmmm....
// Clean up cluster role
IgnoreError(kubeClient.DeleteClusterRole(cr.GetName(), &metav1.DeleteOptions{}))

Expand All @@ -100,11 +108,13 @@ var _ = Describe("Garbage collection for dependent resources", func() {

BeforeEach(func() {
// Delete CRD
Expect(kubeClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Delete(context.TODO(), crd.GetName(), metav1.DeleteOptions{})).To(Succeed())
Eventually(func() bool {
err := kubeClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Delete(context.TODO(), crd.GetName(), metav1.DeleteOptions{})
return k8serrors.IsNotFound(err)
}).Should(BeTrue())
})

It("should delete the associated ClusterRole", func() {

Eventually(func() bool {
_, err := kubeClient.GetClusterRole(cr.GetName())
return k8serrors.IsNotFound(err)
Expand All @@ -124,9 +134,7 @@ var _ = Describe("Garbage collection for dependent resources", func() {
BeforeEach(func() {
group := rand.String(16)

// Create an API Service
var err error
as, err = kubeClient.CreateAPIService(&apiregistrationv1.APIService{
apiService := &apiregistrationv1.APIService{
ObjectMeta: metav1.ObjectMeta{
Name: fmt.Sprintf("v1.%s", group),
},
Expand All @@ -136,17 +144,28 @@ var _ = Describe("Garbage collection for dependent resources", func() {
GroupPriorityMinimum: 1,
VersionPriority: 1,
},
})
Expect(err).NotTo(HaveOccurred())
}
// Create an API Service
var err error
Eventually(func() error {
// TODO(tflannag): Is comparing error to apierrors.IsAlreadyExists prone to masking
// incorrect testing assumptions?
as, err = kubeClient.CreateAPIService(apiService)
return err
}).Should(Succeed())

// Create a ClusterRole
cr, err = kubeClient.CreateClusterRole(&rbacv1.ClusterRole{
cr := &rbacv1.ClusterRole{
ObjectMeta: metav1.ObjectMeta{
GenerateName: "clusterrole-",
OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(as)},
},
})
Expect(err).NotTo(HaveOccurred())
}

Eventually(func() error {
// Create a ClusterRole
cr, err = kubeClient.CreateClusterRole(cr)
return err
}).Should(Succeed())
})

AfterEach(func() {
Expand All @@ -161,7 +180,10 @@ var _ = Describe("Garbage collection for dependent resources", func() {

BeforeEach(func() {
// Delete API service
Expect(kubeClient.DeleteAPIService(as.GetName(), &metav1.DeleteOptions{})).To(Succeed())
Eventually(func() bool {
err := kubeClient.DeleteAPIService(as.GetName(), &metav1.DeleteOptions{})
return k8serrors.IsNotFound(err)
}).Should(BeTrue())
})

It("should delete the associated ClusterRole", func() {
Expand Down Expand Up @@ -193,18 +215,22 @@ var _ = Describe("Garbage collection for dependent resources", func() {
propagation metav1.DeletionPropagation
options metav1.DeleteOptions
)

BeforeEach(func() {

ownerA = newCSV("ownera", testNamespace, "", semver.MustParse("0.0.0"), nil, nil, nil)
ownerB = newCSV("ownerb", testNamespace, "", semver.MustParse("0.0.0"), nil, nil, nil)

// create all owners
var err error
fetchedA, err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(context.TODO(), &ownerA, metav1.CreateOptions{})
Expect(err).NotTo(HaveOccurred())
fetchedB, err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(context.TODO(), &ownerB, metav1.CreateOptions{})
Expect(err).NotTo(HaveOccurred())
Eventually(func() error {
fetchedA, err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(context.TODO(), &ownerA, metav1.CreateOptions{})
return err
}).Should(Succeed())

Eventually(func() error {
fetchedB, err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(context.TODO(), &ownerB, metav1.CreateOptions{})
return err
}).Should(Succeed())

dependent = &corev1.ConfigMap{
ObjectMeta: metav1.ObjectMeta{
Expand All @@ -218,8 +244,10 @@ var _ = Describe("Garbage collection for dependent resources", func() {
ownerutil.AddOwner(dependent, fetchedB, true, false)

// create ConfigMap dependent
_, err = kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Create(context.TODO(), dependent, metav1.CreateOptions{})
Expect(err).NotTo(HaveOccurred(), "dependent could not be created")
Eventually(func() error {
_, err = kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Create(context.TODO(), dependent, metav1.CreateOptions{})
return err
}).Should(Succeed(), "dependent could not be created")

propagation = metav1.DeletePropagationForeground
options = metav1.DeleteOptions{PropagationPolicy: &propagation}
Expand All @@ -229,32 +257,35 @@ var _ = Describe("Garbage collection for dependent resources", func() {

BeforeEach(func() {
// delete ownerA in the foreground (to ensure any "blocking" dependents are deleted before ownerA)
err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), fetchedA.GetName(), options)
Expect(err).NotTo(HaveOccurred())
Eventually(func() bool {
err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), fetchedA.GetName(), options)
return k8serrors.IsNotFound(err)
}).Should(BeTrue())

// wait for deletion of ownerA
Eventually(func() bool {
_, err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Get(context.TODO(), ownerA.GetName(), metav1.GetOptions{})
return k8serrors.IsNotFound(err)
}).Should(BeTrue())

})

It("should not have deleted the dependent since ownerB CSV is still present", func() {
_, err := kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Get(context.TODO(), dependent.GetName(), metav1.GetOptions{})
Expect(err).NotTo(HaveOccurred(), "dependent deleted after one of the owner was deleted")
Eventually(func() error {
_, err := kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Get(context.TODO(), dependent.GetName(), metav1.GetOptions{})
return err
}).Should(Succeed(), "dependent deleted after one of the owner was deleted")
ctx.Ctx().Logf("dependent still exists after one owner was deleted")

})

})

When("removing both the owners using 'Foreground' deletion policy", func() {

BeforeEach(func() {
// delete ownerA in the foreground (to ensure any "blocking" dependents are deleted before ownerA)
err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), fetchedA.GetName(), options)
Expect(err).NotTo(HaveOccurred())
Eventually(func() bool {
err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), fetchedA.GetName(), options)
return k8serrors.IsNotFound(err)
}).Should(BeTrue())

// wait for deletion of ownerA
Eventually(func() bool {
Expand All @@ -263,8 +294,10 @@ var _ = Describe("Garbage collection for dependent resources", func() {
}).Should(BeTrue())

// delete ownerB in the foreground (to ensure any "blocking" dependents are deleted before ownerB)
err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), fetchedB.GetName(), options)
Expect(err).NotTo(HaveOccurred())
Eventually(func() bool {
err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), fetchedB.GetName(), options)
return k8serrors.IsNotFound(err)
}).Should(BeTrue())

// wait for deletion of ownerB
Eventually(func() bool {
Expand All @@ -274,9 +307,10 @@ var _ = Describe("Garbage collection for dependent resources", func() {
})

It("should have deleted the dependent since both the owners were deleted", func() {
_, err := kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Get(context.TODO(), dependent.GetName(), metav1.GetOptions{})
Expect(err).To(HaveOccurred())
Expect(k8serrors.IsNotFound(err)).To(BeTrue())
Eventually(func() bool {
_, err := kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Get(context.TODO(), dependent.GetName(), metav1.GetOptions{})
return k8serrors.IsNotFound(err)
}).Should(BeTrue(), "expected dependency configmap would be properly garabage collected")
ctx.Ctx().Logf("dependent successfully garbage collected after both owners were deleted")
})

Expand Down Expand Up @@ -359,8 +393,10 @@ var _ = Describe("Garbage collection for dependent resources", func() {

BeforeEach(func() {
// Delete subscription first
err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Delete(context.TODO(), subName, metav1.DeleteOptions{})
Expect(err).To(BeNil())
Eventually(func() bool {
err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Delete(context.TODO(), subName, metav1.DeleteOptions{})
return k8serrors.IsNotFound(err)
}).Should(BeTrue())

// wait for deletion
Eventually(func() bool {
Expand All @@ -369,8 +405,10 @@ var _ = Describe("Garbage collection for dependent resources", func() {
}).Should(BeTrue())

// Delete CSV
err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), csvName, metav1.DeleteOptions{})
Expect(err).To(BeNil())
Eventually(func() bool {
err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), csvName, metav1.DeleteOptions{})
return k8serrors.IsNotFound(err)
}).Should(BeTrue())

// wait for deletion
Eventually(func() bool {
Expand Down Expand Up @@ -427,8 +465,11 @@ var _ = Describe("Garbage collection for dependent resources", func() {
},
}

source, err := operatorClient.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred(), "could not create catalog source")
var err error
Eventually(func() error {
source, err = operatorClient.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
return err
}).Should(Succeed(), "could not create catalog source")

// Create a Subscription for package
_ = createSubscriptionForCatalog(operatorClient, source.GetNamespace(), subName, source.GetName(), packageName, channelName, "", v1alpha1.ApprovalAutomatic)
Expand Down Expand Up @@ -457,17 +498,20 @@ var _ = Describe("Garbage collection for dependent resources", func() {
var installPlanRef string

BeforeEach(func() {
// update subscription first
sub, err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Get(context.TODO(), subName, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred(), "could not get subscription")

// update channel on sub
sub.Spec.Channel = upgradeChannelName
_, err = operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Update(context.TODO(), sub, metav1.UpdateOptions{})
Expect(err).ToNot(HaveOccurred(), "could not update subscription")
Eventually(func() error {
// update subscription first
sub, err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Get(context.TODO(), subName, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("could not get subscription")
}
// update channel on sub
sub.Spec.Channel = upgradeChannelName
_, err = operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Update(context.TODO(), sub, metav1.UpdateOptions{})
return err
}).Should(Succeed(), "could not update subscription")

// Wait for the Subscription to succeed
sub, err = fetchSubscription(operatorClient, testNamespace, subName, subscriptionStateAtLatestChecker)
sub, err := fetchSubscription(operatorClient, testNamespace, subName, subscriptionStateAtLatestChecker)
Expect(err).ToNot(HaveOccurred(), "could not get subscription at latest status")

installPlanRef = sub.Status.InstallPlanRef.Name
Expand Down Expand Up @@ -530,8 +574,11 @@ var _ = Describe("Garbage collection for dependent resources", func() {
},
}

source, err := operatorClient.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
Expect(err).ToNot(HaveOccurred(), "could not create catalog source")
var err error
Eventually(func() error {
source, err = operatorClient.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
return err
}).Should(Succeed())

// Create a Subscription for package
_ = createSubscriptionForCatalog(operatorClient, source.GetNamespace(), subName, source.GetName(), packageName, channelName, "", v1alpha1.ApprovalAutomatic)
Expand Down Expand Up @@ -561,17 +608,20 @@ var _ = Describe("Garbage collection for dependent resources", func() {
var installPlanRef string

BeforeEach(func() {
// update subscription first
sub, err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Get(context.TODO(), subName, metav1.GetOptions{})
Expect(err).ToNot(HaveOccurred(), "could not get subscription")

// update channel on sub
sub.Spec.Channel = upgradeChannelName
_, err = operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Update(context.TODO(), sub, metav1.UpdateOptions{})
Expect(err).ToNot(HaveOccurred(), "could not update subscription")
Eventually(func() error {
// update subscription first
sub, err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Get(context.TODO(), subName, metav1.GetOptions{})
if err != nil {
return fmt.Errorf("could not get subscription")
}
// update channel on sub
sub.Spec.Channel = upgradeChannelName
_, err = operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Update(context.TODO(), sub, metav1.UpdateOptions{})
return err
}).Should(Succeed(), "could not update subscription")

// Wait for the Subscription to succeed
sub, err = fetchSubscription(operatorClient, testNamespace, subName, subscriptionStateAtLatestChecker)
sub, err := fetchSubscription(operatorClient, testNamespace, subName, subscriptionStateAtLatestChecker)
Expect(err).ToNot(HaveOccurred(), "could not get subscription at latest status")

installPlanRef = sub.Status.InstallPlanRef.Name
Expand Down

0 comments on commit 9163ac6

Please sign in to comment.