define partition as number of non-updated pods should be reversed (op…

…enkruise#1819)

Signed-off-by: Abner-1 <[email protected]>

apis/apps/v1beta1/statefulset_types.go

@@ -87,9 +87,7 @@ type VolumeClaimUpdateStrategy struct {
 
 // RollingUpdateStatefulSetStrategy is used to communicate parameter for RollingUpdateStatefulSetStrategyType.
 type RollingUpdateStatefulSetStrategy struct {
-	// Partition indicates the ordinal at which the StatefulSet should be partitioned by default.
-	// But if unorderedUpdate has been set:
-	//   - Partition indicates the number of pods with non-updated revisions when rolling update.
+	// Partition indicates the number of pods the StatefulSet should be partitioned by default.
 	//   - It means controller will update $(replicas - partition) number of pod.
 	// Default value is 0.
 	// +optional

config/crd/bases/apps.kruise.io_statefulsets.yaml

@@ -784,9 +784,7 @@ spec:
                         type: integer
                       partition:
                         description: |-
-                          Partition indicates the ordinal at which the StatefulSet should be partitioned by default.
-                          But if unorderedUpdate has been set:
-                            - Partition indicates the number of pods with non-updated revisions when rolling update.
+                          Partition indicates the number of pods the StatefulSet should be partitioned by default.
                             - It means controller will update $(replicas - partition) number of pod.
                           Default value is 0.
                         format: int32

config/crd/bases/apps.kruise.io_uniteddeployments.yaml

@@ -407,9 +407,7 @@ spec:
                                     type: integer
                                   partition:
                                     description: |-
-                                      Partition indicates the ordinal at which the StatefulSet should be partitioned by default.
-                                      But if unorderedUpdate has been set:
-                                        - Partition indicates the number of pods with non-updated revisions when rolling update.
+                                      Partition indicates the number of pods the StatefulSet should be partitioned by default.
                                         - It means controller will update $(replicas - partition) number of pod.
                                       Default value is 0.
                                     format: int32

pkg/controller/statefulset/stateful_set_utils.go

@@ -514,7 +514,16 @@ func isCurrentRevisionNeeded(set *appsv1beta1.StatefulSet, updateRevision string
 		return ordinal < getStartOrdinal(set)+int(set.Status.CurrentReplicas)
 	}
 	if set.Spec.UpdateStrategy.RollingUpdate.UnorderedUpdate == nil {
-		return ordinal < getStartOrdinal(set)+int(*set.Spec.UpdateStrategy.RollingUpdate.Partition)
+		unreservedPodsNum := 0
+		// assume all pods [0, idx) are created and only reserved pods are nil
+		idx := ordinal - getStartOrdinal(set)
+		for i := 0; i < idx; i++ {
+			if replicas[i] != nil {
+				unreservedPodsNum++
+			}
+		}
+		// if all pods [0, idx] are current revision
+		return unreservedPodsNum+1 <= int(*set.Spec.UpdateStrategy.RollingUpdate.Partition)
 	}
 
 	var noUpdatedReplicas int

pkg/controller/statefulset/stateful_set_utils_test.go

@@ -1250,7 +1250,8 @@ func TestIsCurrentRevisionNeeded(t *testing.T) {
 			updateRevision: updatedRevisionHash,
 			ordinal:        0,
 			replicas: func() []*corev1.Pod {
-				pods := newReplicas(2, 2, currentRevisionHash)
+				pods := []*corev1.Pod{nil, nil}
+				pods = append(pods, newReplicas(2, 2, currentRevisionHash)...)
 				return pods
 			}(),
 			expectedRes: true,
@@ -1701,12 +1702,12 @@ func TestIsCurrentRevisionNeeded(t *testing.T) {
 		},
 
 		// with startOrdinal and reservedIds
-		// TODO Abner-1: These cases will be discussed and may be changed
+		// fixes https://github.com/openkruise/kruise/issues/1813
 		{
 			// reservedId 1, replicas 3, partition 2
 			// 3: updated revision
 			// 0: current revision
-			// => 2: should be updated revision
+			// => 2: should be current revision
 			name: "Ordinals start 0, reservedId 1, partition 1, create pod2",
 			statefulSet: &appsv1beta1.StatefulSet{
 				Spec: appsv1beta1.StatefulSetSpec{
@@ -1731,16 +1732,17 @@ func TestIsCurrentRevisionNeeded(t *testing.T) {
 			ordinal:        2,
 			replicas: func() []*corev1.Pod {
 				pods := newReplicas(0, 1, currentRevisionHash)
+				pods = append(pods, nil, nil)
 				pods = append(pods, newReplicas(3, 1, updatedRevisionHash)...)
 				return pods
 			}(),
-			expectedRes: false,
+			expectedRes: true,
 		},
 		{
 			// start ordinal 2, reservedId 3, replicas 3, partition 2
 			// 5: updated revision
 			// 2: current revision
-			// => 4: should be updated revision
+			// => 4: should be current revision
 			name: "Ordinals start 2, reservedId 1, partition 1, create pod2",
 			statefulSet: &appsv1beta1.StatefulSet{
 				Spec: appsv1beta1.StatefulSetSpec{
@@ -1765,10 +1767,11 @@ func TestIsCurrentRevisionNeeded(t *testing.T) {
 			ordinal:        4,
 			replicas: func() []*corev1.Pod {
 				pods := newReplicas(2, 1, currentRevisionHash)
+				pods = append(pods, nil, nil)
 				pods = append(pods, newReplicas(5, 1, updatedRevisionHash)...)
 				return pods
 			}(),
-			expectedRes: false,
+			expectedRes: true,
 		},
 	}
 

pkg/controller/statefulset/stateful_update_utils.go

@@ -30,9 +30,14 @@ func sortPodsToUpdate(rollingUpdateStrategy *appsv1beta1.RollingUpdateStatefulSe
 		updateMin = int(*rollingUpdateStrategy.Partition)
 	}
 
+	maxUpdate := int(totalReplicas) - updateMin
+	if maxUpdate <= 0 {
+		return []int{}
+	}
+
 	if rollingUpdateStrategy == nil || rollingUpdateStrategy.UnorderedUpdate == nil {
 		var indexes []int
-		for target := len(replicas) - 1; target >= updateMin; target-- {
+		for target := len(replicas) - 1; target >= updateMin && len(indexes) < maxUpdate; target-- {
 			if replicas[target] == nil {
 				continue
 			}
@@ -42,10 +47,6 @@ func sortPodsToUpdate(rollingUpdateStrategy *appsv1beta1.RollingUpdateStatefulSe
 	}
 
 	priorityStrategy := rollingUpdateStrategy.UnorderedUpdate.PriorityStrategy
-	maxUpdate := int(totalReplicas) - updateMin
-	if maxUpdate <= 0 {
-		return []int{}
-	}
 
 	var updatedIdxs []int
 	var waitUpdateIdxs []int

pkg/controller/statefulset/stateful_update_utils_test.go

@@ -87,6 +87,7 @@ func TestSortPodsToUpdate(t *testing.T) {
 			expected: []int{2, 1, 0},
 		},
 		{
+			// change the case because we define partition as number of pods with non-updated revision
 			strategy:       &appsv1beta1.RollingUpdateStatefulSetStrategy{Partition: func() *int32 { var i int32 = 2; return &i }()},
 			updateRevision: "r1",
 			totalReplicas:  3,
@@ -97,7 +98,7 @@ func TestSortPodsToUpdate(t *testing.T) {
 				nil,
 				{ObjectMeta: metav1.ObjectMeta{Labels: map[string]string{apps.ControllerRevisionHashLabelKey: "r0"}}},
 			},
-			expected: []int{4, 2},
+			expected: []int{4},
 		},
 		{
 			strategy: &appsv1beta1.RollingUpdateStatefulSetStrategy{