Skip to content

Commit 00ebc0e

Browse files
caribbeantigerxagent003
caribbeantiger
and
xagent003
committed
Return existing reservation if podRef matched - rebase
in case allocation already found for pod in this range we will return it, avoiding IP leak Co-Authored-By: Arjun Baindur <[email protected]>
1 parent 16baf31 commit 00ebc0e

File tree

5 files changed

+66
-20
lines changed

5 files changed

+66
-20
lines changed

‎cmd/whereabouts_test.go‎

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,9 @@ func AllocateAndReleaseAddressesTest(ipVersion string, ipamConf *whereaboutstype
6767
Netns: nspath,
6868
IfName: ifname,
6969
StdinData: cniConf,
70-
Args: cniArgs(podNamespace, podName),
70+
Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,i)),
7171
}
72+
ipamConf.PodName=fmt.Sprintf("%v-%d", podName,i)
7273
client := mutateK8sIPAM(args.ContainerID, ipamConf, wbClient)
7374

7475
// Allocate the IP
@@ -971,9 +972,9 @@ var _ = Describe("Whereabouts operations", func() {
971972
Netns: nspath,
972973
IfName: ifname,
973974
StdinData: []byte(conf),
974-
Args: cniArgs(podNamespace, podName),
975+
Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,i)),
975976
}
976-
977+
ipamConf.PodName=fmt.Sprintf("%v-%d", podName,i)
977978
k8sClient = mutateK8sIPAM(args.ContainerID, ipamConf, wbClient)
978979
r, raw, err := testutils.CmdAddWithArgs(args, func() error {
979980
return cmdAdd(args, k8sClient, cniVersion)
@@ -1000,8 +1001,9 @@ var _ = Describe("Whereabouts operations", func() {
10001001
Netns: nspath,
10011002
IfName: ifname,
10021003
StdinData: []byte(conf),
1003-
Args: cniArgs(podNamespace, podName),
1004+
Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,50)),
10041005
}
1006+
ipamConf.PodName=fmt.Sprintf("%v-%d", podName,50)
10051007
_, _, err = testutils.CmdAddWithArgs(args, func() error {
10061008
return cmdAdd(args, mutateK8sIPAM(args.ContainerID, ipamConf, wbClient), "0.3.1")
10071009
})
@@ -1048,12 +1050,12 @@ var _ = Describe("Whereabouts operations", func() {
10481050
Netns: nspath,
10491051
IfName: ifname,
10501052
StdinData: []byte(conf),
1051-
Args: cniArgs(podNamespace, podName),
1053+
Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,0)),
10521054
}
10531055

10541056
confPath := filepath.Join(tmpDir, "whereabouts.conf")
10551057
Expect(os.WriteFile(confPath, []byte(conf), 0755)).To(Succeed())
1056-
ipamConf, cniVersion, err := config.LoadIPAMConfig([]byte(conf), cniArgs(podNamespace, podName), confPath)
1058+
ipamConf, cniVersion, err := config.LoadIPAMConfig([]byte(conf), cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,0)), confPath)
10571059
Expect(err).NotTo(HaveOccurred())
10581060
Expect(ipamConf.IPRanges).NotTo(BeEmpty())
10591061
wbClient := *kubernetes.NewKubernetesClient(
@@ -1102,12 +1104,12 @@ var _ = Describe("Whereabouts operations", func() {
11021104
Netns: nspath,
11031105
IfName: ifname,
11041106
StdinData: []byte(confsecond),
1105-
Args: cniArgs(podNamespace, podName),
1107+
Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,1)),
11061108
}
11071109

11081110
secondConfPath := filepath.Join(tmpDir, "whereabouts.conf")
11091111
Expect(os.WriteFile(confPath, []byte(confsecond), 0755)).To(Succeed())
1110-
secondIPAMConf, secondCNIVersion, err := config.LoadIPAMConfig([]byte(confsecond), cniArgs(podNamespace, podName), secondConfPath)
1112+
secondIPAMConf, secondCNIVersion, err := config.LoadIPAMConfig([]byte(confsecond), cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,1)), secondConfPath)
11111113
Expect(err).NotTo(HaveOccurred())
11121114

11131115
// Allocate the IP
@@ -1170,12 +1172,12 @@ var _ = Describe("Whereabouts operations", func() {
11701172
Netns: nspath,
11711173
IfName: ifname,
11721174
StdinData: []byte(conf),
1173-
Args: cniArgs(podNamespace, podName),
1175+
Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,0)),
11741176
}
11751177

11761178
confPath := filepath.Join(tmpDir, "whereabouts.conf")
11771179
Expect(os.WriteFile(confPath, []byte(conf), 0755)).To(Succeed())
1178-
ipamConf, cniVersion, err := config.LoadIPAMConfig([]byte(conf), cniArgs(podNamespace, podName), confPath)
1180+
ipamConf, cniVersion, err := config.LoadIPAMConfig([]byte(conf), cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,0)), confPath)
11791181
Expect(err).NotTo(HaveOccurred())
11801182
Expect(ipamConf.IPRanges).NotTo(BeEmpty())
11811183
wbClient := *kubernetes.NewKubernetesClient(
@@ -1224,12 +1226,12 @@ var _ = Describe("Whereabouts operations", func() {
12241226
Netns: nspath,
12251227
IfName: ifname,
12261228
StdinData: []byte(confsecond),
1227-
Args: cniArgs(podNamespace, podName),
1229+
Args: cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,1)),
12281230
}
12291231

12301232
secondConfPath := filepath.Join(tmpDir, "whereabouts.conf")
12311233
Expect(os.WriteFile(confPath, []byte(confsecond), 0755)).To(Succeed())
1232-
secondIPAMConf, secondCNIVersion, err := config.LoadIPAMConfig([]byte(confsecond), cniArgs(podNamespace, podName), secondConfPath)
1234+
secondIPAMConf, secondCNIVersion, err := config.LoadIPAMConfig([]byte(confsecond), cniArgs(podNamespace, fmt.Sprintf("%v-%d", podName,1)), secondConfPath)
12331235
Expect(err).NotTo(HaveOccurred())
12341236

12351237
// Allocate the IP

‎pkg/allocate/allocate.go‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,9 +100,9 @@ func IterateForAssignment(ipnet net.IPNet, rangeStart net.IP, rangeEnd net.IP, r
100100
rangeStart, rangeEnd, ipnet, firstIP, lastIP)
101101

102102
// Build reserved map.
103-
reserved := make(map[string]bool)
103+
reserved := make(map[string]string)
104104
for _, r := range reserveList {
105-
reserved[r.IP.String()] = true
105+
reserved[r.IP.String()] = r.PodRef
106106
}
107107

108108
// 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
119119
// within ipnet, and make sure that ip is smaller than lastIP.
120120
for ip := firstIP; ipnet.Contains(ip) && iphelpers.CompareIPs(ip, lastIP) <= 0; ip = iphelpers.IncIP(ip) {
121121
// If already reserved, skip it.
122-
if reserved[ip.String()] {
123-
continue
122+
ref, exist := reserved[ip.String()]
123+
if exist {
124+
if ref != podRef {
125+
continue
126+
}
127+
logging.Debugf("Found existing reservation %v with matching podRef %s", ip.String(), podRef)
124128
}
125129
// If this IP is within the range of one of the excluded subnets, jump to the exluded subnet's broadcast address
126130
// and skip.

‎pkg/allocate/allocate_test.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,4 +415,24 @@ var _ = Describe("Allocation operations", func() {
415415
})
416416
})
417417
})
418+
419+
420+
It("can IterateForAssignment on an IPv4 address for existing pod Allocation and return same IP", func() {
421+
422+
firstip, ipnet, err := net.ParseCIDR("192.168.1.1/24")
423+
Expect(err).NotTo(HaveOccurred())
424+
425+
// figure out the range start.
426+
calculatedrangestart := net.ParseIP(firstip.Mask(ipnet.Mask).String())
427+
428+
var ipres []types.IPReservation
429+
var exrange []string
430+
podRef := "hello/world-0"
431+
newip, _, err := IterateForAssignment(*ipnet, calculatedrangestart, nil, ipres, exrange, "0xdeadbeef", podRef )
432+
Expect(err).NotTo(HaveOccurred())
433+
newipsec, _, err := IterateForAssignment(*ipnet, calculatedrangestart, nil, ipres, exrange, "0xdeadbeef", podRef )
434+
Expect(err).NotTo(HaveOccurred())
435+
Expect(fmt.Sprint(newip)).To(Equal(fmt.Sprint(newipsec)))
436+
437+
})
418438
})

‎pkg/storage/kubernetes/ipam.go‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -200,13 +200,13 @@ func (i *KubernetesIPAM) GetOverlappingRangeStore() (storage.OverlappingRangeSto
200200
// IsAllocatedInOverlappingRange checks for IP addresses to see if they're allocated cluster wide, for overlapping
201201
// ranges.
202202
func (c *KubernetesOverlappingRangeStore) IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP,
203-
networkName string) (bool, error) {
203+
networkName string , podRef string) (bool, error) {
204204
normalizedIP := normalizeIP(ip, networkName)
205205

206206
logging.Debugf("OverlappingRangewide allocation check; normalized IP: %q, IP: %q, networkName: %q",
207207
normalizedIP, ip, networkName)
208208

209-
_, err := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedIP, metav1.GetOptions{})
209+
clusteripres, err := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedIP, metav1.GetOptions{})
210210
if err != nil && errors.IsNotFound(err) {
211211
// cluster ip reservation does not exist, this appears to be good news.
212212
// logging.Debugf("IP %v is not reserved cluster wide, allowing.", ip)
@@ -216,6 +216,11 @@ func (c *KubernetesOverlappingRangeStore) IsAllocatedInOverlappingRange(ctx cont
216216
return false, fmt.Errorf("k8s get OverlappingRangeIPReservation error: %s", err)
217217
}
218218

219+
if clusteripres.Spec.PodRef == podRef {
220+
logging.Debugf("IP %v matches existing podRef %s", ip, podRef)
221+
return false, nil
222+
}
223+
219224
logging.Debugf("Normalized IP is reserved; normalized IP: %q, IP: %q, networkName: %q",
220225
normalizedIP, ip, networkName)
221226
return true, nil
@@ -245,6 +250,21 @@ func (c *KubernetesOverlappingRangeStore) UpdateOverlappingRangeAllocation(ctx c
245250
_, err = c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Create(
246251
ctx, clusteripres, metav1.CreateOptions{})
247252

253+
if errors.IsAlreadyExists(err) {
254+
logging.Debugf("clusteripres already exists, updating with %v", clusteripres.Spec)
255+
// first get the existing object resourceVersion and then update it https://github.com/kubernetes/kubernetes/issues/70674
256+
clusteripresorig, errorig := c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Get(ctx, normalizedIP, metav1.GetOptions{})
257+
if errorig != nil {
258+
err=errorig
259+
} else {
260+
clusteripres.SetResourceVersion(clusteripresorig.GetResourceVersion())
261+
_, err = c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Update(ctx, clusteripres, metav1.UpdateOptions{})
262+
}
263+
264+
265+
}
266+
267+
248268
case whereaboutstypes.Deallocate:
249269
verb = "deallocate"
250270
err = c.client.WhereaboutsV1alpha1().OverlappingRangeIPReservations(c.namespace).Delete(ctx, clusteripres.GetName(), metav1.DeleteOptions{})
@@ -525,7 +545,7 @@ func IPManagementKubernetesUpdate(ctx context.Context, mode int, ipam *Kubernete
525545
// And we try again.
526546
if ipamConf.OverlappingRanges {
527547
isAllocated, err := overlappingrangestore.IsAllocatedInOverlappingRange(requestCtx, newip.IP,
528-
ipamConf.NetworkName)
548+
ipamConf.NetworkName, podRef)
529549
if err != nil {
530550
logging.Errorf("Error checking overlappingrange allocation: %v", err)
531551
return newips, err

‎pkg/storage/storage.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type Store interface {
3333

3434
// OverlappingRangeStore is an interface for wrapping overlappingrange storage options
3535
type OverlappingRangeStore interface {
36-
IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP, networkName string) (bool, error)
36+
IsAllocatedInOverlappingRange(ctx context.Context, ip net.IP, networkName string, podRef string) (bool, error)
3737
UpdateOverlappingRangeAllocation(ctx context.Context, mode int, ip net.IP, containerID string, podRef,
3838
networkName string) error
3939
}

0 commit comments

Comments
 (0)