diff --git a/api/v1beta2/condition_types.go b/api/v1beta2/condition_types.go index 711469eb8..309699f04 100644 --- a/api/v1beta2/condition_types.go +++ b/api/v1beta2/condition_types.go @@ -19,6 +19,13 @@ package v1beta2 const SourceFinalizer = "finalizers.fluxcd.io" const ( + // AddFailedCondition indicates a transient or persistent failure + // to add an upstream Source. + // If True, the reconciliation failed while adding the source. + // This is a "negative polarity" or "abnormal-true" type, and is only + // present on the resource if it is True. + AddFailedCondition = "AddFailed" + // ArtifactInStorageCondition indicates the availability of the Artifact in // the storage. // If True, the Artifact is stored successfully. diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index 5e117d825..43463de01 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -577,8 +577,8 @@ func (r *HelmRepositoryReconciler) garbageCollect(ctx context.Context, obj *sour // Clean status sub-resource obj.Status.Artifact = nil obj.Status.URL = "" - // Remove the condition as the artifact doesn't exist. - conditions.Delete(obj, sourcev1.ArtifactInStorageCondition) + // Remove any stale conditions + obj.Status.Conditions = nil return nil } if obj.GetArtifact() != nil { diff --git a/controllers/helmrepository_controller_oci.go b/controllers/helmrepository_controller_oci.go index e78984818..c314df669 100644 --- a/controllers/helmrepository_controller_oci.go +++ b/controllers/helmrepository_controller_oci.go @@ -51,18 +51,18 @@ import ( var helmRepositoryOCIReadyCondition = summarize.Conditions{ Target: meta.ReadyCondition, Owned: []string{ - sourcev1.FetchFailedCondition, + sourcev1.AddFailedCondition, meta.ReadyCondition, meta.ReconcilingCondition, meta.StalledCondition, }, Summarize: []string{ - sourcev1.FetchFailedCondition, + sourcev1.AddFailedCondition, meta.StalledCondition, meta.ReconcilingCondition, }, NegativePolarity: []string{ - sourcev1.FetchFailedCondition, + sourcev1.AddFailedCondition, meta.StalledCondition, meta.ReconcilingCondition, }, @@ -71,7 +71,7 @@ var helmRepositoryOCIReadyCondition = summarize.Conditions{ // helmRepositoryOCIFailConditions contains the conditions that represent a // failure. var helmRepositoryOCIFailConditions = []string{ - sourcev1.FetchFailedCondition, + sourcev1.AddFailedCondition, } // +kubebuilder:rbac:groups=source.toolkit.fluxcd.io,resources=helmrepositories,verbs=get;list;watch;create;update;patch;delete @@ -188,7 +188,8 @@ func (r *HelmRepositoryOCIReconciler) Reconcile(ctx context.Context, req ctrl.Re // Examine if a type change has happened and act accordingly if obj.Spec.Type != sourcev1.HelmRepositoryTypeOCI { - // just ignore the object if the type has changed + // remove any stale condition and ignore the object if the type has changed + obj.Status.Conditions = nil recResult, retErr = sreconcile.ResultEmpty, nil return } @@ -258,10 +259,10 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *source func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj *sourcev1.HelmRepository) (sreconcile.Result, error) { if !helmreg.IsOCI(obj.Spec.URL) { e := &serror.Stalling{ - Err: fmt.Errorf("the url scheme is not supported: %s", obj.Spec.URL), + Err: fmt.Errorf("the URL scheme is not supported: %s", obj.Spec.URL), Reason: sourcev1.URLInvalidReason, } - conditions.MarkFalse(obj, meta.ReadyCondition, e.Reason, e.Err.Error()) + conditions.MarkTrue(obj, sourcev1.AddFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -279,7 +280,7 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj * Err: fmt.Errorf("failed to get secret '%s': %w", name.String(), err), Reason: sourcev1.AuthenticationFailedReason, } - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + conditions.MarkTrue(obj, sourcev1.AddFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -290,7 +291,7 @@ func (r *HelmRepositoryOCIReconciler) reconcileSource(ctx context.Context, obj * Err: fmt.Errorf("failed to configure Helm client with secret data: %w", err), Reason: sourcev1.AuthenticationFailedReason, } - conditions.MarkTrue(obj, sourcev1.FetchFailedCondition, e.Reason, e.Err.Error()) + conditions.MarkTrue(obj, sourcev1.AddFailedCondition, e.Reason, e.Err.Error()) // Return err as the content of the secret may change. return sreconcile.ResultEmpty, e } @@ -309,10 +310,10 @@ func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *s registryClient, file, err := r.RegistryClientGenerator(logOpts != nil) if err != nil { e := &serror.Event{ - Err: fmt.Errorf("failed to create registry client:: %w", err), + Err: fmt.Errorf("failed to create registry client: %w", err), Reason: meta.FailedReason, } - conditions.MarkFalse(obj, meta.ReadyCondition, e.Reason, e.Err.Error()) + conditions.MarkTrue(obj, sourcev1.AddFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } @@ -331,23 +332,25 @@ func (r *HelmRepositoryOCIReconciler) validateSource(ctx context.Context, obj *s Err: fmt.Errorf("failed to parse URL '%s': %w", obj.Spec.URL, err), Reason: sourcev1.URLInvalidReason, } - conditions.MarkFalse(obj, meta.ReadyCondition, e.Reason, e.Err.Error()) + conditions.MarkTrue(obj, sourcev1.AddFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } - // Attempt to login to the registry if credentials are provided. if logOpts != nil { err = chartRepo.Login(logOpts...) if err != nil { e := &serror.Event{ - Err: fmt.Errorf("failed to create temporary file: %w", err), + Err: fmt.Errorf("failed to login to repository '%s': %w", obj.Spec.URL, err), Reason: meta.FailedReason, } - conditions.MarkFalse(obj, meta.ReadyCondition, e.Reason, e.Err.Error()) + conditions.MarkTrue(obj, sourcev1.AddFailedCondition, e.Reason, e.Err.Error()) return sreconcile.ResultEmpty, e } } + // Delete any stale failure observation + conditions.Delete(obj, sourcev1.AddFailedCondition) + conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "Helm repository %q is ready", obj.Name) return sreconcile.ResultSuccess, nil diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go index 24484a427..d05fe812d 100644 --- a/controllers/helmrepository_controller_test.go +++ b/controllers/helmrepository_controller_test.go @@ -1179,6 +1179,9 @@ func TestHelmRepositoryReconciler_ReconcileTypeUpdatePredicateFilter(t *testing. return false } readyCondition := conditions.Get(obj, meta.ReadyCondition) + if readyCondition == nil { + return false + } return readyCondition.Status == metav1.ConditionTrue && newGen == readyCondition.ObservedGeneration && newGen == obj.Status.ObservedGeneration