From 090558e7ebaeadeec5d1eaa63ea7914a827484e7 Mon Sep 17 00:00:00 2001 From: Huanyu He <552483776@qq.com> Date: Thu, 21 Sep 2023 20:57:15 +0800 Subject: [PATCH] Merge pull request #403 from mars1024/dev/reserve-after-terminated only reserver ip instance after pod terminated --- pkg/controllers/networking/pod_controller.go | 10 ++ .../networking/pod_controller_test.go | 170 ++++++++++++++++++ pkg/controllers/utils/pod.go | 10 ++ 3 files changed, 190 insertions(+) diff --git a/pkg/controllers/networking/pod_controller.go b/pkg/controllers/networking/pod_controller.go index cb716d0a..6969babc 100644 --- a/pkg/controllers/networking/pod_controller.go +++ b/pkg/controllers/networking/pod_controller.go @@ -144,6 +144,11 @@ func (r *PodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result } if strategy.OwnByStatefulWorkload(ownedObj) { + // Before pod terminated, should not reserve ip instance because of pre-stop + if !utils.PodIsTerminated(pod) { + return ctrl.Result{}, nil + } + if err = r.reserve(ctx, pod); err != nil { return ctrl.Result{}, wrapError("unable to reserve pod", err) } @@ -166,6 +171,11 @@ func (r *PodReconciler) Reconcile(ctx context.Context, req ctrl.Request) (result return ctrl.Result{}, wrapError("unable to remove finalizer", r.removeFinalizer(ctx, pod)) } + // Before pod terminated, should not reserve ip instance because of pre-stop + if !utils.PodIsTerminated(pod) { + return ctrl.Result{}, nil + } + log.V(1).Info("reserve ip for VM pod") if err = r.reserve(ctx, pod, types.DropPodName(true)); err != nil { return ctrl.Result{}, wrapError("unable to reserve pod", err) diff --git a/pkg/controllers/networking/pod_controller_test.go b/pkg/controllers/networking/pod_controller_test.go index 10f451c4..3bb180d3 100644 --- a/pkg/controllers/networking/pod_controller_test.go +++ b/pkg/controllers/networking/pod_controller_test.go @@ -701,6 +701,176 @@ var _ = Describe("Pod controller integration test suite", func() { Expect(k8sClient.Delete(context.Background(), pod, client.GracePeriodSeconds(0))).NotTo(HaveOccurred()) }) + It("Check ip reserved only after pod terminated", func() { + By("create a stateful pod requiring IPv4 address") + var ipInstanceName string + pod := simplePodRender(podName, node1Name) + pod.OwnerReferences = []metav1.OwnerReference{ownerReference} + Expect(k8sClient.Create(context.Background(), pod)).Should(Succeed()) + + By("check the first allocated IPv4 address") + 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] + ipInstanceName = ipInstance.Name + g.Expect(ipInstance.Spec.Address.Version).To(Equal(networkingv1.IPv4)) + 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-%d", idx))) + + g.Expect(ipInstance.Spec.Network).To(Equal(underlayNetworkName)) + g.Expect(ipInstance.Spec.Subnet).To(BeElementOf(underlaySubnetName)) + }). + WithTimeout(30 * time.Second). + WithPolling(time.Second). + Should(Succeed()) + + By("update to make sure pod not terminated") + patch := client.MergeFrom(pod.DeepCopy()) + pod.Status.ContainerStatuses = []corev1.ContainerStatus{ + { + Name: "test", + State: corev1.ContainerState{ + Terminated: nil, + }, + }, + } + Expect(k8sClient.Status().Patch(context.Background(), pod, patch)).NotTo(HaveOccurred()) + + By("try to delete stateful pod into terminating state") + Expect(k8sClient.Delete(context.Background(), pod, client.GracePeriodSeconds(0))).NotTo(HaveOccurred()) + + By("check allocated IPv4 address still not reserved after 30s") + Consistently( + 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.NodeName).NotTo(BeEmpty()) + + }). + WithTimeout(30 * time.Second). + WithPolling(5 * time.Second). + Should(Succeed()) + + By("update to make sure pod terminated") + patch = client.MergeFrom(pod.DeepCopy()) + pod.Status.ContainerStatuses = []corev1.ContainerStatus{ + { + Name: "test", + State: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{}, + }, + }, + } + Expect(k8sClient.Status().Patch(context.Background(), pod, patch)).NotTo(HaveOccurred()) + + By("check the allocated IPv4 address is reserved") + 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.Name).To(Equal(ipInstanceName)) + g.Expect(ipInstance.Spec.Address.Version).To(Equal(networkingv1.IPv4)) + g.Expect(ipInstance.Spec.Binding.PodUID).To(BeEmpty()) + g.Expect(ipInstance.Spec.Binding.PodName).To(Equal(pod.Name)) + g.Expect(ipInstance.Spec.Binding.NodeName).To(BeEmpty()) + 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-%d", idx))) + + g.Expect(ipInstance.Spec.Network).To(Equal(underlayNetworkName)) + g.Expect(ipInstance.Spec.Subnet).To(BeElementOf(underlaySubnetName)) + }). + WithTimeout(30 * time.Second). + WithPolling(time.Second). + Should(Succeed()) + + By("make sure pod deleted") + Eventually( + func(g Gomega) { + err := k8sClient.Get(context.Background(), + types.NamespacedName{ + Namespace: pod.Namespace, + 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("recreate the stateful pod") + pod = simplePodRender(podName, node1Name) + pod.OwnerReferences = []metav1.OwnerReference{ownerReference} + Expect(k8sClient.Create(context.Background(), pod)).NotTo(HaveOccurred()) + + By("check the allocated IPv4 address is retained and reused") + 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.Name).To(Equal(ipInstanceName)) + g.Expect(ipInstance.Spec.Address.Version).To(Equal(networkingv1.IPv4)) + 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-%d", idx))) + + g.Expect(ipInstance.Spec.Network).To(Equal(underlayNetworkName)) + g.Expect(ipInstance.Spec.Subnet).To(BeElementOf(underlaySubnetName)) + }). + WithTimeout(30 * time.Second). + WithPolling(time.Second). + Should(Succeed()) + + By("remove the test 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( diff --git a/pkg/controllers/utils/pod.go b/pkg/controllers/utils/pod.go index fbc84774..120b946f 100644 --- a/pkg/controllers/utils/pod.go +++ b/pkg/controllers/utils/pod.go @@ -51,4 +51,14 @@ func PodIsScheduled(pod *v1.Pod) bool { return len(pod.Spec.NodeName) > 0 } +func PodIsTerminated(pod *v1.Pod) bool { + for i := range pod.Status.ContainerStatuses { + if pod.Status.ContainerStatuses[i].State.Terminated == nil { + return false + } + } + + return true +} + var ParseNetworkConfigOfPodByPriority = utils.ParseNetworkConfigOfPodByPriority