Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OCPBUGS-30233: Filter IPs in majority group validation #6094

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

OCPBUGS-30233: Filter IPs in majority group validation #6094

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/cluster/cluster.go
View file
Open in desktop
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
Open in desktop
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. Good catch

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
Open in desktop
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
Open in desktop
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