From 572bab803c2bed830f71672748db81e463357f0c Mon Sep 17 00:00:00 2001 From: Fang Date: Mon, 27 May 2024 20:39:05 +0800 Subject: [PATCH 1/3] bugfix: recycle unneeded reserved IPs for pre-assigned pods --- pkg/controllers/networking/pod_controller.go | 38 ++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/pkg/controllers/networking/pod_controller.go b/pkg/controllers/networking/pod_controller.go index c84582b0..ba4dbd1e 100644 --- a/pkg/controllers/networking/pod_controller.go +++ b/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, From d4fa109b8f8deb66ef9549ca478a34774d7dd618 Mon Sep 17 00:00:00 2001 From: Fang Date: Mon, 27 May 2024 20:42:22 +0800 Subject: [PATCH 2/3] enhance: avoid misjudgement on DualStack ip family when parsing network config from existing IPInstances --- pkg/webhook/utils/utils.go | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/pkg/webhook/utils/utils.go b/pkg/webhook/utils/utils.go index 1e309bda..0a1f579d 100644 --- a/pkg/webhook/utils/utils.go +++ b/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 From 04909a9cbb1aa8dbfd34901ffb59ed13e89b1559 Mon Sep 17 00:00:00 2001 From: Fang Date: Wed, 10 Jul 2024 17:07:16 +0800 Subject: [PATCH 3/3] integration test: add test cases for possible changes on assigned IPs of stateful workload --- .../networking/pod_controller_test.go | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) diff --git a/pkg/controllers/networking/pod_controller_test.go b/pkg/controllers/networking/pod_controller_test.go index 05f08e59..02f4e5a9 100644 --- a/pkg/controllers/networking/pod_controller_test.go +++ b/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