diff --git a/internal/controller/imageupdateautomation_controller.go b/internal/controller/imageupdateautomation_controller.go index cf8b5516..22448d5c 100644 --- 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 index 5d69cc18..3146169c 100644 --- 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 index 5597a76f..372f49a2 100644 --- 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{