Remove 'Synced' Condition from XR and XRC controllers

crossplane/crossplane-runtime#198 See the above issue for context. Signed-off-by: Nic Cope <[email protected]>
negz · Sep 10, 2020 · 4321f6d · 4321f6d
1 parent 4c898b2
commit 4321f6d
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 49 deletions.
diff --git a/apis/apiextensions/v1alpha1/ccrd/crd_test.go b/apis/apiextensions/v1alpha1/ccrd/crd_test.go
@@ -136,11 +136,6 @@ func TestForCompositeResourceDefinition(t *testing.T) {
 					Type:     "string",
 					JSONPath: ".status.conditions[?(@.type=='Ready')].status",
 				},
-				{
-					Name:     "SYNCED",
-					Type:     "string",
-					JSONPath: ".status.conditions[?(@.type=='Synced')].status",
-				},
 				{
 					Name:     "COMPOSITION",
 					Type:     "string",
@@ -470,11 +465,6 @@ func TestPublishesCompositeResourceDefinition(t *testing.T) {
 					Type:     "string",
 					JSONPath: ".status.conditions[?(@.type=='Ready')].status",
 				},
-				{
-					Name:     "SYNCED",
-					Type:     "string",
-					JSONPath: ".status.conditions[?(@.type=='Synced')].status",
-				},
 				{
 					Name:     "CONNECTION-SECRET",
 					Type:     "string",

diff --git a/apis/apiextensions/v1alpha1/ccrd/schemas.go b/apis/apiextensions/v1alpha1/ccrd/schemas.go
@@ -156,6 +156,8 @@ func CompositeResourceClaimSpecProps() map[string]v1beta1.JSONSchemaProps {
 // infrastructure resources.
 func CompositeResourceStatusProps() map[string]v1beta1.JSONSchemaProps {
 	return map[string]v1beta1.JSONSchemaProps{
+		// TODO(negz): Remove composedResources, readyResources, and
+		// bindingPhase. I believe they're unused.
 		"composedResources": {
 			Type: "integer",
 		},
@@ -200,11 +202,6 @@ func CompositeResourcePrinterColumns() []v1beta1.CustomResourceColumnDefinition
 			Type:     "string",
 			JSONPath: ".status.conditions[?(@.type=='Ready')].status",
 		},
-		{
-			Name:     "SYNCED",
-			Type:     "string",
-			JSONPath: ".status.conditions[?(@.type=='Synced')].status",
-		},
 		{
 			Name:     "COMPOSITION",
 			Type:     "string",
@@ -222,11 +219,6 @@ func CompositeResourceClaimPrinterColumns() []v1beta1.CustomResourceColumnDefini
 			Type:     "string",
 			JSONPath: ".status.conditions[?(@.type=='Ready')].status",
 		},
-		{
-			Name:     "SYNCED",
-			Type:     "string",
-			JSONPath: ".status.conditions[?(@.type=='Synced')].status",
-		},
 		{
 			Name:     "CONNECTION-SECRET",
 			Type:     "string",

diff --git a/pkg/controller/apiextensions/claim/reconciler.go b/pkg/controller/apiextensions/claim/reconciler.go
@@ -303,8 +303,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 					// after a brief wait, in case this was a transient error.
 					log.Debug("Cannot remove finalizer", "error", err, "requeue-after", time.Now().Add(aShortWait))
 					record.Event(cm, event.Warning(reasonDelete, err))
-					cm.SetConditions(v1alpha1.Deleting(), v1alpha1.ReconcileError(err))
-					return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
+					return reconcile.Result{RequeueAfter: aShortWait}, nil
 				}
 
 				// We've successfully deleted our claim and removed our finalizer. If we
@@ -323,7 +322,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 			// implicitly when the composite resource we want to bind to appears.
 			log.Debug("Referenced composite resource not found", "requeue-after", time.Now().Add(aShortWait))
 			record.Event(cm, event.Warning(reasonBind, err))
-			cm.SetConditions(Waiting(), v1alpha1.ReconcileSuccess())
+			cm.SetConditions(Waiting())
 			return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
 		}
 		if err != nil {
@@ -332,8 +331,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 			// after a brief wait, in case this was a transient error.
 			log.Debug("Cannot get referenced composite resource", "error", err, "requeue-after", time.Now().Add(aShortWait))
 			record.Event(cm, event.Warning(reasonBind, err))
-			cm.SetConditions(v1alpha1.ReconcileError(err))
-			return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
+			return reconcile.Result{RequeueAfter: aShortWait}, nil
 		}
 	}
 
@@ -346,8 +344,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 			// after a brief wait, in case this was a transient error.
 			log.Debug("Cannot delete composite resource", "error", err, "requeue-after", time.Now().Add(aShortWait))
 			record.Event(cm, event.Warning(reasonDelete, err))
-			cm.SetConditions(v1alpha1.Deleting(), v1alpha1.ReconcileError(err))
-			return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
+			return reconcile.Result{RequeueAfter: aShortWait}, nil
 		}
 
 		log.Debug("Successfully deleted composite resource")
@@ -359,8 +356,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 			// after a brief wait, in case this was a transient error.
 			log.Debug("Cannot remove finalizer", "error", err, "requeue-after", time.Now().Add(aShortWait))
 			record.Event(cm, event.Warning(reasonDelete, err))
-			cm.SetConditions(v1alpha1.Deleting(), v1alpha1.ReconcileError(err))
-			return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
+			return reconcile.Result{RequeueAfter: aShortWait}, nil
 		}
 
 		// We've successfully deleted our claim and removed our finalizer. If we
@@ -377,7 +373,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 		// after a brief wait, in case this was a transient error.
 		log.Debug("Cannot add composite resource claim finalizer", "error", err, "requeue-after", time.Now().Add(aShortWait))
 		record.Event(cm, event.Warning(reasonBind, err))
-		cm.SetConditions(v1alpha1.Creating(), v1alpha1.ReconcileError(err))
+		cm.SetConditions(v1alpha1.Creating())
 		return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
 	}
 
@@ -397,7 +393,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 			// issue with the resource class was resolved.
 			log.Debug("Cannot configure composite resource", "error", err, "requeue-after", time.Now().Add(aShortWait))
 			record.Event(cm, event.Warning(reasonConfigure, err))
-			cm.SetConditions(v1alpha1.Creating(), v1alpha1.ReconcileError(err))
+			cm.SetConditions(v1alpha1.Creating())
 			return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
 		}
 
@@ -412,7 +408,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 			// after a brief wait, in case this was a transient error.
 			log.Debug("Cannot create composite resource", "error", err, "requeue-after", time.Now().Add(aShortWait))
 			record.Event(cm, event.Warning(reasonConfigure, err))
-			cm.SetConditions(v1alpha1.Creating(), v1alpha1.ReconcileError(err))
+			cm.SetConditions(v1alpha1.Creating())
 			return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
 		}
 
@@ -426,7 +422,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 
 		// We should be watching the composite resource and will have a request
 		// queued if it changes.
-		cm.SetConditions(Waiting(), v1alpha1.ReconcileSuccess())
+		cm.SetConditions(Waiting())
 		return reconcile.Result{}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
 	}
 
@@ -436,7 +432,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 		// wait, in case this was a transient error.
 		log.Debug("Cannot bind to composite resource", "error", err, "requeue-after", time.Now().Add(aShortWait))
 		record.Event(cm, event.Warning(reasonBind, err))
-		cm.SetConditions(Waiting(), v1alpha1.ReconcileError(err))
+		cm.SetConditions(Waiting())
 		return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
 	}
 
@@ -450,14 +446,14 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 		// secret is created.
 		log.Debug("Cannot propagate connection details from composite resource to claim", "error", err, "requeue-after", time.Now().Add(aShortWait))
 		record.Event(cm, event.Warning(reasonPropagate, err))
-		cm.SetConditions(Waiting(), v1alpha1.ReconcileError(err))
+		cm.SetConditions(Waiting())
 		return reconcile.Result{RequeueAfter: aShortWait}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
 	}
 
 	// We have a watch on both the claim and its composite, so there's no
 	// need to requeue here.
 	record.Event(cm, event.Normal(reasonPropagate, "Successfully propagated connection details from composite resource"))
-	cm.SetConditions(v1alpha1.Available(), v1alpha1.ReconcileSuccess())
+	cm.SetConditions(v1alpha1.Available())
 	return reconcile.Result{Requeue: false}, errors.Wrap(r.client.Status().Update(ctx, cm), errUpdateClaimStatus)
 }
 

diff --git a/pkg/controller/apiextensions/composite/reconciler.go b/pkg/controller/apiextensions/composite/reconciler.go
@@ -232,8 +232,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 	if err := r.composite.SelectComposition(ctx, cr); err != nil {
 		log.Debug(errSelectComp, "error", err)
 		r.record.Event(cr, event.Warning(reasonResolve, err))
-		cr.SetConditions(runtimev1alpha1.ReconcileError(errors.Wrap(err, errSelectComp)))
-		return reconcile.Result{RequeueAfter: shortWait}, errors.Wrap(r.client.Status().Update(ctx, cr), errUpdateStatus)
+		return reconcile.Result{RequeueAfter: shortWait}, nil
 	}
 	r.record.Event(cr, event.Normal(reasonResolve, "Successfully selected composition"))
 
@@ -243,15 +242,13 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 	if err := r.client.Get(ctx, meta.NamespacedNameOf(cr.GetCompositionReference()), comp); err != nil {
 		log.Debug(errGetComp, "error", err)
 		r.record.Event(cr, event.Warning(reasonCompose, err))
-		cr.SetConditions(runtimev1alpha1.ReconcileError(errors.Wrap(err, errGetComp)))
-		return reconcile.Result{RequeueAfter: shortWait}, errors.Wrap(r.client.Status().Update(ctx, cr), errUpdateStatus)
+		return reconcile.Result{RequeueAfter: shortWait}, nil
 	}
 
 	if err := r.composite.Configure(ctx, cr, comp); err != nil {
 		log.Debug(errConfigure, "error", err)
 		r.record.Event(cr, event.Warning(reasonCompose, err))
-		cr.SetConditions(runtimev1alpha1.ReconcileError(errors.Wrap(err, errConfigure)))
-		return reconcile.Result{RequeueAfter: shortWait}, errors.Wrap(r.client.Status().Update(ctx, cr), errUpdateStatus)
+		return reconcile.Result{RequeueAfter: shortWait}, nil
 	}
 
 	log = log.WithValues(
@@ -278,8 +275,7 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 		if err != nil {
 			log.Debug(errReconcile, "error", err)
 			r.record.Event(cr, event.Warning(reasonCompose, err))
-			cr.SetConditions(runtimev1alpha1.ReconcileError(errors.Wrap(err, errReconcile)))
-			return reconcile.Result{RequeueAfter: shortWait}, errors.Wrap(r.client.Status().Update(ctx, cr), errUpdateStatus)
+			return reconcile.Result{RequeueAfter: shortWait}, nil
 		}
 
 		for key, val := range obs.ConnectionDetails {
@@ -304,16 +300,14 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 		if err := r.client.Update(ctx, cr); err != nil {
 			log.Debug(errUpdate, "error", err)
 			r.record.Event(cr, event.Warning(reasonCompose, err))
-			cr.SetConditions(runtimev1alpha1.ReconcileError(errors.Wrap(err, errUpdate)))
-			return reconcile.Result{RequeueAfter: shortWait}, errors.Wrap(r.client.Status().Update(ctx, cr), errUpdateStatus)
+			return reconcile.Result{RequeueAfter: shortWait}, nil
 		}
 	}
 
 	if err := r.composite.PublishConnection(ctx, cr, conn); err != nil {
 		log.Debug(errPublish, "error", err)
 		r.record.Event(cr, event.Warning(reasonPublish, err))
-		cr.SetConditions(runtimev1alpha1.ReconcileError(errors.Wrap(err, errPublish)))
-		return reconcile.Result{RequeueAfter: shortWait}, errors.Wrap(r.client.Status().Update(ctx, cr), errUpdateStatus)
+		return reconcile.Result{RequeueAfter: shortWait}, nil
 	}
 
 	// TODO(negz): Update status.composedResources and status.readyResources if
@@ -331,6 +325,5 @@ func (r *Reconciler) Reconcile(req reconcile.Request) (reconcile.Result, error)
 
 	r.record.Event(cr, event.Normal(reasonPublish, "Successfully published connection details"))
 	r.record.Event(cr, event.Normal(reasonCompose, "Successfully composed resources"))
-	cr.SetConditions(runtimev1alpha1.ReconcileSuccess())
 	return reconcile.Result{RequeueAfter: wait}, errors.Wrap(r.client.Status().Update(ctx, cr), errUpdateStatus)
 }