Skip to content

Commit

Permalink
node/manager: Utilize set.SliceSubsetOf in ipcache deletion
Browse files Browse the repository at this point in the history
This cleans up the code to have proper diffing logic when delete node
IPs from the ipcache. Previously, the logic had a potential for a bug
because it assumed that oldIPs and newIPs were sorted.

FWIW, no bug has been reported in regards to the previous code fixed by
this commit, so it's theoretical for now. In the end, this is a
defensive change to prevent it from happening.

Additionally, this commit will ease the changes that will be introduced
in #23208.

Signed-off-by: Chris Tarazi <chris@isovalent.com>
  • Loading branch information
christarazi committed May 11, 2023
1 parent d522562 commit 47fcc78
Showing 1 changed file with 13 additions and 20 deletions.
33 changes: 13 additions & 20 deletions pkg/node/manager/manager.go
Expand Up @@ -33,6 +33,7 @@ import (
nodeTypes "github.com/cilium/cilium/pkg/node/types"
"github.com/cilium/cilium/pkg/option"
"github.com/cilium/cilium/pkg/rand"
"github.com/cilium/cilium/pkg/set"
"github.com/cilium/cilium/pkg/source"
)

Expand Down Expand Up @@ -554,19 +555,9 @@ func (m *manager) upsertIntoIDMD(prefix netip.Prefix, id identity.NumericIdentit
// deleteIPCache deletes the IP addresses from the IPCache with the 'oldSource'
// if they are not found in the newIPs slice.
func (m *manager) deleteIPCache(oldSource source.Source, oldIPs []string, newIPs []string) {
for _, address := range oldIPs {
var found bool
for _, ipAdded := range newIPs {
if ipAdded == address {
found = true
break
}
}
// Delete from the IPCache if the node's IP addresses was not
// added in this update.
if !found {
m.ipcache.Delete(address, oldSource)
}
_, diff := set.SliceSubsetOf(oldIPs, newIPs)
for _, address := range diff {
m.ipcache.Delete(address, oldSource)
}
}

Expand Down Expand Up @@ -603,6 +594,11 @@ func (m *manager) NodeDeleted(n nodeTypes.Node) {
return
}

extraIPs := []net.IP{
entry.node.IPv4HealthIP, entry.node.IPv6HealthIP,
entry.node.IPv4IngressIP, entry.node.IPv6IngressIP,
}
toDelete := make([]string, 0, len(entry.node.IPAddresses)+len(extraIPs))
for _, address := range entry.node.IPAddresses {
if option.Config.NodeIpsetNeeded() && address.Type == addressing.NodeInternalIP {
iptables.RemoveFromNodeIpset(address.IP)
Expand All @@ -618,17 +614,14 @@ func (m *manager) NodeDeleted(n nodeTypes.Node) {
} else {
prefix = ip.IPToNetPrefix(address.IP.To16())
}
m.ipcache.Delete(prefix.String(), n.Source)
toDelete = append(toDelete, prefix.String())
}

for _, address := range []net.IP{
entry.node.IPv4HealthIP, entry.node.IPv6HealthIP,
entry.node.IPv4IngressIP, entry.node.IPv6IngressIP,
} {
for _, address := range extraIPs {
if address != nil {
m.ipcache.Delete(address.String(), n.Source)
toDelete = append(toDelete, address.String())
}
}
m.deleteIPCache(n.Source, toDelete, nil)

m.metricNumNodes.Dec()

Expand Down

0 comments on commit 47fcc78

Please sign in to comment.