From 0b7cbc0e552cc6447880a4158a261b90b11a1890 Mon Sep 17 00:00:00 2001 From: CrystalChun Date: Mon, 18 Mar 2024 14:45:52 -0700 Subject: [PATCH] OCPBUGS-30233: Filter IPs in majority group validation https://issues.redhat.com/browse/OCPBUGS-30233 Filter the IP addresses when creating connectivity groups to hosts that belong within the machine network CIDRs (if they exist) to prevent failing "belongs-to-majority" validation. Previously, if the IPs were not filtered, the validation would fail if hosts had IPs on different NICs that couldn't connect to other hosts. These IPs were not used and caused the "belongs-to-majority" validation to fail. --- internal/cluster/cluster.go | 2 +- internal/network/connectivity_groups.go | 17 ++++--- internal/network/connectivity_groups_test.go | 51 ++++++++++++++++---- internal/network/machine_network_cidr.go | 9 ++++ 4 files changed, 62 insertions(+), 17 deletions(-) diff --git a/internal/cluster/cluster.go b/internal/cluster/cluster.go index 13d33c004a..9ff388df83 100644 --- a/internal/cluster/cluster.go +++ b/internal/cluster/cluster.go @@ -1400,7 +1400,7 @@ func (m *Manager) setConnectivityMajorityGroupsForClusterInternal(cluster *commo } for _, family := range []network.AddressFamily{network.IPv4, network.IPv6} { - majorityGroup, err := network.CreateL3MajorityGroup(hosts, family) + majorityGroup, err := network.CreateL3MajorityGroup(hosts, family, network.GetMachineNetworkCidrs(cluster)) if err != nil { m.log.WithError(err).Warnf("Create L3 majority group for cluster %s failed", cluster.ID.String()) } else { diff --git a/internal/network/connectivity_groups.go b/internal/network/connectivity_groups.go index 7c6afead84..d5c9360575 100644 --- a/internal/network/connectivity_groups.go +++ b/internal/network/connectivity_groups.go @@ -436,7 +436,7 @@ func (l *l3Query) next() strfmt.UUID { foundAddresses[l3.RemoteIPAddress] = true } } - if len(addresses) == len(foundAddresses) { + if len(foundAddresses) > 0 && len(addresses) == len(foundAddresses) { return rh.HostID } } @@ -458,7 +458,7 @@ func (l *l3QueryFactory) create(h *models.Host) (hostQuery, error) { return &ret, nil } -func newL3QueryFactory(hosts []*models.Host, family AddressFamily) (hostQueryFactory, error) { +func newL3QueryFactory(hosts []*models.Host, family AddressFamily, machineNetworkCidrs []string) (hostQueryFactory, error) { nodesAddresses := make(map[strfmt.UUID]map[string]bool) for _, h := range hosts { if h.Inventory == "" { @@ -482,7 +482,10 @@ func newL3QueryFactory(hosts []*models.Host, family AddressFamily) (hostQueryFac if err != nil { return nil, err } - value[ip.String()] = true + + if ipInCidr := IpInCidrs(ip.String(), machineNetworkCidrs); len(machineNetworkCidrs) == 0 || ipInCidr { + value[ip.String()] = true + } } } nodesAddresses[*h.ID] = value @@ -552,7 +555,7 @@ func calculateMajorityGroup(hosts []*models.Host, factory hostQueryFactory) ([]s } /* - * Crate majority for a cidr. A majority group is a the largest group of hosts in a cluster that all of them have full mesh + * Create majority for a cidr. A majority group is a the largest group of hosts in a cluster that all of them have full mesh * to the other group members. * It is done by taking a sorted connectivity group list according to the group size, and from this group take the * largest one @@ -566,16 +569,16 @@ func CreateL2MajorityGroup(cidr string, hosts []*models.Host) ([]strfmt.UUID, er } /* - * Crate majority for address family. A majority group is a the largest group of hosts in a cluster that all of them have full mesh + * Create majority for address family. A majority group is the largest group of hosts in a cluster that all of them have full mesh * to the other group members. * It is done by taking a sorted connectivity group list according to the group size, and from this group take the * largest one */ -func CreateL3MajorityGroup(hosts []*models.Host, family AddressFamily) ([]strfmt.UUID, error) { +func CreateL3MajorityGroup(hosts []*models.Host, family AddressFamily, machineCidrs []string) ([]strfmt.UUID, error) { if !funk.Contains([]AddressFamily{IPv4, IPv6}, family) { return nil, errors.Errorf("Unexpected address family %+v", family) } - factory, err := newL3QueryFactory(hosts, family) + factory, err := newL3QueryFactory(hosts, family, machineCidrs) if err != nil { return nil, err } diff --git a/internal/network/connectivity_groups_test.go b/internal/network/connectivity_groups_test.go index 6204c4a386..c598338d8c 100644 --- a/internal/network/connectivity_groups_test.go +++ b/internal/network/connectivity_groups_test.go @@ -530,6 +530,8 @@ func GenerateL3ConnectivityGroupTests(ipV4 bool, net1CIDR, net2CIDR string) { } else { family = IPv6 } + cidrs := []string{net1CIDR, net2CIDR} + Describe(fmt.Sprintf("connectivity groups %s", ipVersion), func() { BeforeEach(func() { if ipV4 { @@ -560,7 +562,7 @@ func GenerateL3ConnectivityGroupTests(ipV4 bool, net1CIDR, net2CIDR string) { Inventory: makeInventory(nodes[2]), }, } - ret, err := CreateL3MajorityGroup(hosts, family) + ret, err := CreateL3MajorityGroup(hosts, family, cidrs) Expect(err).ToNot(HaveOccurred()) Expect(ret).To(Equal([]strfmt.UUID{})) }) @@ -579,7 +581,7 @@ func GenerateL3ConnectivityGroupTests(ipV4 bool, net1CIDR, net2CIDR string) { Inventory: makeInventory(nodes[2]), }, } - ret, err := CreateL3MajorityGroup(hosts, family) + ret, err := CreateL3MajorityGroup(hosts, family, cidrs) Expect(err).ToNot(HaveOccurred()) Expect(ret).To(Equal([]strfmt.UUID{})) }) @@ -604,7 +606,7 @@ func GenerateL3ConnectivityGroupTests(ipV4 bool, net1CIDR, net2CIDR string) { Inventory: makeInventory(nodes[2]), }, } - ret, err := CreateL3MajorityGroup(hosts, family) + ret, err := CreateL3MajorityGroup(hosts, family, cidrs) Expect(err).ToNot(HaveOccurred()) Expect(ret).To(Equal([]strfmt.UUID{})) }) @@ -632,7 +634,7 @@ func GenerateL3ConnectivityGroupTests(ipV4 bool, net1CIDR, net2CIDR string) { Inventory: makeInventory(nodes[2]), }, } - ret, err := CreateL3MajorityGroup(hosts, family) + ret, err := CreateL3MajorityGroup(hosts, family, cidrs) Expect(err).ToNot(HaveOccurred()) Expect(ret).To(HaveLen(0)) }) @@ -660,7 +662,38 @@ func GenerateL3ConnectivityGroupTests(ipV4 bool, net1CIDR, net2CIDR string) { Inventory: makeInventory(nodes[2]), }, } - ret, err := CreateL3MajorityGroup(hosts, family) + ret, err := CreateL3MajorityGroup(hosts, family, cidrs) + Expect(err).ToNot(HaveOccurred()) + Expect(ret).To(HaveLen(3)) + Expect(ret).To(ContainElement(*nodes[0].id)) + Expect(ret).To(ContainElement(*nodes[1].id)) + Expect(ret).To(ContainElement(*nodes[2].id)) + }) + It("3 with data - two networks - no machine network CIDRs", func() { + hosts := []*models.Host{ + { + ID: nodes[0].id, + Connectivity: createConnectivityReport( + createL3Remote(nodes[1], l3LinkNet1, l3LinkNet2), + createL3Remote(nodes[2], l3LinkNet1, l3LinkNet2)), + Inventory: makeInventory(nodes[0]), + }, + { + ID: nodes[1].id, + Connectivity: createConnectivityReport( + createL3Remote(nodes[0], l3LinkNet1, l3LinkNet2), + createL3Remote(nodes[2], l3LinkNet1, l3LinkNet2)), + Inventory: makeInventory(nodes[1]), + }, + { + ID: nodes[2].id, + Connectivity: createConnectivityReport( + createL3Remote(nodes[0], l3LinkNet1, l3LinkNet2), + createL3Remote(nodes[1], l3LinkNet1, l3LinkNet2)), + Inventory: makeInventory(nodes[2]), + }, + } + ret, err := CreateL3MajorityGroup(hosts, family, nil) Expect(err).ToNot(HaveOccurred()) Expect(ret).To(HaveLen(3)) Expect(ret).To(ContainElement(*nodes[0].id)) @@ -691,7 +724,7 @@ func GenerateL3ConnectivityGroupTests(ipV4 bool, net1CIDR, net2CIDR string) { Inventory: makeInventory(nodes[2]), }, } - ret, err := CreateL3MajorityGroup(hosts, family) + ret, err := CreateL3MajorityGroup(hosts, family, cidrs) Expect(err).ToNot(HaveOccurred()) Expect(ret).To(HaveLen(0)) }) @@ -730,7 +763,7 @@ func GenerateL3ConnectivityGroupTests(ipV4 bool, net1CIDR, net2CIDR string) { Inventory: makeInventory(nodes[3]), }, } - ret, err := CreateL3MajorityGroup(hosts, family) + ret, err := CreateL3MajorityGroup(hosts, family, cidrs) Expect(err).ToNot(HaveOccurred()) Expect(ret).To(HaveLen(4)) Expect(ret).To(ContainElement(*nodes[0].id)) @@ -774,7 +807,7 @@ func GenerateL3ConnectivityGroupTests(ipV4 bool, net1CIDR, net2CIDR string) { Inventory: makeInventory(nodes[3]), }, } - ret, err := CreateL3MajorityGroup(hosts, family) + ret, err := CreateL3MajorityGroup(hosts, family, cidrs) Expect(err).ToNot(HaveOccurred()) Expect(ret).To(HaveLen(4)) Expect(ret).To(ContainElement(*nodes[0].id)) @@ -815,7 +848,7 @@ func GenerateL3ConnectivityGroupTests(ipV4 bool, net1CIDR, net2CIDR string) { Inventory: makeInventory(nodes[3]), }, } - ret, err := CreateL3MajorityGroup(hosts, family) + ret, err := CreateL3MajorityGroup(hosts, family, cidrs) Expect(err).ToNot(HaveOccurred()) Expect(ret).To(HaveLen(3)) Expect(ret).To(ContainElement(*nodes[0].id)) diff --git a/internal/network/machine_network_cidr.go b/internal/network/machine_network_cidr.go index 0c4285ff26..92e4e20962 100644 --- a/internal/network/machine_network_cidr.go +++ b/internal/network/machine_network_cidr.go @@ -281,6 +281,15 @@ func IpInCidr(ipAddr, cidr string) (bool, error) { return ipNet.Contains(ip), nil } +func IpInCidrs(ipAddr string, cidrs []string) bool { + for _, cidr := range cidrs { + if isInCidr, _ := IpInCidr(ipAddr, cidr); isInCidr { + return true + } + } + return false +} + func belongsToNetwork(log logrus.FieldLogger, h *models.Host, machineIpnet *net.IPNet) bool { var inventory models.Inventory err := json.Unmarshal([]byte(h.Inventory), &inventory)