From 8fb5a1ed56b09570f82cf9f689c968532d960ba9 Mon Sep 17 00:00:00 2001 From: Sunny Date: Thu, 2 Feb 2023 21:15:01 +0000 Subject: [PATCH] helmrepo-oci: check before rec on type switching When a HelmRepository with "default" spec.type is switched to "oci", the existing HelmRepository is processed by HelmRepositoryReconciler by running reconcileDelete() which removes all the previous status information and allows the HelmRepositoryOCIReconciler to process the object and add its own status data. But at times, when HelmRepositoryOCIReconciler starts processing a HelmRepository with stale status data from the client cache, it contains the stale conditions that are owned only by HelmRepositoryReconciler and isn't managed by HelmRepositoryOCIReconciler, it results in situations where Ready is marked as True with the latest generation of the object and the unmanaged stale conditions remain in the previous generation, resulting in unexpected status conditions. In the observed flaky tests, TestHelmRepositoryReconciler_ReconcileTypeUpdatePredicateFilter would fail because of stale ArtifactInStorage condition with previous generation value. This change adds a check in the HelmRepositoryOCIReconciler to start processing the object only once the stale conditions owned by HelmRepositoryReconciler have been removed. Signed-off-by: Sunny --- controllers/helmrepository_controller_oci.go | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index e971a11eb..d3e8f9870 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -67,6 +67,14 @@ var helmRepositoryOCINegativeConditions = []string{ meta.ReconcilingCondition, } +// Conditions owned by HelmRepositoryReconciler. +var helmRepositoryOnlyConditions = []string{ + sourcev1.StorageOperationFailedCondition, + sourcev1.FetchFailedCondition, + sourcev1.ArtifactOutdatedCondition, + sourcev1.ArtifactInStorageCondition, +} + // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmrepositories,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmrepositories/status,verbs=get;update;patch // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmrepositories/finalizers,verbs=get;create;update;patch;delete @@ -124,6 +132,15 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re return ctrl.Result{}, client.IgnoreNotFound(err) } + // If the object contains any of the conditions managed by + // HelmRepositoryReconciler, requeue and wait for those conditions to be + // removed first before processing the object. + // NOTE: This will happen only when a HelmRepository's spec.type is switched + // from "default" to "oci". + if conditions.HasAny(obj, helmRepositoryOnlyConditions) { + return ctrl.Result{RequeueAfter: time.Second}, nil + } + // Record suspended status metric r.RecordSuspend(ctx, obj, obj.Spec.Suspend)