bugfix: selective scale in both new and origin pods (#315)

* selective scale in both new and origin pods

* improve e2e

* minor

* minor

* minor

Author: ColdsteelRail

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

test/e2e/apps/collaset.go

@@ -1831,7 +1831,7 @@ var _ = SIGDescribe("CollaSet", func() {
 			}, 30*time.Second, 3*time.Second).Should(BeTrue())
 		})
 
-		framework.ConformanceIt("scaleIn new pod", func() {
+		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"
@@ -1848,7 +1848,64 @@ 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("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) {
+				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 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 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"
+			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 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")