Skip to content

Commit

Permalink
OCPBUGS-30233: Filter IPs in majority group validation
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@CrystalChun
CrystalChun committed Mar 21, 2024
1 parent 100a86f commit 0b7cbc0
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 17 deletions.
2 changes: 1 addition & 1 deletion internal/cluster/cluster.go
View file
Expand Up @@ -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 {
Expand Down
17 changes: 10 additions & 7 deletions internal/network/connectivity_groups.go
View file
Expand Up @@ -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
}
}
Expand All @@ -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 == "" {
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
}
Expand Down
51 changes: 42 additions & 9 deletions internal/network/connectivity_groups_test.go
View file
Expand Up @@ -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 {
Expand Down Expand Up @@ -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{}))
})
Expand All @@ -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{}))
})
Expand All @@ -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{}))
})
Expand Down Expand Up @@ -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))
})
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
})
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down
9 changes: 9 additions & 0 deletions internal/network/machine_network_cidr.go
View file
Expand Up @@ -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)
Expand Down

0 comments on commit 0b7cbc0

Please sign in to comment.