MGMT-21202: Enable dual-stack

Author: danmanor

api/v1alpha1/imageclusterinstall_types.go

@@ -94,9 +94,18 @@ type ImageClusterInstallSpec struct {
 	// MachineNetwork is the subnet provided by user for the ocp cluster.
 	// This will be used to create the node network and choose ip address for the node.
 	// Equivalent to install-config.yaml's machineNetwork.
+	// Deprecated: Use MachineNetworks instead.
 	// +optional.
 	MachineNetwork string `json:"machineNetwork,omitempty"`
 
+	// MachineNetworks is the list of IP address pools for machines.
+	// This enables dual-stack support by allowing multiple networks.
+	// If both MachineNetwork and MachineNetworks are specified, MachineNetwork should match
+	// the first element of MachineNetworks.
+	// Use instead of MachineNetwork.
+	// +optional
+	MachineNetworks []MachineNetworkEntry `json:"machineNetworks,omitempty"`
+
 	// Proxy defines the proxy settings to be applied in relocated cluster
 	// +optional
 	Proxy *Proxy `json:"proxy,omitempty"`
@@ -162,6 +171,12 @@ type Proxy struct {
 	NoProxy string `json:"noProxy,omitempty"`
 }
 
+// MachineNetworkEntry is a single IP address block for node IP blocks.
+type MachineNetworkEntry struct {
+	// CIDR is the IP block address pool for machines within the cluster.
+	CIDR string `json:"cidr"`
+}
+
 //+kubebuilder:object:root=true
 
 // ImageClusterInstallList contains a list of ImageClusterInstall

api/v1alpha1/imageclusterinstall_webhook.go

@@ -23,6 +23,7 @@ import (
 	"reflect"
 	"strings"
 
+	"github.com/go-logr/logr"
 	"golang.org/x/crypto/ssh"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/util/validation"
@@ -90,10 +91,12 @@ func (r *ImageClusterInstall) validate() error {
 		return fmt.Errorf("invalid hostname: %w", err)
 	}
 
-	if err := isValidMachineNetwork(r.Spec.MachineNetwork); err != nil {
+	if err := isValidMachineNetworks(r.Spec.MachineNetwork, r.Spec.MachineNetworks); err != nil {
 		return fmt.Errorf("invalid machine network: %w", err)
 	}
-	if err := isValidProxy(r.Spec.Proxy, r.Spec.MachineNetwork); err != nil {
+
+	effectiveMachineNetworks := getEffectiveMachineNetworks(r.Spec.MachineNetwork, r.Spec.MachineNetworks, icilog)
+	if err := isValidProxy(r.Spec.Proxy, effectiveMachineNetworks); err != nil {
 		return fmt.Errorf("invalid proxy: %w", err)
 	}
 	return nil
@@ -139,20 +142,56 @@ func isValidHostname(hostname string) error {
 	return nil
 }
 
-func isValidMachineNetwork(machineNetwork string) error {
-	if machineNetwork == "" {
-		return nil
+// isValidMachineNetworks validates both legacy MachineNetwork and new MachineNetworks fields
+func isValidMachineNetworks(legacyMachineNetwork string, machineNetworks []MachineNetworkEntry) error {
+	// If both are specified, MachineNetworks takes precedence but we still validate both
+	if legacyMachineNetwork != "" {
+		if err := isValidNetworkCidr(legacyMachineNetwork); err != nil {
+			return err
+		}
+	}
+
+	for i, network := range machineNetworks {
+		if err := isValidNetworkCidr(network.CIDR); err != nil {
+			return fmt.Errorf("machine network %d: %w", i, err)
+		}
+	}
+
+	return nil
+}
+
+// getEffectiveMachineNetworks returns the effective machine networks for other validations
+func getEffectiveMachineNetworks(legacyMachineNetwork string, machineNetworks []MachineNetworkEntry, log logr.Logger) []string {
+	if len(machineNetworks) > 0 {
+		networks := make([]string, len(machineNetworks))
+		for i, network := range machineNetworks {
+			networks[i] = network.CIDR
+		}
+		return networks
+	}
+
+	if legacyMachineNetwork != "" {
+		log.Info("Using legacy MachineNetwork field", "legacyMachineNetwork", legacyMachineNetwork)
+		return []string{legacyMachineNetwork}
+	}
+
+	return nil
+}
+
+func isValidNetworkCidr(networkCIDR string) error {
+	if networkCIDR == "" {
+		return fmt.Errorf("network CIDR is empty")
 	}
 
-	_, _, err := net.ParseCIDR(machineNetwork)
+	_, _, err := net.ParseCIDR(networkCIDR)
 	if err != nil {
-		return fmt.Errorf("error parsing machine network, check that it is valid cidr: %w", err)
+		return fmt.Errorf("error parsing network, check that it is valid CIDR: %w", err)
 	}
 	return nil
 }
 
-func isValidProxy(proxy *Proxy, machineNetwork string) error {
-	if proxy != nil && machineNetwork == "" {
+func isValidProxy(proxy *Proxy, machineNetworks []string) error {
+	if proxy != nil && len(machineNetworks) == 0 {
 		return errors.New("machine network must be set when proxy is defined")
 	}
 	return nil

api/v1alpha1/imageclusterinstall_webhook_test.go

@@ -192,6 +192,65 @@ var _ = Describe("ValidateUpdate", func() {
 		Expect(err.Error()).To(ContainSubstring("invalid machine network"))
 	})
 
+	It("create success when dual-stack machine networks are valid", func() {
+		newClusterInstall := &ImageClusterInstall{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "config",
+				Namespace: "test-namespace",
+			},
+			Spec: ImageClusterInstallSpec{
+				MachineNetworks: []MachineNetworkEntry{
+					{CIDR: "192.0.2.0/24"},
+					{CIDR: "2001:db8::/64"},
+				},
+			},
+		}
+
+		warns, err := newClusterInstall.ValidateCreate()
+		Expect(warns).To(BeNil())
+		Expect(err).To(BeNil())
+	})
+
+	It("create fail when machine networks have invalid CIDR", func() {
+		newClusterInstall := &ImageClusterInstall{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "config",
+				Namespace: "test-namespace",
+			},
+			Spec: ImageClusterInstallSpec{
+				MachineNetworks: []MachineNetworkEntry{
+					{CIDR: "192.0.2.0/24"},
+					{CIDR: "invalid_cidr"},
+				},
+			},
+		}
+
+		warns, err := newClusterInstall.ValidateCreate()
+		Expect(warns).To(BeNil())
+		Expect(err.Error()).To(ContainSubstring("invalid machine network"))
+		Expect(err.Error()).To(ContainSubstring("machine network 1"))
+	})
+
+	It("create success when both legacy and new machine network fields are valid", func() {
+		newClusterInstall := &ImageClusterInstall{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      "config",
+				Namespace: "test-namespace",
+			},
+			Spec: ImageClusterInstallSpec{
+				MachineNetwork: "192.0.2.0/24", // Legacy field
+				MachineNetworks: []MachineNetworkEntry{ // New field takes precedence
+					{CIDR: "192.0.2.0/24"},
+					{CIDR: "2001:db8::/64"},
+				},
+			},
+		}
+
+		warns, err := newClusterInstall.ValidateCreate()
+		Expect(warns).To(BeNil())
+		Expect(err).To(BeNil())
+	})
+
 	It("create fail when proxy is invalid", func() {
 		newClusterInstall := &ImageClusterInstall{
 			ObjectMeta: metav1.ObjectMeta{

api/v1alpha1/zz_generated.deepcopy.go

@@ -282,7 +282,26 @@ spec:
                   MachineNetwork is the subnet provided by user for the ocp cluster.
                   This will be used to create the node network and choose ip address for the node.
                   Equivalent to install-config.yaml's machineNetwork.
+                  Deprecated: Use MachineNetworks instead.
                 type: string
+              machineNetworks:
+                description: |-
+                  MachineNetworks is the list of IP address pools for machines.
+                  This enables dual-stack support by allowing multiple networks.
+                  If both MachineNetwork and MachineNetworks are specified, MachineNetworks takes precedence.
+                  Use instead of MachineNetwork.
+                items:
+                  description: MachineNetworkEntry is a single IP address block for
+                    node IP blocks.
+                  properties:
+                    cidr:
+                      description: CIDR is the IP block address pool for machines
+                        within the cluster.
+                      type: string
+                  required:
+                  - cidr
+                  type: object
+                type: array
               nodeIP:
                 description: |-
                   NodeIP is the desired IP for the host

config/crd/bases/extensions.hive.openshift.io_imageclusterinstalls.yaml

@@ -13,3 +13,6 @@ spec:
   bareMetalHostRef:
     name: host-0
     namespace: test-sno
+  machineNetworks:
+    - cidr: "192.0.2.0/24"
+    - cidr: "2001:db8::/64"

config/samples/extensions_v1alpha1_imageclusterinstall.yaml

@@ -448,27 +448,134 @@ func (r *ImageClusterInstallReconciler) validateBMH(
 	}
 
 	// do not requeue in case of invalid BMH
-	err := r.validateBMHMachineNetwork(ici.Spec.MachineNetwork, *bmh.Status.HardwareDetails)
+	err := r.validateBMHMachineNetworks(ici.Spec.MachineNetwork, ici.Spec.MachineNetworks, *bmh.Status.HardwareDetails)
 	if err != nil {
 		cond.Message = err.Error()
 		return ctrl.Result{}, err
 	}
 	return ctrl.Result{}, nil
 }
 
-func (r *ImageClusterInstallReconciler) validateBMHMachineNetwork(
-	machineNetwork string,
+func (r *ImageClusterInstallReconciler) validateBMHMachineNetworks(
+	legacyMachineNetwork string,
+	machineNetworks []v1alpha1.MachineNetworkEntry,
 	hwDetails bmh_v1alpha1.HardwareDetails) error {
-	if machineNetwork == "" {
+
+	effectiveNetworks, err := getEffectiveMachineNetworksCIDRs(legacyMachineNetwork, machineNetworks, r.Log)
+	if err != nil {
+		return err
+	}
+	if len(effectiveNetworks) == 0 {
 		return nil
 	}
-	for _, nic := range hwDetails.NIC {
-		inCIDR, _ := ipInCidr(nic.IP, machineNetwork)
-		if inCIDR {
-			return nil
+
+	if len(effectiveNetworks) == 1 {
+		// Single-stack: check if any NIC has an IP in the machine network
+		for _, nic := range hwDetails.NIC {
+			if nic.IP == "" {
+				continue
+			}
+			inCIDR, _ := ipInCidr(nic.IP, effectiveNetworks[0])
+			if inCIDR {
+				return nil
+			}
+		}
+		return fmt.Errorf("bmh host doesn't have any nic with ip in provided machineNetwork %v", effectiveNetworks[0])
+	}
+
+	// Dual-stack: need to find a NIC that has both IPv4 and IPv6 addresses
+	// in the corresponding machine networks
+	return r.validateDualStackMachineNetworks(effectiveNetworks, hwDetails.NIC)
+}
+
+// validateDualStackMachineNetworks validates that there's a NIC with both
+// IPv4 and IPv6 addresses in their corresponding machine networks
+func (r *ImageClusterInstallReconciler) validateDualStackMachineNetworks(networks []string, nics []bmh_v1alpha1.NIC) error {
+	var ipv4Networks, ipv6Networks []string
+	for _, network := range networks {
+		if isIPv4CIDR(network) {
+			ipv4Networks = append(ipv4Networks, network)
+		} else {
+			ipv6Networks = append(ipv6Networks, network)
+		}
+	}
+
+	if len(ipv4Networks) == 0 || len(ipv6Networks) == 0 {
+		return fmt.Errorf("dual-stack configuration requires both IPv4 and IPv6 machine networks, got IPv4: %v, IPv6: %v", ipv4Networks, ipv6Networks)
+	}
+
+	nicsByMAC := make(map[string][]bmh_v1alpha1.NIC)
+	for _, nic := range nics {
+		if nic.MAC != "" && nic.IP != "" {
+			nicsByMAC[nic.MAC] = append(nicsByMAC[nic.MAC], nic)
 		}
 	}
-	return fmt.Errorf("bmh host doesn't have any nic with ip in provided machineNetwork %s", machineNetwork)
+
+	for _, macNics := range nicsByMAC {
+		hasIPv4InNetwork := false
+		hasIPv6InNetwork := false
+
+		for _, nic := range macNics {
+			for _, ipv4Net := range ipv4Networks {
+				if inCIDR, _ := ipInCidr(nic.IP, ipv4Net); inCIDR {
+					hasIPv4InNetwork = true
+					break
+				}
+			}
+
+			for _, ipv6Net := range ipv6Networks {
+				if inCIDR, _ := ipInCidr(nic.IP, ipv6Net); inCIDR {
+					hasIPv6InNetwork = true
+					break
+				}
+			}
+
+			if hasIPv4InNetwork && hasIPv6InNetwork {
+				return nil
+			}
+		}
+	}
+
+	return fmt.Errorf("bmh host doesn't have a nic with both IPv4 and IPv6 addresses in the provided dual-stack machineNetworks. IPv4 networks: %v, IPv6 networks: %v", ipv4Networks, ipv6Networks)
+}
+
+// isIPv4CIDR determines if a CIDR string represents an IPv4 network
+func isIPv4CIDR(cidr string) bool {
+	_, ipNet, err := net.ParseCIDR(cidr)
+	if err != nil {
+		return false
+	}
+	return ipNet.IP.To4() != nil
+}
+
+// getEffectiveMachineNetworksCIDRs returns the effective machine network CIDRs.
+// If both MachineNetwork and MachineNetworks are specified, MachineNetwork should match
+// the first element of MachineNetworks.
+func getEffectiveMachineNetworksCIDRs(
+	legacyMachineNetwork string,
+	machineNetworks []v1alpha1.MachineNetworkEntry,
+	log logrus.FieldLogger,
+) ([]string, error) {
+	if len(machineNetworks) > 0 {
+		if legacyMachineNetwork != "" {
+			first := machineNetworks[0].CIDR
+			if legacyMachineNetwork != first {
+				return nil, fmt.Errorf("MachineNetwork (%s) does not match the first MachineNetworks entry (%s)", legacyMachineNetwork, first)
+			}
+		}
+		networks := make([]string, len(machineNetworks))
+		for i, network := range machineNetworks {
+			networks[i] = network.CIDR
+		}
+		return networks, nil
+	}
+
+	if legacyMachineNetwork != "" {
+		log.Infof("Using legacy MachineNetwork field '%s' - consider migrating to MachineNetworks field", legacyMachineNetwork)
+		return []string{legacyMachineNetwork}, nil
+	}
+
+	return nil, nil
 }
 
 func (r *ImageClusterInstallReconciler) mapBMHToICI(ctx context.Context, obj client.Object) []reconcile.Request {