From 518a8465967967d56057ee8081ab4ba7412efccf Mon Sep 17 00:00:00 2001 From: Kaden Nelson Date: Wed, 11 May 2022 08:48:36 -0500 Subject: [PATCH] Add a finalizer to the ImagePolicy and ImageRepository resources Fixes https://github.com/fluxcd/image-reflector-controller/issues/225 This PR adds a finalizer to the ImagePolicy and ImageRepository resources. This is to properly record the Deleted reconciliation status when the object is deleted from the cluster. Without this change, the resource would be deleted before the image reflector controller has a chance to properly report the resource's status in the metrics. As a result, end-users may see falsely reported metrics. Signed-off-by: Kaden Nelson --- api/v1beta1/imagepolicy_types.go | 1 + api/v1beta1/imagerepository_types.go | 1 + controllers/imagepolicy_controller.go | 21 +++++++++++++++++++++ controllers/imagerepository_controller.go | 21 +++++++++++++++++++++ 4 files changed, 44 insertions(+) diff --git a/api/v1beta1/imagepolicy_types.go b/api/v1beta1/imagepolicy_types.go index c709d05f..91b74907 100644 --- a/api/v1beta1/imagepolicy_types.go +++ b/api/v1beta1/imagepolicy_types.go @@ -24,6 +24,7 @@ import ( ) const ImagePolicyKind = "ImagePolicy" +const ImagePolicyFinalizer = "finalizers.fluxcd.io" // ImagePolicySpec defines the parameters for calculating the // ImagePolicy diff --git a/api/v1beta1/imagerepository_types.go b/api/v1beta1/imagerepository_types.go index f2ea888e..3c63dbd9 100644 --- a/api/v1beta1/imagerepository_types.go +++ b/api/v1beta1/imagerepository_types.go @@ -27,6 +27,7 @@ import ( ) const ImageRepositoryKind = "ImageRepository" +const ImageRepositoryFinalizer = "finalizers.fluxcd.io" // ImageRepositorySpec defines the parameters for scanning an image // repository, e.g., `fluxcd/flux`. diff --git a/controllers/imagepolicy_controller.go b/controllers/imagepolicy_controller.go index 229556fa..f6d0f936 100644 --- a/controllers/imagepolicy_controller.go +++ b/controllers/imagepolicy_controller.go @@ -30,6 +30,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/handler" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/source" @@ -89,6 +90,26 @@ func (r *ImagePolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) } defer r.recordReadinessMetric(ctx, &pol) + // Add our finalizer if it does not exist. + if !controllerutil.ContainsFinalizer(&pol, imagev1.ImagePolicyFinalizer) { + patch := client.MergeFrom(pol.DeepCopy()) + controllerutil.AddFinalizer(&pol, imagev1.ImagePolicyFinalizer) + if err := r.Patch(ctx, &pol, patch); err != nil { + log.Error(err, "unable to register finalizer") + return ctrl.Result{}, err + } + } + + // If the object is under deletion, record the readiness, and remove our finalizer. + if !pol.ObjectMeta.DeletionTimestamp.IsZero() { + r.recordReadinessMetric(ctx, &pol) + controllerutil.RemoveFinalizer(&pol, imagev1.ImagePolicyFinalizer) + if err := r.Update(ctx, &pol); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil + } + var repo imagev1.ImageRepository repoNamespacedName := types.NamespacedName{ Namespace: pol.Namespace, diff --git a/controllers/imagerepository_controller.go b/controllers/imagerepository_controller.go index 6ccedd3a..3e9ff2a1 100644 --- a/controllers/imagerepository_controller.go +++ b/controllers/imagerepository_controller.go @@ -46,6 +46,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/controller" + "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" "sigs.k8s.io/controller-runtime/pkg/predicate" "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm" @@ -128,6 +129,26 @@ func (r *ImageRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Requ log := ctrl.LoggerFrom(ctx) + // Add our finalizer if it does not exist. + if !controllerutil.ContainsFinalizer(&imageRepo, imagev1.ImageRepositoryFinalizer) { + patch := client.MergeFrom(imageRepo.DeepCopy()) + controllerutil.AddFinalizer(&imageRepo, imagev1.ImageRepositoryFinalizer) + if err := r.Patch(ctx, &imageRepo, patch); err != nil { + log.Error(err, "unable to register finalizer") + return ctrl.Result{}, err + } + } + + // If the object is under deletion, record the readiness, and remove our finalizer. + if !imageRepo.ObjectMeta.DeletionTimestamp.IsZero() { + r.recordReadinessMetric(ctx, &imageRepo) + controllerutil.RemoveFinalizer(&imageRepo, imagev1.ImageRepositoryFinalizer) + if err := r.Update(ctx, &imageRepo); err != nil { + return ctrl.Result{}, err + } + return ctrl.Result{}, nil + } + if imageRepo.Spec.Suspend { msg := "ImageRepository is suspended, skipping reconciliation" imagev1.SetImageRepositoryReadiness(