rollouts: add status conditions to remote root sync (#3849)

Author: ChristopherFry

rollouts/api/v1alpha1/remoterootsync_types.go

@@ -70,7 +70,11 @@ type RemoteRootSyncStatus struct {
 	// Conditions describes the reconciliation state of the object.
 	Conditions []metav1.Condition `json:"conditions,omitempty"`
 
+	// SyncStatus describes the observed state of external sync.
 	SyncStatus string `json:"syncStatus,omitempty"`
+
+	// Internal only. SyncCreated describes if the external sync has been created.
+	SyncCreated bool `json:"syncCreated"`
 }
 
 //+kubebuilder:object:root=true

rollouts/config/crd/bases/gitops.kpt.dev_remoterootsyncs.yaml

@@ -191,8 +191,15 @@ spec:
                   this file'
                 format: int64
                 type: integer
+              syncCreated:
+                description: Internal only. SyncCreated describes if the external
+                  sync has been created.
+                type: boolean
               syncStatus:
+                description: SyncStatus describes the observed state of external sync.
                 type: string
+            required:
+            - syncCreated
             type: object
         type: object
     served: true

rollouts/controllers/remoterootsync_controller.go

@@ -63,7 +63,12 @@ var (
 )
 
 const (
-	externalSyncCreatedConditionType = "ExternalSyncCreated"
+	conditionReconciling = "Reconciling"
+	conditionStalled     = "Stalled"
+
+	reasonCreateSync = "CreateSync"
+	reasonUpdateSync = "UpdateSync"
+	reasonError      = "Error"
 )
 
 // RemoteRootSyncReconciler reconciles a RemoteRootSync object
@@ -121,10 +126,16 @@ func (r *RemoteRootSyncReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		// The object is being deleted
 		if controllerutil.ContainsFinalizer(&remoterootsync, myFinalizerName) {
 			// our finalizer is present, so lets handle any external dependency
-			if meta.IsStatusConditionTrue(remoterootsync.Status.Conditions, externalSyncCreatedConditionType) {
+			if remoterootsync.Status.SyncCreated {
 				// Delete the external sync resource
 				err := r.deleteExternalResources(ctx, &remoterootsync)
 				if err != nil && !apierrors.IsNotFound(err) {
+					statusError := r.updateStatus(ctx, &remoterootsync, "", err)
+
+					if statusError != nil {
+						logger.Error(statusError, "Failed to update status")
+					}
+
 					// if fail to delete the external dependency here, return with error
 					// so that it can be retried
 					return ctrl.Result{}, fmt.Errorf("have problem to delete external resource: %w", err)
@@ -145,42 +156,66 @@ func (r *RemoteRootSyncReconciler) Reconcile(ctx context.Context, req ctrl.Reque
 		return ctrl.Result{}, nil
 	}
 
-	clusterRef := &remoterootsync.Spec.ClusterRef
-	dynCl, err := r.getDynamicClientForCluster(ctx, clusterRef)
-	if err != nil {
-		return ctrl.Result{}, err
-	}
+	syncStatus, syncError := r.syncExternalSync(ctx, &remoterootsync)
 
-	if err := r.patchRootSync(ctx, dynCl, req.Name, &remoterootsync); err != nil {
+	if err := r.updateStatus(ctx, &remoterootsync, syncStatus, syncError); err != nil {
+		logger.Error(err, "Failed to update status")
 		return ctrl.Result{}, err
 	}
 
-	r.setupWatches(ctx, remoterootsync.Name, remoterootsync.Namespace, remoterootsync.Spec.ClusterRef)
+	return ctrl.Result{}, syncError
+}
+
+func (r *RemoteRootSyncReconciler) syncExternalSync(ctx context.Context, rrs *gitopsv1alpha1.RemoteRootSync) (string, error) {
+	syncName := rrs.Name
+	clusterRef := &rrs.Spec.ClusterRef
 
-	syncStatus, err := checkSyncStatus(ctx, dynCl, req.Name)
+	dynCl, err := r.getDynamicClientForCluster(ctx, clusterRef)
 	if err != nil {
-		return ctrl.Result{}, err
+		return "", fmt.Errorf("failed to create client: %w", err)
 	}
 
-	if err := r.updateStatus(ctx, &remoterootsync, syncStatus); err != nil {
-		logger.Error(err, "Failed to update status")
-		return ctrl.Result{}, err
+	if err := r.patchRootSync(ctx, dynCl, syncName, rrs); err != nil {
+		return "", fmt.Errorf("failed to create/update sync: %w", err)
 	}
 
-	return ctrl.Result{}, nil
+	r.setupWatches(ctx, rrs.Name, rrs.Namespace, rrs.Spec.ClusterRef)
+
+	syncStatus, err := checkSyncStatus(ctx, dynCl, syncName)
+	if err != nil {
+		return "", fmt.Errorf("faild to check status: %w", err)
+	}
+
+	return syncStatus, nil
 }
 
-func (r *RemoteRootSyncReconciler) updateStatus(ctx context.Context, rrs *gitopsv1alpha1.RemoteRootSync, syncStatus string) error {
+func (r *RemoteRootSyncReconciler) updateStatus(ctx context.Context, rrs *gitopsv1alpha1.RemoteRootSync, syncStatus string, syncError error) error {
 	logger := klog.FromContext(ctx)
 
-	// Don't update if there are no changes.
-
 	rrsPrior := rrs.DeepCopy()
+	conditions := &rrs.Status.Conditions
 
-	rrs.Status.SyncStatus = syncStatus
-	rrs.Status.ObservedGeneration = rrs.Generation
+	if syncError == nil {
+		rrs.Status.SyncStatus = syncStatus
+		rrs.Status.SyncCreated = true
+
+		meta.RemoveStatusCondition(conditions, conditionReconciling)
+		meta.RemoveStatusCondition(conditions, conditionStalled)
+	} else {
+		reconcileReason := reasonUpdateSync
+
+		rrs.Status.SyncStatus = "Unknown"
+
+		if !rrs.Status.SyncCreated {
+			rrs.Status.SyncStatus = ""
+			reconcileReason = reasonCreateSync
+		}
 
-	meta.SetStatusCondition(&rrs.Status.Conditions, metav1.Condition{Type: externalSyncCreatedConditionType, Status: metav1.ConditionTrue, Reason: "SyncCreated"})
+		meta.SetStatusCondition(conditions, metav1.Condition{Type: conditionReconciling, Status: metav1.ConditionTrue, Reason: reconcileReason})
+		meta.SetStatusCondition(conditions, metav1.Condition{Type: conditionStalled, Status: metav1.ConditionTrue, Reason: reasonError, Message: syncError.Error()})
+	}
+
+	rrs.Status.ObservedGeneration = rrs.Generation
 
 	if reflect.DeepEqual(rrs.Status, rrsPrior.Status) {
 		return nil