removed progressive status from error cases

Author: Skarlso

api/v1alpha1/condition_types.go

@@ -20,7 +20,7 @@ const (
 	// ConvertComponentDescriptorFailedReason is used when the Component Descriptor cannot be converted.
 	ConvertComponentDescriptorFailedReason = "ConvertComponentDescriptorFailed"
 
-	// NameGenerationFailedReason is used when the componentn name could not be generated.
+	// NameGenerationFailedReason is used when the component name could not be generated.
 	NameGenerationFailedReason = "NameGenerationFailed"
 
 	// CreateOrUpdateComponentDescriptorFailedReason is used when the Component Descriptor cannot be created or updated on the resource.
@@ -44,6 +44,12 @@ const (
 	// ComponentDescriptorNotFoundReason is used when the component descriptor cannot be found.
 	ComponentDescriptorNotFoundReason = "ComponentDescriptorNotFound"
 
+	// ComponentVersionNotFoundReason is used when the component version cannot be found.
+	ComponentVersionNotFoundReason = "ComponentVersionNotFound"
+
+	// ComponentVersionNotReadyReason is used when the component version is not ready.
+	ComponentVersionNotReadyReason = "ComponentVersionNotReady"
+
 	// CreateOrUpdateSnapshotFailedReason is used when the snapshot cannot be created or updated.
 	CreateOrUpdateSnapshotFailedReason = "CreateOrUpdateSnapshotFailed"
 

api/v1alpha1/localization_types.go

@@ -28,6 +28,17 @@ type Localization struct {
 	Status MutationStatus `json:"status,omitempty"`
 }
 
+func (in *Localization) GetVID() map[string]string {
+	metadata := make(map[string]string)
+	metadata[GroupVersion.Group+"/localization_digest"] = in.Status.LatestSnapshotDigest
+
+	return metadata
+}
+
+func (in *Localization) SetObservedGeneration(v int64) {
+	in.Status.ObservedGeneration = v
+}
+
 // GetConditions returns the conditions of the Localization.
 func (in *Localization) GetConditions() []metav1.Condition {
 	return in.Status.Conditions

api/v1alpha1/resource_types.go

@@ -74,6 +74,17 @@ type Resource struct {
 	Status ResourceStatus `json:"status,omitempty"`
 }
 
+func (in *Resource) GetVID() map[string]string {
+	metadata := make(map[string]string)
+	metadata[GroupVersion.Group+"/resource_version"] = in.Status.LastAppliedResourceVersion
+
+	return metadata
+}
+
+func (in *Resource) SetObservedGeneration(v int64) {
+	in.Status.ObservedGeneration = v
+}
+
 // GetConditions returns the conditions of the Resource.
 func (in *Resource) GetConditions() []metav1.Condition {
 	return in.Status.Conditions

api/v1alpha1/snapshot_types.go

@@ -53,6 +53,17 @@ type SnapshotStatus struct {
 	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
 }
 
+func (in *Snapshot) GetVID() map[string]string {
+	metadata := make(map[string]string)
+	metadata[GroupVersion.Group+"/snapshot_digest"] = in.Status.LastReconciledDigest
+
+	return metadata
+}
+
+func (in *Snapshot) SetObservedGeneration(v int64) {
+	in.Status.ObservedGeneration = v
+}
+
 // GetComponentVersion returns the component version for the snapshot
 func (in Snapshot) GetComponentVersion() string {
 	return in.Spec.Identity[ComponentVersionKey]

controllers/componentversion_controller.go

@@ -101,10 +101,11 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	if err != nil {
 		// we don't fail here, because all manifests might have been applied at once or the secret
 		// for authentication is being reconciled.
-		_ = r.markAsFailed(
+		MarkAsFailed(
+			r.EventRecorder,
 			obj,
 			v1alpha1.AuthenticatedContextCreationFailedReason,
-			fmt.Errorf("authentication failed for repository: %s with error: %s", obj.Spec.Repository.URL, err),
+			fmt.Sprintf("authentication failed for repository: %s with error: %s", obj.Spec.Repository.URL, err),
 		)
 
 		return ctrl.Result{
@@ -116,10 +117,11 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	update, version, err := r.checkVersion(ctx, octx, obj)
 	if err != nil {
 		// The component might not be there yet. We don't fail but keep polling instead.
-		_ = r.markAsFailed(
+		MarkAsFailed(
+			r.EventRecorder,
 			obj,
 			v1alpha1.CheckVersionFailedReason,
-			fmt.Errorf("version check failed for %s %s with error: %s", obj.Spec.Component, obj.Spec.Version.Semver, err),
+			fmt.Sprintf("version check failed for %s %s with error: %s", obj.Spec.Component, obj.Spec.Version.Semver, err),
 		)
 
 		return ctrl.Result{
@@ -144,10 +146,11 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 
 	ok, err := r.OCMClient.VerifyComponent(ctx, octx, obj, version)
 	if err != nil {
-		_ = r.markAsFailed(
+		MarkAsFailed(
+			r.EventRecorder,
 			obj,
 			v1alpha1.VerificationFailedReason,
-			fmt.Errorf("failed to verify %s with constraint %s with error: %s", obj.Spec.Component, obj.Spec.Version.Semver, err),
+			fmt.Sprintf("failed to verify %s with constraint %s with error: %s", obj.Spec.Component, obj.Spec.Version.Semver, err),
 		)
 
 		return ctrl.Result{
@@ -156,10 +159,11 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	}
 
 	if !ok {
-		_ = r.markAsFailed(
+		MarkAsFailed(
+			r.EventRecorder,
 			obj,
 			v1alpha1.VerificationFailedReason,
-			errors.New("attempted to verify component, but the digest didn't match"),
+			"attempted to verify component, but the digest didn't match",
 		)
 
 		return ctrl.Result{
@@ -178,11 +182,14 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
 
 	cv, err := r.OCMClient.GetComponentVersion(ctx, octx, obj, obj.Spec.Component, version)
 	if err != nil {
-		return ctrl.Result{}, r.markAsFailed(
+		err = fmt.Errorf("failed to get component version: %w", err)
+		MarkAsFailed(
+			r.EventRecorder,
 			obj,
 			v1alpha1.ComponentVersionInvalidReason,
-			fmt.Errorf("failed to get component version: %w", err),
+			err.Error(),
 		)
+		return ctrl.Result{}, err
 	}
 
 	defer cv.Close()
@@ -191,23 +198,29 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
 	dv := &compdesc.DescriptorVersion{}
 	cd, err := dv.ConvertFrom(cv.GetDescriptor())
 	if err != nil {
-		return ctrl.Result{}, r.markAsFailed(
+		err = fmt.Errorf("failed to convert component descriptor: %w", err)
+		MarkAsFailed(
+			r.EventRecorder,
 			obj,
 			v1alpha1.ConvertComponentDescriptorFailedReason,
-			fmt.Errorf("failed to convert component descriptor: %w", err),
+			err.Error(),
 		)
+		return ctrl.Result{}, err
 	}
 
 	rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "component fetched, creating descriptors")
 
 	// setup the component descriptor kubernetes resource
 	componentName, err := component.ConstructUniqueName(cd.GetName(), cd.GetVersion(), nil)
 	if err != nil {
-		return ctrl.Result{}, r.markAsFailed(
+		err = fmt.Errorf("failed to generate name: %w", err)
+		MarkAsFailed(
+			r.EventRecorder,
 			obj,
 			v1alpha1.NameGenerationFailedReason,
-			fmt.Errorf("failed to generate name: %w", err),
+			err.Error(),
 		)
+		return ctrl.Result{}, err
 	}
 
 	descriptor := &v1alpha1.ComponentDescriptor{
@@ -237,11 +250,14 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
 	})
 
 	if err != nil {
-		return ctrl.Result{}, r.markAsFailed(
+		err = fmt.Errorf("failed to create or update component descriptor: %w", err)
+		MarkAsFailed(
+			r.EventRecorder,
 			obj,
 			v1alpha1.CreateOrUpdateComponentDescriptorFailedReason,
-			fmt.Errorf("failed to create or update component descriptor: %w", err),
+			err.Error(),
 		)
+		return ctrl.Result{}, err
 	}
 
 	componentDescriptor := v1alpha1.Reference{
@@ -258,11 +274,14 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
 
 		componentDescriptor.References, err = r.parseReferences(ctx, octx, obj, cv.GetDescriptor().References)
 		if err != nil {
-			return ctrl.Result{}, r.markAsFailed(
+			err = fmt.Errorf("failed to parse references: %w", err)
+			MarkAsFailed(
+				r.EventRecorder,
 				obj,
 				v1alpha1.ParseReferencesFailedReason,
-				fmt.Errorf("failed to parse references: %w", err),
+				err.Error(),
 			)
+			return ctrl.Result{}, err
 		}
 	}
 
@@ -403,10 +422,3 @@ func (r *ComponentVersionReconciler) createComponentDescriptor(ctx context.Conte
 
 	return descriptor, nil
 }
-
-func (r *ComponentVersionReconciler) markAsFailed(obj *v1alpha1.ComponentVersion, reason string, err error) error {
-	conditions.MarkFalse(obj, meta.ReadyCondition, reason, err.Error())
-	event.New(r.EventRecorder, obj, eventv1.EventSeverityError, err.Error(), nil)
-
-	return err
-}

controllers/configuration_controller.go

@@ -10,13 +10,11 @@ import (
 	"fmt"
 	"time"
 
-	eventv1 "github.com/fluxcd/pkg/apis/event/v1beta1"
 	"github.com/fluxcd/pkg/apis/meta"
 	"github.com/fluxcd/pkg/runtime/conditions"
 	"github.com/fluxcd/pkg/runtime/patch"
 	rreconcile "github.com/fluxcd/pkg/runtime/reconcile"
 	sourcev1 "github.com/fluxcd/source-controller/api/v1"
-	"github.com/open-component-model/ocm-controller/pkg/event"
 	"golang.org/x/exp/slices"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -30,7 +28,6 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/handler"
-	"sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
@@ -153,8 +150,6 @@ func (r *ConfigurationReconciler) Reconcile(
 	ctx context.Context,
 	req ctrl.Request,
 ) (result ctrl.Result, err error) {
-	logger := log.FromContext(ctx).WithName("configuration-controller")
-
 	obj := &v1alpha1.Configuration{}
 	if err = r.Client.Get(ctx, req.NamespacedName, obj); err != nil {
 		if apierrors.IsNotFound(err) {
@@ -166,8 +161,7 @@ func (r *ConfigurationReconciler) Reconcile(
 
 	// return early if obj is suspended
 	if obj.Spec.Suspend {
-		logger.Info("configuration object suspended")
-		return result, nil
+		return ctrl.Result{}, nil
 	}
 
 	patchHelper := patch.NewSerialPatcher(obj, r.Client)
@@ -187,25 +181,40 @@ func (r *ConfigurationReconciler) Reconcile(
 	// check dependencies are ready
 	ready, err := r.checkReadiness(ctx, obj.GetNamespace(), &obj.Spec.SourceRef)
 	if err != nil {
-		r.markAsFailed(obj, "SourceRefNotReadyWithError", err.Error(), "source ref not yet ready with error: %s", obj.Spec.SourceRef.Name)
+		MarkAsFailed(r.EventRecorder, obj, "SourceRefNotReadyWithError", err.Error())
 
 		return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
 	}
 	if !ready {
-		r.markAsFailed(obj, "SourceRefNotReady", "source not ready yet", "source ref not yet ready: %s", obj.Spec.SourceRef.Name)
+		MarkAsFailed(
+			r.EventRecorder,
+			obj,
+			"SourceRefNotReady",
+			fmt.Sprintf("source ref not yet ready: %s", obj.Spec.SourceRef.Name),
+		)
 
 		return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
 	}
 
 	if obj.Spec.ConfigRef != nil {
 		ready, err := r.checkReadiness(ctx, obj.GetNamespace(), obj.Spec.ConfigRef)
 		if err != nil {
-			r.markAsFailed(obj, "ConfigRefNotReadyWithError", err.Error(), "config ref not yet ready with error: %s", obj.Spec.ConfigRef.Name)
+			MarkAsFailed(
+				r.EventRecorder,
+				obj,
+				"ConfigRefNotReadyWithError",
+				fmt.Sprintf("config ref not yet ready with error: %s: %s", obj.Spec.ConfigRef.Name, err),
+			)
 
 			return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
 		}
 		if !ready {
-			r.markAsFailed(obj, "ConfigRefNotReady", "config ref not ready", "config ref not yet ready: %s", obj.Spec.ConfigRef.Name)
+			MarkAsFailed(
+				r.EventRecorder,
+				obj,
+				"ConfigRefNotReady",
+				fmt.Sprintf("config ref not yet ready: %s", obj.Spec.ConfigRef.Name),
+			)
 
 			return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
 		}
@@ -214,13 +223,23 @@ func (r *ConfigurationReconciler) Reconcile(
 	if obj.Spec.PatchStrategicMerge != nil {
 		ready, err := r.checkFluxSourceReadiness(ctx, obj.Spec.PatchStrategicMerge.Source.SourceRef)
 		if err != nil {
-			r.markAsFailed(obj, "PatchStrategicMergeSourceRefNotReadyWithError", err.Error(), "patch strategic merge source ref not yet ready with error: %s", obj.Spec.PatchStrategicMerge.Source.SourceRef.Name)
+			MarkAsFailed(
+				r.EventRecorder,
+				obj,
+				"PatchStrategicMergeSourceRefNotReadyWithError",
+				fmt.Sprintf("patch strategic merge source ref not yet ready with error: %s: %s", obj.Spec.PatchStrategicMerge.Source.SourceRef.Name, err),
+			)
 
 			return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
 		}
 
 		if !ready {
-			r.markAsFailed(obj, "PatchStrategicMergeSourceRefNotReady", "patch strategic merge source not ready yet", "patch strategic merge source ref not yet ready: %s", obj.Spec.PatchStrategicMerge.Source.SourceRef.Name)
+			MarkAsFailed(
+				r.EventRecorder,
+				obj,
+				"PatchStrategicMergeSourceRefNotReady",
+				fmt.Sprintf("patch strategic merge source ref not yet ready: %s", obj.Spec.PatchStrategicMerge.Source.SourceRef.Name),
+			)
 
 			return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
 		}
@@ -231,7 +250,8 @@ func (r *ConfigurationReconciler) Reconcile(
 	if obj.GetSnapshotName() == "" {
 		name, err := snapshot.GenerateSnapshotName(obj.GetName())
 		if err != nil {
-			r.markAsFailed(obj, "GenerateSnapshotNameError", err.Error(), "failed to generate snapshot name for: %s", obj.GetName())
+			err := fmt.Errorf("failed to generate snapshot name for: %s: %s", obj.GetName(), err)
+			MarkAsFailed(r.EventRecorder, obj, v1alpha1.NameGenerationFailedReason, err.Error())
 
 			return ctrl.Result{}, err
 		}
@@ -248,8 +268,6 @@ func (r *ConfigurationReconciler) reconcile(
 	ctx context.Context,
 	obj *v1alpha1.Configuration,
 ) (ctrl.Result, error) {
-	rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress")
-
 	if obj.Generation != obj.Status.ObservedGeneration {
 		rreconcile.ProgressiveStatus(
 			false,
@@ -269,13 +287,13 @@ func (r *ConfigurationReconciler) reconcile(
 
 		if errors.Is(err, errTar) {
 			err = fmt.Errorf("source resource is not a tar archive: %w", err)
-			r.markAsFailed(obj, v1alpha1.SourceReasonNotATarArchiveReason, err.Error(), "source is not a tar archive")
+			MarkAsFailed(r.EventRecorder, obj, v1alpha1.SourceReasonNotATarArchiveReason, err.Error())
 
 			return ctrl.Result{}, err
 		}
 
 		err = fmt.Errorf("failed to reconcile mutation object: %w", err)
-		r.markAsFailed(obj, v1alpha1.ReconcileMutationObjectFailedReason, err.Error(), "failed to reconcile mutation object")
+		MarkAsFailed(r.EventRecorder, obj, v1alpha1.ReconcileMutationObjectFailedReason, err.Error())
 
 		return ctrl.Result{}, err
 	}
@@ -452,9 +470,3 @@ func makeRequestsForConfigurations(ll ...v1alpha1.Configuration) []reconcile.Req
 
 	return requests
 }
-
-func (r *ConfigurationReconciler) markAsFailed(obj *v1alpha1.Configuration, reason, msg, format string, messageArgs ...any) {
-	rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, format, messageArgs...)
-	conditions.MarkFalse(obj, meta.ReadyCondition, reason, msg)
-	event.New(r.EventRecorder, obj, eventv1.EventSeverityError, msg, nil)
-}