Skip to content

Commit cf49231

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 cf49231

File tree

1 file changed

+122
-72
lines changed

1 file changed

+122
-72
lines changed

‎test/e2e/gc_e2e_test.go

Lines changed: 122 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,22 +71,32 @@ 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() {
9198

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

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

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

106117
It("should delete the associated ClusterRole", func() {
107-
108118
Eventually(func() bool {
109119
_, err := kubeClient.GetClusterRole(cr.GetName())
110120
return k8serrors.IsNotFound(err)
@@ -117,16 +127,14 @@ var _ = Describe("Garbage collection for dependent resources", func() {
117127
Context("Given a ClusterRole owned by a APIService", func() {
118128

119129
var (
120-
as *apiregistrationv1.APIService
121-
cr *rbacv1.ClusterRole
130+
apiService *apiregistrationv1.APIService
131+
cr *rbacv1.ClusterRole
122132
)
123133

124134
BeforeEach(func() {
125135
group := rand.String(16)
126136

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

142-
// Create a ClusterRole
143-
cr, err = kubeClient.CreateClusterRole(&rbacv1.ClusterRole{
157+
cr = &rbacv1.ClusterRole{
144158
ObjectMeta: metav1.ObjectMeta{
145159
GenerateName: "clusterrole-",
146-
OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(as)},
160+
OwnerReferences: []metav1.OwnerReference{ownerutil.NonBlockingOwner(apiService)},
147161
},
148-
})
149-
Expect(err).NotTo(HaveOccurred())
162+
}
163+
164+
Eventually(func() error {
165+
// Create a ClusterRole
166+
cr, err = kubeClient.CreateClusterRole(cr)
167+
return err
168+
}).Should(Succeed())
150169
})
151170

152171
AfterEach(func() {
153172

154173
IgnoreError(kubeClient.DeleteClusterRole(cr.GetName(), &metav1.DeleteOptions{}))
155174

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

158177
})
159178

160179
When("APIService is deleted", func() {
161180

162181
BeforeEach(func() {
163182
// Delete API service
164-
Expect(kubeClient.DeleteAPIService(as.GetName(), &metav1.DeleteOptions{})).To(Succeed())
183+
Eventually(func() bool {
184+
err := kubeClient.DeleteAPIService(apiService.GetName(), &metav1.DeleteOptions{})
185+
return k8serrors.IsNotFound(err)
186+
}).Should(BeTrue())
165187
})
166188

167189
It("should delete the associated ClusterRole", func() {
@@ -193,18 +215,22 @@ var _ = Describe("Garbage collection for dependent resources", func() {
193215
propagation metav1.DeletionPropagation
194216
options metav1.DeleteOptions
195217
)
196-
197218
BeforeEach(func() {
198219

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

202223
// create all owners
203224
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())
225+
Eventually(func() error {
226+
fetchedA, err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(context.TODO(), &ownerA, metav1.CreateOptions{})
227+
return err
228+
}).Should(Succeed())
229+
230+
Eventually(func() error {
231+
fetchedB, err = operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Create(context.TODO(), &ownerB, metav1.CreateOptions{})
232+
return err
233+
}).Should(Succeed())
208234

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

220246
// 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")
247+
Eventually(func() error {
248+
_, err = kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Create(context.TODO(), dependent, metav1.CreateOptions{})
249+
return err
250+
}).Should(Succeed(), "dependent could not be created")
223251

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

230258
BeforeEach(func() {
231259
// 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())
260+
Eventually(func() bool {
261+
err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), fetchedA.GetName(), options)
262+
return k8serrors.IsNotFound(err)
263+
}).Should(BeTrue())
234264

235265
// wait for deletion of ownerA
236266
Eventually(func() bool {
237267
_, err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Get(context.TODO(), ownerA.GetName(), metav1.GetOptions{})
238268
return k8serrors.IsNotFound(err)
239269
}).Should(BeTrue())
240-
241270
})
242271

243272
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")
273+
Eventually(func() error {
274+
_, err := kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Get(context.TODO(), dependent.GetName(), metav1.GetOptions{})
275+
return err
276+
}).Should(Succeed(), "dependent deleted after one of the owner was deleted")
246277
ctx.Ctx().Logf("dependent still exists after one owner was deleted")
247-
248278
})
249-
250279
})
251280

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

254283
BeforeEach(func() {
255284
// 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())
285+
Eventually(func() bool {
286+
err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), fetchedA.GetName(), options)
287+
return k8serrors.IsNotFound(err)
288+
}).Should(BeTrue())
258289

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

265296
// 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())
297+
Eventually(func() bool {
298+
err := operatorClient.OperatorsV1alpha1().ClusterServiceVersions(testNamespace).Delete(context.TODO(), fetchedB.GetName(), options)
299+
return k8serrors.IsNotFound(err)
300+
}).Should(BeTrue())
268301

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

276309
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())
310+
Eventually(func() bool {
311+
_, err := kubeClient.KubernetesInterface().CoreV1().ConfigMaps(testNamespace).Get(context.TODO(), dependent.GetName(), metav1.GetOptions{})
312+
return k8serrors.IsNotFound(err)
313+
}).Should(BeTrue(), "expected dependency configmap would be properly garabage collected")
280314
ctx.Ctx().Logf("dependent successfully garbage collected after both owners were deleted")
281315
})
282316

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

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

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

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

375413
// wait for deletion
376414
Eventually(func() bool {
@@ -427,8 +465,11 @@ var _ = Describe("Garbage collection for dependent resources", func() {
427465
},
428466
}
429467

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

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

459500
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")
501+
Eventually(func() error {
502+
// update subscription first
503+
sub, err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Get(context.TODO(), subName, metav1.GetOptions{})
504+
if err != nil {
505+
return fmt.Errorf("could not get subscription")
506+
}
507+
// update channel on sub
508+
sub.Spec.Channel = upgradeChannelName
509+
_, err = operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Update(context.TODO(), sub, metav1.UpdateOptions{})
510+
return err
511+
}).Should(Succeed(), "could not update subscription")
468512

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

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

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

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

563610
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")
611+
Eventually(func() error {
612+
// update subscription first
613+
sub, err := operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Get(context.TODO(), subName, metav1.GetOptions{})
614+
if err != nil {
615+
return fmt.Errorf("could not get subscription")
616+
}
617+
// update channel on sub
618+
sub.Spec.Channel = upgradeChannelName
619+
_, err = operatorClient.OperatorsV1alpha1().Subscriptions(testNamespace).Update(context.TODO(), sub, metav1.UpdateOptions{})
620+
return err
621+
}).Should(Succeed(), "could not update subscription")
572622

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

577627
installPlanRef = sub.Status.InstallPlanRef.Name

0 commit comments

Comments
 (0)