Merge pull request #418 from sjtufl/fix/pre-assign-recycle-ips

Bugfix:  recycle unneeded reserved IPs for pre-assigned pods

Author: mars1024

pkg/controllers/networking/pod_controller.go

@@ -27,6 +27,7 @@ import (
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	apitypes "k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/record"
 	"k8s.io/client-go/util/retry"
 	kubevirtv1 "kubevirt.io/api/core/v1"
@@ -566,9 +567,46 @@ func (r *PodReconciler) vmAllocate(ctx context.Context, pod *corev1.Pod, vmName,
 		types.AdditionalLabels(vmLabels), types.OwnerReference(*vmiOwnerReference)))
 }
 
+// recycleNonCandidateReservedIPs recycles IPs that should NO LONGER serve given pod, i.e., release
+// reserved IPs that does not appear in candidates.
+func (r *PodReconciler) recycleNonCandidateReservedIPs(ctx context.Context, pod *corev1.Pod, ipCandidates []ipCandidate) (err error) {
+	var (
+		allocatedIPs []*networkingv1.IPInstance
+		toReleaseIPs []*networkingv1.IPInstance
+		toStayIPs    = sets.NewString()
+	)
+	for _, candidate := range ipCandidates {
+		toStayIPs.Insert(candidate.ip)
+	}
+	if allocatedIPs, err = utils.ListAllocatedIPInstancesOfPod(ctx, r, pod); err != nil {
+		return fmt.Errorf("failed to list allocated ip instances for pod %v/%v: %v", pod.Namespace, pod.Name, err)
+	}
+	for i := range allocatedIPs {
+		if toStayIPs.Has(string(globalutils.StringToIPNet(allocatedIPs[i].Spec.Address.IP).IP)) {
+			continue
+		}
+		toReleaseIPs = append(toReleaseIPs, allocatedIPs[i])
+	}
+	if len(toReleaseIPs) > 0 {
+		err = r.release(ctx, pod, transform.TransferIPInstancesForIPAM(toReleaseIPs))
+		if err != nil {
+			return wrapError("failed to release ips that no longer serve current pod", err)
+		}
+	}
+	return nil
+}
+
 // assign means some allocated or pre-assigned IPs will be assigned to a specified pod
 func (r *PodReconciler) assign(ctx context.Context, pod *corev1.Pod, networkName string, ipCandidates []ipCandidate, force bool,
 	ipFamily types.IPFamilyMode, reCoupleOptions ...types.ReCoupleOption) (err error) {
+	if force {
+		// recycle non-candidate reserved IPs, as pre-assigned IPs for current pod
+		// could be changed.
+		if err = r.recycleNonCandidateReservedIPs(ctx, pod, ipCandidates); err != nil {
+			return
+		}
+	}
+
 	// try to assign candidate IPs to pod
 	var AssignedIPs []*types.IP
 	if AssignedIPs, err = r.IPAMManager.Assign(networkName,

pkg/controllers/networking/pod_controller_test.go

@@ -2108,6 +2108,157 @@ var _ = Describe("Pod controller integration test suite", func() {
 		})
 	})
 
+	Context("Specify IP pool for stateful pod and check reserved ip instances", func() {
+		var podName string
+		var ownerReference = statefulOwnerReferenceRender()
+		var idx = 0
+		var ipPool = []string{
+			"100.10.0.151",
+			"100.10.0.161",
+		}
+
+		BeforeEach(func() {
+			podName = fmt.Sprintf("pod-sts-%d", idx)
+		})
+
+		It("Change assigned IP for stateful pod", func() {
+			By("create a stateful pod with special annotations")
+			pod := simplePodRender(podName, node1Name)
+			pod.OwnerReferences = []metav1.OwnerReference{ownerReference}
+			pod.Annotations = map[string]string{
+				constants.AnnotationSpecifiedNetwork: overlayNetworkName,
+				constants.AnnotationIPPool:           strings.Join(ipPool, ","),
+			}
+			Expect(k8sClient.Create(context.Background(), pod)).Should(Succeed())
+
+			By("check the allocated ip instance")
+			Eventually(
+				func(g Gomega) {
+					ipInstances, err := utils.ListAllocatedIPInstancesOfPod(context.Background(), k8sClient, pod)
+					g.Expect(err).NotTo(HaveOccurred())
+					g.Expect(ipInstances).To(HaveLen(1))
+
+					ipInstance := ipInstances[0]
+					g.Expect(ipInstance.Spec.Binding.PodUID).To(Equal(pod.UID))
+					g.Expect(ipInstance.Spec.Binding.PodName).To(Equal(pod.Name))
+					g.Expect(ipInstance.Spec.Binding.NodeName).To(Equal(node1Name))
+					g.Expect(ipInstance.Spec.Binding.ReferredObject).To(Equal(networkingv1.ObjectMeta{
+						Kind: ownerReference.Kind,
+						Name: ownerReference.Name,
+						UID:  ownerReference.UID,
+					}))
+
+					g.Expect(ipInstance.Spec.Binding.Stateful).NotTo(BeNil())
+					g.Expect(ipInstance.Spec.Binding.Stateful.Index).NotTo(BeNil())
+
+					idx := *ipInstance.Spec.Binding.Stateful.Index
+					g.Expect(pod.Name).To(Equal(fmt.Sprintf("pod-sts-%d", idx)))
+
+					g.Expect(ipInstance.Spec.Network).To(Equal(overlayNetworkName))
+					g.Expect(ipInstance.Spec.Subnet).To(Equal(overlayIPv4SubnetName))
+
+					g.Expect(ipInstance.Spec.Address.Version).To(Equal(networkingv1.IPv4))
+					g.Expect(ipInstance.Spec.Address.IP).To(Equal(ipPool[idx] + "/24"))
+				}).
+				WithTimeout(30 * time.Second).
+				WithPolling(time.Second).
+				Should(Succeed())
+
+			By("remove stateful pod")
+			Expect(k8sClient.Delete(context.Background(), pod, client.GracePeriodSeconds(0))).NotTo(HaveOccurred())
+
+			By("make sure the pod is cleaned")
+			Eventually(
+				func(g Gomega) {
+					err := k8sClient.Get(context.Background(),
+						types.NamespacedName{
+							Namespace: "default",
+							Name:      podName,
+						},
+						&corev1.Pod{})
+					g.Expect(err).NotTo(BeNil())
+					g.Expect(errors.IsNotFound(err)).To(BeTrue())
+				}).
+				WithTimeout(30 * time.Second).
+				WithPolling(time.Second).
+				Should(Succeed())
+
+			By("change specified IP and recreate the pod")
+			ipPool[idx] = "100.10.0.152"
+			pod = simplePodRender(podName, node1Name)
+			pod.OwnerReferences = []metav1.OwnerReference{ownerReference}
+			pod.Annotations = map[string]string{
+				constants.AnnotationSpecifiedNetwork: overlayNetworkName,
+				constants.AnnotationIPPool:           strings.Join(ipPool, ","),
+			}
+			Expect(k8sClient.Create(context.Background(), pod)).Should(Succeed())
+
+			By("check the allocated ip instance again")
+			Eventually(
+				func(g Gomega) {
+					ipInstances, err := utils.ListAllocatedIPInstancesOfPod(context.Background(), k8sClient, pod)
+					g.Expect(err).NotTo(HaveOccurred())
+					g.Expect(ipInstances).To(HaveLen(1))
+
+					ipInstance := ipInstances[0]
+					g.Expect(ipInstance.Spec.Binding.PodUID).To(Equal(pod.UID))
+					g.Expect(ipInstance.Spec.Binding.PodName).To(Equal(pod.Name))
+					g.Expect(ipInstance.Spec.Binding.NodeName).To(Equal(node1Name))
+					g.Expect(ipInstance.Spec.Binding.ReferredObject).To(Equal(networkingv1.ObjectMeta{
+						Kind: ownerReference.Kind,
+						Name: ownerReference.Name,
+						UID:  ownerReference.UID,
+					}))
+
+					g.Expect(ipInstance.Spec.Binding.Stateful).NotTo(BeNil())
+					g.Expect(ipInstance.Spec.Binding.Stateful.Index).NotTo(BeNil())
+
+					idx := *ipInstance.Spec.Binding.Stateful.Index
+					g.Expect(pod.Name).To(Equal(fmt.Sprintf("pod-sts-%d", idx)))
+
+					g.Expect(ipInstance.Spec.Network).To(Equal(overlayNetworkName))
+					g.Expect(ipInstance.Spec.Subnet).To(Equal(overlayIPv4SubnetName))
+
+					g.Expect(ipInstance.Spec.Address.Version).To(Equal(networkingv1.IPv4))
+					g.Expect(ipInstance.Spec.Address.IP).To(Equal(ipPool[idx] + "/24"))
+				}).
+				WithTimeout(30 * time.Second).
+				WithPolling(time.Second).
+				Should(Succeed())
+
+			By("clean up stateful pod")
+			Expect(k8sClient.Delete(context.Background(), pod, client.GracePeriodSeconds(0))).NotTo(HaveOccurred())
+		})
+
+		AfterEach(func() {
+			By("make sure test ip instances cleaned up")
+			Expect(k8sClient.DeleteAllOf(
+				context.Background(),
+				&networkingv1.IPInstance{},
+				client.MatchingLabels{
+					constants.LabelPod: transform.TransferPodNameForLabelValue(podName),
+				},
+				client.InNamespace("default"),
+			)).NotTo(HaveOccurred())
+
+			By("make sure test pod cleaned up")
+			Eventually(
+				func(g Gomega) {
+					err := k8sClient.Get(context.Background(),
+						types.NamespacedName{
+							Namespace: "default",
+							Name:      podName,
+						},
+						&corev1.Pod{})
+					g.Expect(err).NotTo(BeNil())
+					g.Expect(errors.IsNotFound(err)).To(BeTrue())
+				}).
+				WithTimeout(30 * time.Second).
+				WithPolling(time.Second).
+				Should(Succeed())
+		})
+	})
+
 	Context("Specify MAC address pool for pod", func() {
 		var podName string
 		var ownerReference metav1.OwnerReference

pkg/webhook/utils/utils.go

@@ -263,7 +263,22 @@ func parseNetworkConfigByExistIPInstances(ctx context.Context, c client.Reader,
 			ipFamily = ipamtypes.IPv4
 		}
 	case 2:
-		ipFamily = ipamtypes.DualStack
+		var (
+			v4Count = 0
+			v6Count = 0
+		)
+		for i := range validIPList {
+			if networkingv1.IsIPv6IPInstance(&validIPList[i]) {
+				v6Count++
+			} else {
+				v4Count++
+			}
+		}
+		if v4Count == 1 && v6Count == 1 {
+			ipFamily = ipamtypes.DualStack
+		} else {
+			err = fmt.Errorf("more than two ip instances are of the same family type, ipv4 count %d, ipv6 count %d", v4Count, v6Count)
+		}
 	default:
 		err = fmt.Errorf("more than two reserve ip exist for list options %v", opts)
 		return