Skip to content

Commit

Permalink
Return existing reservation if podRef matched - rebase
Browse files Browse the repository at this point in the history 
in case allocation already found for pod in this range we will return it, avoiding IP leak

Co-Authored-By: Arjun Baindur <15885540+xagent003@users.noreply.github.com>
  • Loading branch information
@caribbeantiger @xagent003
caribbeantiger and xagent003 committed Sep 12, 2023
1 parent 16baf31 commit 00ebc0e
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 20 deletions.
26 changes: 14 additions & 12 deletions cmd/whereabouts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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")
})
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions pkg/allocate/allocate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand All @@ -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.
Expand Down
20 changes: 20 additions & 0 deletions pkg/allocate/allocate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)))

})
})
26 changes: 23 additions & 3 deletions pkg/storage/kubernetes/ipam.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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{})
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/storage/storage.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 00ebc0e

Please sign in to comment.