Merge pull request #10 from open-component-model/apply-conditions

feat(ref): add conditions to git-sync-controller

Author: Skarlso

api/v1alpha1/condition_types.go

@@ -0,0 +1,20 @@
+// Copyright 2022.
+// SPDX-FileCopyrightText: 2022 SAP SE or an SAP affiliate company and Open Component Model contributors.
+//
+// SPDX-License-Identifier: Apache-2.0
+
+package v1alpha1
+
+const (
+	// PatchFailedReason is used when we couldn't patch an object.
+	PatchFailedReason = "PatchFailed"
+
+	// SnapshotGetFailedReason is used when the needed snapshot does not exist.
+	SnapshotGetFailedReason = "SnapshotGetFailed"
+
+	// AuthenticateGetFailedReason is used when the needed authentication does not exist.
+	CredentialsNotFoundReason = "CredentialsNotFound"
+
+	// GitRepositoryPushFailedReason is used when the needed pushing to a git repository failed.
+	GitRepositoryPushFailedReason = "GitRepositoryPushFailed"
+)

api/v1alpha1/gitsync_types.go

@@ -6,6 +6,8 @@
 package v1alpha1
 
 import (
+	"time"
+
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 )
 
@@ -37,6 +39,30 @@ type GitSyncSpec struct {
 // GitSyncStatus defines the observed state of GitSync
 type GitSyncStatus struct {
 	Digest string `json:"digest,omitempty"`
+
+	// ObservedGeneration is the last reconciled generation.
+	// +optional
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
+	// +optional
+	// +kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].status",description=""
+	// +kubebuilder:printcolumn:name="Status",type="string",JSONPath=".status.conditions[?(@.type==\"Ready\")].message",description=""
+	Conditions []metav1.Condition `json:"conditions,omitempty"`
+}
+
+// GetConditions returns the conditions of the ComponentVersion.
+func (in *GitSync) GetConditions() []metav1.Condition {
+	return in.Status.Conditions
+}
+
+// SetConditions sets the conditions of the ComponentVersion.
+func (in *GitSync) SetConditions(conditions []metav1.Condition) {
+	in.Status.Conditions = conditions
+}
+
+// GetRequeueAfter returns the duration after which the ComponentVersion must be
+// reconciled again.
+func (in GitSync) GetRequeueAfter() time.Duration {
+	return in.Spec.Interval.Duration
 }
 
 //+kubebuilder:object:root=true

api/v1alpha1/zz_generated.deepcopy.go

@@ -94,8 +94,80 @@ spec:
           status:
             description: GitSyncStatus defines the observed state of GitSync
             properties:
+              conditions:
+                items:
+                  description: "Condition contains details for one aspect of the current
+                    state of this API Resource. --- This struct is intended for direct
+                    use as an array at the field path .status.conditions.  For example,
+                    \n type FooStatus struct{ // Represents the observations of a
+                    foo's current state. // Known .status.conditions.type are: \"Available\",
+                    \"Progressing\", and \"Degraded\" // +patchMergeKey=type // +patchStrategy=merge
+                    // +listType=map // +listMapKey=type Conditions []metav1.Condition
+                    `json:\"conditions,omitempty\" patchStrategy:\"merge\" patchMergeKey:\"type\"
+                    protobuf:\"bytes,1,rep,name=conditions\"` \n // other fields }"
+                  properties:
+                    lastTransitionTime:
+                      description: lastTransitionTime is the last time the condition
+                        transitioned from one status to another. This should be when
+                        the underlying condition changed.  If that is not known, then
+                        using the time when the API field changed is acceptable.
+                      format: date-time
+                      type: string
+                    message:
+                      description: message is a human readable message indicating
+                        details about the transition. This may be an empty string.
+                      maxLength: 32768
+                      type: string
+                    observedGeneration:
+                      description: observedGeneration represents the .metadata.generation
+                        that the condition was set based upon. For instance, if .metadata.generation
+                        is currently 12, but the .status.conditions[x].observedGeneration
+                        is 9, the condition is out of date with respect to the current
+                        state of the instance.
+                      format: int64
+                      minimum: 0
+                      type: integer
+                    reason:
+                      description: reason contains a programmatic identifier indicating
+                        the reason for the condition's last transition. Producers
+                        of specific condition types may define expected values and
+                        meanings for this field, and whether the values are considered
+                        a guaranteed API. The value should be a CamelCase string.
+                        This field may not be empty.
+                      maxLength: 1024
+                      minLength: 1
+                      pattern: ^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
+                      type: string
+                    status:
+                      description: status of the condition, one of True, False, Unknown.
+                      enum:
+                      - "True"
+                      - "False"
+                      - Unknown
+                      type: string
+                    type:
+                      description: type of condition in CamelCase or in foo.example.com/CamelCase.
+                        --- Many .condition.type values are consistent across resources
+                        like Available, but because arbitrary conditions can be useful
+                        (see .node.status.conditions), the ability to deconflict is
+                        important. The regex it matches is (dns1123SubdomainFmt/)?(qualifiedNameFmt)
+                      maxLength: 316
+                      pattern: ^([a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/)?(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])$
+                      type: string
+                  required:
+                  - lastTransitionTime
+                  - message
+                  - reason
+                  - status
+                  - type
+                  type: object
+                type: array
               digest:
                 type: string
+              observedGeneration:
+                description: ObservedGeneration is the last reconciled generation.
+                format: int64
+                type: integer
             type: object
         type: object
     served: true

config/crd/bases/delivery.ocm.software_gitsyncs.yaml

@@ -2,7 +2,7 @@ apiVersion: v1
 kind: Namespace
 metadata:
   labels:
-    control-plane: git-sync-controller
+    app: git-sync-controller
   name: ocm-system
 ---
 apiVersion: apps/v1
@@ -11,18 +11,18 @@ metadata:
   name: git-sync-controller
   namespace: ocm-system
   labels:
-    control-plane: git-sync-controller
+    app: git-sync-controller
 spec:
   selector:
     matchLabels:
-      control-plane: git-sync-controller
+      app: git-sync-controller
   replicas: 1
   template:
     metadata:
       annotations:
         kubectl.kubernetes.io/default-container: manager
       labels:
-        control-plane: git-sync-controller
+        app: git-sync-controller
     spec:
       securityContext:
         runAsNonRoot: true

config/manager/deployment.yaml

@@ -7,12 +7,15 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
-	"time"
 
+	"github.com/fluxcd/pkg/apis/meta"
+	"github.com/fluxcd/pkg/runtime/conditions"
 	"github.com/fluxcd/pkg/runtime/patch"
 	corev1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -44,71 +47,139 @@ type GitSyncReconciler struct {
 // Reconcile is part of the main kubernetes reconciliation loop which aims to
 // move the current state of the cluster closer to the desired state.
 func (r *GitSyncReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
+	var (
+		result ctrl.Result
+		retErr error
+	)
+
 	log := log.FromContext(ctx)
 	log.V(4).Info("starting reconcile loop for snapshot")
-	gitSync := &v1alpha1.GitSync{}
-	if err := r.Get(ctx, req.NamespacedName, gitSync); err != nil {
+	obj := &v1alpha1.GitSync{}
+	if err := r.Get(ctx, req.NamespacedName, obj); err != nil {
 		if apierrors.IsNotFound(err) {
 			return ctrl.Result{}, nil
 		}
 		return ctrl.Result{}, fmt.Errorf("failed to get git sync object: %w", err)
 	}
-	log.V(4).Info("found reconciling object", "gitSync", gitSync)
+	log.V(4).Info("found reconciling object", "gitSync", obj)
+
+	// The replication controller doesn't need a shouldReconcile, because it should always reconcile,
+	// that is its purpose.
+	patchHelper, err := patch.NewHelper(obj, r.Client)
+	if err != nil {
+		retErr = errors.Join(retErr, err)
+		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.PatchFailedReason, err.Error())
+
+		return ctrl.Result{}, retErr
+	}
+
+	// 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 gitSync.Status.Digest != "" {
-		log.Info("GitSync object already synced; status contains digest information", "digest", gitSync.Status.Digest)
+		if condition := conditions.Get(obj, meta.StalledCondition); condition != nil && condition.Status == metav1.ConditionTrue {
+			conditions.Delete(obj, meta.ReconcilingCondition)
+		}
+
+		// Check if it's a successful reconciliation.
+		// We don't set Requeue in case of error, so we can safely check for Requeue.
+		if result.RequeueAfter == obj.GetRequeueAfter() && !result.Requeue && retErr == nil {
+			// Remove the reconciling condition if it's set.
+			conditions.Delete(obj, meta.ReconcilingCondition)
+
+			// Set the return err as the ready failure message if the resource is not ready, but also not reconciling or stalled.
+			if ready := conditions.Get(obj, meta.ReadyCondition); ready != nil && ready.Status == metav1.ConditionFalse && !conditions.IsStalled(obj) {
+				retErr = errors.New(conditions.GetMessage(obj, meta.ReadyCondition))
+			}
+		}
+
+		// 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)
+		}
+
+		// If not reconciling or stalled than mark Ready=True
+		if !conditions.IsReconciling(obj) &&
+			!conditions.IsStalled(obj) &&
+			retErr == nil {
+			conditions.MarkTrue(obj, meta.ReadyCondition, meta.SucceededReason, "Reconciliation success")
+		}
+
+		// Set status observed generation option if the component is stalled or ready.
+		if conditions.IsStalled(obj) || conditions.IsReady(obj) {
+			obj.Status.ObservedGeneration = obj.Generation
+		}
+
+		// Update the object.
+		if err := patchHelper.Patch(ctx, obj); err != nil {
+			retErr = errors.Join(retErr, err)
+		}
+	}()
+
+	// it's important that this happens here so any residual status condition can be overwritten / set.
+	if obj.Status.Digest != "" {
+		log.Info("GitSync object already synced; status contains digest information", "digest", obj.Status.Digest)
 		return ctrl.Result{}, nil
 	}
 
 	snapshot := &ocmv1.Snapshot{}
 	if err := r.Get(ctx, types.NamespacedName{
-		Namespace: gitSync.Spec.SnapshotRef.Namespace,
-		Name:      gitSync.Spec.SnapshotRef.Name,
+		Namespace: obj.Spec.SnapshotRef.Namespace,
+		Name:      obj.Spec.SnapshotRef.Name,
 	}, snapshot); err != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to find snapshot: %w", err)
+		retErr = fmt.Errorf("failed to find snapshot: %w", err)
+		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.SnapshotGetFailedReason, retErr.Error())
+
+		return ctrl.Result{}, retErr
 	}
+
 	authSecret := &corev1.Secret{}
 	if err := r.Get(ctx, types.NamespacedName{
-		Namespace: gitSync.Spec.AuthRef.Namespace,
-		Name:      gitSync.Spec.AuthRef.Name,
+		Namespace: obj.Spec.AuthRef.Namespace,
+		Name:      obj.Spec.AuthRef.Name,
 	}, authSecret); err != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to find authentication secret: %w", err)
+		retErr = fmt.Errorf("failed to find authentication secret: %w", err)
+		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.CredentialsNotFoundReason, retErr.Error())
+
+		return ctrl.Result{}, retErr
 	}
 
 	// trim any trailing `/` and then just add.
 	log.V(4).Info("crafting artifact URL to download from", "url", snapshot.Status.RepositoryURL)
 	opts := &providers.PushOptions{
-		URL:      gitSync.Spec.URL,
-		Message:  gitSync.Spec.CommitTemplate.Message,
-		Name:     gitSync.Spec.CommitTemplate.Name,
-		Email:    gitSync.Spec.CommitTemplate.Email,
+		URL:      obj.Spec.URL,
+		Message:  obj.Spec.CommitTemplate.Message,
+		Name:     obj.Spec.CommitTemplate.Name,
+		Email:    obj.Spec.CommitTemplate.Email,
 		Snapshot: snapshot,
-		Branch:   gitSync.Spec.Branch,
-		SubPath:  gitSync.Spec.SubPath,
+		Branch:   obj.Spec.Branch,
+		SubPath:  obj.Spec.SubPath,
 	}
+
 	r.parseAuthSecret(authSecret, opts)
 
 	digest, err := r.Git.Push(ctx, opts)
 	if err != nil {
-		return ctrl.Result{}, fmt.Errorf("failed to push to git repository: %w", err)
-	}
-	// Initialize the patch helper.
-	patchHelper, err := patch.NewHelper(gitSync, r.Client)
-	if err != nil {
-		return ctrl.Result{
-			RequeueAfter: 1 * time.Minute,
-		}, fmt.Errorf("failed to create patch helper: %w", err)
-	}
+		retErr = fmt.Errorf("failed to push to git repository: %w", err)
+		conditions.MarkFalse(obj, meta.ReadyCondition, v1alpha1.GitRepositoryPushFailedReason, retErr.Error())
 
-	gitSync.Status.Digest = digest
-	if err := patchHelper.Patch(ctx, gitSync); err != nil {
-		return ctrl.Result{
-			RequeueAfter: 1 * time.Minute,
-		}, fmt.Errorf("failed to patch git sync object: %w", err)
+		return ctrl.Result{}, retErr
 	}
-	log.V(4).Info("patch successful")
 
-	return ctrl.Result{}, nil
+	obj.Status.Digest = digest
+
+	// Remove any stale Ready condition, most likely False, set above. Its value
+	// is derived from the overall result of the reconciliation in the deferred
+	// block at the very end.
+	conditions.Delete(obj, meta.ReadyCondition)
+
+	return result, retErr
 }
 
 // SetupWithManager sets up the controller with the Manager.