From 0b3321f1b73d0ed19d4ccca52bf7c4a079085b76 Mon Sep 17 00:00:00 2001 From: Gabriel Mougard Date: Thu, 5 Dec 2024 11:32:44 +0100 Subject: [PATCH 1/5] cmd/microcloud: Introduce a new 'Network' helper struct This type will be used to store a variety of subnet informations and which local network interface they will use. This will be used both for configuring MicroCloud but for running validation as well (e.g, interface collisions within a member, subnet collisions between members of a cluster) Signed-off-by: Gabriel Mougard --- cmd/microcloud/ask.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/cmd/microcloud/ask.go b/cmd/microcloud/ask.go index 280a8540..86f0ee42 100644 --- a/cmd/microcloud/ask.go +++ b/cmd/microcloud/ask.go @@ -911,6 +911,22 @@ func (c *initConfig) askRemotePool(sh *service.Handler) error { return nil } +// Network is a helper struct to store an IP address, its subnet and +// its corresponding network interface name. +type Network struct { + // Interface is the name of the network interface. An example + // of why this is useful in MicroCloud is when we want to check that different network types (OVN, Ceph, etc) + // are on different network interfaces. + Interface net.Interface + // IP is the IP address of the network. An example of why this is useful in MicroCloud is when we want to store + // a member OVN underlay network IP address. + IP net.IP + // Subnet is the subnet of the network. An example of why this is useful in MicroCloud is when we want to check + // that we don't have subnet collisions between different network types (OVN, Ceph, etc) in a cluster. + // For example, we don't want a member using 'subnet A' for OVN and an other member using 'subnet A' for Ceph. + Subnet *net.IPNet +} + func (c *initConfig) askOVNNetwork(sh *service.Handler) error { if sh.Services[types.MicroOVN] == nil { return nil From 6af10b3f3c3eff647d2ab35bcf843b10789c2ec9 Mon Sep 17 00:00:00 2001 From: Gabriel Mougard Date: Thu, 5 Dec 2024 11:37:14 +0100 Subject: [PATCH 2/5] service: Introduce the 'FindInterfaceForSubnet' function This will be needed to get a network interface info from a CIDR subnet notation (e.g, mostly in the case of configuring MicroCloud internal and public networks) Signed-off-by: Gabriel Mougard --- service/lxd_config.go | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/service/lxd_config.go b/service/lxd_config.go index 7a1a4607..ba122bb5 100644 --- a/service/lxd_config.go +++ b/service/lxd_config.go @@ -308,3 +308,37 @@ func (s LXDService) DefaultCephFSStoragePoolJoinConfig() api.ClusterMemberConfig Value: DefaultCephFSOSDPool, } } + +// FindInterfaceForSubnet takes a subnet in CIDR notation (e.g., "192.168.1.0/24") +// and returns the network interface that has an IP address in that subnet. +func (s LXDService) FindInterfaceForSubnet(cidrSubnet string) (*net.Interface, error) { + _, targetNet, err := net.ParseCIDR(cidrSubnet) + if err != nil { + return nil, fmt.Errorf("Invalid subnet notation: %v", err) + } + + interfaces, err := net.Interfaces() + if err != nil { + return nil, fmt.Errorf("Failed to list interfaces: %v", err) + } + + for _, iface := range interfaces { + addrs, err := iface.Addrs() + if err != nil { + continue + } + + for _, addr := range addrs { + ip, _, err := net.ParseCIDR(addr.String()) + if err != nil { + return nil, fmt.Errorf("Failed to parse IP address %q: %w", addr.String(), err) + } + + if targetNet.Contains(ip) { + return &iface, nil + } + } + } + + return nil, fmt.Errorf("No interface found for CIDR subnet %q", cidrSubnet) +} From cad47873a1b64bf8f0db97535ab025d58e55bc56 Mon Sep 17 00:00:00 2001 From: Gabriel Mougard Date: Thu, 5 Dec 2024 11:39:50 +0100 Subject: [PATCH 3/5] cmd/microcloud: refactoring to respectively use `OVNGeneveNetwork`, `MicroCephPublicNetwork` and `MicroCephInternalNetwork` instead of `OVNGeneveAddr`, `MicroCephPublicNetworkSubnet` and `MicroCephInternalNetworkSubnet` Signed-off-by: Gabriel Mougard --- cmd/microcloud/ask.go | 56 ++++++++++++++++++++++++---------- cmd/microcloud/main_init.go | 60 ++++++++++++++++++++++++------------- cmd/microcloud/preseed.go | 41 +++++++++++++++++++++++-- 3 files changed, 118 insertions(+), 39 deletions(-) diff --git a/cmd/microcloud/ask.go b/cmd/microcloud/ask.go index 86f0ee42..3c0be4f3 100644 --- a/cmd/microcloud/ask.go +++ b/cmd/microcloud/ask.go @@ -1149,7 +1149,7 @@ func (c *initConfig) askOVNNetwork(sh *service.Handler) error { } } - var ovnUnderlaySelectedIPs map[string]string + var ovnUnderlaySelectedNets map[string]*Network ovnUnderlayData := [][]string{} for peer, system := range c.systems { // skip any systems that have already been clustered, but are available for other configuration. @@ -1175,7 +1175,6 @@ func (c *initConfig) askOVNNetwork(sh *service.Handler) error { if wantsDedicatedUnderlay { header = []string{"LOCATION", "IFACE", "TYPE", "IP ADDRESS (CIDR)"} - ovnUnderlaySelectedIPs = map[string]string{} err = c.askRetry("Retry selecting underlay network interfaces?", func() error { table := tui.NewSelectableTable(header, ovnUnderlayData) answers, err := table.Render(context.Background(), c.asker, "Select exactly one network interface from each cluster member:") @@ -1183,20 +1182,22 @@ func (c *initConfig) askOVNNetwork(sh *service.Handler) error { return err } - ovnUnderlaySelectedIPs = map[string]string{} + ovnUnderlaySelectedNets = make(map[string]*Network) for _, answer := range answers { target := answer["LOCATION"] ipAddr := answer["IP ADDRESS (CIDR)"] - if ovnUnderlaySelectedIPs[target] != "" { + ifaceName := answer["IFACE"] + + if ovnUnderlaySelectedNets[target] != nil { return fmt.Errorf("Failed to configure OVN underlay traffic: Selected more than one interface for target %q", target) } - ip, _, err := net.ParseCIDR(ipAddr) + ip, ipNet, err := net.ParseCIDR(ipAddr) if err != nil { return err } - ovnUnderlaySelectedIPs[target] = ip.String() + ovnUnderlaySelectedNets[target] = &Network{Interface: net.Interface{Name: ifaceName}, IP: ip, Subnet: ipNet} } return nil @@ -1207,11 +1208,11 @@ func (c *initConfig) askOVNNetwork(sh *service.Handler) error { } } - if len(ovnUnderlaySelectedIPs) > 0 { + if len(ovnUnderlaySelectedNets) > 0 { for peer := range askSystems { - underlayIP, ok := ovnUnderlaySelectedIPs[peer] + underlayNetwork, ok := ovnUnderlaySelectedNets[peer] if ok { - fmt.Printf(" Using %q for OVN underlay traffic on %q\n", underlayIP, peer) + fmt.Printf(" Using %q for OVN underlay traffic on %q\n", underlayNetwork.IP.String(), peer) } } @@ -1248,10 +1249,10 @@ func (c *initConfig) askOVNNetwork(sh *service.Handler) error { system.Networks = append(system.Networks, finalConfigs...) } - if ovnUnderlaySelectedIPs != nil { - ovnUnderlayIpAddr, ok := ovnUnderlaySelectedIPs[peer] + if ovnUnderlaySelectedNets != nil { + ovnUnderlayNet, ok := ovnUnderlaySelectedNets[peer] if ok { - system.OVNGeneveAddr = ovnUnderlayIpAddr + system.OVNGeneveNetwork = ovnUnderlayNet } } @@ -1409,14 +1410,24 @@ func (c *initConfig) askCephNetwork(sh *service.Handler) error { return err } + internalCephNetworkInterface, err := lxd.FindInterfaceForSubnet(internalCephSubnet) + if err != nil { + return fmt.Errorf("Failed to find interface for subnet %q: %w", internalCephSubnet, err) + } + if internalCephSubnet != microCloudInternalNetworkAddrCIDR { err = c.validateCephInterfacesForSubnet(lxd, availableCephNetworkInterfaces, internalCephSubnet) if err != nil { return err } + internalCephIP, internalCephNet, err := net.ParseCIDR(internalCephSubnet) + if err != nil { + return fmt.Errorf("Failed to parse the internal Ceph network: %w", err) + } + bootstrapSystem := c.systems[sh.Name] - bootstrapSystem.MicroCephInternalNetworkSubnet = internalCephSubnet + bootstrapSystem.MicroCephInternalNetwork = &Network{Interface: *internalCephNetworkInterface, Subnet: internalCephNet, IP: internalCephIP} c.systems[sh.Name] = bootstrapSystem } @@ -1425,6 +1436,11 @@ func (c *initConfig) askCephNetwork(sh *service.Handler) error { return err } + publicCephNetworkInterface, err := lxd.FindInterfaceForSubnet(publicCephSubnet) + if err != nil { + return fmt.Errorf("Failed to find interface for subnet %q: %w", publicCephSubnet, err) + } + if publicCephSubnet != internalCephSubnet { err = c.validateCephInterfacesForSubnet(lxd, availableCephNetworkInterfaces, publicCephSubnet) if err != nil { @@ -1433,15 +1449,25 @@ func (c *initConfig) askCephNetwork(sh *service.Handler) error { } if publicCephSubnet != microCloudInternalNetworkAddrCIDR { + publicCephIP, publicCephNet, err := net.ParseCIDR(publicCephSubnet) + if err != nil { + return fmt.Errorf("Failed to parse the public Ceph network: %w", err) + } + bootstrapSystem := c.systems[sh.Name] - bootstrapSystem.MicroCephPublicNetworkSubnet = publicCephSubnet + bootstrapSystem.MicroCephPublicNetwork = &Network{Interface: *publicCephNetworkInterface, Subnet: publicCephNet, IP: publicCephIP} c.systems[sh.Name] = bootstrapSystem // This is to avoid the situation where the internal network for Ceph has been skipped, but the public network has been set. // Ceph will automatically set the internal network to the public Ceph network if the internal network is not set, which is not what we want. // Instead, we still want to keep the internal Ceph network to use the MicroCloud internal network as a default. if internalCephSubnet == microCloudInternalNetworkAddrCIDR { - bootstrapSystem.MicroCephInternalNetworkSubnet = microCloudInternalNetworkAddrCIDR + microcloudInternalIP, microcloudNet, err := net.ParseCIDR(microCloudInternalNetworkAddrCIDR) + if err != nil { + return fmt.Errorf("Failed to parse the internal MicroCloud network: %w", err) + } + + bootstrapSystem.MicroCephInternalNetwork = &Network{Interface: bootstrapSystem.MicroCloudInternalNetwork.Interface, Subnet: microcloudNet, IP: microcloudInternalIP} c.systems[sh.Name] = bootstrapSystem } } diff --git a/cmd/microcloud/main_init.go b/cmd/microcloud/main_init.go index 456bdcdf..a33740fa 100644 --- a/cmd/microcloud/main_init.go +++ b/cmd/microcloud/main_init.go @@ -53,19 +53,24 @@ type InitSystem struct { AvailableDisks []lxdAPI.ResourcesStorageDisk // MicroCephDisks contains the disks intended to be passed to MicroCeph. MicroCephDisks []cephTypes.DisksPost - // MicroCephPublicNetworkSubnet is an optional subnet (IPv4/IPv6 CIDR notation) for the Ceph public network. - MicroCephPublicNetworkSubnet string - // MicroCephClusterNetworkSubnet is an optional subnet (IPv4/IPv6 CIDR notation) for the Ceph cluster network. - MicroCephInternalNetworkSubnet string + // MicroCephPublicNetwork contains an optional subnet (IPv4/IPv6 CIDR notation) for the Ceph public network and + // the network interface name to use for the Ceph public network and its IP address within the subnet. + MicroCephPublicNetwork *Network + // MicroCephInternalNetwork contains an optional subnet (IPv4/IPv6 CIDR notation) for the Ceph cluster network and + // the network interface name to use for the Ceph cluster network and its IP address within the subnet. + MicroCephInternalNetwork *Network + // MicroCloudInternalNetwork contains the network configuration for the MicroCloud internal network. + MicroCloudInternalNetwork *Network // TargetNetworks contains the network configuration for the target system. TargetNetworks []lxdAPI.NetworksPost // TargetStoragePools contains the storage pool configuration for the target system. TargetStoragePools []lxdAPI.StoragePoolsPost // Networks is the cluster-wide network configuration. Networks []lxdAPI.NetworksPost - // OVNGeneveAddr represents an IP address to use for the OVN (if OVN is supported) Geneve tunnel on this system. + // OVNGeneveNetwork contains an IP address to use for the OVN (if OVN is supported) Geneve tunnel on this system. // If left empty, the system will choose to route the Geneve traffic through the management network. - OVNGeneveAddr string + // It also contains the network interface name to use for the OVN Geneve tunnel and the network subnet. + OVNGeneveNetwork *Network // StoragePools is the cluster-wide storage pool configuration. StoragePools []lxdAPI.StoragePoolsPost // StorageVolumes is the cluster-wide storage volume configuration. @@ -214,6 +219,20 @@ func (c *initConfig) RunInteractive(cmd *cobra.Command, args []string) error { services[s.Type()] = version } + // Also register the MicroCloud internal network in the bootstrap system information. + lxd := s.Services[types.MicroCloud].(*service.LXDService) + microCloudInternalNetworkAddr := c.lookupSubnet.IP.Mask(c.lookupSubnet.Mask) + ones, _ := c.lookupSubnet.Mask.Size() + microCloudInternalNetworkAddrCIDR := fmt.Sprintf("%s/%d", microCloudInternalNetworkAddr.String(), ones) + microcloudNetworkInterface, err := lxd.FindInterfaceForSubnet(microCloudInternalNetworkAddrCIDR) + if err != nil { + return fmt.Errorf("Failed to find interface for subnet %q: %w", microCloudInternalNetworkAddrCIDR, err) + } + + bootstrapSystem := c.systems[c.name] + bootstrapSystem.MicroCloudInternalNetwork = &Network{Interface: *microcloudNetworkInterface, Subnet: c.lookupSubnet, IP: microCloudInternalNetworkAddr} + c.systems[c.name] = bootstrapSystem + var reverter *revert.Reverter if c.setupMany { err = c.runSession(context.Background(), s, types.SessionInitiating, c.sessionTimeout, func(gw *cloudClient.WebsocketGateway) error { @@ -388,9 +407,9 @@ func (c *initConfig) addPeers(sh *service.Handler) (revert.Hook, error) { CephConfig: info.MicroCephDisks, } - if info.OVNGeneveAddr != "" { + if info.OVNGeneveNetwork != nil { p := joinConfig[peer] - p.OVNConfig = map[string]string{"ovn-encap-ip": info.OVNGeneveAddr} + p.OVNConfig = map[string]string{"ovn-encap-ip": info.OVNGeneveNetwork.IP.String()} joinConfig[peer] = p } } @@ -547,23 +566,22 @@ func validateGatewayNet(config map[string]string, ipPrefix string, cidrValidator func (c *initConfig) validateSystems(s *service.Handler) (err error) { for _, sys := range c.systems { - if sys.MicroCephInternalNetworkSubnet == "" || sys.OVNGeneveAddr == "" { + if sys.MicroCephInternalNetwork == nil || sys.OVNGeneveNetwork == nil { continue } - _, subnet, err := net.ParseCIDR(sys.MicroCephInternalNetworkSubnet) + _, subnet, err := net.ParseCIDR(sys.MicroCephInternalNetwork.Subnet.String()) if err != nil { return fmt.Errorf("Failed to parse available network interface CIDR address: %q: %w", subnet, err) } - underlayIP := net.ParseIP(sys.OVNGeneveAddr) + underlayIP := net.ParseIP(sys.OVNGeneveNetwork.IP.String()) if underlayIP == nil { - return fmt.Errorf("OVN underlay IP %q is invalid", sys.OVNGeneveAddr) + return fmt.Errorf("OVN underlay IP %q is invalid", sys.OVNGeneveNetwork.IP.String()) } - if subnet.Contains(underlayIP) { - tui.PrintWarning(fmt.Sprintf("OVN underlay IP (%s) is shared with the Ceph cluster network (%s)\n", underlayIP.String(), subnet.String())) - + if sys.MicroCephInternalNetwork.Subnet.Contains(sys.OVNGeneveNetwork.IP) { + tui.PrintWarning(fmt.Sprintf("OVN underlay IP (%s) is shared with the Ceph cluster network (%s)\n", sys.OVNGeneveNetwork.IP.String(), sys.MicroCephInternalNetwork.Subnet.String())) break } } @@ -694,12 +712,12 @@ func (c *initConfig) setupCluster(s *service.Handler) error { if s.Type() == types.MicroCeph { microCephBootstrapConf := make(map[string]string) - if bootstrapSystem.MicroCephInternalNetworkSubnet != "" { - microCephBootstrapConf["ClusterNet"] = bootstrapSystem.MicroCephInternalNetworkSubnet + if bootstrapSystem.MicroCephInternalNetwork != nil { + microCephBootstrapConf["ClusterNet"] = bootstrapSystem.MicroCephInternalNetwork.Subnet.String() } - if bootstrapSystem.MicroCephPublicNetworkSubnet != "" { - microCephBootstrapConf["PublicNet"] = bootstrapSystem.MicroCephPublicNetworkSubnet + if bootstrapSystem.MicroCephPublicNetwork != nil { + microCephBootstrapConf["PublicNet"] = bootstrapSystem.MicroCephPublicNetwork.Subnet.String() } if len(microCephBootstrapConf) > 0 { @@ -709,8 +727,8 @@ func (c *initConfig) setupCluster(s *service.Handler) error { if s.Type() == types.MicroOVN { microOvnBootstrapConf := make(map[string]string) - if bootstrapSystem.OVNGeneveAddr != "" { - microOvnBootstrapConf["ovn-encap-ip"] = bootstrapSystem.OVNGeneveAddr + if bootstrapSystem.OVNGeneveNetwork != nil { + microOvnBootstrapConf["ovn-encap-ip"] = bootstrapSystem.OVNGeneveNetwork.IP.String() } if len(microOvnBootstrapConf) > 0 { diff --git a/cmd/microcloud/preseed.go b/cmd/microcloud/preseed.go index 2cee2af0..f06533ad 100644 --- a/cmd/microcloud/preseed.go +++ b/cmd/microcloud/preseed.go @@ -794,6 +794,8 @@ func (p *Preseed) Parse(s *service.Handler, c *initConfig, installedServices map return nil, fmt.Errorf("Failed to parse supplied underlay IP %q", sys.UnderlayIP) } + ovnUnderlayIfaceByPeer := make(map[string]string) + ovnUnderlaySubnetByPeer := make(map[string]*net.IPNet) for _, iface := range addressedInterfaces[sys.Name] { for _, cidr := range iface.Addresses { _, subnet, err := net.ParseCIDR(cidr) @@ -803,6 +805,8 @@ func (p *Preseed) Parse(s *service.Handler, c *initConfig, installedServices map if subnet.Contains(underlayIP) { assignedSystems[sys.Name] = true + ovnUnderlayIfaceByPeer[sys.Name] = iface.Network.Name + ovnUnderlaySubnetByPeer[sys.Name] = subnet break } } @@ -812,8 +816,13 @@ func (p *Preseed) Parse(s *service.Handler, c *initConfig, installedServices map return nil, fmt.Errorf("No available interface found for OVN underlay IP %q", sys.UnderlayIP) } + ifaceName, ok := ovnUnderlayIfaceByPeer[sys.Name] + if !ok { + return nil, fmt.Errorf("Failed to find OVN underlay interface name for system %q", sys.Name) + } + system := c.systems[sys.Name] - system.OVNGeneveAddr = sys.UnderlayIP + system.OVNGeneveNetwork = &Network{Interface: net.Interface{Name: ifaceName}, Subnet: ovnUnderlaySubnetByPeer[sys.Name], IP: underlayIP} c.systems[sys.Name] = system } } @@ -1095,14 +1104,30 @@ func (p *Preseed) Parse(s *service.Handler, c *initConfig, installedServices map internalCephNetwork = customTargetCephInternalNetwork } + // Also register the MicroCloud internal network in the bootstrap system information. + + bootstrapSystem := c.systems[s.Name] + bootstrapSystem.MicroCloudInternalNetwork = &Network{Interface: net.Interface{Name: c.lookupIface.Name}, Subnet: c.lookupSubnet, IP: nil} + c.systems[s.Name] = bootstrapSystem + if internalCephNetwork != "" { err = c.validateCephInterfacesForSubnet(lxd, addressedInterfaces, internalCephNetwork) if err != nil { return nil, err } + iface, err := lxd.FindInterfaceForSubnet(internalCephNetwork) + if err != nil { + return nil, err + } + + _, internalCephSubnet, err := net.ParseCIDR(internalCephNetwork) + if err != nil { + return nil, fmt.Errorf("Invalid CIDR for internal Ceph subnet: %v", err) + } + bootstrapSystem := c.systems[s.Name] - bootstrapSystem.MicroCephInternalNetworkSubnet = internalCephNetwork + bootstrapSystem.MicroCephInternalNetwork = &Network{Interface: net.Interface{Name: iface.Name}, Subnet: internalCephSubnet, IP: nil} c.systems[s.Name] = bootstrapSystem } @@ -1119,8 +1144,18 @@ func (p *Preseed) Parse(s *service.Handler, c *initConfig, installedServices map return nil, err } + iface, err := lxd.FindInterfaceForSubnet(publicCephNetwork) + if err != nil { + return nil, err + } + + _, publicCephSubnet, err := net.ParseCIDR(publicCephNetwork) + if err != nil { + return nil, fmt.Errorf("Invalid CIDR for public Ceph subnet: %w", err) + } + bootstrapSystem := c.systems[s.Name] - bootstrapSystem.MicroCephPublicNetworkSubnet = publicCephNetwork + bootstrapSystem.MicroCephPublicNetwork = &Network{Interface: *iface, Subnet: publicCephSubnet, IP: nil} c.systems[s.Name] = bootstrapSystem } } else { From 395b848a89513098a6449726d9f1e5b57b0a277a Mon Sep 17 00:00:00 2001 From: Gabriel Mougard Date: Fri, 31 Jan 2025 18:09:32 +0100 Subject: [PATCH 4/5] cmd/microcloud: Add the `detectCollisions` function This function detect the local network interface collisions and the global subet collisions Signed-off-by: Gabriel Mougard --- cmd/microcloud/main_init.go | 127 ++++++++++++++++++++++++++++++++---- 1 file changed, 114 insertions(+), 13 deletions(-) diff --git a/cmd/microcloud/main_init.go b/cmd/microcloud/main_init.go index a33740fa..8c986cc4 100644 --- a/cmd/microcloud/main_init.go +++ b/cmd/microcloud/main_init.go @@ -6,6 +6,7 @@ import ( "fmt" "net" "os" + "sort" "strings" "sync" "time" @@ -564,25 +565,125 @@ func validateGatewayNet(config map[string]string, ipPrefix string, cidrValidator return ovnIPRanges, nil } -func (c *initConfig) validateSystems(s *service.Handler) (err error) { - for _, sys := range c.systems { - if sys.MicroCephInternalNetwork == nil || sys.OVNGeneveNetwork == nil { - continue +// detectCollisions checks for network collisions across systems. +// This tracks subnet usage across all systems using a global map and ensure collisions are +// detected when different systems use the same subnet for different network types. +// This also flag interface collisions within each individual system. +func detectCollisions(systems map[string]InitSystem) []string { + // subnetsCollisionMap maps a subnet CIDR notation to a map of subnet type to peer names. + // + // Example: + // { + // "10.0.1.0/24": { + // "OVN underlay": ["system1", "system2"], + // "Ceph cluster network": ["system3"] + // }, + // "10.0.2.0/24": { + // "Ceph public network": ["system1", "system2", "system3"], + // } + // } + // + // In this example, we have a collision for the subnet "10.0.1.0/24": + // - system1 and system2 are using it for the OVN underlay, whereas system3 is using it for the Ceph cluster network. + // Everything is fine for the subnet "10.0.2.0/24" as all systems are using it for the Ceph public network. + subnetsCollisionMap := make(map[string]map[string]struct{}) + interfaceCollisionMap := make(map[string]map[string]struct{}) + subnetToGlobalTypes := make(map[string]map[string]struct{}) + + sortedKeys := func(set map[string]struct{}) []string { + keys := make([]string, 0, len(set)) + for k := range set { + keys = append(keys, k) } - _, subnet, err := net.ParseCIDR(sys.MicroCephInternalNetwork.Subnet.String()) - if err != nil { - return fmt.Errorf("Failed to parse available network interface CIDR address: %q: %w", subnet, err) + sort.Strings(keys) + return keys + } + + for _, sys := range systems { + // if sys.MicroCephInternalNetwork == nil || sys.OVNGeneveNetwork == nil || sys.MicroCloudInternalNetwork == nil { + // continue + // } + + netTypeToNet := map[string]*Network{ + "Ceph cluster network": sys.MicroCephInternalNetwork, + "OVN underlay": sys.OVNGeneveNetwork, + "MicroCloud internal network": sys.MicroCloudInternalNetwork, } - underlayIP := net.ParseIP(sys.OVNGeneveNetwork.IP.String()) - if underlayIP == nil { - return fmt.Errorf("OVN underlay IP %q is invalid", sys.OVNGeneveNetwork.IP.String()) + if sys.MicroCephPublicNetwork != nil { + netTypeToNet["Ceph public network"] = sys.MicroCephPublicNetwork } - if sys.MicroCephInternalNetwork.Subnet.Contains(sys.OVNGeneveNetwork.IP) { - tui.PrintWarning(fmt.Sprintf("OVN underlay IP (%s) is shared with the Ceph cluster network (%s)\n", sys.OVNGeneveNetwork.IP.String(), sys.MicroCephInternalNetwork.Subnet.String())) - break + // Track interface collisions (local to each system) + interfaceToTypes := make(map[string][]string) + for netType, net := range netTypeToNet { + if net == nil { + continue + } + + interfaceToTypes[net.Interface.Name] = append(interfaceToTypes[net.Interface.Name], netType) + } + + for iface, types := range interfaceToTypes { + if len(types) > 1 { + _, ok := interfaceCollisionMap[iface] + if !ok { + interfaceCollisionMap[iface] = make(map[string]struct{}) + } + + for _, t := range types { + interfaceCollisionMap[iface][t] = struct{}{} + } + } + } + + // Track subnets globally across systems + for netType, net := range netTypeToNet { + if net == nil { + continue + } + + subnetStr := net.Subnet.String() + _, exists := subnetToGlobalTypes[subnetStr] + if !exists { + subnetToGlobalTypes[subnetStr] = make(map[string]struct{}) + } + + subnetToGlobalTypes[subnetStr][netType] = struct{}{} + } + } + + // Build subnet collisions from global subnets + for subnet, types := range subnetToGlobalTypes { + if len(types) > 1 { + subnetsCollisionMap[subnet] = types + } + } + + warnings := make([]string, 0) + + // Format interface collisions + for iface, typesSet := range interfaceCollisionMap { + types := sortedKeys(typesSet) + warnings = append(warnings, fmt.Sprintf("- %s sharing network interface %q", strings.Join(types, ", "), iface)) + } + + // Format subnet collisions + for subnet, typesSet := range subnetsCollisionMap { + types := sortedKeys(typesSet) + warnings = append(warnings, fmt.Sprintf("- %s sharing subnet %q", strings.Join(types, ", "), subnet)) + } + + return warnings +} + +func (c *initConfig) validateSystems(s *service.Handler) (err error) { + warnings := detectCollisions(c.systems) + if len(warnings) > 0 { + fmt.Println("! Network collision:") + for _, warning := range warnings { + fmt.Println(warning) } } From c6ab1d73ceffe10bf32a8c3bf2e27e7bd46365bc Mon Sep 17 00:00:00 2001 From: Gabriel Mougard Date: Wed, 29 Jan 2025 16:06:44 +0100 Subject: [PATCH 5/5] cmd/microcloud: Add unit tests for `detectCollisions` Signed-off-by: Gabriel Mougard --- cmd/microcloud/main_init_test.go | 102 +++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/cmd/microcloud/main_init_test.go b/cmd/microcloud/main_init_test.go index 4cc8360f..fa81b5d6 100644 --- a/cmd/microcloud/main_init_test.go +++ b/cmd/microcloud/main_init_test.go @@ -1,9 +1,11 @@ package main import ( + "net" "testing" lxdAPI "github.com/canonical/lxd/shared/api" + "github.com/stretchr/testify/assert" "github.com/canonical/microcloud/microcloud/multicast" "github.com/canonical/microcloud/microcloud/service" @@ -226,3 +228,103 @@ func TestValidateSystemsMultiSystem(t *testing.T) { t.Fatalf("sys4 with conflicting management IP and ipv6.ovn.ranges passed validation") } } + +func newNetwork(ifaceName, ipStr string) *Network { + ip, subnet, _ := net.ParseCIDR(ipStr) + return &Network{ + Interface: net.Interface{Name: ifaceName}, + IP: ip, + Subnet: subnet, + } +} + +func TestValidateSystemsNetworkCollision(t *testing.T) { + tests := []struct { + name string + systems map[string]InitSystem + expectedWarnings []string + }{ + { + name: "no collisions", + systems: map[string]InitSystem{ + "system1": { + MicroCephPublicNetwork: newNetwork("eth0", "10.0.1.0/24"), + MicroCephInternalNetwork: newNetwork("eth1", "10.0.2.0/24"), + MicroCloudInternalNetwork: newNetwork("eth2", "10.0.3.0/24"), + OVNGeneveNetwork: newNetwork("eth3", "10.0.4.0/24"), + }, + "system2": { + MicroCephPublicNetwork: newNetwork("eth0", "10.1.1.0/24"), + MicroCephInternalNetwork: newNetwork("eth1", "10.1.2.0/24"), + MicroCloudInternalNetwork: newNetwork("eth2", "10.1.3.0/24"), + OVNGeneveNetwork: newNetwork("eth3", "10.1.4.0/24"), + }, + }, + expectedWarnings: nil, + }, + { + name: "single system interface collision", + systems: map[string]InitSystem{ + "system1": { + MicroCephInternalNetwork: newNetwork("eth0", "10.0.1.0/24"), + OVNGeneveNetwork: newNetwork("eth0", "10.0.2.0/24"), // Same interface + MicroCloudInternalNetwork: newNetwork("eth1", "10.0.3.0/24"), + }, + }, + expectedWarnings: []string{ + "- Ceph cluster network, OVN underlay sharing network interface \"eth0\"", + }, + }, + { + name: "cross-system subnet collision", + systems: map[string]InitSystem{ + "system1": { + MicroCephInternalNetwork: newNetwork("eth0", "10.0.1.0/24"), // Same subnet + OVNGeneveNetwork: newNetwork("eth1", "10.0.2.0/24"), + MicroCloudInternalNetwork: newNetwork("eth2", "10.0.3.0/24"), + }, + "system2": { + OVNGeneveNetwork: newNetwork("eth3", "10.0.1.0/24"), // Same subnet for different network type + MicroCloudInternalNetwork: newNetwork("eth4", "10.0.4.0/24"), + }, + }, + expectedWarnings: []string{ + "- Ceph cluster network, OVN underlay sharing subnet \"10.0.1.0/24\"", + }, + }, + { + name: "mixed interface and cross-subnet collision", + systems: map[string]InitSystem{ + "system1": { + MicroCephInternalNetwork: newNetwork("eth0", "10.0.1.0/24"), // Interface and subnet collision + OVNGeneveNetwork: newNetwork("eth0", "10.0.1.0/24"), // Same interface and subnet + MicroCloudInternalNetwork: newNetwork("eth1", "10.0.2.0/24"), + }, + "system2": { + OVNGeneveNetwork: newNetwork("eth2", "10.0.1.0/24"), // Same subnet globally + }, + }, + expectedWarnings: []string{ + "- Ceph cluster network, OVN underlay sharing network interface \"eth0\"", + "- Ceph cluster network, OVN underlay sharing subnet \"10.0.1.0/24\"", + }, + }, + { + name: "ignore systems with missing networks", + systems: map[string]InitSystem{ + "system1": { // Incomplete networks (skipped in checks) + OVNGeneveNetwork: newNetwork("eth0", "10.0.1.0/24"), + MicroCloudInternalNetwork: newNetwork("eth1", "10.0.2.0/24"), + }, + }, + expectedWarnings: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + warnings := detectCollisions(tt.systems) + assert.ElementsMatch(t, tt.expectedWarnings, warnings) + }) + } +}