More comments

util/conditions/v1beta2/patch.go

@@ -30,14 +30,14 @@ import (
 // Patch defines a list of operations to change a list of conditions into another.
 type Patch []PatchOperation
 
-// PatchOperation define an operation that changes a single condition.
+// PatchOperation defines an operation that changes a single condition.
 type PatchOperation struct {
 	Before *metav1.Condition
 	After  *metav1.Condition
 	Op     PatchOperationType
 }
 
-// PatchOperationType defines patch operation types.
+// PatchOperationType defines a condition patch operation type.
 type PatchOperationType string
 
 const (
@@ -131,15 +131,15 @@ func (p Patch) Apply(latest Setter, options ...ApplyOption) error {
 	for _, conditionPatch := range p {
 		switch conditionPatch.Op {
 		case AddConditionPatch:
-			// If the conditions is owned, always keep the after value.
+			// If the condition is owned, always keep the after value.
 			if applyOpt.forceOverwrite || applyOpt.isOwnedConditionType(conditionPatch.After.Type) {
 				meta.SetStatusCondition(&latestConditions, *conditionPatch.After)
 				continue
 			}
 
 			// If the condition is already on latest, check if latest and after agree on the change; if not, this is a conflict.
 			if latestCondition := meta.FindStatusCondition(latestConditions, conditionPatch.After.Type); latestCondition != nil {
-				// If latest and after disagreeWithForceOverwrite on the change, then it is a conflict.
+				// If latest and after disagree on the change, then it is a conflict
 				if !hasSameState(latestCondition, conditionPatch.After) {
 					return errors.Errorf("error patching conditions: The condition %q was modified by a different process and this caused a merge/AddCondition conflict: %v", conditionPatch.After.Type, cmp.Diff(latestCondition, conditionPatch.After))
 				}

util/patch/patch.go

@@ -78,7 +78,7 @@ func NewHelper(obj client.Object, crClient client.Client) (*Helper, error) {
 	}
 
 	// Identify location of the condition fields according to the canonical place defined by the proposal for
-	// // improving status in Cluster API conditions.
+	// improving status in Cluster API conditions.
 	metav1ConditionsFieldPath, clusterv1ConditionsFieldPath, err := identifyConditionsFieldsPath(obj)
 	if err != nil {
 		return nil, errors.Wrapf(err, "failed to identify condition fields for object %s", klog.KObj(obj))
@@ -260,7 +260,7 @@ func (h *Helper) patchStatusConditions(ctx context.Context, obj client.Object, f
 			after,
 		)
 		if err != nil {
-			return errors.Wrapf(err, "%s can not be patched", h.gvk.Kind)
+			return errors.Wrapf(err, "%s %s can not be patched", h.gvk.Kind, klog.KObj(before))
 		}
 		if !diff.IsZero() {
 			clusterv1ApplyPatch = func(latest client.Object) error {
@@ -295,7 +295,7 @@ func (h *Helper) patchStatusConditions(ctx context.Context, obj client.Object, f
 			after,
 		)
 		if err != nil {
-			return errors.Wrapf(err, "%s can not be patched", h.gvk.Kind)
+			return errors.Wrapf(err, "%s %s can not be patched", h.gvk.Kind, klog.KObj(h.beforeObject))
 		}
 
 		if !diff.IsZero() {

util/patch/patch_test.go

@@ -237,7 +237,7 @@ func TestPatchHelper(t *testing.T) {
 
 				objCopy := obj.DeepCopy()
 
-				t.Log("Marking Ready=False")
+				t.Log("Marking TestCondition=False")
 				conditions.MarkFalse(objCopy, clusterv1.ConditionType("TestCondition"), "reason", clusterv1.ConditionSeverityInfo, "message")
 				g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed())
 
@@ -299,7 +299,7 @@ func TestPatchHelper(t *testing.T) {
 
 				objCopy := obj.DeepCopy()
 
-				t.Log("Marking Ready=False")
+				t.Log("Marking TestCondition=False")
 				conditions.MarkFalse(objCopy, clusterv1.ConditionType("TestCondition"), "reason", clusterv1.ConditionSeverityInfo, "message")
 				g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed())
 
@@ -367,7 +367,7 @@ func TestPatchHelper(t *testing.T) {
 
 				objCopy := obj.DeepCopy()
 
-				t.Log("Marking ")
+				t.Log("Marking Ready=False")
 				conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message")
 				g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed())
 
@@ -422,7 +422,7 @@ func TestPatchHelper(t *testing.T) {
 
 				objCopy := obj.DeepCopy()
 
-				t.Log("Marking ")
+				t.Log("Marking Ready=False")
 				conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message")
 				g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed())
 
@@ -471,7 +471,7 @@ func TestPatchHelper(t *testing.T) {
 
 				objCopy := obj.DeepCopy()
 
-				t.Log("Marking ")
+				t.Log("Marking Ready=False")
 				conditions.MarkFalse(objCopy, clusterv1.ReadyCondition, "reason", clusterv1.ConditionSeverityInfo, "message")
 				g.Expect(env.Status().Update(ctx, objCopy)).To(Succeed())