Skip to content

Commit ccb66a6

Browse files
Syspretor玖宇
Syspretor
and
玖宇
authored
chore(rbg): migrate updateStrategy field type (#114)
Co-authored-by: 玖宇 <guotongyu.gty@alibaba-inc.com>
1 parent bc8aa6a commit ccb66a6

20 files changed

Lines changed: 209 additions & 149 deletions

‎api/workloads/v1alpha1/rolebasedgroup_types.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ type RollingUpdate struct {
163163
//
164164
// +optional
165165
// +kubebuilder:default=0
166-
Partition *int32 `json:"partition,omitempty"`
166+
Partition *intstr.IntOrString `json:"partition,omitempty"`
167167

168168
// The maximum number of replicas that can be unavailable during the update.
169169
// Value can be an absolute number (ex: 5) or a percentage of total replicas at the start of update (ex: 10%).
@@ -178,7 +178,7 @@ type RollingUpdate struct {
178178
//
179179
// +kubebuilder:validation:XIntOrString
180180
// +kubebuilder:default=1
181-
MaxUnavailable intstr.IntOrString `json:"maxUnavailable,omitempty"`
181+
MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"`
182182

183183
// The maximum number of replicas that can be scheduled above the original number of
184184
// replicas.
@@ -194,7 +194,7 @@ type RollingUpdate struct {
194194
//
195195
// +kubebuilder:validation:XIntOrString
196196
// +kubebuilder:default=0
197-
MaxSurge intstr.IntOrString `json:"maxSurge,omitempty"`
197+
MaxSurge *intstr.IntOrString `json:"maxSurge,omitempty"`
198198

199199
// Paused indicates that the InstanceSet is paused.
200200
// Default value is false

‎api/workloads/v1alpha1/zz_generated.deepcopy.go‎

Lines changed: 11 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎client-go/applyconfiguration/workloads/v1alpha1/rollingupdate.go‎

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎config/crd/bases/workloads.x-k8s.io_rolebasedgroups.yaml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,12 +1644,14 @@ spec:
16441644
Value can be an absolute number (ex: 5) or a percentage of total replicas at the start of update (ex: 10%).
16451645
x-kubernetes-int-or-string: true
16461646
partition:
1647+
anyOf:
1648+
- type: integer
1649+
- type: string
16471650
default: 0
16481651
description: |-
16491652
Partition indicates the ordinal at which the role should be partitioned for updates.
16501653
During a rolling update, all the groups from ordinal Partition to Replicas-1 will be updated.
1651-
format: int32
1652-
type: integer
1654+
x-kubernetes-int-or-string: true
16531655
paused:
16541656
description: |-
16551657
Paused indicates that the InstanceSet is paused.

‎config/crd/bases/workloads.x-k8s.io_rolebasedgroupsets.yaml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,12 +1682,14 @@ spec:
16821682
Value can be an absolute number (ex: 5) or a percentage of total replicas at the start of update (ex: 10%).
16831683
x-kubernetes-int-or-string: true
16841684
partition:
1685+
anyOf:
1686+
- type: integer
1687+
- type: string
16851688
default: 0
16861689
description: |-
16871690
Partition indicates the ordinal at which the role should be partitioned for updates.
16881691
During a rolling update, all the groups from ordinal Partition to Replicas-1 will be updated.
1689-
format: int32
1690-
type: integer
1692+
x-kubernetes-int-or-string: true
16911693
paused:
16921694
description: |-
16931695
Paused indicates that the InstanceSet is paused.

‎deploy/helm/rbgs/crds/workloads.x-k8s.io_rolebasedgroups.yaml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1644,12 +1644,14 @@ spec:
16441644
Value can be an absolute number (ex: 5) or a percentage of total replicas at the start of update (ex: 10%).
16451645
x-kubernetes-int-or-string: true
16461646
partition:
1647+
anyOf:
1648+
- type: integer
1649+
- type: string
16471650
default: 0
16481651
description: |-
16491652
Partition indicates the ordinal at which the role should be partitioned for updates.
16501653
During a rolling update, all the groups from ordinal Partition to Replicas-1 will be updated.
1651-
format: int32
1652-
type: integer
1654+
x-kubernetes-int-or-string: true
16531655
paused:
16541656
description: |-
16551657
Paused indicates that the InstanceSet is paused.

‎deploy/helm/rbgs/crds/workloads.x-k8s.io_rolebasedgroupsets.yaml‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1682,12 +1682,14 @@ spec:
16821682
Value can be an absolute number (ex: 5) or a percentage of total replicas at the start of update (ex: 10%).
16831683
x-kubernetes-int-or-string: true
16841684
partition:
1685+
anyOf:
1686+
- type: integer
1687+
- type: string
16851688
default: 0
16861689
description: |-
16871690
Partition indicates the ordinal at which the role should be partitioned for updates.
16881691
During a rolling update, all the groups from ordinal Partition to Replicas-1 will be updated.
1689-
format: int32
1690-
type: integer
1692+
x-kubernetes-int-or-string: true
16911693
paused:
16921694
description: |-
16931695
Paused indicates that the InstanceSet is paused.

‎internal/controller/workloads/rolebasedgroup_controller.go‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -619,15 +619,17 @@ func (r *RoleBasedGroupReconciler) calculateRollingUpdateForCoordination(
619619
strategy = *(role.RolloutStrategy.RollingUpdate.DeepCopy())
620620
}
621621
if coordination.Strategy.RollingUpdate.MaxUnavailable != nil {
622-
strategy.MaxUnavailable = intstr.FromString(*coordination.Strategy.RollingUpdate.MaxUnavailable)
622+
strategy.MaxUnavailable = ptr.To(intstr.FromString(*coordination.Strategy.RollingUpdate.MaxUnavailable))
623623
}
624624
if coordination.Strategy.RollingUpdate.Partition != nil {
625625
partitionPercent := intstr.FromString(*coordination.Strategy.RollingUpdate.Partition)
626626
partition, err := utils.CalculatePartitionReplicas(&partitionPercent, role.Replicas)
627627
if err != nil {
628628
return nil, err
629629
}
630-
strategy.Partition = func(a int32) *int32 { return &a }(int32(partition))
630+
strategy.Partition = func(a int32) *intstr.IntOrString {
631+
return ptr.To(intstr.FromInt32(a))
632+
}(int32(partition))
631633
}
632634
strategyRollingUpdate[role.Name] = strategy
633635
}
@@ -649,16 +651,20 @@ func (r *RoleBasedGroupReconciler) calculateRollingUpdateForCoordination(
649651
// finalPartition is the user-settings target partition that should be respected by any coordination.
650652
finalPartition := int32(0)
651653
if strategy.Partition != nil {
652-
finalPartition = *strategy.Partition
654+
finalPartitionInt, err := intstr.GetScaledValueFromIntOrPercent(strategy.Partition, int(*role.Replicas), true)
655+
if err != nil {
656+
return nil, err
657+
}
658+
finalPartition = int32(finalPartitionInt)
653659
}
654660

655661
// stepPartition is the calculated partition based on the maxSkew.
656662
stepPartition := max(*role.Replicas-updatedTarget, 0)
657663

658664
if stepPartition > finalPartition {
659-
strategy.Partition = &stepPartition
665+
strategy.Partition = ptr.To(intstr.FromInt32(stepPartition))
660666
} else {
661-
strategy.Partition = &finalPartition
667+
strategy.Partition = ptr.To(intstr.FromInt32(finalPartition))
662668
}
663669
strategyRollingUpdate[role.Name] = strategy
664670
}
@@ -771,18 +777,18 @@ func mergeStrategyRollingUpdate(strategiesA, strategiesB map[string]workloadsv1a
771777
merged[role] = strategyB
772778
continue
773779
}
774-
maxUnavailableA, _ := intstr.GetScaledValueFromIntOrPercent(&strategyA.MaxUnavailable, 100, true)
775-
maxUnavailableB, _ := intstr.GetScaledValueFromIntOrPercent(&strategyB.MaxUnavailable, 100, true)
780+
maxUnavailableA, _ := intstr.GetScaledValueFromIntOrPercent(strategyA.MaxUnavailable, 100, true)
781+
maxUnavailableB, _ := intstr.GetScaledValueFromIntOrPercent(strategyB.MaxUnavailable, 100, true)
776782
if maxUnavailableA > maxUnavailableB {
777783
strategyA.MaxUnavailable = strategyB.MaxUnavailable
778784
}
779785

780786
partitionA, partitionB := 0, 0
781787
if strategyA.Partition != nil {
782-
partitionA = int(*strategyA.Partition)
788+
partitionA, _ = intstr.GetScaledValueFromIntOrPercent(strategyA.Partition, 100, true)
783789
}
784790
if strategyB.Partition != nil {
785-
partitionB = int(*strategyB.Partition)
791+
partitionB, _ = intstr.GetScaledValueFromIntOrPercent(strategyB.Partition, 100, true)
786792
}
787793
if partitionA < partitionB {
788794
strategyA.Partition = strategyB.Partition

‎internal/controller/workloads/rolebasedgroup_controller_test.go‎

Lines changed: 45 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1074,145 +1074,145 @@ func Test_mergeStrategyRollingUpdate(t *testing.T) {
10741074
name: "merge with no overlap",
10751075
strategiesA: map[string]workloadsv1alpha1.RollingUpdate{
10761076
"role1": {
1077-
MaxUnavailable: intstr.FromInt(5),
1078-
Partition: ptr.To(int32(10)),
1077+
MaxUnavailable: ptr.To(intstr.FromInt32(5)),
1078+
Partition: ptr.To(intstr.FromInt32(10)),
10791079
},
10801080
},
10811081
strategiesB: map[string]workloadsv1alpha1.RollingUpdate{
10821082
"role2": {
1083-
MaxUnavailable: intstr.FromInt(3),
1084-
Partition: ptr.To(int32(5)),
1083+
MaxUnavailable: ptr.To(intstr.FromInt32(3)),
1084+
Partition: ptr.To(intstr.FromInt32(5)),
10851085
},
10861086
},
10871087
expected: map[string]workloadsv1alpha1.RollingUpdate{
10881088
"role1": {
1089-
MaxUnavailable: intstr.FromInt(5),
1090-
Partition: ptr.To(int32(10)),
1089+
MaxUnavailable: ptr.To(intstr.FromInt32(5)),
1090+
Partition: ptr.To(intstr.FromInt32(10)),
10911091
},
10921092
"role2": {
1093-
MaxUnavailable: intstr.FromInt(3),
1094-
Partition: ptr.To(int32(5)),
1093+
MaxUnavailable: ptr.To(intstr.FromInt32(3)),
1094+
Partition: ptr.To(intstr.FromInt32(5)),
10951095
},
10961096
},
10971097
},
10981098
{
10991099
name: "merge with overlap, B has smaller maxUnavailable",
11001100
strategiesA: map[string]workloadsv1alpha1.RollingUpdate{
11011101
"role1": {
1102-
MaxUnavailable: intstr.FromInt(10),
1103-
Partition: ptr.To(int32(5)),
1102+
MaxUnavailable: ptr.To(intstr.FromInt32(10)),
1103+
Partition: ptr.To(intstr.FromInt32(5)),
11041104
},
11051105
},
11061106
strategiesB: map[string]workloadsv1alpha1.RollingUpdate{
11071107
"role1": {
1108-
MaxUnavailable: intstr.FromInt(5),
1109-
Partition: ptr.To(int32(3)),
1108+
MaxUnavailable: ptr.To(intstr.FromInt32(5)),
1109+
Partition: ptr.To(intstr.FromInt32(3)),
11101110
},
11111111
},
11121112
expected: map[string]workloadsv1alpha1.RollingUpdate{
11131113
"role1": {
1114-
MaxUnavailable: intstr.FromInt(5), // Smaller value
1115-
Partition: ptr.To(int32(5)), // Larger partition
1114+
MaxUnavailable: ptr.To(intstr.FromInt32(5)), // Smaller value
1115+
Partition: ptr.To(intstr.FromInt32(5)), // Larger partition
11161116
},
11171117
},
11181118
},
11191119
{
11201120
name: "merge with overlap, B has larger partition",
11211121
strategiesA: map[string]workloadsv1alpha1.RollingUpdate{
11221122
"role1": {
1123-
MaxUnavailable: intstr.FromInt(5),
1124-
Partition: ptr.To(int32(3)),
1123+
MaxUnavailable: ptr.To(intstr.FromInt32(5)),
1124+
Partition: ptr.To(intstr.FromInt32(3)),
11251125
},
11261126
},
11271127
strategiesB: map[string]workloadsv1alpha1.RollingUpdate{
11281128
"role1": {
1129-
MaxUnavailable: intstr.FromInt(5),
1130-
Partition: ptr.To(int32(10)),
1129+
MaxUnavailable: ptr.To(intstr.FromInt32(5)),
1130+
Partition: ptr.To(intstr.FromInt32(10)),
11311131
},
11321132
},
11331133
expected: map[string]workloadsv1alpha1.RollingUpdate{
11341134
"role1": {
1135-
MaxUnavailable: intstr.FromInt(5),
1136-
Partition: ptr.To(int32(10)), // Larger partition
1135+
MaxUnavailable: ptr.To(intstr.FromInt32(5)),
1136+
Partition: ptr.To(intstr.FromInt32(10)), // Larger partition
11371137
},
11381138
},
11391139
},
11401140
{
11411141
name: "merge with percentage maxUnavailable",
11421142
strategiesA: map[string]workloadsv1alpha1.RollingUpdate{
11431143
"role1": {
1144-
MaxUnavailable: intstr.FromString("20%"),
1145-
Partition: ptr.To(int32(5)),
1144+
MaxUnavailable: ptr.To(intstr.FromString("20%")),
1145+
Partition: ptr.To(intstr.FromInt32(5)),
11461146
},
11471147
},
11481148
strategiesB: map[string]workloadsv1alpha1.RollingUpdate{
11491149
"role1": {
1150-
MaxUnavailable: intstr.FromString("10%"),
1151-
Partition: ptr.To(int32(3)),
1150+
MaxUnavailable: ptr.To(intstr.FromString("10%")),
1151+
Partition: ptr.To(intstr.FromInt32(3)),
11521152
},
11531153
},
11541154
expected: map[string]workloadsv1alpha1.RollingUpdate{
11551155
"role1": {
1156-
MaxUnavailable: intstr.FromString("10%"), // Smaller value
1157-
Partition: ptr.To(int32(5)), // Larger partition
1156+
MaxUnavailable: ptr.To(intstr.FromString("10%")), // Smaller value
1157+
Partition: ptr.To(intstr.FromInt32(5)), // Larger partition
11581158
},
11591159
},
11601160
},
11611161
{
11621162
name: "merge with nil partition",
11631163
strategiesA: map[string]workloadsv1alpha1.RollingUpdate{
11641164
"role1": {
1165-
MaxUnavailable: intstr.FromInt(5),
1165+
MaxUnavailable: ptr.To(intstr.FromInt32(5)),
11661166
Partition: nil,
11671167
},
11681168
},
11691169
strategiesB: map[string]workloadsv1alpha1.RollingUpdate{
11701170
"role1": {
1171-
MaxUnavailable: intstr.FromInt(3),
1172-
Partition: ptr.To(int32(10)),
1171+
MaxUnavailable: ptr.To(intstr.FromInt32(3)),
1172+
Partition: ptr.To(intstr.FromInt32(10)),
11731173
},
11741174
},
11751175
expected: map[string]workloadsv1alpha1.RollingUpdate{
11761176
"role1": {
1177-
MaxUnavailable: intstr.FromInt(3),
1178-
Partition: ptr.To(int32(10)), // B's partition
1177+
MaxUnavailable: ptr.To(intstr.FromInt32(3)),
1178+
Partition: ptr.To(intstr.FromInt32(10)), // B's partition
11791179
},
11801180
},
11811181
},
11821182
{
11831183
name: "merge multiple roles",
11841184
strategiesA: map[string]workloadsv1alpha1.RollingUpdate{
11851185
"role1": {
1186-
MaxUnavailable: intstr.FromInt(10),
1187-
Partition: ptr.To(int32(5)),
1186+
MaxUnavailable: ptr.To(intstr.FromInt32(10)),
1187+
Partition: ptr.To(intstr.FromInt32(5)),
11881188
},
11891189
"role2": {
1190-
MaxUnavailable: intstr.FromInt(5),
1191-
Partition: ptr.To(int32(3)),
1190+
MaxUnavailable: ptr.To(intstr.FromInt32(5)),
1191+
Partition: ptr.To(intstr.FromInt32(3)),
11921192
},
11931193
},
11941194
strategiesB: map[string]workloadsv1alpha1.RollingUpdate{
11951195
"role2": {
1196-
MaxUnavailable: intstr.FromInt(3),
1197-
Partition: ptr.To(int32(10)),
1196+
MaxUnavailable: ptr.To(intstr.FromInt32(3)),
1197+
Partition: ptr.To(intstr.FromInt32(10)),
11981198
},
11991199
"role3": {
1200-
MaxUnavailable: intstr.FromInt(7),
1201-
Partition: ptr.To(int32(8)),
1200+
MaxUnavailable: ptr.To(intstr.FromInt32(7)),
1201+
Partition: ptr.To(intstr.FromInt32(8)),
12021202
},
12031203
},
12041204
expected: map[string]workloadsv1alpha1.RollingUpdate{
12051205
"role1": {
1206-
MaxUnavailable: intstr.FromInt(10),
1207-
Partition: ptr.To(int32(5)),
1206+
MaxUnavailable: ptr.To(intstr.FromInt32(10)),
1207+
Partition: ptr.To(intstr.FromInt32(5)),
12081208
},
12091209
"role2": {
1210-
MaxUnavailable: intstr.FromInt(3),
1211-
Partition: ptr.To(int32(10)),
1210+
MaxUnavailable: ptr.To(intstr.FromInt32(3)),
1211+
Partition: ptr.To(intstr.FromInt32(10)),
12121212
},
12131213
"role3": {
1214-
MaxUnavailable: intstr.FromInt(7),
1215-
Partition: ptr.To(int32(8)),
1214+
MaxUnavailable: ptr.To(intstr.FromInt32(7)),
1215+
Partition: ptr.To(intstr.FromInt32(8)),
12161216
},
12171217
},
12181218
},

0 commit comments

Comments
 (0)