feat: add kstatus conforment status updates

Author: Skarlso

api/v1alpha1/componentversion_types.go

@@ -5,6 +5,7 @@
 package v1alpha1
 
 import (
+	"fmt"
 	"time"
 
 	"github.com/fluxcd/pkg/apis/meta"
@@ -145,11 +146,23 @@ type ComponentVersionStatus struct {
 	// +optional
 	ReconciledVersion string `json:"reconciledVersion,omitempty"`
 
-	// Verified is a boolean indicating whether all of the specified signatures have been verified and are valid.
+	// Verified is a boolean indicating whether all the specified signatures have been verified and are valid.
 	// +optional
 	Verified bool `json:"verified,omitempty"`
 }
 
+func (in *ComponentVersion) GetVID() map[string]string {
+	vid := fmt.Sprintf("%s:%s", in.Status.ComponentDescriptor.Name, in.Status.ReconciledVersion)
+	metadata := make(map[string]string)
+	metadata[GroupVersion.Group+"/component_version"] = vid
+
+	return metadata
+}
+
+func (in *ComponentVersion) SetObservedGeneration(v int64) {
+	in.Status.ObservedGeneration = v
+}
+
 // GetComponentName returns the name of the component
 func (in *ComponentVersion) GetComponentName() string {
 	return in.Spec.Component

api/v1alpha1/identifiable_contract.go

@@ -0,0 +1,20 @@
+package v1alpha1
+
+import (
+	"github.com/fluxcd/pkg/runtime/conditions"
+)
+
+// IdentifiableClientObject defines an object which can create an identity for itself.
+type IdentifiableClientObject interface {
+	StatusMutator
+	conditions.Setter
+
+	// GetVID constructs an identifier for an object.
+	GetVID() map[string]string
+}
+
+// StatusMutator allows mutating specific status fields of an object.
+type StatusMutator interface {
+	// SetObservedGeneration mutates the observed generation field of an object.
+	SetObservedGeneration(v int64)
+}

controllers/componentversion_controller.go

@@ -19,7 +19,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	kuberecorder "k8s.io/client-go/tools/record"
-	"k8s.io/klog/v2"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -84,51 +83,29 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		return
 	}
 
-	patchHelper, err := patch.NewHelper(obj, r.Client)
-	if err != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to create patchhelper: %w", err)
-	}
+	patchHelper := patch.NewSerialPatcher(obj, r.Client)
 
 	// Always attempt to patch the object and status after each reconciliation.
 	defer func() {
-		// Patching has not been set up, or the controller errored earlier.
-		if patchHelper == nil {
-			return
-		}
-
-		// If still reconciling then reconciliation did not succeed, set to ProgressingWithRetry to
-		// indicate that reconciliation will be retried.
-		if conditions.IsReconciling(obj) {
-			reconciling := conditions.Get(obj, meta.ReconcilingCondition)
-			reconciling.Reason = meta.ProgressingWithRetryReason
-			conditions.Set(obj, reconciling)
-			msg := fmt.Sprintf("Reconciliation did not succeed, retrying in %s", obj.GetRequeueAfter())
-			event.New(r.EventRecorder, obj, eventv1.EventSeverityError, msg, nil)
-		}
-
-		// Set status observed generation option if the component is ready.
-		if conditions.IsReady(obj) {
-			obj.Status.ObservedGeneration = obj.Generation
-			msg := fmt.Sprintf("Reconciliation finished, next run in %s", obj.GetRequeueAfter())
-			vid := fmt.Sprintf("%s:%s", obj.Status.ComponentDescriptor.Name, obj.Status.ReconciledVersion)
-			metadata := make(map[string]string)
-			metadata[v1alpha1.GroupVersion.Group+"/component_version"] = vid
-			event.New(r.EventRecorder, obj, eventv1.EventSeverityInfo, msg, metadata)
-		}
-
-		// Update the object.
-		if err := patchHelper.Patch(ctx, obj); err != nil {
-			retErr = errors.Join(retErr, err)
+		if derr := DeferredStatusUpdate(ctx, patchHelper, obj, r.EventRecorder, obj.GetRequeueAfter()); derr != nil {
+			retErr = errors.Join(retErr, derr)
 		}
 	}()
 
-	rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconcilation in progress for component: %s", obj.Spec.Component)
+	// Starts the progression by setting ReconcilingCondition.
+	// This will be checked in defer.
+	// Should only be deleted on a success.
+	rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "reconciliation in progress for component: %s", obj.Spec.Component)
 
 	octx, err := r.OCMClient.CreateAuthenticatedOCMContext(ctx, obj)
 	if err != nil {
-		msg := fmt.Sprintf("authentication failed for repository: %s with error: %s", obj.Spec.Repository.URL, err)
-		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.AuthenticatedContextCreationFailedReason, msg)
-		event.New(r.EventRecorder, obj, eventv1.EventSeverityError, msg, 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(
+			obj,
+			v1alpha1.AuthenticatedContextCreationFailedReason,
+			fmt.Errorf("authentication failed for repository: %s with error: %s", obj.Spec.Repository.URL, err),
+		)
 
 		return ctrl.Result{
 			RequeueAfter: obj.GetRequeueAfter(),
@@ -138,9 +115,12 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 	// reconcile the version before calling reconcile func
 	update, version, err := r.checkVersion(ctx, octx, obj)
 	if err != nil {
-		msg := fmt.Sprintf("version check failed for %s %s with error: %s", obj.Spec.Component, obj.Spec.Version.Semver, err)
-		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.CheckVersionFailedReason, msg)
-		event.New(r.EventRecorder, obj, eventv1.EventSeverityError, msg, nil)
+		// The component might not be there yet. We don't fail but keep polling instead.
+		_ = r.markAsFailed(
+			obj,
+			v1alpha1.CheckVersionFailedReason,
+			fmt.Errorf("version check failed for %s %s with error: %s", obj.Spec.Component, obj.Spec.Version.Semver, err),
+		)
 
 		return ctrl.Result{
 			RequeueAfter: obj.GetRequeueAfter(),
@@ -152,58 +132,57 @@ func (r *ComponentVersionReconciler) Reconcile(ctx context.Context, req ctrl.Req
 		conditions.MarkTrue(obj,
 			meta.ReadyCondition,
 			meta.SucceededReason,
-			fmt.Sprintf("Applied version: %s", version))
+			"Applied version: %s",
+			version)
 
 		return ctrl.Result{
 			RequeueAfter: obj.GetRequeueAfter(),
 		}, nil
 	}
 
+	rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason, "updating component to new version: %s: %s", obj.Spec.Component, version)
+
 	ok, err := r.OCMClient.VerifyComponent(ctx, octx, obj, version)
 	if err != nil {
-		msg := fmt.Sprintf("failed to verify %s with constraint %s with error: %s", obj.Spec.Component, obj.Spec.Version.Semver, err)
-		conditions.Delete(obj, meta.ReconcilingCondition)
-		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.VerificationFailedReason, msg)
-		event.New(r.EventRecorder, obj, eventv1.EventSeverityError, fmt.Sprintf("%s, retrying in %s", err.Error(), obj.GetRequeueAfter()), nil)
+		_ = r.markAsFailed(
+			obj,
+			v1alpha1.VerificationFailedReason,
+			fmt.Errorf("failed to verify %s with constraint %s with error: %s", obj.Spec.Component, obj.Spec.Version.Semver, err),
+		)
 
 		return ctrl.Result{
 			RequeueAfter: obj.GetRequeueAfter(),
 		}, nil
 	}
 
 	if !ok {
-		msg := "attempted to verify component, but the digest didn't match"
-		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.VerificationFailedReason, msg)
-		event.New(r.EventRecorder, obj, eventv1.EventSeverityError, fmt.Sprintf("%s, retrying in %s", msg, obj.GetRequeueAfter()), nil)
+		_ = r.markAsFailed(
+			obj,
+			v1alpha1.VerificationFailedReason,
+			errors.New("attempted to verify component, but the digest didn't match"),
+		)
 
 		return ctrl.Result{
 			RequeueAfter: obj.GetRequeueAfter(),
 		}, nil
 	}
 
-	// update the result for the defer call to have the latest information
-	rresult, err := r.reconcile(ctx, octx, obj, version)
-	if err != nil {
-		event.New(r.EventRecorder, obj, eventv1.EventSeverityError, fmt.Sprintf("Reconciliation failed: %s, retrying in %s", err.Error(), obj.GetRequeueAfter()), nil)
-	}
-
-	return rresult, err
+	return r.reconcile(ctx, octx, obj, version)
 }
 
 func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Context, obj *v1alpha1.ComponentVersion, version string) (ctrl.Result, error) {
 	if obj.Generation != obj.Status.ObservedGeneration {
-		// don't have to patch here since we patch the object in the outer reconcile call.
 		rreconcile.ProgressiveStatus(false, obj, meta.ProgressingReason,
 			"processing object: new generation %d -> %d", obj.Status.ObservedGeneration, obj.Generation)
 	}
 
 	cv, err := r.OCMClient.GetComponentVersion(ctx, octx, obj, obj.Spec.Component, version)
 	if err != nil {
-		err = fmt.Errorf("failed to get component version: %w", err)
-		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.ComponentVersionInvalidReason, err.Error())
-		event.New(r.EventRecorder, obj, eventv1.EventSeverityError, err.Error(), nil)
-
-		return ctrl.Result{}, err
+		return ctrl.Result{}, r.markAsFailed(
+			obj,
+			v1alpha1.ComponentVersionInvalidReason,
+			fmt.Errorf("failed to get component version: %w", err),
+		)
 	}
 
 	defer cv.Close()
@@ -212,18 +191,25 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
 	dv := &compdesc.DescriptorVersion{}
 	cd, err := dv.ConvertFrom(cv.GetDescriptor())
 	if err != nil {
-		err = fmt.Errorf("failed to convert component descriptor: %w", err)
-		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.ConvertComponentDescriptorFailedReason, err.Error())
-		return ctrl.Result{}, err
+		return ctrl.Result{}, r.markAsFailed(
+			obj,
+			v1alpha1.ConvertComponentDescriptorFailedReason,
+			fmt.Errorf("failed to convert component descriptor: %w", 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 {
-		err = fmt.Errorf("failed to generate name: %w", err)
-		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.NameGenerationFailedReason, err.Error())
-		return ctrl.Result{}, err
+		return ctrl.Result{}, r.markAsFailed(
+			obj,
+			v1alpha1.NameGenerationFailedReason,
+			fmt.Errorf("failed to generate name: %w", err),
+		)
 	}
+
 	descriptor := &v1alpha1.ComponentDescriptor{
 		ObjectMeta: metav1.ObjectMeta{
 			Namespace: obj.GetNamespace(),
@@ -251,10 +237,11 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
 	})
 
 	if err != nil {
-		err = fmt.Errorf("failed to create or update component descriptor: %w", err)
-		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.CreateOrUpdateComponentDescriptorFailedReason, err.Error())
-		event.New(r.EventRecorder, obj, eventv1.EventSeverityError, err.Error(), nil)
-		return ctrl.Result{}, err
+		return ctrl.Result{}, r.markAsFailed(
+			obj,
+			v1alpha1.CreateOrUpdateComponentDescriptorFailedReason,
+			fmt.Errorf("failed to create or update component descriptor: %w", err),
+		)
 	}
 
 	componentDescriptor := v1alpha1.Reference{
@@ -269,10 +256,11 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
 	if obj.Spec.References.Expand {
 		componentDescriptor.References, err = r.parseReferences(ctx, octx, obj, cv.GetDescriptor().References)
 		if err != nil {
-			err = fmt.Errorf("failed to parse references: %w", err)
-			conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.ParseReferencesFailedReason, err.Error())
-			event.New(r.EventRecorder, obj, eventv1.EventSeverityError, err.Error(), nil)
-			return ctrl.Result{}, err
+			return ctrl.Result{}, r.markAsFailed(
+				obj,
+				v1alpha1.ParseReferencesFailedReason,
+				fmt.Errorf("failed to parse references: %w", err),
+			)
 		}
 	}
 
@@ -283,21 +271,22 @@ func (r *ComponentVersionReconciler) reconcile(ctx context.Context, octx ocm.Con
 	conditions.MarkTrue(obj,
 		meta.ReadyCondition,
 		meta.SucceededReason,
-		fmt.Sprintf("Applied version: %s", version))
+		"Applied version: %s",
+		version)
 
 	conditions.Delete(obj, meta.ReconcilingCondition)
 
 	return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil
 }
 
 func (r *ComponentVersionReconciler) checkVersion(ctx context.Context, octx ocm.Context, obj *v1alpha1.ComponentVersion) (bool, string, error) {
-	log := log.FromContext(ctx).WithName("ocm-component-version-reconcile")
+	logger := log.FromContext(ctx).WithName("ocm-component-version-reconcile")
 
 	latest, err := r.OCMClient.GetLatestValidComponentVersion(ctx, octx, obj)
 	if err != nil {
 		return false, "", fmt.Errorf("failed to get latest component version: %w", err)
 	}
-	log.V(4).Info("got latest version of component", "version", latest)
+	logger.V(4).Info("got latest version of component", "version", latest)
 
 	latestSemver, err := semver.NewVersion(latest)
 	if err != nil {
@@ -312,11 +301,11 @@ func (r *ComponentVersionReconciler) checkVersion(ctx context.Context, octx ocm.
 	if err != nil {
 		return false, "", fmt.Errorf("failed to parse reconciled version: %w", err)
 	}
-	log.V(4).Info("current reconciled version is", "reconciled", current.String())
+	logger.V(4).Info("current reconciled version is", "reconciled", current.String())
 
 	if latestSemver.Equal(current) || current.GreaterThan(latestSemver) {
-		log.V(4).Info("Reconciled version equal to or greater than newest available version", "version", latestSemver)
-		return false, "", nil
+		logger.V(4).Info("Reconciled version equal to or greater than newest available version", "version", latestSemver)
+		return false, latest, nil
 	}
 
 	event.New(r.EventRecorder, obj, eventv1.EventSeverityInfo, fmt.Sprintf("Version check succeeded, found latest version: %s", latest), nil)
@@ -380,7 +369,6 @@ func (r *ComponentVersionReconciler) createComponentDescriptor(ctx context.Conte
 		return nil, fmt.Errorf("failed to convert component descriptor: %w", err)
 	}
 
-	log := log.FromContext(ctx)
 	// setup the component descriptor kubernetes resource
 	componentName, err := component.ConstructUniqueName(ref.ComponentName, ref.Version, ref.GetMeta().ExtraIdentity)
 	if err != nil {
@@ -397,19 +385,26 @@ func (r *ComponentVersionReconciler) createComponentDescriptor(ctx context.Conte
 		},
 	}
 
-	if err := controllerutil.SetOwnerReference(parent, descriptor, r.Scheme); err != nil {
-		return nil, fmt.Errorf("failed to set owner reference: %w", err)
-	}
-
 	// create or update the component descriptor kubernetes resource
 	// we don't need to update it
-	op, err := controllerutil.CreateOrUpdate(ctx, r.Client, descriptor, func() error {
+	if _, err = controllerutil.CreateOrUpdate(ctx, r.Client, descriptor, func() error {
+		if descriptor.ObjectMeta.CreationTimestamp.IsZero() {
+			if err := controllerutil.SetOwnerReference(parent, descriptor, r.Scheme); err != nil {
+				return fmt.Errorf("failed to set owner reference: %w", err)
+			}
+		}
+
 		return nil
-	})
-	if err != nil {
+	}); err != nil {
 		return nil, fmt.Errorf("failed to create/update component descriptor: %w", err)
 	}
-	log.V(4).Info(fmt.Sprintf("%s(ed) descriptor", op), "descriptor", klog.KObj(descriptor))
 
 	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
+}