From 0a1a13ec62377ae7a8f56a900b1032fc3e9e20d5 Mon Sep 17 00:00:00 2001 From: ColdsteelRail <574252631@qq.com> Date: Thu, 6 Feb 2025 16:14:47 +0800 Subject: [PATCH 1/5] selective scale in both new and origin pods --- pkg/controllers/collaset/synccontrol/scale.go | 2 +- test/e2e/apps/collaset.go | 56 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) diff --git a/pkg/controllers/collaset/synccontrol/scale.go b/pkg/controllers/collaset/synccontrol/scale.go index 78e2e14b..62d46168 100644 --- a/pkg/controllers/collaset/synccontrol/scale.go +++ b/pkg/controllers/collaset/synccontrol/scale.go @@ -59,7 +59,7 @@ func getPodsToDelete(filteredPods []*collasetutils.PodWrapper, replaceMapping ma if replacePairPod, exist := replaceMapping[pod.Name]; exist && replacePairPod != nil { // don't selective scaleIn newPod (and its originPod) until replace finished - if replacePairPod.ToDelete { + if replacePairPod.ToDelete && !pod.ToDelete { continue } // when scaleIn origin Pod, newPod should be deleted if not service available diff --git a/test/e2e/apps/collaset.go b/test/e2e/apps/collaset.go index ed1f3105..80451741 100644 --- a/test/e2e/apps/collaset.go +++ b/test/e2e/apps/collaset.go @@ -1831,6 +1831,62 @@ var _ = SIGDescribe("CollaSet", func() { }, 30*time.Second, 3*time.Second).Should(BeTrue()) }) + framework.ConformanceIt("scaleIn origin and new pod", func() { + cls := tester.NewCollaSet("collaset-"+randStr, 1, appsv1alpha1.UpdateStrategy{}) + // use bad image to mock new replace pod unavailable + cls.Spec.Template.Spec.Containers[0].Image = "nginx:non-exist" + Expect(tester.CreateCollaSet(cls)).NotTo(HaveOccurred()) + + By("Wait for status replicas satisfied") + Eventually(func() error { return tester.ExpectedStatusReplicas(cls, 1, 0, 0, 1, 1) }, 30*time.Second, 3*time.Second).ShouldNot(HaveOccurred()) + + By("Replace pod by label") + pods, err := tester.ListPodsForCollaSet(cls) + Expect(err).NotTo(HaveOccurred()) + podToReplace := pods[0] + Expect(tester.UpdatePod(podToReplace, func(pod *v1.Pod) { + pod.Labels[appsv1alpha1.PodReplaceIndicationLabelKey] = "true" + })).NotTo(HaveOccurred()) + + By("Wait for replace new pod created") + Eventually(func() error { return tester.ExpectedStatusReplicas(cls, 2, 0, 0, 2, 2) }, 30*time.Second, 3*time.Second).ShouldNot(HaveOccurred()) + + By("Selective scaleIn new pod") + pods, err = tester.ListPodsForCollaSet(cls) + Expect(err).NotTo(HaveOccurred()) + var newPod *v1.Pod + for _, pod := range pods { + if pod.Name != podToReplace.Name { + newPod = pod + } + } + Expect(tester.UpdateCollaSet(cls, func(cls *appsv1alpha1.CollaSet) { + cls.Spec.Replicas = int32Pointer(0) + cls.Spec.ScaleStrategy = appsv1alpha1.ScaleStrategy{ + PodToDelete: []string{newPod.Name, podToReplace.Name}, + } + })).NotTo(HaveOccurred()) + + By("Wait for CollaSet reconciled") + Eventually(func() bool { + if err := tester.GetCollaSet(cls); err != nil { + return false + } + return cls.Generation == cls.Status.ObservedGeneration + }, 10*time.Second, 3*time.Second).Should(Equal(true)) + + By("Wait for pods are deleted") + Eventually(func() error { return tester.ExpectedStatusReplicas(cls, 0, 0, 0, 0, 0) }, 30*time.Second, 3*time.Second).ShouldNot(HaveOccurred()) + + By("Check resourceContext") + var currResourceContexts []*appsv1alpha1.ResourceContext + Eventually(func() bool { + currResourceContexts, err = tester.ListResourceContextsForCollaSet(cls) + Expect(err).Should(BeNil()) + return len(currResourceContexts) == 0 + }, 30*time.Second, 3*time.Second).Should(BeTrue()) + }) + framework.ConformanceIt("scaleIn new pod", func() { cls := tester.NewCollaSet("collaset-"+randStr, 1, appsv1alpha1.UpdateStrategy{}) // use bad image to mock new replace pod unavailable From 72d2b679eb6517c02e21be30adf85b27ecf87f43 Mon Sep 17 00:00:00 2001 From: ColdsteelRail <574252631@qq.com> Date: Thu, 6 Feb 2025 16:16:20 +0800 Subject: [PATCH 2/5] improve e2e --- test/e2e/apps/collaset.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/apps/collaset.go b/test/e2e/apps/collaset.go index 80451741..9d47ac36 100644 --- a/test/e2e/apps/collaset.go +++ b/test/e2e/apps/collaset.go @@ -1887,7 +1887,7 @@ var _ = SIGDescribe("CollaSet", func() { }, 30*time.Second, 3*time.Second).Should(BeTrue()) }) - framework.ConformanceIt("scaleIn new pod", func() { + framework.ConformanceIt("scaleIn new pod only", func() { cls := tester.NewCollaSet("collaset-"+randStr, 1, appsv1alpha1.UpdateStrategy{}) // use bad image to mock new replace pod unavailable cls.Spec.Template.Spec.Containers[0].Image = "nginx:non-exist" From 78e17e57a648569fedafac47afa1056e6f626568 Mon Sep 17 00:00:00 2001 From: ColdsteelRail <574252631@qq.com> Date: Fri, 7 Feb 2025 16:34:24 +0800 Subject: [PATCH 3/5] minor --- test/e2e/apps/collaset.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/e2e/apps/collaset.go b/test/e2e/apps/collaset.go index 9d47ac36..45f4fb36 100644 --- a/test/e2e/apps/collaset.go +++ b/test/e2e/apps/collaset.go @@ -1848,16 +1848,17 @@ var _ = SIGDescribe("CollaSet", func() { pod.Labels[appsv1alpha1.PodReplaceIndicationLabelKey] = "true" })).NotTo(HaveOccurred()) - By("Wait for replace new pod created") + By("Wait for new pod created by OperationJob") Eventually(func() error { return tester.ExpectedStatusReplicas(cls, 2, 0, 0, 2, 2) }, 30*time.Second, 3*time.Second).ShouldNot(HaveOccurred()) - By("Selective scaleIn new pod") + By("scaleIn the specified new pod") pods, err = tester.ListPodsForCollaSet(cls) Expect(err).NotTo(HaveOccurred()) var newPod *v1.Pod for _, pod := range pods { if pod.Name != podToReplace.Name { newPod = pod + break } } Expect(tester.UpdateCollaSet(cls, func(cls *appsv1alpha1.CollaSet) { @@ -1875,7 +1876,7 @@ var _ = SIGDescribe("CollaSet", func() { return cls.Generation == cls.Status.ObservedGeneration }, 10*time.Second, 3*time.Second).Should(Equal(true)) - By("Wait for pods are deleted") + By("Wait for pods deleted") Eventually(func() error { return tester.ExpectedStatusReplicas(cls, 0, 0, 0, 0, 0) }, 30*time.Second, 3*time.Second).ShouldNot(HaveOccurred()) By("Check resourceContext") From 0a6f25f6bf36a437aedef5e6d2ea2dd1bb4c41de Mon Sep 17 00:00:00 2001 From: ColdsteelRail <574252631@qq.com> Date: Fri, 7 Feb 2025 16:36:08 +0800 Subject: [PATCH 4/5] minor --- test/e2e/apps/collaset.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/e2e/apps/collaset.go b/test/e2e/apps/collaset.go index 45f4fb36..d3e81815 100644 --- a/test/e2e/apps/collaset.go +++ b/test/e2e/apps/collaset.go @@ -1905,10 +1905,10 @@ var _ = SIGDescribe("CollaSet", func() { pod.Labels[appsv1alpha1.PodReplaceIndicationLabelKey] = "true" })).NotTo(HaveOccurred()) - By("Wait for replace new pod created") + By("Wait for new pod created by OperationJob") Eventually(func() error { return tester.ExpectedStatusReplicas(cls, 2, 0, 0, 2, 2) }, 30*time.Second, 3*time.Second).ShouldNot(HaveOccurred()) - By("Selective scaleIn new pod") + By("scaleIn the specified new pod") pods, err = tester.ListPodsForCollaSet(cls) Expect(err).NotTo(HaveOccurred()) var newPod *v1.Pod @@ -1940,7 +1940,7 @@ var _ = SIGDescribe("CollaSet", func() { newPod.Labels[appsv1alpha1.PodServiceAvailableLabel] = "true" })).NotTo(HaveOccurred()) - By("Wait for pods are deleted") + By("Wait for pods deleted") Eventually(func() error { return tester.ExpectedStatusReplicas(cls, 0, 0, 0, 0, 0) }, 30*time.Second, 3*time.Second).ShouldNot(HaveOccurred()) By("Check resourceContext") From f14a64dd549013af4f17d3296f3f9d56f19075f7 Mon Sep 17 00:00:00 2001 From: ColdsteelRail <574252631@qq.com> Date: Fri, 7 Feb 2025 16:38:19 +0800 Subject: [PATCH 5/5] minor --- test/e2e/apps/collaset.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/e2e/apps/collaset.go b/test/e2e/apps/collaset.go index d3e81815..3f1f8fed 100644 --- a/test/e2e/apps/collaset.go +++ b/test/e2e/apps/collaset.go @@ -1908,7 +1908,7 @@ var _ = SIGDescribe("CollaSet", func() { By("Wait for new pod created by OperationJob") Eventually(func() error { return tester.ExpectedStatusReplicas(cls, 2, 0, 0, 2, 2) }, 30*time.Second, 3*time.Second).ShouldNot(HaveOccurred()) - By("scaleIn the specified new pod") + By("Selective scaleIn new pod") pods, err = tester.ListPodsForCollaSet(cls) Expect(err).NotTo(HaveOccurred()) var newPod *v1.Pod @@ -1940,7 +1940,7 @@ var _ = SIGDescribe("CollaSet", func() { newPod.Labels[appsv1alpha1.PodServiceAvailableLabel] = "true" })).NotTo(HaveOccurred()) - By("Wait for pods deleted") + By("Wait for pods are deleted") Eventually(func() error { return tester.ExpectedStatusReplicas(cls, 0, 0, 0, 0, 0) }, 30*time.Second, 3*time.Second).ShouldNot(HaveOccurred()) By("Check resourceContext")