diff --git a/cmd/whereabouts_test.go b/cmd/whereabouts_test.go index 92ba77914..ae951af74 100644 --- a/cmd/whereabouts_test.go +++ b/cmd/whereabouts_test.go @@ -67,8 +67,9 @@ func AllocateAndReleaseAddressesTest(ipVersion string, ipamConf *whereaboutstype Netns: nspath, IfName: ifname, StdinData: cniConf, - Args: cniArgs(podNamespace, podName), + Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,i)), } + ipamConf.PodName=fmt.Sprintf("%v-%d", podName,i) client := mutateK8sIPAM(args.ContainerID, ipamConf, wbClient) // Allocate the IP @@ -971,9 +972,9 @@ var _ = Describe("Whereabouts operations", func() { Netns: nspath, IfName: ifname, StdinData: []byte(conf), - Args: cniArgs(podNamespace, podName), + Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,i)), } - + ipamConf.PodName=fmt.Sprintf("%v-%d", podName,i) k8sClient = mutateK8sIPAM(args.ContainerID, ipamConf, wbClient) r, raw, err := testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args, k8sClient, cniVersion) @@ -1000,8 +1001,9 @@ var _ = Describe("Whereabouts operations", func() { Netns: nspath, IfName: ifname, StdinData: []byte(conf), - Args: cniArgs(podNamespace, podName), + Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,50)), } + ipamConf.PodName=fmt.Sprintf("%v-%d", podName,50) _, _, err = testutils.CmdAddWithArgs(args, func() error { return cmdAdd(args, mutateK8sIPAM(args.ContainerID, ipamConf, wbClient), "0.3.1") }) @@ -1048,12 +1050,12 @@ var _ = Describe("Whereabouts operations", func() { Netns: nspath, IfName: ifname, StdinData: []byte(conf), - Args: cniArgs(podNamespace, podName), + Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,0)), } confPath := filepath.Join(tmpDir, "whereabouts.conf") Expect(os.WriteFile(confPath, []byte(conf), 0755)).To(Succeed()) - ipamConf, cniVersion, err := config.LoadIPAMConfig([]byte(conf), cniArgs(podNamespace, podName), confPath) + ipamConf, cniVersion, err := config.LoadIPAMConfig([]byte(conf), cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,0)), confPath) Expect(err).NotTo(HaveOccurred()) Expect(ipamConf.IPRanges).NotTo(BeEmpty()) wbClient := *kubernetes.NewKubernetesClient( @@ -1102,12 +1104,12 @@ var _ = Describe("Whereabouts operations", func() { Netns: nspath, IfName: ifname, StdinData: []byte(confsecond), - Args: cniArgs(podNamespace, podName), + Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,1)), } secondConfPath := filepath.Join(tmpDir, "whereabouts.conf") Expect(os.WriteFile(confPath, []byte(confsecond), 0755)).To(Succeed()) - secondIPAMConf, secondCNIVersion, err := config.LoadIPAMConfig([]byte(confsecond), cniArgs(podNamespace, podName), secondConfPath) + secondIPAMConf, secondCNIVersion, err := config.LoadIPAMConfig([]byte(confsecond), cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,1)), secondConfPath) Expect(err).NotTo(HaveOccurred()) // Allocate the IP @@ -1170,12 +1172,12 @@ var _ = Describe("Whereabouts operations", func() { Netns: nspath, IfName: ifname, StdinData: []byte(conf), - Args: cniArgs(podNamespace, podName), + Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,0)), } confPath := filepath.Join(tmpDir, "whereabouts.conf") Expect(os.WriteFile(confPath, []byte(conf), 0755)).To(Succeed()) - ipamConf, cniVersion, err := config.LoadIPAMConfig([]byte(conf), cniArgs(podNamespace, podName), confPath) + ipamConf, cniVersion, err := config.LoadIPAMConfig([]byte(conf), cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,0)), confPath) Expect(err).NotTo(HaveOccurred()) Expect(ipamConf.IPRanges).NotTo(BeEmpty()) wbClient := *kubernetes.NewKubernetesClient( @@ -1224,12 +1226,12 @@ var _ = Describe("Whereabouts operations", func() { Netns: nspath, IfName: ifname, StdinData: []byte(confsecond), - Args: cniArgs(podNamespace, podName), + Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,1)), } secondConfPath := filepath.Join(tmpDir, "whereabouts.conf") Expect(os.WriteFile(confPath, []byte(confsecond), 0755)).To(Succeed()) - secondIPAMConf, secondCNIVersion, err := config.LoadIPAMConfig([]byte(confsecond), cniArgs(podNamespace, podName), secondConfPath) + secondIPAMConf, secondCNIVersion, err := config.LoadIPAMConfig([]byte(confsecond), cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,1)), secondConfPath) Expect(err).NotTo(HaveOccurred()) // Allocate the IP diff --git a/pkg/allocate/allocate.go b/pkg/allocate/allocate.go index 88dead47f..76ac2ce64 100644 --- a/pkg/allocate/allocate.go +++ b/pkg/allocate/allocate.go @@ -100,9 +100,9 @@ func IterateForAssignment(ipnet net.IPNet, rangeStart net.IP, rangeEnd net.IP, r rangeStart, rangeEnd, ipnet, firstIP, lastIP) // Build reserved map. - reserved := make(map[string]bool) + reserved := make(map[string]string) for _, r := range reserveList { - reserved[r.IP.String()] = true + reserved[r.IP.String()] = r.PodRef } // Build excluded list, "192.168.2.229/30", "192.168.1.229/30". @@ -119,8 +119,12 @@ func IterateForAssignment(ipnet net.IPNet, rangeStart net.IP, rangeEnd net.IP, r // within ipnet, and make sure that ip is smaller than lastIP. for ip := firstIP; ipnet.Contains(ip) && iphelpers.CompareIPs(ip, lastIP) <= 0; ip = iphelpers.IncIP(ip) { // If already reserved, skip it. - if reserved[ip.String()] { - continue + ref, exist := reserved[ip.String()] + if exist { + if ref != podRef { + continue + } + logging.Debugf("Found existing reservation %v with matching podRef %s", ip.String(), podRef) } // If this IP is within the range of one of the excluded subnets, jump to the exluded subnet's broadcast address // and skip. diff --git a/pkg/allocate/allocate_test.go b/pkg/allocate/allocate_test.go index d88ed9696..bde58ea01 100644 --- a/pkg/allocate/allocate_test.go +++ b/pkg/allocate/allocate_test.go @@ -415,4 +415,24 @@ var _ = Describe("Allocation operations", func() { }) }) }) + + + It("can IterateForAssignment on an IPv4 address for existing pod Allocation and return same IP", func() { + + firstip, ipnet, err := net.ParseCIDR("192.168.1.1/24") + Expect(err).NotTo(HaveOccurred()) + + // figure out the range start. + calculatedrangestart := net.ParseIP(firstip.Mask(ipnet.Mask).String()) + + var ipres []types.IPReservation + var exrange []string + podRef := "hello/world-0" + newip, _, err := IterateForAssignment(*ipnet, calculatedrangestart, nil, ipres, exrange, "0xdeadbeef", podRef ) + Expect(err).NotTo(HaveOccurred()) + newipsec, _, err := IterateForAssignment(*ipnet, calculatedrangestart, nil, ipres, exrange, "0xdeadbeef", podRef ) + Expect(err).NotTo(HaveOccurred()) + Expect(fmt.Sprint(newip)).To(Equal(fmt.Sprint(newipsec))) + + }) }) diff --git a/pkg/storage/kubernetes/ipam.go b/pkg/storage/kubernetes/ipam.go index 299c3ee58..3fadcf81d 100644 --- a/pkg/storage/kubernetes/ipam.go +++ b/pkg/storage/kubernetes/ipam.go @@ -200,13 +200,13 @@ func (i *KubernetesIPAM) GetOverlappingRangeStore() (storage.OverlappingRangeSto // IsAllocatedInOverlappingRange checks for IP addresses to see if they're allocated cluster wide, for overlapping // ranges. func (c *KubernetesOverlappingRangeStore) IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP, - networkName string) (bool, error) { + networkName string , podRef string) (bool, error) { normalizedIP := normalizeIP(ip, networkName) logging.Debugf("OverlappingRangewide allocation check; normalized IP: %q, IP: %q, networkName: %q", normalizedIP, ip, networkName) - _, err := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedIP, metav1.GetOptions{}) + clusteripres, err := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedIP, metav1.GetOptions{}) if err != nil && errors.IsNotFound(err) { // cluster ip reservation does not exist, this appears to be good news. // logging.Debugf("IP %v is not reserved cluster wide, allowing.", ip) @@ -216,6 +216,11 @@ func (c *KubernetesOverlappingRangeStore) IsAllocatedInOverlappingRange(ctx cont return false, fmt.Errorf("k8s get OverlappingRangeIPReservation error: %s", err) } + if clusteripres.Spec.PodRef == podRef { + logging.Debugf("IP %v matches existing podRef %s", ip, podRef) + return false, nil + } + logging.Debugf("Normalized IP is reserved; normalized IP: %q, IP: %q, networkName: %q", normalizedIP, ip, networkName) return true, nil @@ -245,6 +250,21 @@ func (c *KubernetesOverlappingRangeStore) UpdateOverlappingRangeAllocation(ctx c _, err = c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Create( ctx, clusteripres, metav1.CreateOptions{}) + if errors.IsAlreadyExists(err) { + logging.Debugf("clusteripres already exists, updating with %v", clusteripres.Spec) + // first get the existing object resourceVersion and then update it https://github.com/kubernetes/kubernetes/issues/70674 + clusteripresorig, errorig := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedIP, metav1.GetOptions{}) + if errorig != nil { + err=errorig + } else { + clusteripres.SetResourceVersion(clusteripresorig.GetResourceVersion()) + _, err = c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Update(ctx, clusteripres, metav1.UpdateOptions{}) + } + + + } + + case whereaboutstypes.Deallocate: verb = "deallocate" err = c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Delete(ctx, clusteripres.GetName(), metav1.DeleteOptions{}) @@ -331,7 +351,7 @@ func (p *KubernetesIPPool) Update(ctx context.Context, reservations []whereabout } // apply the patch - _, err = p.client.WhereaboutsV1alpha1().IPPools(orig.GetNamespace()).Patch(ctx, orig.GetName(), types.JSONPatchType, patchData, metav1.PatchOptions{}) + patchresult, err := p.client.WhereaboutsV1alpha1().IPPools(orig.GetNamespace()).Patch(ctx, orig.GetName(), types.JSONPatchType, patchData, metav1.PatchOptions{}) if err != nil { if errors.IsInvalid(err) { // expect "invalid" errors if any of the jsonpatch "test" Operations fail @@ -525,7 +545,7 @@ func IPManagementKubernetesUpdate(ctx context.Context, mode int, ipam *Kubernete // And we try again. if ipamConf.OverlappingRanges { isAllocated, err := overlappingrangestore.IsAllocatedInOverlappingRange(requestCtx, newip.IP, - ipamConf.NetworkName) + ipamConf.NetworkName, podRef) if err != nil { logging.Errorf("Error checking overlappingrange allocation: %v", err) return newips, err diff --git a/pkg/storage/storage.go b/pkg/storage/storage.go index 52660eee2..2d679a37c 100644 --- a/pkg/storage/storage.go +++ b/pkg/storage/storage.go @@ -33,7 +33,7 @@ type Store interface { // OverlappingRangeStore is an interface for wrapping overlappingrange storage options type OverlappingRangeStore interface { - IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP, networkName string) (bool, error) + IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP, networkName string, podRef string) (bool, error) UpdateOverlappingRangeAllocation(ctx context.Context, mode int, ip net.IP, containerID string, podRef, networkName string) error }