Allow ChildReconciler to reflect invalid api errors (#418)

API errors with an "invalid" response from the server are now passed to
ReflectChildStatusOnParent. They can be detected with:

    apierrs.IsInvalid(err)

Signed-off-by: Scott Andrews <andrewssc@vmware.com>

reconcilers/child.go

@@ -103,6 +103,7 @@ type ChildReconciler[Type, ChildType client.Object, ChildListType client.ObjectL
 	// ReflectChildStatusOnParent updates the reconciled resource's status with values from the
 	// child. Select types of errors are passed, including:
 	//   - apierrs.IsAlreadyExists
+	//   - apierrs.IsInvalid
 	//
 	// Most errors are returned directly, skipping this method. The set of handled error types
 	// may grow, implementations should be defensive rather than assuming the error type.
@@ -165,6 +166,9 @@ func (r *ChildReconciler[T, CT, CLT]) init() {
 		if r.Name == "" {
 			r.Name = fmt.Sprintf("%sChildReconciler", typeName(r.ChildType))
 		}
+		if r.Sanitize == nil {
+			r.Sanitize = func(child CT) interface{} { return child }
+		}
 		if r.stamp == nil {
 			r.stamp = &ResourceManager[CT]{
 				Name:                     r.Name,
@@ -251,7 +255,8 @@ func (r *ChildReconciler[T, CT, CLT]) Reconcile(ctx context.Context, resource T)
 		return Result{}, err
 	}
 	if err != nil {
-		if apierrs.IsAlreadyExists(err) {
+		switch {
+		case apierrs.IsAlreadyExists(err):
 			// check if the resource blocking create is owned by the reconciled resource.
 			// the created child from a previous turn may be slow to appear in the informer cache, but shouldn't appear
 			// on the reconciled resource as being not ready.
@@ -265,6 +270,9 @@ func (r *ChildReconciler[T, CT, CLT]) Reconcile(ctx context.Context, resource T)
 			log.Info("unable to reconcile child, not owned", "child", namespaceName(conflicted), "ownerRefs", conflicted.GetOwnerReferences())
 			r.ReflectChildStatusOnParent(ctx, resource, child, err)
 			return Result{}, nil
+		case apierrs.IsInvalid(err):
+			r.ReflectChildStatusOnParent(ctx, resource, child, err)
+			return Result{}, nil
 		}
 		log.Error(err, "unable to reconcile child")
 		return Result{}, err

reconcilers/child_test.go

@@ -25,6 +25,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -89,9 +90,13 @@ func TestChildReconciler(t *testing.T) {
 			},
 			ReflectChildStatusOnParent: func(ctx context.Context, parent *resources.TestResource, child *corev1.ConfigMap, err error) {
 				if err != nil {
-					if apierrs.IsAlreadyExists(err) {
+					switch {
+					case apierrs.IsAlreadyExists(err):
 						name := err.(apierrs.APIStatus).Status().Details.Name
 						parent.Status.MarkNotReady(ctx, "NameConflict", "%q already exists", name)
+					case apierrs.IsInvalid(err):
+						name := err.(apierrs.APIStatus).Status().Details.Name
+						parent.Status.MarkNotReady(ctx, "InvalidChild", "%q was rejected by the api server", name)
 					}
 					return
 				}
@@ -657,6 +662,43 @@ func TestChildReconciler(t *testing.T) {
 				},
 			},
 		},
+		"invalid child": {
+			Resource: resourceReady.
+				SpecDie(func(d *dies.TestResourceSpecDie) {
+					d.AddField("foo", "bar")
+				}).
+				DieReleasePtr(),
+			WithReactors: []rtesting.ReactionFunc{
+				rtesting.InduceFailure("create", "ConfigMap", rtesting.InduceFailureOpts{
+					Error: apierrs.NewInvalid(schema.GroupKind{}, testName, field.ErrorList{
+						field.Invalid(field.NewPath("metadata", "name"), testName, ""),
+					}),
+				}),
+			},
+			Metadata: map[string]interface{}{
+				"SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] {
+					return defaultChildReconciler(c)
+				},
+			},
+			ExpectResource: resourceReady.
+				SpecDie(func(d *dies.TestResourceSpecDie) {
+					d.AddField("foo", "bar")
+				}).
+				StatusDie(func(d *dies.TestResourceStatusDie) {
+					d.ConditionsDie(
+						diemetav1.ConditionBlank.Type(apis.ConditionReady).Status(metav1.ConditionFalse).
+							Reason("InvalidChild").Message(`"test-resource" was rejected by the api server`),
+					)
+				}).
+				DieReleasePtr(),
+			ExpectEvents: []rtesting.Event{
+				rtesting.NewEvent(resource, scheme, corev1.EventTypeWarning, "CreationFailed",
+					"Failed to create ConfigMap %q:  %q is invalid: metadata.name: Invalid value: %q", testName, testName, testName),
+			},
+			ExpectCreates: []client.Object{
+				configMapCreate,
+			},
+		},
 		"child name collision": {
 			Resource: resourceReady.
 				SpecDie(func(d *dies.TestResourceSpecDie) {

reconcilers/childset.go

@@ -89,6 +89,7 @@ type ChildSetReconciler[Type, ChildType client.Object, ChildListType client.Obje
 	// ReflectChildrenStatusOnParent updates the reconciled resource's status with values from the
 	// child reconciliations. Select types of errors are captured, including:
 	//   - apierrs.IsAlreadyExists
+	//   - apierrs.IsInvalid
 	//
 	// Most errors are returned directly, skipping this method. The set of handled error types
 	// may grow, implementations should be defensive rather than assuming the error type.