fix: a smaller maxUnavaila will block the sidecarSet from updating po…

…ds (#1834) Signed-off-by: liheng.zms <[email protected]>
openkruise · Nov 21, 2024 · 4661b6e · 4661b6e
1 parent 54a769f
commit 4661b6e
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 2 deletions.
diff --git a/pkg/controller/sidecarset/sidecarset_strategy.go b/pkg/controller/sidecarset/sidecarset_strategy.go
@@ -139,7 +139,8 @@ func calculateUpgradeCount(coreControl sidecarcontrol.SidecarControl, waitUpdate
 	// default partition = 0, indicates all pods will been upgraded
 	var partition int
 	if strategy.Partition != nil {
-		partition, _ = intstrutil.GetValueFromIntOrPercent(strategy.Partition, totalReplicas, false)
+		totalInt32 := int32(totalReplicas)
+		partition, _ = util.CalculatePartitionReplicas(strategy.Partition, &totalInt32)
 	}
 	// indicates the partition pods will not be upgraded for the time
 	if len(waitUpdateIndexes)-partition <= 0 {
@@ -150,7 +151,7 @@ func calculateUpgradeCount(coreControl sidecarcontrol.SidecarControl, waitUpdate
 	// max unavailable pods number, default is 1
 	maxUnavailable := 1
 	if strategy.MaxUnavailable != nil {
-		maxUnavailable, _ = intstrutil.GetValueFromIntOrPercent(strategy.MaxUnavailable, totalReplicas, false)
+		maxUnavailable, _ = intstrutil.GetValueFromIntOrPercent(strategy.MaxUnavailable, totalReplicas, true)
 	}
 
 	var upgradeAndNotReadyCount int

diff --git a/pkg/controller/sidecarset/sidecarset_strategy_test.go b/pkg/controller/sidecarset/sidecarset_strategy_test.go
@@ -355,6 +355,65 @@ func testGetNextUpgradePods(t *testing.T, factoryPods FactoryPods, factorySideca
 			exceptNeedUpgradeCount:   0,
 			exceptNotUpgradableCount: 100,
 		},
+		{
+			name: "only maxUnavailable(5%), and pods(count=5, upgraded=0, upgradedAndReady=0)",
+			getPods: func() []*corev1.Pod {
+				pods := factoryPods(5, 0, 0)
+				return Random(pods)
+			},
+			getSidecarset: func() *appsv1alpha1.SidecarSet {
+				sidecarSet := factorySidecar()
+				sidecarSet.Spec.UpdateStrategy.MaxUnavailable = &intstr.IntOrString{
+					Type:   intstr.String,
+					StrVal: "5%",
+				}
+				return sidecarSet
+			},
+			exceptNeedUpgradeCount:   1,
+			exceptNotUpgradableCount: 0,
+		},
+		{
+			name: "maxUnavailable(5) partition(99%), and pods(count=5, upgraded=0, upgradedAndReady=0)",
+			getPods: func() []*corev1.Pod {
+				pods := factoryPods(5, 0, 0)
+				return Random(pods)
+			},
+			getSidecarset: func() *appsv1alpha1.SidecarSet {
+				sidecarSet := factorySidecar()
+				sidecarSet.Spec.UpdateStrategy.MaxUnavailable = &intstr.IntOrString{
+					Type:   intstr.Int,
+					IntVal: 5,
+				}
+				sidecarSet.Spec.UpdateStrategy.Partition = &intstr.IntOrString{
+					Type:   intstr.String,
+					StrVal: "99%",
+				}
+				return sidecarSet
+			},
+			exceptNeedUpgradeCount:   1,
+			exceptNotUpgradableCount: 0,
+		},
+		{
+			name: "maxUnavailable(5) partition(1%), and pods(count=5, upgraded=0, upgradedAndReady=0)",
+			getPods: func() []*corev1.Pod {
+				pods := factoryPods(5, 0, 0)
+				return Random(pods)
+			},
+			getSidecarset: func() *appsv1alpha1.SidecarSet {
+				sidecarSet := factorySidecar()
+				sidecarSet.Spec.UpdateStrategy.MaxUnavailable = &intstr.IntOrString{
+					Type:   intstr.Int,
+					IntVal: 5,
+				}
+				sidecarSet.Spec.UpdateStrategy.Partition = &intstr.IntOrString{
+					Type:   intstr.String,
+					StrVal: "1%",
+				}
+				return sidecarSet
+			},
+			exceptNeedUpgradeCount:   4,
+			exceptNotUpgradableCount: 0,
+		},
 	}
 	strategy := NewStrategy()
 	for _, cs := range cases {