Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ipam clean all pod nic ip address and mac even if just delete a nic #3453

Merged
merged 4 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions pkg/controller/gc.go
Original file line number Diff line number Diff line change
Expand Up @@ -353,20 +353,31 @@ func (c *Controller) markAndCleanLSP() error {
}

klog.Infof("gc logical switch port %s", lsp.Name)
if err := c.OVNNbClient.DeleteLogicalSwitchPort(lsp.Name); err != nil {
klog.Errorf("failed to delete lsp %s: %v", lsp.Name, err)
ipCr, err := c.config.KubeOvnClient.KubeovnV1().IPs().Get(context.Background(), lsp.Name, metav1.GetOptions{})
if err != nil {
if k8serrors.IsNotFound(err) {
// ip cr not found, skip lsp gc
continue
}
klog.Errorf("failed to get ip %s, %v", lsp.Name, err)
return err
}

if err := c.config.KubeOvnClient.KubeovnV1().IPs().Delete(context.Background(), lsp.Name, metav1.DeleteOptions{}); err != nil {
if !k8serrors.IsNotFound(err) {
klog.Errorf("failed to delete ip %s, %v", lsp.Name, err)
return err
klog.Infof("gc ip %s", ipCr.Name)
if err := c.config.KubeOvnClient.KubeovnV1().IPs().Delete(context.Background(), ipCr.Name, metav1.DeleteOptions{}); err != nil {
if k8serrors.IsNotFound(err) {
// ip cr not found, skip lsp gc
continue
}
klog.Errorf("failed to delete ip %s, %v", ipCr.Name, err)
return err
}
if ipCr.Spec.Subnet == "" {
klog.Errorf("ip %s has no subnet", ipCr.Name)
// ip cr no subnet, skip lsp gc
continue
}

if key := lsp.ExternalIDs["pod"]; key != "" {
c.ipam.ReleaseAddressByPod(key)
c.ipam.ReleaseAddressByPod(key, ipCr.Spec.Subnet)
}
}
lastNoPodLSP = noPodLSP
Expand Down
3 changes: 2 additions & 1 deletion pkg/controller/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,8 @@ func (c *Controller) handleDeleteNode(key string) error {
return err
}

c.ipam.ReleaseAddressByPod(portName)
klog.Infof("release node port %s", portName)
c.ipam.ReleaseAddressByPod(portName, c.config.NodeSwitch)

providerNetworks, err := c.providerNetworksLister.List(labels.Everything())
if err != nil && !k8serrors.IsNotFound(err) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/controller/ovn_eip.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func (c *Controller) handleDelOvnEip(key string) error {
}
}

c.ipam.ReleaseAddressByPod(eip.Name)
c.ipam.ReleaseAddressByPod(eip.Name, eip.Spec.ExternalSubnet)
c.updateSubnetStatusQueue.Add(eip.Spec.ExternalSubnet)
return nil
}
Expand Down
6 changes: 4 additions & 2 deletions pkg/controller/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,8 @@ func (c *Controller) changeVMSubnet(vmName, namespace, providerName, subnetName
return err
}
}
c.ipam.ReleaseAddressByPod(key)
klog.Infof("after changing subnet, release ip %s", ipName)
c.ipam.ReleaseAddressByPod(key, ipCr.Spec.Subnet)
}
}
return nil
Expand Down Expand Up @@ -1084,7 +1085,8 @@ func (c *Controller) handleDeletePod(key string) error {
}
}

c.ipam.ReleaseAddressByPod(podKey)
klog.Infof("release all ip address for deleting pod %s", podKey)
c.ipam.ReleaseAddressByPod(podKey, "")

podNets, err := c.getPodKubeovnNets(pod)
if err != nil {
Expand Down
6 changes: 4 additions & 2 deletions pkg/controller/subnet.go
Original file line number Diff line number Diff line change
Expand Up @@ -1931,7 +1931,8 @@ func (c *Controller) reconcileU2OInterconnectionIP(subnet *kubeovnv1.Subnet) err
}
} else if subnet.Spec.U2OInterconnectionIP != "" && subnet.Status.U2OInterconnectionIP != subnet.Spec.U2OInterconnectionIP {
if subnet.Status.U2OInterconnectionIP != "" {
c.ipam.ReleaseAddressByPod(u2oInterconnName)
klog.Infof("release underlay to overlay interconnection ip address %s for subnet %s", subnet.Status.U2OInterconnectionIP, subnet.Name)
c.ipam.ReleaseAddressByPod(u2oInterconnName, subnet.Name)
}

v4ip, v6ip, _, err = c.acquireStaticIPAddress(subnet.Name, u2oInterconnName, u2oInterconnLrpName, subnet.Spec.U2OInterconnectionIP)
Expand Down Expand Up @@ -1959,7 +1960,8 @@ func (c *Controller) reconcileU2OInterconnectionIP(subnet *kubeovnv1.Subnet) err
}
} else if subnet.Status.U2OInterconnectionIP != "" {
u2oInterconnName := fmt.Sprintf(util.U2OInterconnName, subnet.Spec.Vpc, subnet.Name)
c.ipam.ReleaseAddressByPod(u2oInterconnName)
klog.Infof("release underlay to overlay interconnection ip address %s for subnet %s", subnet.Status.U2OInterconnectionIP, subnet.Name)
c.ipam.ReleaseAddressByPod(u2oInterconnName, subnet.Name)
subnet.Status.U2OInterconnectionIP = ""

if err := c.config.KubeOvnClient.KubeovnV1().IPs().Delete(context.Background(), u2oInterconnName, metav1.DeleteOptions{}); err != nil {
Expand Down
4 changes: 2 additions & 2 deletions pkg/controller/vip.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ func (c *Controller) handleDelVirtualIP(vip *kubeovnv1.Vip) error {
klog.Errorf("delete virtual logical switch port %s from logical switch %s: %v", vip.Name, vip.Spec.Subnet, err)
return err
}
c.ipam.ReleaseAddressByPod(vip.Name)
c.ipam.ReleaseAddressByPod(vip.Name, vip.Spec.Subnet)
c.updateSubnetStatusQueue.Add(vip.Spec.Subnet)
return nil
}
Expand Down Expand Up @@ -635,7 +635,7 @@ func (c *Controller) podReuseVip(key, portName string, keepVIP bool) error {
klog.Errorf("failed to patch label for vip '%s', %v", vip.Name, err)
return err
}
c.ipam.ReleaseAddressByPod(key)
c.ipam.ReleaseAddressByPod(key, vip.Spec.Subnet)
c.updateSubnetStatusQueue.Add(vip.Spec.Subnet)
return nil
}
Expand Down
8 changes: 4 additions & 4 deletions pkg/controller/vpc_nat_gw_eip.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ func (c *Controller) handleUpdateIptablesEip(key string) error {
externalNetwork := util.GetExternalNetwork(cachedEip.Spec.ExternalSubnet)
// should delete
if !cachedEip.DeletionTimestamp.IsZero() {
klog.V(3).Infof("clean eip '%s' in pod", key)
klog.Infof("clean eip %q in pod", key)
v4Cidr, err := c.getEipV4Cidr(cachedEip.Status.IP, externalNetwork)
if err != nil {
klog.Errorf("failed to clean eip %s, %v", key, err)
Expand All @@ -318,9 +318,10 @@ func (c *Controller) handleUpdateIptablesEip(key string) error {
klog.Errorf("failed to handle del finalizer for eip %s, %v", key, err)
return err
}
c.ipam.ReleaseAddressByPod(key, cachedEip.Spec.ExternalSubnet)
return nil
}
klog.V(3).Infof("handle update eip %s", key)
klog.Infof("handle update eip %s", key)
// eip change ip
if c.eipChangeIP(cachedEip) {
err := fmt.Errorf("not support eip change ip, old ip '%s', new ip '%s'", cachedEip.Status.IP, cachedEip.Spec.V4ip)
Expand Down Expand Up @@ -400,8 +401,7 @@ func (c *Controller) handleUpdateIptablesEip(key string) error {
}

func (c *Controller) handleDelIptablesEip(key string) error {
c.ipam.ReleaseAddressByPod(key)
klog.V(3).Infof("deleted vpc nat eip %s", key)
klog.Infof("handle delete iptables eip %s", key)
return nil
}

Expand Down
12 changes: 9 additions & 3 deletions pkg/ipam/ipam.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,17 @@ func checkAndAppendIpsForDual(ips []IP, mac, podName, nicName string, subnet *Su
return newIps, err
}

func (ipam *IPAM) ReleaseAddressByPod(podName string) {
func (ipam *IPAM) ReleaseAddressByPod(podName, subnetName string) {
ipam.mutex.RLock()
defer ipam.mutex.RUnlock()
for _, subnet := range ipam.Subnets {
subnet.ReleaseAddress(podName)
if subnetName != "" {
if subnet, ok := ipam.Subnets[subnetName]; ok {
subnet.ReleaseAddress(podName)
}
} else {
for _, subnet := range ipam.Subnets {
subnet.ReleaseAddress(podName)
}
}
}

Expand Down
30 changes: 15 additions & 15 deletions test/unittest/ipam/ipam.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ var _ = Describe("[IPAM]", func() {
Expect(err).Should(MatchError(ipam.ErrConflict))

By("release pod with multiple nics")
im.ReleaseAddressByPod(pod2)
im.ReleaseAddressByPod(pod2, "")
ip2, err := ipam.NewIP(freeIP2)
Expect(err).ShouldNot(HaveOccurred())
ip3, err := ipam.NewIP(freeIP3)
Expand All @@ -120,7 +120,7 @@ var _ = Describe("[IPAM]", func() {
Expect(im.Subnets[subnetName].IPPools[""].V4Released.Contains(ip3)).Should(BeTrue())

By("release pod with single nic")
im.ReleaseAddressByPod(pod1)
im.ReleaseAddressByPod(pod1, "")
ip1, err := ipam.NewIP(freeIP1)
Expect(err).ShouldNot(HaveOccurred())
Expect(im.Subnets[subnetName].IPPools[""].V4Released.Contains(ip1)).To(BeTrue())
Expand Down Expand Up @@ -166,12 +166,12 @@ var _ = Describe("[IPAM]", func() {
Expect(err).ShouldNot(HaveOccurred())
Expect(ip).To(Equal("10.16.0.1"))

im.ReleaseAddressByPod("pod1.ns")
im.ReleaseAddressByPod("pod1.ns", "")
ip, _, _, err = im.GetRandomAddress("pod1.ns", "pod1.ns", nil, subnetName, "", nil, true)
Expect(err).ShouldNot(HaveOccurred())
Expect(ip).To(Equal("10.16.0.2"))

im.ReleaseAddressByPod("pod1.ns")
im.ReleaseAddressByPod("pod1.ns", "")
ip, _, _, err = im.GetRandomAddress("pod1.ns", "pod1.ns", nil, subnetName, "", nil, true)
Expect(err).ShouldNot(HaveOccurred())
Expect(ip).To(Equal("10.16.0.1"))
Expand All @@ -186,7 +186,7 @@ var _ = Describe("[IPAM]", func() {
Expect(err).ShouldNot(HaveOccurred())
Expect(ip).To(Equal("10.16.0.1"))

im.ReleaseAddressByPod("pod1.ns")
im.ReleaseAddressByPod("pod1.ns", "")
err = im.AddOrUpdateSubnet(subnetName, "10.16.0.0/30", v4Gw, []string{"10.16.0.1..10.16.0.2"})
Expect(err).ShouldNot(HaveOccurred())

Expand Down Expand Up @@ -266,7 +266,7 @@ var _ = Describe("[IPAM]", func() {
Expect(err).Should(MatchError(ipam.ErrConflict))

By("release pod with multiple nics")
im.ReleaseAddressByPod(pod2)
im.ReleaseAddressByPod(pod2, "")
ip2, err := ipam.NewIP(freeIP2)
Expect(err).ShouldNot(HaveOccurred())
ip3, err := ipam.NewIP(freeIP3)
Expand All @@ -275,7 +275,7 @@ var _ = Describe("[IPAM]", func() {
Expect(im.Subnets[subnetName].IPPools[""].V6Released.Contains(ip3)).Should(BeTrue())

By("release pod with single nic")
im.ReleaseAddressByPod(pod1)
im.ReleaseAddressByPod(pod1, "")
ip1, err := ipam.NewIP(freeIP1)
Expect(err).ShouldNot(HaveOccurred())
Expect(im.Subnets[subnetName].IPPools[""].V6Released.Contains(ip1)).Should(BeTrue())
Expand Down Expand Up @@ -321,12 +321,12 @@ var _ = Describe("[IPAM]", func() {
Expect(err).ShouldNot(HaveOccurred())
Expect(ip).To(Equal("fd00::1"))

im.ReleaseAddressByPod("pod1.ns")
im.ReleaseAddressByPod("pod1.ns", "")
_, ip, _, err = im.GetRandomAddress("pod1.ns", "pod1.ns", nil, subnetName, "", nil, true)
Expect(err).ShouldNot(HaveOccurred())
Expect(ip).To(Equal("fd00::2"))

im.ReleaseAddressByPod("pod1.ns")
im.ReleaseAddressByPod("pod1.ns", "")
_, ip, _, err = im.GetRandomAddress("pod1.ns", "pod1.ns", nil, subnetName, "", nil, true)
Expect(err).ShouldNot(HaveOccurred())
Expect(ip).To(Equal("fd00::1"))
Expand All @@ -341,7 +341,7 @@ var _ = Describe("[IPAM]", func() {
Expect(err).ShouldNot(HaveOccurred())
Expect(ip).To(Equal("fd00::1"))

im.ReleaseAddressByPod("pod1.ns")
im.ReleaseAddressByPod("pod1.ns", "")
err = im.AddOrUpdateSubnet(subnetName, "fd00::/126", v6Gw, []string{"fd00::1..fd00::2"})
Expect(err).ShouldNot(HaveOccurred())

Expand Down Expand Up @@ -440,7 +440,7 @@ var _ = Describe("[IPAM]", func() {
Expect(err).Should(MatchError(ipam.ErrConflict))

By("release pod with multiple nics")
im.ReleaseAddressByPod(pod2)
im.ReleaseAddressByPod(pod2, "")
ip42, err := ipam.NewIP(freeIP42)
Expect(err).ShouldNot(HaveOccurred())
ip43, err := ipam.NewIP(freeIP43)
Expand All @@ -455,7 +455,7 @@ var _ = Describe("[IPAM]", func() {
Expect(im.Subnets[subnetName].IPPools[""].V6Released.Contains(ip63)).Should(BeTrue())

By("release pod with single nic")
im.ReleaseAddressByPod(pod1)
im.ReleaseAddressByPod(pod1, "")
ip41, err := ipam.NewIP(freeIP41)
Expect(err).ShouldNot(HaveOccurred())
ip61, err := ipam.NewIP(freeIP61)
Expand Down Expand Up @@ -504,13 +504,13 @@ var _ = Describe("[IPAM]", func() {
Expect(ipv4).To(Equal("10.16.0.1"))
Expect(ipv6).To(Equal("fd00::1"))

im.ReleaseAddressByPod("pod1.ns")
im.ReleaseAddressByPod("pod1.ns", "")
ipv4, ipv6, _, err = im.GetRandomAddress("pod1.ns", "pod1.ns", nil, subnetName, "", nil, true)
Expect(err).ShouldNot(HaveOccurred())
Expect(ipv4).To(Equal("10.16.0.2"))
Expect(ipv6).To(Equal("fd00::2"))

im.ReleaseAddressByPod("pod1.ns")
im.ReleaseAddressByPod("pod1.ns", "")
ipv4, ipv6, _, err = im.GetRandomAddress("pod1.ns", "pod1.ns", nil, subnetName, "", nil, true)
Expect(err).ShouldNot(HaveOccurred())
Expect(ipv4).To(Equal("10.16.0.1"))
Expand All @@ -527,7 +527,7 @@ var _ = Describe("[IPAM]", func() {
Expect(ipv4).To(Equal("10.16.0.1"))
Expect(ipv6).To(Equal("fd00::1"))

im.ReleaseAddressByPod("pod1.ns")
im.ReleaseAddressByPod("pod1.ns", "")
err = im.AddOrUpdateSubnet(subnetName, "10.16.0.2/30,fd00::/126", dualGw, []string{"10.16.0.1..10.16.0.2", "fd00::1..fd00::2"})
Expect(err).ShouldNot(HaveOccurred())

Expand Down
4 changes: 2 additions & 2 deletions test/unittest/ipam_bench/ipam_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ func delPodAddressCapacity(b *testing.B, im *ipam.IPAM, isTimeTrace bool) {
fmt.Printf("delete %d to %d pods spent time %ds \n", n+1-step, n, currentTime-startTime)
startTime = currentTime
}
im.ReleaseAddressByPod(podName)
im.ReleaseAddressByPod(podName, "")
}
}

Expand Down Expand Up @@ -332,7 +332,7 @@ func benchmarkAllocFreeAddrParallel(b *testing.B, podNumber int, protocol string
}
for n := 0; n < podNumber; n++ {
podName := fmt.Sprintf("pod%d_%d", key, n)
im.ReleaseAddressByPod(podName)
im.ReleaseAddressByPod(podName, "")
}
}
})
Expand Down
Loading