fix: surface errors from mutator failures

Signed-off-by: Tarun Gupta Akirala <[email protected]>
kubernetes-sigs · Mar 1, 2023 · 97fbdc8 · 97fbdc8
1 parent 7b5b587
commit 97fbdc8
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 6 deletions.
diff --git a/cmd/clusterctl/client/cluster/mover.go b/cmd/clusterctl/client/cluster/mover.go
@@ -43,7 +43,7 @@ import (
 )
 
 // ResourceMutatorFunc holds the type for mutators to be applied on resources during a move operation.
-type ResourceMutatorFunc func(u *unstructured.Unstructured)
+type ResourceMutatorFunc func(u *unstructured.Unstructured) error
 
 // ObjectMover defines methods for moving Cluster API objects to another management cluster.
 type ObjectMover interface {
@@ -606,7 +606,10 @@ func patchCluster(proxy Proxy, n *node, patch client.Patch, mutators ...Resource
 	clusterObj.SetName(n.identity.Name)
 	clusterObj.SetNamespace(n.identity.Namespace)
 	for _, mutator := range mutators {
-		mutator(clusterObj)
+		if err = mutator(clusterObj); err != nil {
+			return errors.Wrapf(err, "error applying resource mutator to %q %s/%s",
+				clusterObj.GroupVersionKind(), clusterObj.GetNamespace(), clusterObj.GetName())
+		}
 	}
 
 	if err := cFrom.Get(ctx, client.ObjectKeyFromObject(clusterObj), clusterObj); err != nil {
@@ -635,7 +638,10 @@ func pauseClusterClass(proxy Proxy, n *node, pause bool, mutators ...ResourceMut
 	clusterClass.SetName(n.identity.Name)
 	clusterClass.SetNamespace(n.identity.Namespace)
 	for _, mutator := range mutators {
-		mutator(clusterClass)
+		if err = mutator(clusterClass); err != nil {
+			return errors.Wrapf(err, "error applying resource mutator to %q %s/%s",
+				clusterClass.GroupVersionKind(), clusterClass.GetNamespace(), clusterClass.GetName())
+		}
 	}
 	if err := cFrom.Get(ctx, client.ObjectKeyFromObject(clusterClass), clusterClass); err != nil {
 		return errors.Wrapf(err, "error reading ClusterClass %s/%s", n.identity.Namespace, n.identity.Name)
@@ -887,7 +893,10 @@ func (o *objectMover) createTargetObject(nodeToCreate *node, toProxy Proxy, muta
 	}
 
 	for _, mutator := range mutators {
-		mutator(obj)
+		if err = mutator(obj); err != nil {
+			return errors.Wrapf(err, "error applying resource mutator to %q %s/%s",
+				obj.GroupVersionKind(), obj.GetNamespace(), obj.GetName())
+		}
 	}
 	// Applying mutators MAY change the namespace, so ensure the namespace exists before creating the resource.
 	if !nodeToCreate.isGlobal && !existingNamespaces.Has(obj.GetNamespace()) {

diff --git a/cmd/clusterctl/client/cluster/mover_test.go b/cmd/clusterctl/client/cluster/mover_test.go
@@ -1185,9 +1185,9 @@ func Test_objectMover_move(t *testing.T) {
 					{"spec", "infrastructureRef", "namespace"},
 				},
 			}
-			var namespaceMutator ResourceMutatorFunc = func(u *unstructured.Unstructured) {
+			var namespaceMutator ResourceMutatorFunc = func(u *unstructured.Unstructured) error {
 				if u == nil || u.Object == nil {
-					return
+					return nil
 				}
 				if u.GetNamespace() != "" {
 					u.SetNamespace(toNamespace)
@@ -1201,6 +1201,7 @@ func Test_objectMover_move(t *testing.T) {
 						}
 					}
 				}
+				return nil
 			}
 
 			// Create an objectGraph bound a source cluster with all the CRDs for the types involved in the test.
@@ -1226,6 +1227,7 @@ func Test_objectMover_move(t *testing.T) {
 			if includeMutator {
 				mutators = append(mutators, namespaceMutator)
 			}
+
 			err := mover.move(graph, toProxy, mutators...)
 
 			if tt.wantErr {