Skip to content

Commit b25b06f

Browse files
timflannagantimflannagan
committed
test/e2e: Wrap flake-prone garbage collection tests in eventually assertions
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 <[email protected]>
1 parent 07dd3a0 commit b25b06f

File tree

1 file changed

+121
-72
lines changed

1 file changed

+121
-72
lines changed

‎test/e2e/gc_e2e_test.go

Lines changed: 121 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,7 @@ var _ = Describe("Garbage collection for dependent resources", func() {
4747
BeforeEach(func() {
4848
group := fmt.Sprintf("%s.com", rand.String(16))
4949

50-
// Create a CustomResourceDefinition
51-
var err error
52-
crd, err = kubeClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Create(context.TODO(), &apiextensionsv1.CustomResourceDefinition{
50+
crd = &apiextensionsv1.CustomResourceDefinition{
5351
ObjectMeta: metav1.ObjectMeta{
5452
Name: fmt.Sprintf("plural.%s", group),
5553
},
@@ -73,18 +71,27 @@ var _ = Describe("Garbage collection for dependent resources", func() {
7371
ListKind: "KindList",
7472
},
7573
},
76-
}, metav1.CreateOptions{})
77-
Expect(err).NotTo(HaveOccurred())
74+
}
7875

79-
// Create a ClusterRole for the crd
80-
cr, err = kubeClient.CreateClusterRole(&rbacv1.ClusterRole{
76+
// Create a CustomResourceDefinition
77+
var err error
78+
Eventually(func() error {
79+
crd, err = kubeClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Create(context.TODO(), crd, metav1.CreateOptions{})
80+
return err
81+
}).Should(Succeed())
82+
83+
cr = &rbacv1.ClusterRole{
8184
ObjectMeta: metav1.ObjectMeta{
8285
GenerateName: "clusterrole-",
8386
OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(crd)},
8487
},
85-
})
86-
Expect(err).NotTo(HaveOccurred())
88+
}
8789

90+
// Create a ClusterRole for the crd
91+
Eventually(func() error {
92+
cr, err = kubeClient.CreateClusterRole(cr)
93+
return err
94+
}).Should(Succeed())
8895
})
8996

9097
AfterEach(func() {
@@ -100,11 +107,13 @@ var _ = Describe("Garbage collection for dependent resources", func() {
100107

101108
BeforeEach(func() {
102109
// Delete CRD
103-
Expect(kubeClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Delete(context.TODO(), crd.GetName(), metav1.DeleteOptions{})).To(Succeed())
110+
Eventually(func() bool {
111+
err := kubeClient.ApiextensionsInterface().ApiextensionsV1().CustomResourceDefinitions().Delete(context.TODO(), crd.GetName(), metav1.DeleteOptions{})
112+
return k8serrors.IsNotFound(err)
113+
}).Should(BeTrue())
104114
})
105115

106116
It("should delete the associated ClusterRole", func() {
107-
108117
Eventually(func() bool {
109118
_, err := kubeClient.GetClusterRole(cr.GetName())
110119
return k8serrors.IsNotFound(err)
@@ -117,16 +126,14 @@ var _ = Describe("Garbage collection for dependent resources", func() {
117126
Context("Given a ClusterRole owned by a APIService", func() {
118127

119128
var (
120-
as *apiregistrationv1.APIService
121-
cr *rbacv1.ClusterRole
129+
apiService *apiregistrationv1.APIService
130+
cr *rbacv1.ClusterRole
122131
)
123132

124133
BeforeEach(func() {
125134
group := rand.String(16)
126135

127-
// Create an API Service
128-
var err error
129-
as, err = kubeClient.CreateAPIService(&apiregistrationv1.APIService{
136+
apiService = &apiregistrationv1.APIService{
130137
ObjectMeta: metav1.ObjectMeta{
131138
Name: fmt.Sprintf("v1.%s", group),
132139
},
@@ -136,32 +143,46 @@ var _ = Describe("Garbage collection for dependent resources", func() {
136143
GroupPriorityMinimum: 1,
137144
VersionPriority: 1,
138145
},
139-
})
140-
Expect(err).NotTo(HaveOccurred())
146+
}
147+
// Create an API Service
148+
var err error
149+
Eventually(func() error {
150+
// TODO(tflannag): Is comparing error to apierrors.IsAlreadyExists prone to masking
151+
// incorrect testing assumptions?
152+
apiService, err = kubeClient.CreateAPIService(apiService)
153+
return err
154+
}).Should(Succeed())
141155

142-
// Create a ClusterRole
143-
cr, err = kubeClient.CreateClusterRole(&rbacv1.ClusterRole{
156+
cr = &rbacv1.ClusterRole{
144157
ObjectMeta: metav1.ObjectMeta{
145158
GenerateName: "clusterrole-",
146-
OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(as)},
159+
OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(apiService)},
147160
},
148-
})
149-
Expect(err).NotTo(HaveOccurred())
161+
}
162+
163+
Eventually(func() error {
164+
// Create a ClusterRole
165+
cr, err = kubeClient.CreateClusterRole(cr)
166+
return err
167+
}).Should(Succeed())
150168
})
151169

152170
AfterEach(func() {
153171

154172
IgnoreError(kubeClient.DeleteClusterRole(cr.GetName(), &metav1.DeleteOptions{}))
155173

156-
IgnoreError(kubeClient.DeleteAPIService(as.GetName(), &metav1.DeleteOptions{}))
174+
IgnoreError(kubeClient.DeleteAPIService(apiService.GetName(), &metav1.DeleteOptions{}))
157175

158176
})
159177

160178
When("APIService is deleted", func() {
161179

162180
BeforeEach(func() {
163181
// Delete API service
164-
Expect(kubeClient.DeleteAPIService(as.GetName(), &metav1.DeleteOptions{})).To(Succeed())
182+
Eventually(func() bool {
183+
err := kubeClient.DeleteAPIService(apiService.GetName(), &metav1.DeleteOptions{})
184+
return k8serrors.IsNotFound(err)
185+
}).Should(BeTrue())
165186
})
166187

167188
It("should delete the associated ClusterRole", func() {
@@ -193,18 +214,22 @@ var _ = Describe("Garbage collection for dependent resources", func() {
193214
propagation metav1.DeletionPropagation
194215
options metav1.DeleteOptions
195216
)
196-
197217
BeforeEach(func() {
198218

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

202222
// create all owners
203223
var err error
204-
fetchedA, err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(context.TODO(), &ownerA, metav1.CreateOptions{})
205-
Expect(err).NotTo(HaveOccurred())
206-
fetchedB, err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(context.TODO(), &ownerB, metav1.CreateOptions{})
207-
Expect(err).NotTo(HaveOccurred())
224+
Eventually(func() error {
225+
fetchedA, err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(context.TODO(), &ownerA, metav1.CreateOptions{})
226+
return err
227+
}).Should(Succeed())
228+
229+
Eventually(func() error {
230+
fetchedB, err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(context.TODO(), &ownerB, metav1.CreateOptions{})
231+
return err
232+
}).Should(Succeed())
208233

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

220245
// create ConfigMap dependent
221-
_, err = kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Create(context.TODO(), dependent, metav1.CreateOptions{})
222-
Expect(err).NotTo(HaveOccurred(), "dependent could not be created")
246+
Eventually(func() error {
247+
_, err = kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Create(context.TODO(), dependent, metav1.CreateOptions{})
248+
return err
249+
}).Should(Succeed(), "dependent could not be created")
223250

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

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

235264
// wait for deletion of ownerA
236265
Eventually(func() bool {
237266
_, err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Get(context.TODO(), ownerA.GetName(), metav1.GetOptions{})
238267
return k8serrors.IsNotFound(err)
239268
}).Should(BeTrue())
240-
241269
})
242270

243271
It("should not have deleted the dependent since ownerB CSV is still present", func() {
244-
_, err := kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Get(context.TODO(), dependent.GetName(), metav1.GetOptions{})
245-
Expect(err).NotTo(HaveOccurred(), "dependent deleted after one of the owner was deleted")
272+
Eventually(func() error {
273+
_, err := kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Get(context.TODO(), dependent.GetName(), metav1.GetOptions{})
274+
return err
275+
}).Should(Succeed(), "dependent deleted after one of the owner was deleted")
246276
ctx.Ctx().Logf("dependent still exists after one owner was deleted")
247-
248277
})
249-
250278
})
251279

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

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

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

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

269301
// wait for deletion of ownerB
270302
Eventually(func() bool {
@@ -274,9 +306,10 @@ var _ = Describe("Garbage collection for dependent resources", func() {
274306
})
275307

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

@@ -359,8 +392,10 @@ var _ = Describe("Garbage collection for dependent resources", func() {
359392

360393
BeforeEach(func() {
361394
// Delete subscription first
362-
err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Delete(context.TODO(), subName, metav1.DeleteOptions{})
363-
Expect(err).To(BeNil())
395+
Eventually(func() bool {
396+
err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Delete(context.TODO(), subName, metav1.DeleteOptions{})
397+
return k8serrors.IsNotFound(err)
398+
}).Should(BeTrue())
364399

365400
// wait for deletion
366401
Eventually(func() bool {
@@ -369,8 +404,10 @@ var _ = Describe("Garbage collection for dependent resources", func() {
369404
}).Should(BeTrue())
370405

371406
// Delete CSV
372-
err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), csvName, metav1.DeleteOptions{})
373-
Expect(err).To(BeNil())
407+
Eventually(func() bool {
408+
err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), csvName, metav1.DeleteOptions{})
409+
return k8serrors.IsNotFound(err)
410+
}).Should(BeTrue())
374411

375412
// wait for deletion
376413
Eventually(func() bool {
@@ -427,8 +464,11 @@ var _ = Describe("Garbage collection for dependent resources", func() {
427464
},
428465
}
429466

430-
source, err := operatorClient.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
431-
Expect(err).ToNot(HaveOccurred(), "could not create catalog source")
467+
var err error
468+
Eventually(func() error {
469+
source, err = operatorClient.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
470+
return err
471+
}).Should(Succeed(), "could not create catalog source")
432472

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

459499
BeforeEach(func() {
460-
// update subscription first
461-
sub, err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Get(context.TODO(), subName, metav1.GetOptions{})
462-
Expect(err).ToNot(HaveOccurred(), "could not get subscription")
463-
464-
// update channel on sub
465-
sub.Spec.Channel = upgradeChannelName
466-
_, err = operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Update(context.TODO(), sub, metav1.UpdateOptions{})
467-
Expect(err).ToNot(HaveOccurred(), "could not update subscription")
500+
Eventually(func() error {
501+
// update subscription first
502+
sub, err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Get(context.TODO(), subName, metav1.GetOptions{})
503+
if err != nil {
504+
return fmt.Errorf("could not get subscription")
505+
}
506+
// update channel on sub
507+
sub.Spec.Channel = upgradeChannelName
508+
_, err = operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Update(context.TODO(), sub, metav1.UpdateOptions{})
509+
return err
510+
}).Should(Succeed(), "could not update subscription")
468511

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

473516
installPlanRef = sub.Status.InstallPlanRef.Name
@@ -530,8 +573,11 @@ var _ = Describe("Garbage collection for dependent resources", func() {
530573
},
531574
}
532575

533-
source, err := operatorClient.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
534-
Expect(err).ToNot(HaveOccurred(), "could not create catalog source")
576+
var err error
577+
Eventually(func() error {
578+
source, err = operatorClient.OperatorsV1alpha1().CatalogSources(source.GetNamespace()).Create(context.TODO(), source, metav1.CreateOptions{})
579+
return err
580+
}).Should(Succeed())
535581

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

563609
BeforeEach(func() {
564-
// update subscription first
565-
sub, err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Get(context.TODO(), subName, metav1.GetOptions{})
566-
Expect(err).ToNot(HaveOccurred(), "could not get subscription")
567-
568-
// update channel on sub
569-
sub.Spec.Channel = upgradeChannelName
570-
_, err = operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Update(context.TODO(), sub, metav1.UpdateOptions{})
571-
Expect(err).ToNot(HaveOccurred(), "could not update subscription")
610+
Eventually(func() error {
611+
// update subscription first
612+
sub, err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Get(context.TODO(), subName, metav1.GetOptions{})
613+
if err != nil {
614+
return fmt.Errorf("could not get subscription")
615+
}
616+
// update channel on sub
617+
sub.Spec.Channel = upgradeChannelName
618+
_, err = operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Update(context.TODO(), sub, metav1.UpdateOptions{})
619+
return err
620+
}).Should(Succeed(), "could not update subscription")
572621

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

577626
installPlanRef = sub.Status.InstallPlanRef.Name

0 commit comments

Comments
 (0)