Return the allocated IP to a pod with the same namespace/name

[pliurh]

For statefulset pods, it will reuse the pod names after a node reboot. Whereabouts shall always return the same IP address that has already been allocated, when the pods start.

This patch also changes the IP deallocation logic. Now, the allocation record is identified by podRef instead of containerID.

[pliurh]

fix #176

[pliurh]

@andreaskaris PTAL

[andreaskaris]

wrap in retry?

[pliurh]

It will return an error and fail the CNI call. So kubelet will retry the CNI call. Additionally, we don't have the retry for other API calls either. Therefore, if we want to implement a retry logic, we shall address it comprehensively.

[andreaskaris]

yeah o.k. I misunderstood this anyway given that it's a CNI plugin it doesn't make a lot of sense

[andreaskaris]

Looks o.k. to me but I'm horrible at reviewing without actually testing the code. If I have time I'll test this tomorrow, but IMO this looks simpler (and thus better) than the earlier approach

[dougbtv]

I tried to surf around the history to figure out if there was a reason we didn't use podRef, but, I can't find any smoking gun.

I think this approach of switching to base on podRef is an improvement, thanks.

Thanks Peng for the PR, and Andreas for the review, as well.

[coveralls]

Pull Request Test Coverage Report for Build 8612252580

Details

• 7 of 12 (58.33%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.04%) to 72.317%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
pkg/allocate/allocate.go	6	11	54.55%

Totals
Change from base Build 8282575075:	-0.04%
Covered Lines:	1139
Relevant Lines:	1575

---

💛 - Coveralls

[wwcd]

When a pod references multiple virtual function interfaces and all are allocated addresses, they will all be assigned the same address.

[pliurh]

When a pod references multiple virtual function interfaces and all are allocated addresses, they will be assigned the same address.

Yeah, it is a limitation when you have a pod with multiple interfaces using the same IP range. A workaround is to divide the IP range into sections and assign different ranges to each interface of the pod.

If we want to fix that, we need to update the API to store interface names. So that we can identify the IP allocation per interface. @dougbtv @andreaskaris @maiqueb WDYT

[andreaskaris]

I'm trying to reproduce https://issues.redhat.com/browse/OCPBUGS-29648 or #176 with upstream/master whereabouts on kind, to get a baseline for my testing.
I can reproduce OCPBUGS-29648 on OCP 4.14, but I cannot reproduce the same issue on kind; but I wouldn't exclude that I'm doing something wrong ... @pliurh could you repro the issues on upstream prior to applying the PR?

[pliurh]

@andreaskaris I couldn't reproduce with a kind cluster either. I reproduced it with OCP.

[andreaskaris]

I just tested the reboot --force scenario from https://issues.redhat.com/browse/OCPBUGS-29648 manually and everything works for me with the fix.

WRT attaching the same network 2x:

 25   template:                                                                                                             
 26     metadata:                                                                                                           
 27       annotations:                                                                                                      
 28         k8s.v1.cni.cncf.io/networks: test-ipv4,test-ipv4

I get:

[akaris@workstation whereabouts-cni ((dfaf5249...))]$ oc get pods tools-ipv4-0 --output=custom-columns="NAME:.metadata.name,NETWORK:.metadata.annotations.k8s\.v1\.cni\.cncf\.io\/network-status"
NAME           NETWORK
tools-ipv4-0   [{
    "name": "ovn-kubernetes",
    "interface": "eth0",
    "ips": [
        "10.128.0.101",
        "fd01:0:0:1::65"
    ],
    "mac": "0a:58:0a:80:00:65",
    "default": true,
    "dns": {}
},{
    "name": "test/test-ipv4",
    "interface": "net1",
    "ips": [
        "192.168.10.140"
    ],
    "mac": "ae:3d:0f:f3:28:4e",
    "dns": {}
},{
    "name": "test/test-ipv4",
    "interface": "net2",
    "ips": [
        "192.168.10.140"
    ],
    "mac": "22:3b:b7:71:d5:c9",
    "dns": {}
}]
[akaris@workstation whereabouts-cni ((dfaf5249...))]$ oc exec tools-ipv4-0 -- ip a | grep 192
    inet 192.168.10.140/24 brd 192.168.10.255 scope global net1
    inet 192.168.10.140/24 brd 192.168.10.255 scope global net2

That's indeed a regression to how it worked before, though:

[akaris@workstation whereabouts-cni ((dfaf5249...))]$ oc get pods tools-ipv4-0 --output=custom-columns="NAME:.metadata.name,NETWORK:.metadata.annotations.k8s\.v1\.cni\.cncf\.io\/network-status"
NAME           NETWORK
tools-ipv4-0   [{
    "name": "ovn-kubernetes",
    "interface": "eth0",
    "ips": [
        "10.128.0.108",
        "fd01:0:0:1::6c"
    ],
    "mac": "0a:58:0a:80:00:6c",
    "default": true,
    "dns": {}
},{
    "name": "test/test-ipv4",
    "interface": "net1",
    "ips": [
        "192.168.10.140"
    ],
    "mac": "56:31:3a:61:53:8f",
    "dns": {}
},{
    "name": "test/test-ipv4",
    "interface": "net2",
    "ips": [
        "192.168.10.141"
    ],
    "mac": "ce:84:dd:e5:91:d4",
    "dns": {}
}]
[akaris@workstation whereabouts-cni ((dfaf5249...))]$ oc exec tools-ipv4-0 -- ip a | grep 192
    inet 192.168.10.140/24 brd 192.168.10.255 scope global net1
    inet 192.168.10.141/24 brd 192.168.10.255 scope global net2