fix(instanceset): stabilize rolling update participants

controllers/workloads/instanceset_controller_2_test.go

@@ -371,7 +371,7 @@ var _ = Describe("InstanceSet Controller 2", func() {
 			}).Should(Succeed())
 		})
 
-		It("uses status role order for a roleful rolling-update window", func() {
+		It("keeps a roleful rolling-update window stable when roles change", func() {
 			createITSObj(itsName, func(f *testapps.MockInstanceSetFactory) {
 				f.SetRoles([]workloads.ReplicaRole{
 					{
@@ -405,6 +405,8 @@ var _ = Describe("InstanceSet Controller 2", func() {
 			})).Should(Succeed())
 
 			By("update its spec")
+			beforeUpdate := time.Now()
+			time.Sleep(time.Second)
 			Expect(testapps.GetAndChangeObj(&testCtx, itsKey, func(its *workloads.InstanceSet) {
 				its.Spec.Template.Spec.DNSPolicy = corev1.DNSClusterFirstWithHostNet
 			})()).ShouldNot(HaveOccurred())
@@ -415,7 +417,24 @@ var _ = Describe("InstanceSet Controller 2", func() {
 				g.Expect(inst.Spec.Template.Spec.DNSPolicy).Should(Equal(corev1.DNSClusterFirstWithHostNet))
 			})).Should(Succeed())
 
-			By("keep leaders outside the rolling-update window")
+			By("make the updated member ready and switch the follower role")
+			Eventually(testapps.CheckObj(&testCtx, followerKey, func(g Gomega, pod *corev1.Pod) {
+				g.Expect(pod.CreationTimestamp.After(beforeUpdate)).Should(BeTrue())
+			})).Should(Succeed())
+			mockPodReadyNAvailableWithRole(itsObj.Namespace, podName(0), "leader", 0)
+			mockPodReadyNAvailableWithRole(itsObj.Namespace, podName(1), "follower", 0)
+			Eventually(func(g Gomega) {
+				leader := &workloads.Instance{}
+				g.Expect(testCtx.Cli.Get(testCtx.Ctx, followerKey, leader)).Should(Succeed())
+				g.Expect(leader.Status.Role).Should(Equal("leader"))
+
+				newFollower := &workloads.Instance{}
+				newFollowerKey := types.NamespacedName{Namespace: itsObj.Namespace, Name: podName(1)}
+				g.Expect(testCtx.Cli.Get(testCtx.Ctx, newFollowerKey, newFollower)).Should(Succeed())
+				g.Expect(newFollower.Status.Role).Should(Equal("follower"))
+			}).Should(Succeed())
+
+			By("keep the original participant and leave the new follower outside the window")
 			Consistently(func(g Gomega) {
 				for i := int32(1); i < replicas; i++ {
 					inst := &workloads.Instance{}

pkg/controller/instanceset/instance_util.go

@@ -43,6 +43,7 @@ import (
 	"github.com/apecloud/kubeblocks/pkg/controller/builder"
 	"github.com/apecloud/kubeblocks/pkg/controller/instancetemplate"
 	"github.com/apecloud/kubeblocks/pkg/controller/model"
+	"github.com/apecloud/kubeblocks/pkg/controller/rollingupdate"
 	intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil"
 )
 
@@ -555,9 +556,15 @@ func buildInstanceTemplateRevision(template *corev1.PodTemplateSpec, parent *wor
 		mutateTemplateFn(templateCopy)
 	}
 	podTemplate := filterInPlaceFields(templateCopy)
+	annotations := make(map[string]string, len(parent.Annotations))
+	for key, value := range parent.Annotations {
+		if key != rollingupdate.WindowAnnotationKey {
+			annotations[key] = value
+		}
+	}
 	its := builder.NewInstanceSetBuilder(parent.Namespace, parent.Name).
 		SetUID(parent.UID).
-		AddAnnotationsInMap(parent.Annotations).
+		AddAnnotationsInMap(annotations).
 		SetSelectorMatchLabel(parent.Labels).
 		SetTemplate(*podTemplate).
 		GetObject()

pkg/controller/instanceset/instance_util_test.go

@@ -38,6 +38,7 @@ import (
 	"github.com/apecloud/kubeblocks/pkg/constant"
 	"github.com/apecloud/kubeblocks/pkg/controller/builder"
 	"github.com/apecloud/kubeblocks/pkg/controller/instancetemplate"
+	"github.com/apecloud/kubeblocks/pkg/controller/rollingupdate"
 )
 
 var _ = Describe("instance util test", func() {
@@ -84,6 +85,21 @@ var _ = Describe("instance util test", func() {
 		})
 	})
 
+	Context("buildInstanceTemplateRevision", func() {
+		It("ignores the internal rolling-update window annotation", func() {
+			before, err := buildInstanceTemplateRevision(&its.Spec.Template, its, nil)
+			Expect(err).ShouldNot(HaveOccurred())
+
+			if its.Annotations == nil {
+				its.Annotations = make(map[string]string)
+			}
+			its.Annotations[rollingupdate.WindowAnnotationKey] = `{"rolloutID":"2","replicas":1,"participants":["pod-0"]}`
+			after, err := buildInstanceTemplateRevision(&its.Spec.Template, its, nil)
+			Expect(err).ShouldNot(HaveOccurred())
+			Expect(after).Should(Equal(before))
+		})
+	})
+
 	Context("configsToUpdate", func() {
 		It("treats nil and empty config hash as equal", func() {
 			its := builder.NewInstanceSetBuilder(namespace, name).

pkg/controller/instanceset/reconciler_update.go

@@ -37,6 +37,7 @@ import (
 	"github.com/apecloud/kubeblocks/pkg/controller/kubebuilderx"
 	"github.com/apecloud/kubeblocks/pkg/controller/lifecycle"
 	"github.com/apecloud/kubeblocks/pkg/controller/model"
+	"github.com/apecloud/kubeblocks/pkg/controller/rollingupdate"
 	intctrlutil "github.com/apecloud/kubeblocks/pkg/controllerutil"
 )
 
@@ -62,6 +63,14 @@ func (r *updateReconciler) PreCondition(tree *kubebuilderx.ObjectTree) *kubebuil
 
 func (r *updateReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilderx.Result, error) {
 	its, _ := tree.GetRoot().(*workloads.InstanceSet)
+	// OnDelete ends the rolling-update lifecycle even while the instance set is
+	// temporarily unaligned (for example, during scaling).
+	if its.Spec.InstanceUpdateStrategy != nil && its.Spec.InstanceUpdateStrategy.Type == kbappsv1.OnDeleteStrategyType {
+		if rollingupdate.Reset(its) {
+			return kubebuilderx.Commit, nil
+		}
+		return kubebuilderx.Continue, nil
+	}
 	itsExt, err := instancetemplate.BuildInstanceSetExt(its, tree)
 	if err != nil {
 		return kubebuilderx.Continue, err
@@ -97,13 +106,7 @@ func (r *updateReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilder
 		return kubebuilderx.Continue, nil
 	}
 
-	// 3. do update
-	// do nothing if update strategy type is 'OnDelete'
-	if its.Spec.InstanceUpdateStrategy != nil && its.Spec.InstanceUpdateStrategy.Type == kbappsv1.OnDeleteStrategyType {
-		return kubebuilderx.Continue, nil
-	}
-
-	// handle 'RollingUpdate'
+	// 3. handle 'RollingUpdate'
 	rollingUpdateQuota, unavailableQuota, err := r.rollingUpdateQuota(its, oldPodList)
 	if err != nil {
 		return kubebuilderx.Continue, err
@@ -117,12 +120,24 @@ func (r *updateReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilder
 
 	priorities := ComposeRolePriorityMap(its.Spec.Roles)
 	sortObjects(oldPodList, priorities, false)
+	orderedNames := make([]string, len(oldPodList))
+	for i, pod := range oldPodList {
+		orderedNames[i] = pod.Name
+	}
+	updateRevisions, err := GetRevisions(its.Status.UpdateRevisions)
+	if err != nil {
+		return kubebuilderx.Continue, err
+	}
+	participants, windowChanged := rollingupdate.Participants(its, updateRevisions, rollingUpdateQuota, orderedNames)
+	if windowChanged {
+		return kubebuilderx.Commit, nil
+	}
 
 	// treat old and Pending pod as a special case, as they can be updated without a consequence
 	// PodUpdatePolicy is ignored here since in-place update for a pending pod doesn't make much sense.
-	for i, pod := range oldPodList {
-		if i >= rollingUpdateQuota {
-			break
+	for _, pod := range oldPodList {
+		if !participants.Has(pod.Name) {
+			continue
 		}
 		updatePolicy, _, _, err := getPodUpdatePolicy(its, pod)
 		if err != nil {
@@ -135,15 +150,12 @@ func (r *updateReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilder
 		}
 	}
 
-	// updatedPods tracks the positions already covered by the rolling-update
-	// window, while updatingPods tracks actual updates admitted in this round.
-	updatedPods := 0
 	updatingPods := 0
 	isBlocked := false
 	needRetry := false
 	for _, pod := range oldPodList {
-		if updatedPods >= rollingUpdateQuota {
-			break
+		if !participants.Has(pod.Name) {
+			continue
 		}
 		if updatingPods >= unavailableQuota {
 			break
@@ -224,7 +236,6 @@ func (r *updateReconciler) Reconcile(tree *kubebuilderx.ObjectTree) (kubebuilder
 			}
 			updatingPods++
 		}
-		updatedPods++
 	}
 
 	if !isBlocked {