Make podRef to be unique

We used podRef with format /<pod_name> to identify the IP
allocation. However, in scenarios like replicaSet, a different
pod can reuse the pod name. This patch changes the format of podRef
to /<pod_name>:<pod_uid> to ensure each ip allocation will have a
unique identifier. The legacy podRef format will be updated
automatically during an upgrade.

Signed-off-by: Peng Liu <[email protected]>

cmd/controlloop/controlloop.go

@@ -79,6 +79,9 @@ func main() {
 	}
 	defer watcher.Close()
 
+	// trigger one immediate reconcile before cron job start
+	reconciler.ReconcileIPs(errorChan)
+
 	reconcilerConfigWatcher, err := reconciler.NewConfigWatcher(
 		reconcilerCronConfiguration,
 		s,

pkg/config/config.go

@@ -52,6 +52,7 @@ func LoadIPAMConfig(bytes []byte, envArgs string, extraConfigPaths ...string) (*
 	}
 	n.IPAM.PodName = string(args.K8S_POD_NAME)
 	n.IPAM.PodNamespace = string(args.K8S_POD_NAMESPACE)
+	n.IPAM.PodUID = string(args.K8S_POD_UID)
 
 	flatipam, foundflatfile, err := GetFlatIPAM(false, n.IPAM, extraConfigPaths...)
 	if err != nil {

pkg/controlloop/entity_generators.go

@@ -1,5 +1,3 @@
-//go:build test
-// +build test
 
 package controlloop
 
@@ -10,6 +8,7 @@ import (
 
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	types "k8s.io/apimachinery/pkg/types"
 
 	nad "github.com/k8snetworkplumbingwg/network-attachment-definition-client/pkg/apis/k8s.cni.cncf.io/v1"
 
@@ -58,12 +57,13 @@ func nodeSpec(name string) *v1.Node {
 	}
 }
 
-func podSpec(name string, namespace string, nodeName string, networks ...string) *v1.Pod {
+func podSpec(name string, namespace string, uid string, nodeName string, networks ...string) *v1.Pod {
 	return &v1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
 			Name:        name,
 			Namespace:   namespace,
 			Annotations: podNetworkSelectionElements(networks...),
+			UID:         types.UID(uid),
 		},
 		Spec: v1.PodSpec{
 			NodeName: nodeName,

pkg/controlloop/pod.go

@@ -177,6 +177,7 @@ func (pc *PodController) processNextWorkItem() bool {
 func (pc *PodController) garbageCollectPodIPs(pod *v1.Pod) error {
 	podNamespace := pod.GetNamespace()
 	podName := pod.GetName()
+	podUID := pod.GetUID()
 
 	ifaceStatuses, err := podNetworkStatus(pod)
 	if err != nil {
@@ -199,7 +200,7 @@ func (pc *PodController) garbageCollectPodIPs(pod *v1.Pod) error {
 		}
 
 		logging.Verbosef("the NAD's config: %s", nad.Spec)
-		ipamConfig, err := ipamConfiguration(nad, podNamespace, podName, mountPath)
+		ipamConfig, err := ipamConfiguration(nad, podNamespace, podName, string(podUID), mountPath)
 		if err != nil && isInvalidPluginType(err) {
 			logging.Debugf("error while computing something: %v", err)
 			continue
@@ -370,7 +371,7 @@ func podNetworkStatus(pod *v1.Pod) ([]nadv1.NetworkStatus, error) {
 	return ifaceStatuses, nil
 }
 
-func ipamConfiguration(nad *nadv1.NetworkAttachmentDefinition, podNamespace string, podName string, mountPath string) (*types.IPAMConfig, error) {
+func ipamConfiguration(nad *nadv1.NetworkAttachmentDefinition, podNamespace string, podName string, podUID string, mountPath string) (*types.IPAMConfig, error) {
 	mounterWhereaboutsConfigFilePath := mountPath + whereaboutsConfigPath
 
 	ipamConfig, err := config.LoadIPAMConfiguration([]byte(nad.Spec.Config), "", mounterWhereaboutsConfigFilePath)
@@ -379,6 +380,7 @@ func ipamConfiguration(nad *nadv1.NetworkAttachmentDefinition, podNamespace stri
 	}
 	ipamConfig.PodName = podName
 	ipamConfig.PodNamespace = podNamespace
+	ipamConfig.PodUID = podUID
 	ipamConfig.Kubernetes.KubeConfigPath = mountPath + ipamConfig.Kubernetes.KubeConfigPath // must use the mount path
 
 	return ipamConfig, nil

pkg/controlloop/pod_controller_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/k8snetworkplumbingwg/whereabouts/pkg/api/whereabouts.cni.cncf.io/v1alpha1"
 	wbclient "github.com/k8snetworkplumbingwg/whereabouts/pkg/client/clientset/versioned"
 	fakewbclient "github.com/k8snetworkplumbingwg/whereabouts/pkg/client/clientset/versioned/fake"
+	"github.com/k8snetworkplumbingwg/whereabouts/pkg/reconciler"
 	"github.com/k8snetworkplumbingwg/whereabouts/pkg/storage/kubernetes"
 )
 
@@ -62,6 +63,7 @@ var _ = Describe("IPControlLoop", func() {
 		const (
 			networkName = "meganet"
 			podName     = "tiny-winy-pod"
+			podUID      = "tiny-winy-pod-uid"
 			nodeName    = "hypernode"
 		)
 
@@ -74,7 +76,7 @@ var _ = Describe("IPControlLoop", func() {
 		)
 
 		BeforeEach(func() {
-			pod = podSpec(podName, namespace, nodeName, networkName)
+			pod = podSpec(podName, namespace, nodeName, podUID, networkName)
 			node = nodeSpec(nodeName)
 			k8sClient = fakek8sclient.NewSimpleClientset(pod, node)
 			os.Setenv("NODENAME", nodeName)
@@ -120,7 +122,7 @@ var _ = Describe("IPControlLoop", func() {
 			)
 
 			BeforeEach(func() {
-				dummyNetworkPool = ipPool(kubernetes.PoolIdentifier{IpRange: dummyNetIPRange, NetworkName: kubernetes.UnnamedNetwork}, ipPoolsNamespace(), podReference(pod))
+				dummyNetworkPool = ipPool(kubernetes.PoolIdentifier{IpRange: dummyNetIPRange, NetworkName: kubernetes.UnnamedNetwork}, ipPoolsNamespace(), reconciler.ComposePodRef(*pod))
 				wbClient = fakewbclient.NewSimpleClientset(dummyNetworkPool)
 			})
 
@@ -288,10 +290,6 @@ func newFakeNetAttachDefClient(namespace string, networkAttachments ...nad.Netwo
 	return netAttachDefClient, nil
 }
 
-func podReference(pod *v1.Pod) string {
-	return fmt.Sprintf("%s/%s", pod.GetNamespace(), pod.GetName())
-}
-
 func dummyWhereaboutsConfig() string {
 	return `{
       "datastore": "kubernetes",

pkg/reconciler/ip_test.go

@@ -8,6 +8,8 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/google/uuid"
+
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
@@ -18,6 +20,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
+	kubetypes "k8s.io/apimachinery/pkg/types"
 	k8sclient "k8s.io/client-go/kubernetes"
 	fakek8sclient "k8s.io/client-go/kubernetes/fake"
 
@@ -44,11 +47,9 @@ var _ = Describe("Whereabouts IP reconciler", func() {
 	var (
 		reconcileLooper *ReconcileLooper
 		k8sClientSet    k8sclient.Interface
+		pod             *v1.Pod
 	)
-
 	Context("reconciling IP pools with a single running pod", func() {
-		var pod *v1.Pod
-
 		BeforeEach(func() {
 			pod = generatePod(namespace, podName, ipInNetwork{ip: firstIPInRange, networkName: networkName})
 			k8sClientSet = fakek8sclient.NewSimpleClientset(pod)
@@ -63,7 +64,7 @@ var _ = Describe("Whereabouts IP reconciler", func() {
 			)
 
 			BeforeEach(func() {
-				pool = generateIPPoolSpec(ipRange, namespace, poolName, pod.Name)
+				pool = generateIPPoolSpec(ipRange, namespace, poolName, pod)
 				wbClient = fakewbclient.NewSimpleClientset(pool)
 			})
 
@@ -102,7 +103,7 @@ var _ = Describe("Whereabouts IP reconciler", func() {
 			secondIPInRange = "10.10.10.2"
 		)
 
-		var pods []v1.Pod
+		var pods []*v1.Pod
 
 		BeforeEach(func() {
 			pods = nil
@@ -115,7 +116,7 @@ var _ = Describe("Whereabouts IP reconciler", func() {
 				if i == livePodIndex {
 					k8sClientSet = fakek8sclient.NewSimpleClientset(pod)
 				}
-				pods = append(pods, *pod)
+				pods = append(pods, pod)
 			}
 		})
 
@@ -128,11 +129,7 @@ var _ = Describe("Whereabouts IP reconciler", func() {
 			)
 
 			BeforeEach(func() {
-				var podNames []string
-				for _, pod := range pods {
-					podNames = append(podNames, pod.Name)
-				}
-				pool = generateIPPoolSpec(ipRange, namespace, poolName, podNames...)
+				pool = generateIPPoolSpec(ipRange, namespace, poolName, pods...)
 				wbClient = fakewbclient.NewSimpleClientset(pool)
 			})
 
@@ -159,7 +156,7 @@ var _ = Describe("Whereabouts IP reconciler", func() {
 
 					remainingAllocation := map[string]v1alpha1.IPAllocation{
 						"2": {
-							PodRef: fmt.Sprintf("%s/%s", namespace, pods[livePodIndex].Name),
+							PodRef: ComposePodRef(*pods[livePodIndex]),
 						},
 					}
 					Expect(poolAfterCleanup.Spec.Allocations).To(Equal(remainingAllocation))
@@ -211,8 +208,8 @@ var _ = Describe("Whereabouts IP reconciler", func() {
 		})
 
 		BeforeEach(func() {
-			firstPool := generateIPPoolSpec(firstNetworkRange, namespace, firstPoolName, pods[0].GetName(), pods[2].GetName())
-			secondPool := generateIPPoolSpec(secondNetworkRange, namespace, secondPoolName, pods[1].GetName())
+			firstPool := generateIPPoolSpec(firstNetworkRange, namespace, firstPoolName, pods[0], pods[2])
+			secondPool := generateIPPoolSpec(secondNetworkRange, namespace, secondPoolName, pods[1])
 			wbClient = fakewbclient.NewSimpleClientset(firstPool, secondPool)
 			pools = append(pools, *firstPool, *secondPool)
 		})
@@ -221,7 +218,7 @@ var _ = Describe("Whereabouts IP reconciler", func() {
 			podIPs := []string{firstIPInRange, secondIPInRange, thirdIPInRange}
 			for i := 0; i < numberOfPods; i++ {
 				var clusterWideIP v1alpha1.OverlappingRangeIPReservation
-				ownerPodRef := fmt.Sprintf("%s/%s", namespace, pods[i].GetName())
+				ownerPodRef := ComposePodRef(*pods[i])
 				_, err := wbClient.WhereaboutsV1alpha1().OverlappingRangeIPReservations(namespace).Create(context.TODO(), generateClusterWideIPReservation(namespace, podIPs[i], ownerPodRef), metav1.CreateOptions{})
 				Expect(err).NotTo(HaveOccurred())
 				clusterWideIPs = append(clusterWideIPs, clusterWideIP)
@@ -279,7 +276,7 @@ var _ = Describe("Whereabouts IP reconciler", func() {
 		})
 
 		BeforeEach(func() {
-			firstPool := generateIPPoolSpec(networkRange, namespace, poolName, pods[0].GetName())
+			firstPool := generateIPPoolSpec(networkRange, namespace, poolName, pods[0])
 			wbClient = fakewbclient.NewSimpleClientset(firstPool)
 			pools = append(pools, *firstPool)
 		})
@@ -288,7 +285,7 @@ var _ = Describe("Whereabouts IP reconciler", func() {
 			podIPs := []string{ipv6FirstIPInRange}
 			for i := 0; i < numberOfPods; i++ {
 				var clusterWideIP v1alpha1.OverlappingRangeIPReservation
-				ownerPodRef := fmt.Sprintf("%s/%s", namespace, pods[i].GetName())
+				ownerPodRef := ComposePodRef(*pods[i])
 				_, err := wbClient.WhereaboutsV1alpha1().OverlappingRangeIPReservations(namespace).Create(context.TODO(), generateClusterWideIPReservation(namespace, podIPs[i], ownerPodRef), metav1.CreateOptions{})
 				Expect(err).NotTo(HaveOccurred())
 				clusterWideIPs = append(clusterWideIPs, clusterWideIP)
@@ -319,20 +316,19 @@ var _ = Describe("Whereabouts IP reconciler", func() {
 
 		BeforeEach(func() {
 			var err error
-			pod, err = k8sClientSet.CoreV1().Pods(namespace).Create(
-				context.TODO(),
-				generatePendingPod(namespace, podName),
-				metav1.CreateOptions{})
-			Expect(err).NotTo(HaveOccurred())
+			pod = generatePendingPod(namespace, podName)
+			k8sClientSet = fakek8sclient.NewSimpleClientset(pod)
 
-			pool = generateIPPoolSpec(ipRange, namespace, poolName, pod.Name)
+			pool = generateIPPoolSpec(ipRange, namespace, poolName, pod)
 			wbClient = fakewbclient.NewSimpleClientset(pool)
 			reconcileLooper, err = NewReconcileLooperWithClient(context.TODO(), kubernetes.NewKubernetesClient(wbClient, k8sClientSet, timeout), timeout)
 			Expect(err).NotTo(HaveOccurred())
 		})
 
 		It("can be reconciled", func() {
-			Expect(reconcileLooper.ReconcileIPPools(context.TODO())).NotTo(BeEmpty())
+			ips, err := reconcileLooper.ReconcileIPPools(context.TODO())
+			Expect(ips).NotTo(BeEmpty())
+			Expect(err).NotTo(HaveOccurred())
 		})
 	})
 })
@@ -380,10 +376,11 @@ var _ = Describe("IPReconciler", func() {
 			ipCIDR         = "192.168.14.0/24"
 			namespace      = "default"
 			podName        = "pod1"
+			podUID         = "pod1-uid"
 		)
 
 		BeforeEach(func() {
-			podRef := "default/pod1"
+			podRef := "default/pod1:pod1-uid"
 			reservations := generateIPReservation(firstIPInRange, podRef)
 
 			pool := generateIPPool(ipCIDR, podRef)
@@ -403,10 +400,10 @@ var _ = Describe("IPReconciler", func() {
 
 		Context("and they are actually multiple IPs", func() {
 			BeforeEach(func() {
-				podRef := "default/pod2"
+				podRef := "default/pod2:pod2-uid"
 				reservations := generateIPReservation("192.168.14.2", podRef)
 
-				pool := generateIPPool(ipCIDR, podRef, "default/pod2", "default/pod3")
+				pool := generateIPPool(ipCIDR, podRef, "default/pod2:pod2-uid", "default/pod3")
 				orphanedIPAddr := OrphanedIPReservations{
 					Pool:        dummyPool{orphans: reservations, pool: pool},
 					Allocations: reservations,
@@ -425,8 +422,8 @@ var _ = Describe("IPReconciler", func() {
 		Context("but the IP reservation owner does not match", func() {
 			var reservationPodRef string
 			BeforeEach(func() {
-				reservationPodRef = "default/pod2"
-				podRef := "default/pod1"
+				reservationPodRef = "default/pod2:pod2-uid"
+				podRef := "default/pod1:pod1-uid"
 				reservations := generateIPReservation(firstIPInRange, podRef)
 				erroredReservations := generateIPReservation(firstIPInRange, reservationPodRef)
 
@@ -448,11 +445,11 @@ var _ = Describe("IPReconciler", func() {
 	})
 })
 
-func generateIPPoolSpec(ipRange string, namespace string, poolName string, podNames ...string) *v1alpha1.IPPool {
+func generateIPPoolSpec(ipRange string, namespace string, poolName string, pods ...*v1.Pod) *v1alpha1.IPPool {
 	allocations := map[string]v1alpha1.IPAllocation{}
-	for i, podName := range podNames {
+	for i, pod := range pods {
 		allocations[fmt.Sprintf("%d", i+1)] = v1alpha1.IPAllocation{
-			PodRef: fmt.Sprintf("%s/%s", namespace, podName),
+			PodRef: ComposePodRef(*pod),
 		}
 	}
 	return &v1alpha1.IPPool{
@@ -484,6 +481,7 @@ func generatePod(namespace string, podName string, ipNetworks ...ipInNetwork) *v
 			Name:        podName,
 			Namespace:   namespace,
 			Annotations: generatePodAnnotations(ipNetworks...),
+			UID:         kubetypes.UID(uuid.NewString()),
 		},
 		Spec: v1.PodSpec{
 			Containers: []v1.Container{