NCN-108428: When failureDomain is used, machineDetails "cluster" and "subnets" should be optional

yanhua121 · yanhua121 · commit 250a2bc498e3 · 2025-07-17T00:54:06.000-04:00
instead of required
diff --git a/api/v1alpha1/crds/caren.nutanix.com_nutanixclusterconfigs.yaml b/api/v1alpha1/crds/caren.nutanix.com_nutanixclusterconfigs.yaml
@@ -511,9 +511,7 @@ spec:
                               format: int32
                               type: integer
                           required:
-                            - cluster
                             - memorySize
-                            - subnets
                             - systemDiskSize
                             - vcpuSockets
                             - vcpusPerSocket
diff --git a/api/v1alpha1/crds/caren.nutanix.com_nutanixworkernodeconfigs.yaml b/api/v1alpha1/crds/caren.nutanix.com_nutanixworkernodeconfigs.yaml
@@ -234,9 +234,7 @@ spec:
                           format: int32
                           type: integer
                       required:
-                        - cluster
                         - memorySize
-                        - subnets
                         - systemDiskSize
                         - vcpuSockets
                         - vcpusPerSocket
diff --git a/api/v1alpha1/nutanix_node_types.go b/api/v1alpha1/nutanix_node_types.go
@@ -50,13 +50,15 @@ type NutanixMachineDetails struct {
 
 	// cluster identifies the Prism Element in which the machine will be created.
 	// The identifier (uuid or name) can be obtained from the console or API.
-	// +kubebuilder:validation:Required
-	Cluster capxv1.NutanixResourceIdentifier `json:"cluster"`
+	// +kubebuilder:validation:Optional
+	// +optional
+	Cluster *capxv1.NutanixResourceIdentifier `json:"cluster,omitempty"`
 
 	// subnet identifies the network subnet to use for the machine.
 	// The identifier (uuid or name) can be obtained from the console or API.
-	// +kubebuilder:validation:Required
-	Subnets []capxv1.NutanixResourceIdentifier `json:"subnets"`
+	// +kubebuilder:validation:Optional
+	// +optional
+	Subnets []capxv1.NutanixResourceIdentifier `json:"subnets,omitempty"`
 
 	// List of categories that need to be added to the machines. Categories must already
 	// exist in Prism Central. One category key can have more than one value.
diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go
diff --git a/pkg/handlers/generic/mutation/kubeproxymode/variables_test.go b/pkg/handlers/generic/mutation/kubeproxymode/variables_test.go
@@ -162,13 +162,18 @@ func minimalNutanixClusterConfigSpec() v1alpha1.NutanixClusterConfigSpec {
 						Type: capxv1.NutanixIdentifierName,
 						Name: ptr.To("fake-image"),
 					},
-					Cluster: capxv1.NutanixResourceIdentifier{
+					Cluster: &capxv1.NutanixResourceIdentifier{
 						Type: capxv1.NutanixIdentifierName,
 						Name: ptr.To("fake-pe-cluster"),
 					},
 					MemorySize:     resource.MustParse("8Gi"),
 					SystemDiskSize: resource.MustParse("40Gi"),
-					Subnets:        []capxv1.NutanixResourceIdentifier{},
+					Subnets: []capxv1.NutanixResourceIdentifier{
+						{
+							Type: capxv1.NutanixIdentifierName,
+							Name: ptr.To("fake-subnet"),
+						},
+					},
 				},
 			},
 		},
diff --git a/pkg/handlers/nutanix/mutation/controlplanefailuredomains/variables_test.go b/pkg/handlers/nutanix/mutation/controlplanefailuredomains/variables_test.go
@@ -50,13 +50,18 @@ func minimumClusterConfigSpec() v1alpha1.NutanixClusterConfigSpec {
 						Type: capxv1.NutanixIdentifierName,
 						Name: ptr.To("fake-image"),
 					},
-					Cluster: capxv1.NutanixResourceIdentifier{
+					Cluster: &capxv1.NutanixResourceIdentifier{
 						Type: capxv1.NutanixIdentifierName,
 						Name: ptr.To("fake-pe-cluster"),
 					},
 					MemorySize:     resource.MustParse("8Gi"),
 					SystemDiskSize: resource.MustParse("40Gi"),
-					Subnets:        []capxv1.NutanixResourceIdentifier{},
+					Subnets: []capxv1.NutanixResourceIdentifier{
+						{
+							Type: capxv1.NutanixIdentifierName,
+							Name: ptr.To("fake-subnet"),
+						},
+					},
 				},
 			},
 		},
diff --git a/pkg/handlers/nutanix/mutation/machinedetails/inject.go b/pkg/handlers/nutanix/mutation/machinedetails/inject.go
@@ -97,7 +97,14 @@ func (h *nutanixMachineDetailsPatchHandler) Mutate(
 			spec := obj.Spec.Template.Spec
 
 			spec.BootType = nutanixMachineDetailsVar.BootType
-			spec.Cluster = nutanixMachineDetailsVar.Cluster
+			if nutanixMachineDetailsVar.Cluster != nil {
+				spec.Cluster = *nutanixMachineDetailsVar.Cluster
+			} else {
+				// Have to set the required Type field.
+				spec.Cluster = capxv1.NutanixResourceIdentifier{
+					Type: capxv1.NutanixIdentifierName,
+				}
+			}
 
 			switch {
 			case nutanixMachineDetailsVar.Image != nil:
@@ -118,7 +125,6 @@ func (h *nutanixMachineDetailsPatchHandler) Mutate(
 			spec.Subnets = slices.Clone(nutanixMachineDetailsVar.Subnets)
 			spec.AdditionalCategories = slices.Clone(nutanixMachineDetailsVar.AdditionalCategories)
 			spec.GPUs = slices.Clone(nutanixMachineDetailsVar.GPUs)
-
 			spec.Project = nutanixMachineDetailsVar.Project.DeepCopy()
 
 			obj.Spec.Template.Spec = spec
diff --git a/pkg/handlers/nutanix/mutation/machinedetails/inject_control_plane_test.go b/pkg/handlers/nutanix/mutation/machinedetails/inject_control_plane_test.go
@@ -27,7 +27,7 @@ var (
 			Type: capxv1.NutanixIdentifierName,
 			Name: ptr.To("fake-image"),
 		},
-		Cluster: capxv1.NutanixResourceIdentifier{
+		Cluster: &capxv1.NutanixResourceIdentifier{
 			Type: capxv1.NutanixIdentifierName,
 			Name: ptr.To("fake-pe-cluster"),
 		},
@@ -72,7 +72,7 @@ var (
 		ImageLookup: &capxv1.NutanixImageLookup{
 			BaseOS: "rockylinux-9",
 		},
-		Cluster: capxv1.NutanixResourceIdentifier{
+		Cluster: &capxv1.NutanixResourceIdentifier{
 			Type: capxv1.NutanixIdentifierName,
 			Name: ptr.To("fake-pe-cluster"),
 		},
diff --git a/pkg/handlers/nutanix/mutation/machinedetails/variables_test.go b/pkg/handlers/nutanix/mutation/machinedetails/variables_test.go
@@ -109,13 +109,18 @@ func minimumClusterConfigSpec() v1alpha1.NutanixClusterConfigSpec {
 						Type: capxv1.NutanixIdentifierName,
 						Name: ptr.To("fake-image"),
 					},
-					Cluster: capxv1.NutanixResourceIdentifier{
+					Cluster: &capxv1.NutanixResourceIdentifier{
 						Type: capxv1.NutanixIdentifierName,
 						Name: ptr.To("fake-pe-cluster"),
 					},
 					MemorySize:     resource.MustParse("8Gi"),
 					SystemDiskSize: resource.MustParse("40Gi"),
-					Subnets:        []capxv1.NutanixResourceIdentifier{},
+					Subnets: []capxv1.NutanixResourceIdentifier{
+						{
+							Type: capxv1.NutanixIdentifierName,
+							Name: ptr.To("fake-subnet"),
+						},
+					},
 				},
 			},
 		},
diff --git a/pkg/webhook/cluster/nutanix_validator.go b/pkg/webhook/cluster/nutanix_validator.go
@@ -11,6 +11,7 @@ import (
 	"net/netip"
 
 	v1 "k8s.io/api/admission/v1"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	clusterv1 "sigs.k8s.io/cluster-api/api/v1beta1"
 	ctrlclient "sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/webhook/admission"
@@ -89,9 +90,115 @@ func (a *nutanixValidator) validate(
 		}
 	}
 
+	if err := validateFailureDomainRelatedConfig(clusterConfig, cluster); err != nil {
+		return admission.Denied(err.Error())
+	}
+
 	return admission.Allowed("")
 }
 
+// validateFailureDomainRelatedConfig validates the failure domain related configuration in cluster topology.
+func validateFailureDomainRelatedConfig(
+	clusterConfig *variables.ClusterConfigSpec,
+	cluster *clusterv1.Cluster,
+) error {
+	fldErrs := field.ErrorList{}
+	fldPath := field.NewPath("spec", "topology")
+
+	// Validate that either failureDomains is set, or cluster and subnets are set with machineDetails, for control plane.
+	if clusterConfig.ControlPlane != nil &&
+		clusterConfig.ControlPlane.Nutanix != nil &&
+		len(clusterConfig.ControlPlane.Nutanix.FailureDomains) == 0 {
+
+		machineDetails := clusterConfig.ControlPlane.Nutanix.MachineDetails
+		if machineDetails.Cluster == nil || !(machineDetails.Cluster.IsName() || machineDetails.Cluster.IsUUID()) {
+			fldErrs = append(fldErrs, field.Required(
+				fldPath.Child(
+					"variables",
+					"clusterConfig",
+					"value",
+					"controlPlane",
+					"nutanix",
+					"machineDetails",
+					"cluster",
+				),
+				"\"cluster\" must set when failureDomains is not configured.",
+			))
+		}
+
+		if len(machineDetails.Subnets) == 0 {
+			fldErrs = append(fldErrs, field.Required(
+				fldPath.Child(
+					"variables",
+					"clusterConfig",
+					"value",
+					"controlPlane",
+					"nutanix",
+					"machineDetails",
+					"subnets",
+				),
+				"\"subnets\" must set when failureDomains is not configured.",
+			))
+		}
+	}
+
+	// Validate either failureDomains is set, or cluster and sebnets are set with machineDetails, for workers.
+	if cluster.Spec.Topology.Workers != nil {
+		for i := range cluster.Spec.Topology.Workers.MachineDeployments {
+			md := cluster.Spec.Topology.Workers.MachineDeployments[i]
+			if md.FailureDomain != nil && *md.FailureDomain != "" {
+				// failureDomain is configured
+				continue
+			}
+
+			if md.Variables != nil && len(md.Variables.Overrides) > 0 {
+				workerConfig, err := variables.UnmarshalWorkerConfigVariable(md.Variables.Overrides)
+				if err != nil {
+					fldErrs = append(fldErrs, field.InternalError(
+						fldPath.Child("workers", "machineDeployments", "variables", "workerConfig"),
+						fmt.Errorf("failed to unmarshal worker topology variable: %w", err)))
+				}
+				if workerConfig.Nutanix == nil {
+					continue
+				}
+
+				machineDetails := workerConfig.Nutanix.MachineDetails
+				if machineDetails.Cluster == nil ||
+					!(machineDetails.Cluster.IsName() || machineDetails.Cluster.IsUUID()) {
+					fldErrs = append(fldErrs, field.Required(
+						fldPath.Child(
+							"workers",
+							"machineDeployments",
+							"variables",
+							"workerConfig",
+							"nutanix",
+							"machineDetails",
+							"cluster",
+						),
+						"\"cluster\" must set when failureDomain is not configured.",
+					))
+				}
+				if len(machineDetails.Subnets) == 0 {
+					fldErrs = append(fldErrs, field.Required(
+						fldPath.Child(
+							"workers",
+							"machineDeployments",
+							"variables",
+							"workerConfig",
+							"nutanix",
+							"machineDetails",
+							"subnets",
+						),
+						"\"subnets\" must set when failureDomain is not configured.",
+					))
+				}
+			}
+		}
+	}
+
+	return fldErrs.ToAggregate()
+}
+
 // validatePrismCentralIPNotInLoadBalancerIPRange checks if the Prism Central IP is not
 // in the MetalLB Load Balancer IP range and error out if it is.
 func validatePrismCentralIPNotInLoadBalancerIPRange(
diff --git a/pkg/webhook/preflight/nutanix/failuredomain.go b/pkg/webhook/preflight/nutanix/failuredomain.go
@@ -61,9 +61,12 @@ func newFailureDomainChecks(cd *checkDependencies) []preflight.Check {
 				checks = append(checks, &failureDomainCheck{
 					failureDomainName: *md.FailureDomain,
 					namespace:         cd.cluster.Namespace,
-					field:             "$.spec.topology.workers.machineDeployments[?@.name==%q].failureDomain",
-					kclient:           cd.kclient,
-					nclient:           cd.nclient,
+					field: fmt.Sprintf(
+						"$.spec.topology.workers.machineDeployments[?@.name==%q].failureDomain",
+						md.Name,
+					),
+					kclient: cd.kclient,
+					nclient: cd.nclient,
 				})
 			}
 		}
diff --git a/pkg/webhook/preflight/nutanix/storagecontainer.go b/pkg/webhook/preflight/nutanix/storagecontainer.go
@@ -45,7 +45,7 @@ func (c *storageContainerCheck) Run(ctx context.Context) preflight.CheckResult {
 		return result
 	}
 
-	clusterIdentifier := &c.machineSpec.Cluster
+	clusterIdentifier := c.machineSpec.Cluster
 
 	// To avoid unnecessary API calls, we delay the retrieval of clusters until we actually
 	// need to check for storage containers.
diff --git a/pkg/webhook/preflight/nutanix/storagecontainer_test.go b/pkg/webhook/preflight/nutanix/storagecontainer_test.go
