Skip to content

Commit

Permalink
User podRef as indentifier in IP allocation/deallocation
Browse files Browse the repository at this point in the history 
The containerID of a pod will change after node reboot. It caused
the IP allocation/deallocation operation cannot find the IP that
has been reserved by the pod.

Signed-off-by: Peng Liu <[email protected]>
  • Loading branch information
@pliurh
pliurh committed Mar 11, 2024
1 parent 886c060 commit b823fee
Show file tree
Hide file tree
Showing 5 changed files with 141 additions and 62 deletions.
28 changes: 17 additions & 11 deletions cmd/whereabouts_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ func AllocateAndReleaseAddressesTest(ipVersion string, ipamConf *whereaboutstype
ifname string = "eth0"
nspath string = "/some/where"
cniVersion = "0.3.1"
podName = "dummyPOD"
podNamespace = "dummyNS"
)

Expand All @@ -62,12 +61,13 @@ func AllocateAndReleaseAddressesTest(ipVersion string, ipamConf *whereaboutstype
fakek8sclient.NewSimpleClientset(),
0)
for i := 0; i < len(expectedAddresses); i++ {
ipamConf.PodName = fmt.Sprintf("pod-%d", i)
args := &skel.CmdArgs{
ContainerID: fmt.Sprintf("dummy-%d", i),
Netns: nspath,
IfName: ifname,
StdinData: cniConf,
Args: cniArgs(podNamespace, podName),
Args: cniArgs(podNamespace, ipamConf.PodName),
}
client := mutateK8sIPAM(args.ContainerID, ipamConf, wbClient)

Expand Down Expand Up @@ -956,6 +956,8 @@ var _ = Describe("Whereabouts operations", func() {
var ipArgs []*skel.CmdArgs
// allocate 8 IPs (192.168.1.5 - 192.168.1.12); the entirety of the pool defined above
for i := 0; i < 8; i++ {
podName := fmt.Sprintf("pod-%d", i)
ipamConf.PodName = podName
args := &skel.CmdArgs{
ContainerID: fmt.Sprintf("dummy-%d", i),
Netns: nspath,
Expand All @@ -982,7 +984,7 @@ var _ = Describe("Whereabouts operations", func() {
}))
ipArgs = append(ipArgs, args)
}

ipamConf.PodName = podName
// assigning more IPs should result in error due to the defined range_start - range_end
args := &skel.CmdArgs{
ContainerID: "dummy-failure",
Expand Down Expand Up @@ -1014,6 +1016,8 @@ var _ = Describe("Whereabouts operations", func() {
It("detects IPv4 addresses used in other ranges, to allow for overlapping IP address ranges", func() {
const ifname string = "eth0"
const nspath string = "/some/where"
const podName1 string = "pod-1"
const podName2 string = "pod-2"

// ----------------------------- range 1

Expand All @@ -1037,12 +1041,12 @@ var _ = Describe("Whereabouts operations", func() {
Netns: nspath,
IfName: ifname,
StdinData: []byte(conf),
Args: cniArgs(podNamespace, podName),
Args: cniArgs(podNamespace, podName1),
}

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, podName1), confPath)
Expect(err).NotTo(HaveOccurred())
Expect(ipamConf.IPRanges).NotTo(BeEmpty())
wbClient := *kubernetes.NewKubernetesClient(
Expand Down Expand Up @@ -1090,12 +1094,12 @@ var _ = Describe("Whereabouts operations", func() {
Netns: nspath,
IfName: ifname,
StdinData: []byte(confsecond),
Args: cniArgs(podNamespace, podName),
Args: cniArgs(podNamespace, podName2),
}

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, podName2), secondConfPath)
Expect(err).NotTo(HaveOccurred())

// Allocate the IP
Expand Down Expand Up @@ -1134,6 +1138,8 @@ var _ = Describe("Whereabouts operations", func() {
It("detects IPv6 addresses used in other ranges, to allow for overlapping IP address ranges", func() {
const ifname string = "eth0"
const nspath string = "/some/where"
const podName1 string = "pod-1"
const podName2 string = "pod-2"

// ----------------------------- range 1

Expand All @@ -1157,12 +1163,12 @@ var _ = Describe("Whereabouts operations", func() {
Netns: nspath,
IfName: ifname,
StdinData: []byte(conf),
Args: cniArgs(podNamespace, podName),
Args: cniArgs(podNamespace, podName1),
}

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, podName1), confPath)
Expect(err).NotTo(HaveOccurred())
Expect(ipamConf.IPRanges).NotTo(BeEmpty())
wbClient := *kubernetes.NewKubernetesClient(
Expand Down Expand Up @@ -1210,12 +1216,12 @@ var _ = Describe("Whereabouts operations", func() {
Netns: nspath,
IfName: ifname,
StdinData: []byte(confsecond),
Args: cniArgs(podNamespace, podName),
Args: cniArgs(podNamespace, podName2),
}

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, podName2), secondConfPath)
Expect(err).NotTo(HaveOccurred())

// Allocate the IP
Expand Down
24 changes: 16 additions & 8 deletions pkg/allocate/allocate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ func AssignIP(ipamConf types.RangeConfiguration, reservelist []types.IPReservati
}

// DeallocateIP assigns an IP using a range and a reserve list.
func DeallocateIP(reservelist []types.IPReservation, containerID string) ([]types.IPReservation, net.IP, error) {
func DeallocateIP(reservelist []types.IPReservation, podRef string) ([]types.IPReservation, net.IP, error) {

updatedreservelist, hadip, err := IterateForDeallocation(reservelist, containerID, getMatchingIPReservationIndex)
updatedreservelist, hadip, err := IterateForDeallocation(reservelist, podRef, getMatchingIPReservationIndex)
if err != nil {
return nil, nil, err
}
Expand All @@ -52,13 +52,13 @@ func DeallocateIP(reservelist []types.IPReservation, containerID string) ([]type
// IterateForDeallocation iterates overs currently reserved IPs and the deallocates given the container id.
func IterateForDeallocation(
reservelist []types.IPReservation,
containerID string,
podRef string,
matchingFunction func(reservation []types.IPReservation, id string) int) ([]types.IPReservation, net.IP, error) {

foundidx := matchingFunction(reservelist, containerID)
foundidx := matchingFunction(reservelist, podRef)
// Check if it's a valid index
if foundidx < 0 {
return reservelist, nil, fmt.Errorf("did not find reserved IP for container %v", containerID)
return reservelist, nil, fmt.Errorf("did not find reserved IP for container %v", podRef)
}

returnip := reservelist[foundidx].IP
Expand All @@ -70,7 +70,7 @@ func IterateForDeallocation(
func getMatchingIPReservationIndex(reservelist []types.IPReservation, id string) int {
foundidx := -1
for idx, v := range reservelist {
if v.ContainerID == id {
if v.PodRef == id {
foundidx = idx
break
}
Expand Down Expand Up @@ -102,6 +102,15 @@ func IterateForAssignment(ipnet net.IPNet, rangeStart net.IP, rangeEnd net.IP, r
// Build reserved map.
reserved := make(map[string]bool)
for _, r := range reserveList {
if r.PodRef == podRef {
// If this IP has been reserved for this pod, return it.
if r.ContainerID != containerID {
// update the containerID if it has changed
r.ContainerID = containerID
}
logging.Debugf("Returning reserved IP: |%v|", r.IP.String()+" "+podRef)
return r.IP, reserveList, nil
}
reserved[r.IP.String()] = true
}

Expand All @@ -118,7 +127,6 @@ func IterateForAssignment(ipnet net.IPNet, rangeStart net.IP, rangeEnd net.IP, r
// Iterate over every IP address in the range, accounting for reserved IPs and exclude ranges. Make sure that ip is
// 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
}
Expand All @@ -129,7 +137,7 @@ func IterateForAssignment(ipnet net.IPNet, rangeStart net.IP, rangeEnd net.IP, r
continue
}
// Assign and reserve the IP and return.
logging.Debugf("Reserving IP: |%v|", ip.String()+" "+containerID)
logging.Debugf("Reserving IP: |%v|", ip.String()+" "+podRef)
reserveList = append(reserveList, types.IPReservation{IP: ip, ContainerID: containerID, PodRef: podRef})
return ip, reserveList, nil
}
Expand Down
Loading

0 comments on commit b823fee

Please sign in to comment.