Merge pull request #564 from fluxcd/delete-before-finalize

Handle delete before adding finalizer
fluxcd · Jul 31, 2023 · 09aa2ac · 09aa2ac
2 parents 205065a + eabd20b
commit 09aa2ac
Show file tree

Hide file tree

Showing 3 changed files with 60 additions and 11 deletions.
diff --git a/internal/controller/imageupdateautomation_controller.go b/internal/controller/imageupdateautomation_controller.go
@@ -110,7 +110,18 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr
  return ctrl.Result{}, client.IgnoreNotFound(err)
  }
 
+ // If the object is under deletion, record the readiness, and remove our finalizer.
+ if !auto.ObjectMeta.DeletionTimestamp.IsZero() {
+ controllerutil.RemoveFinalizer(&auto, imagev1.ImageUpdateAutomationFinalizer)
+ if err := r.Update(ctx, &auto); err != nil {
+ return ctrl.Result{}, err
+ }
+ return ctrl.Result{}, nil
+ }
+
  // Add our finalizer if it does not exist.
+ // Note: Finalizers in general can only be added when the deletionTimestamp
+ // is not set.
  if !controllerutil.ContainsFinalizer(&auto, imagev1.ImageUpdateAutomationFinalizer) {
  patch := client.MergeFrom(auto.DeepCopy())
  controllerutil.AddFinalizer(&auto, imagev1.ImageUpdateAutomationFinalizer)
@@ -120,15 +131,6 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr
  }
  }
 
- // If the object is under deletion, record the readiness, and remove our finalizer.
- if !auto.ObjectMeta.DeletionTimestamp.IsZero() {
- controllerutil.RemoveFinalizer(&auto, imagev1.ImageUpdateAutomationFinalizer)
- if err := r.Update(ctx, &auto); err != nil {
- return ctrl.Result{}, err
- }
- return ctrl.Result{}, nil
- }
-
  // record suspension metrics
  r.RecordSuspend(ctx, &auto, auto.Spec.Suspend)
 

diff --git a/internal/controller/suite_test.go b/internal/controller/suite_test.go
@@ -27,6 +27,7 @@ import (
  utilruntime "k8s.io/apimachinery/pkg/util/runtime"
  "k8s.io/client-go/kubernetes/scheme"
  ctrl "sigs.k8s.io/controller-runtime"
+ "sigs.k8s.io/controller-runtime/pkg/client"
 
  imagev1_reflect "github.com/fluxcd/image-reflector-controller/api/v1beta2"
  "github.com/fluxcd/pkg/runtime/controller"
@@ -44,8 +45,9 @@ import (
 // Gomega.
 
 var (
- testEnv *testenv.Environment
- ctx = ctrl.SetupSignalHandler()
+ k8sClient client.Client
+ testEnv *testenv.Environment
+ ctx = ctrl.SetupSignalHandler()
 )
 
 func init() {
@@ -74,6 +76,13 @@ func runTestsWithFeatures(m *testing.M, feats map[string]bool) int {
  testenv.WithMaxConcurrentReconciles(2),
  )
 
+ var err error
+ // Initialize a cacheless client for tests that need the latest objects.
+ k8sClient, err = client.New(testEnv.Config, client.Options{Scheme: scheme.Scheme})
+ if err != nil {
+ panic(fmt.Sprintf("failed to create k8s client: %v", err))
+ }
+
  controllerName := "image-automation-controller"
  if err := (&ImageUpdateAutomationReconciler{
  Client: testEnv,

diff --git a/internal/controller/update_test.go b/internal/controller/update_test.go
@@ -47,6 +47,7 @@ import (
  apimeta "k8s.io/apimachinery/pkg/api/meta"
  metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
  "k8s.io/apimachinery/pkg/types"
+ "k8s.io/client-go/tools/record"
  ctrl "sigs.k8s.io/controller-runtime"
  "sigs.k8s.io/controller-runtime/pkg/client"
  fakeclient "sigs.k8s.io/controller-runtime/pkg/client/fake"
@@ -124,6 +125,43 @@ func randStringRunes(n int) string {
  return string(b)
 }
 
+func TestImageUpdateAutomationReconciler_deleteBeforeFinalizer(t *testing.T) {
+ g := NewWithT(t)
+
+ namespaceName := "imageupdate-" + randStringRunes(5)
+ namespace := &corev1.Namespace{
+ ObjectMeta: metav1.ObjectMeta{Name: namespaceName},
+ }
+ g.Expect(k8sClient.Create(ctx, namespace)).ToNot(HaveOccurred())
+ t.Cleanup(func() {
+ g.Expect(k8sClient.Delete(ctx, namespace)).NotTo(HaveOccurred())
+ })
+
+ imageUpdate := &imagev1.ImageUpdateAutomation{}
+ imageUpdate.Name = "test-imageupdate"
+ imageUpdate.Namespace = namespaceName
+ imageUpdate.Spec = imagev1.ImageUpdateAutomationSpec{
+ Interval: metav1.Duration{Duration: time.Second},
+ SourceRef: imagev1.CrossNamespaceSourceReference{
+ Kind: "GitRepository",
+ Name: "foo",
+ },
+ }
+ // Add a test finalizer to prevent the object from getting deleted.
+ imageUpdate.SetFinalizers([]string{"test-finalizer"})
+ g.Expect(k8sClient.Create(ctx, imageUpdate)).NotTo(HaveOccurred())
+ // Add deletion timestamp by deleting the object.
+ g.Expect(k8sClient.Delete(ctx, imageUpdate)).NotTo(HaveOccurred())
+
+ r := &ImageUpdateAutomationReconciler{
+ Client: k8sClient,
+ EventRecorder: record.NewFakeRecorder(32),
+ }
+ // NOTE: Only a real API server responds with an error in this scenario.
+ _, err := r.Reconcile(ctx, ctrl.Request{NamespacedName: client.ObjectKeyFromObject(imageUpdate)})
+ g.Expect(err).NotTo(HaveOccurred())
+}
+
 func TestImageAutomationReconciler_commitMessage(t *testing.T) {
  policySpec := imagev1_reflect.ImagePolicySpec{
  ImageRepositoryRef: meta.NamespacedObjectReference{