Skip to content

Commit

Permalink
helmrepo-oci: check before rec on type switching
Browse files Browse the repository at this point in the history
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 <darkowlzz@protonmail.com>
  • Loading branch information
darkowlzz committed Feb 2, 2023
1 parent 9d640b6 commit 8fb5a1e
Showing 1 changed file with 17 additions and 0 deletions.
17 changes: 17 additions & 0 deletions controllers/helmrepository_controller_oci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down

0 comments on commit 8fb5a1e

Please sign in to comment.