Skip to content

Commit

Permalink
Fix flaky test related to patching finalizer
Browse files Browse the repository at this point in the history
In the TestAlertReconciler and TestProviderReconciler migration to
static object tests, the objects are patched with specific finalizers
and checked if they get removed by the migration reconciler. The test
set up step, that doesn't involve the reconciler, results in a timeout
waiting for the patched finalizer to appear on the object. Increasing
the timeout doesn't help. Updating the object, instead of patching works
around this issue.
A similar issue has been observed in fluxcd/pkg/runtime/patch package
tests where the patching behavior is tested, likely related to
finalizers, result in flaky tests.

Signed-off-by: Sunny <[email protected]>
  • Loading branch information
darkowlzz committed Oct 25, 2023
1 parent ee3b54f commit 823a0b8
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 12 deletions.
22 changes: 16 additions & 6 deletions internal/controller/alert_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
. "github.com/onsi/gomega"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/util/retry"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

Expand Down Expand Up @@ -81,15 +82,24 @@ func TestAlertReconciler(t *testing.T) {

// Remove finalizer at delete.

patchHelper, err = patch.NewHelper(alert, testEnv.Client)
g.Expect(err).ToNot(HaveOccurred())

// Suspend the alert to prevent finalizer from getting removed.
// Ensure only flux finalizer is set to allow the object to be garbage
// collected at the end.
alert.ObjectMeta.Finalizers = []string{apiv1.NotificationFinalizer}
alert.Spec.Suspend = true
g.Expect(patchHelper.Patch(ctx, alert)).ToNot(HaveOccurred())
suspendAndFinalize := func(a *apiv1beta3.Alert) {
a.ObjectMeta.Finalizers = []string{apiv1.NotificationFinalizer}
a.Spec.Suspend = true
}
// NOTE: Use Update() instead of Patch() here because patch results in flaky
// tests. The patched finalizers don't seem to register sometimes due to
// which the test times out waiting for the patched finalizer to appear.
// This is not handled by the reconciler. Maybe a bug in PATCH requests
// handling in the envtest kube-apiserver.
err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {
g.Expect(k8sClient.Get(ctx, alertKey, alert)).ToNot(HaveOccurred())
suspendAndFinalize(alert)
return testEnv.Update(ctx, alert)
})
g.Expect(err).ToNot(HaveOccurred())

// Verify that finalizer exists on the object using a live client.
g.Eventually(func() bool {
Expand Down
22 changes: 16 additions & 6 deletions internal/controller/provider_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
. "github.com/onsi/gomega"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/client-go/util/retry"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"

Expand Down Expand Up @@ -79,15 +80,24 @@ func TestProviderReconciler(t *testing.T) {

// Remove finalizer at delete.

patchHelper, err = patch.NewHelper(provider, testEnv.Client)
g.Expect(err).ToNot(HaveOccurred())

// Suspend the provider to prevent finalizer from getting removed.
// Ensure only flux finalizer is set to allow the object to be garbage
// collected at the end.
provider.ObjectMeta.Finalizers = []string{apiv1.NotificationFinalizer}
provider.Spec.Suspend = true
g.Expect(patchHelper.Patch(ctx, provider)).ToNot(HaveOccurred())
suspendAndFinalize := func(p *apiv1beta3.Provider) {
p.ObjectMeta.Finalizers = []string{apiv1.NotificationFinalizer}
p.Spec.Suspend = true
}
// NOTE: Use Update() instead of Patch() here because patch results in flaky
// tests. The patched finalizers don't seem to register sometimes due to
// which the test times out waiting for the patched finalizer to appear.
// This is not handled by the reconciler. Maybe a bug in PATCH requests
// handling in the envtest kube-apiserver.
err = retry.RetryOnConflict(retry.DefaultBackoff, func() error {
g.Expect(k8sClient.Get(ctx, providerKey, provider)).ToNot(HaveOccurred())
suspendAndFinalize(provider)
return testEnv.Update(ctx, provider)
})
g.Expect(err).ToNot(HaveOccurred())

// Verify that finalizer exists on the object using a live client.
g.Eventually(func() bool {
Expand Down

0 comments on commit 823a0b8

Please sign in to comment.