daemon, node: Remove old, discarded router IPs from cilium_host

[christarazi]

In the previous commit (referenced below), we forgot to remove the old
router IPs from the actual interface (cilium_host). This caused
connectivity issues in user environments where the discarded, stale IPs
were reassigned to pods, causing the ipcache entries for those IPs to
have remote-node identity.

To fix this, we remove all IPs from the cilium_host interface that
weren't restored during the router IP restoration process. This step
correctly finalizes the restoration process for router IPs.

Fixes: ff63b07 ("daemon, node: Fix faulty router IP restoration
logic")

Signed-off-by: Chris Tarazi chris@isovalent.com

[christarazi]

/test

[joestringer]

I think this will do the right thing in the common circumstances, although I would like to know that the log messages will include all possibly-relevant information in them (I found this a little hard to tell at face value).

Most of the discussion below is just looking over this code with fresh eyes and I found it a bit confusing for the case where the overall IP allocation CIDR range changes. Maybe we're not so worried about that case, but it looks like it could cause some hiccups if anyone did change the CIDR.

[joestringer]

LGTM, thanks.

[pchaigno]

/test

[christarazi]

/test

[qmonnet]

• gke-stable looks very much like an old/rare flake documented in CI: k8s 1.10: Chaos Restart: establish control: are you sure there is a netserver listening on 10.10.0.45 at port 12865? #6128 (opened in 2018, closed in 2019)
• k8s-1.16-kernel-netnext is CI: failed to ensure kubectl version: failed to download kubectl #18060 (new, but also encountered on the v1.11 backport PR)
• k8s-1.22-kernel-4.19 looks like CI: K8sServicesTest Checks service across nodes Tests NodePort BPF Tests with direct routing Tests NodePort #18014

[qmonnet]

/test-gke

[qmonnet]

/test-1.16-netnext

[qmonnet]

/test-1.22-4.19

Job 'Cilium-PR-K8s-GKE' hit: #17353 (88.79% similarity)

Job 'Cilium-PR-K8s-GKE' failed and has not been observed before, so may be related to your PR:

Click to show.

Test Name

K8sDatapathConfig Host firewall With native routing

Failure Output

FAIL: Pods are not ready in time: timed out waiting for pods with filter  to be ready: 4m0s timeout expired

If it is a flake, comment /mlh new-flake Cilium-PR-K8s-GKE so I can create a new GitHub issue to track it.

[qmonnet]

Job 'Cilium-PR-K8s-GKE' hit: #17353 (88.79% similarity)

No, the stack trace is closer to the one in #17545.

/test-gke

[christarazi]

GKE hit #17307

[houminz]

@christarazi Hi chris, I found another problem when remove old, discard router IPs from cilium_host.

In my scenario #20879, restoredIP is nil, thus leading family to IPv6. However old router IP is IPv4, we could not list IPv4 address, thus old router IPs are not removed.

Maybe we should re-consider the AddrList logic?

func removeOldRouterState(restoredIP net.IP) error {
	l, err := netlink.LinkByName(defaults.HostDevice)
	if err != nil {
		return err
	}

	family := netlink.FAMILY_V6
	if restoredIP.To4() != nil {
		family = netlink.FAMILY_V4
	}
	addrs, err := netlink.AddrList(l, family)
	if err != nil {
		return err
	}
	if len(addrs) > 1 {
		log.Info("More than one router IP was found on the cilium_host device after restoration, cleaning up old router IPs.")
	}

	var errs []error
	for _, a := range addrs {
		if restoredIP != nil && restoredIP.Equal(a.IP) {
			continue
		}
		if err := netlink.AddrDel(l, &a); err != nil {
			errs = append(errs, fmt.Errorf("failed to remove IP %s: %w", a.IP, err))
		}
	}
	if len(errs) > 0 {
		return fmt.Errorf("failed to remove all old router IPs: %v", errs)
	}

	return nil
}