Skip to content

Commit d51996a

Browse files
AiRanthemAiRanthem
committed
resolve some conversations
Signed-off-by: AiRanthem <zhongtianyun.zty@alibaba-inc.com>
1 parent 8841c0d commit d51996a

File tree

3 files changed

+54
-28
lines changed

3 files changed

+54
-28
lines changed

‎pkg/controller/batchrelease/labelpatch/patcher.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,11 +121,17 @@ func (r *realPatcher) patchPodBatchLabel(pods []*corev1.Pod, ctx *batchcontext.B
121121

122122
podBatchID, err := strconv.Atoi(labels[v1beta1.RolloutBatchIDLabel])
123123
if err != nil {
124-
klog.InfoS("Pod batchID is not a number, skip patching", "pod", klog.KObj(pod), "rollout", r.logKey)
124+
klog.InfoS("Pod batchID is not a number, will overwrite it", "pod", klog.KObj(pod),
125+
"rollout", r.logKey, "batchID", labels[v1beta1.RolloutBatchIDLabel])
126+
updatedButUnpatchedPods = append(updatedButUnpatchedPods, pod)
125127
continue
126128
}
127129
if podBatchID > 0 && podBatchID <= len(plannedUpdatedReplicasForBatches) {
128130
plannedUpdatedReplicasForBatches[podBatchID-1]--
131+
} else {
132+
klog.InfoS("Pod batchID is not valid, will overwrite it", "pod", klog.KObj(pod),
133+
"rollout", r.logKey, "batchID", labels[v1beta1.RolloutBatchIDLabel])
134+
updatedButUnpatchedPods = append(updatedButUnpatchedPods, pod)
129135
}
130136
}
131137
klog.InfoS("updatedButUnpatchedPods amount calculated", "amount", len(updatedButUnpatchedPods),

‎pkg/controller/batchrelease/labelpatch/patcher_test.go‎

Lines changed: 45 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ func TestLabelPatcher(t *testing.T) {
7171
},
7272
Batches: []v1beta1.ReleaseBatch{
7373
{
74-
CanaryReplicas: intstr.FromInt(5),
74+
CanaryReplicas: intstr.FromInt32(5),
7575
},
7676
},
7777
expectedPatched: []int{5},
@@ -91,7 +91,7 @@ func TestLabelPatcher(t *testing.T) {
9191
},
9292
Batches: []v1beta1.ReleaseBatch{
9393
{
94-
CanaryReplicas: intstr.FromInt(5),
94+
CanaryReplicas: intstr.FromInt32(5),
9595
},
9696
},
9797
expectedPatched: []int{5},
@@ -111,7 +111,7 @@ func TestLabelPatcher(t *testing.T) {
111111
},
112112
Batches: []v1beta1.ReleaseBatch{
113113
{
114-
CanaryReplicas: intstr.FromInt(5),
114+
CanaryReplicas: intstr.FromInt32(5),
115115
},
116116
},
117117
expectedPatched: []int{5},
@@ -131,7 +131,7 @@ func TestLabelPatcher(t *testing.T) {
131131
},
132132
Batches: []v1beta1.ReleaseBatch{
133133
{
134-
CanaryReplicas: intstr.FromInt(5),
134+
CanaryReplicas: intstr.FromInt32(5),
135135
},
136136
},
137137
expectedPatched: []int{7},
@@ -151,7 +151,7 @@ func TestLabelPatcher(t *testing.T) {
151151
},
152152
Batches: []v1beta1.ReleaseBatch{
153153
{
154-
CanaryReplicas: intstr.FromInt(5),
154+
CanaryReplicas: intstr.FromInt32(5),
155155
},
156156
},
157157
expectedPatched: []int{2},
@@ -171,7 +171,7 @@ func TestLabelPatcher(t *testing.T) {
171171
},
172172
Batches: []v1beta1.ReleaseBatch{
173173
{
174-
CanaryReplicas: intstr.FromInt(5),
174+
CanaryReplicas: intstr.FromInt32(5),
175175
},
176176
},
177177
expectedPatched: []int{5},
@@ -191,8 +191,8 @@ func TestLabelPatcher(t *testing.T) {
191191
return ctx
192192
},
193193
Batches: []v1beta1.ReleaseBatch{
194-
{CanaryReplicas: intstr.FromInt(2)},
195-
{CanaryReplicas: intstr.FromInt(5)},
194+
{CanaryReplicas: intstr.FromInt32(2)},
195+
{CanaryReplicas: intstr.FromInt32(5)},
196196
},
197197
expectedPatched: []int{2, 3},
198198
},
@@ -211,8 +211,8 @@ func TestLabelPatcher(t *testing.T) {
211211
return ctx
212212
},
213213
Batches: []v1beta1.ReleaseBatch{
214-
{CanaryReplicas: intstr.FromInt(2)},
215-
{CanaryReplicas: intstr.FromInt(5)},
214+
{CanaryReplicas: intstr.FromInt32(2)},
215+
{CanaryReplicas: intstr.FromInt32(5)},
216216
},
217217
expectedPatched: []int{2, 3},
218218
},
@@ -231,11 +231,31 @@ func TestLabelPatcher(t *testing.T) {
231231
return ctx
232232
},
233233
Batches: []v1beta1.ReleaseBatch{
234-
{CanaryReplicas: intstr.FromInt(2)},
235-
{CanaryReplicas: intstr.FromInt(5)},
234+
{CanaryReplicas: intstr.FromInt32(2)},
235+
{CanaryReplicas: intstr.FromInt32(5)},
236236
},
237237
expectedPatched: []int{3, 2},
238238
},
239+
"batch-id out of range, equal to no pods are labelled": {
240+
batchContext: func() *batchcontext.BatchContext {
241+
ctx := &batchcontext.BatchContext{
242+
RolloutID: "rollout-1",
243+
UpdateRevision: "version-1",
244+
PlannedUpdatedReplicas: 5,
245+
CurrentBatch: 1,
246+
Replicas: 10,
247+
}
248+
pods := generatePods(1, 5, 3,
249+
"rollout-1", strconv.Itoa(3), ctx.UpdateRevision)
250+
ctx.Pods = pods
251+
return ctx
252+
},
253+
Batches: []v1beta1.ReleaseBatch{
254+
{CanaryReplicas: intstr.FromInt32(2)},
255+
{CanaryReplicas: intstr.FromInt32(5)},
256+
},
257+
expectedPatched: []int{2, 3},
258+
},
239259
}
240260

241261
for name, cs := range cases {
@@ -255,7 +275,7 @@ func TestLabelPatcher(t *testing.T) {
255275
patched := make([]int, ctx.CurrentBatch+1)
256276
for _, pod := range podList.Items {
257277
if pod.Labels[v1beta1.RolloutIDLabel] == ctx.RolloutID {
258-
if batchID, err := strconv.Atoi(pod.Labels[v1beta1.RolloutBatchIDLabel]); err == nil {
278+
if batchID, err := strconv.Atoi(pod.Labels[v1beta1.RolloutBatchIDLabel]); err == nil && batchID <= len(patched) {
259279
patched[batchID-1]++
260280
}
261281
}
@@ -311,7 +331,7 @@ func TestDeploymentPatch(t *testing.T) {
311331
},
312332
Batches: []v1beta1.ReleaseBatch{
313333
{
314-
CanaryReplicas: intstr.FromInt(5),
334+
CanaryReplicas: intstr.FromInt32(5),
315335
},
316336
},
317337
expectedPatched: []int{5},
@@ -331,7 +351,7 @@ func TestDeploymentPatch(t *testing.T) {
331351
},
332352
Batches: []v1beta1.ReleaseBatch{
333353
{
334-
CanaryReplicas: intstr.FromInt(5),
354+
CanaryReplicas: intstr.FromInt32(5),
335355
},
336356
},
337357
expectedPatched: []int{5},
@@ -351,7 +371,7 @@ func TestDeploymentPatch(t *testing.T) {
351371
},
352372
Batches: []v1beta1.ReleaseBatch{
353373
{
354-
CanaryReplicas: intstr.FromInt(5),
374+
CanaryReplicas: intstr.FromInt32(5),
355375
},
356376
},
357377
expectedPatched: []int{5},
@@ -371,7 +391,7 @@ func TestDeploymentPatch(t *testing.T) {
371391
},
372392
Batches: []v1beta1.ReleaseBatch{
373393
{
374-
CanaryReplicas: intstr.FromInt(5),
394+
CanaryReplicas: intstr.FromInt32(5),
375395
},
376396
},
377397
expectedPatched: []int{7},
@@ -391,7 +411,7 @@ func TestDeploymentPatch(t *testing.T) {
391411
},
392412
Batches: []v1beta1.ReleaseBatch{
393413
{
394-
CanaryReplicas: intstr.FromInt(5),
414+
CanaryReplicas: intstr.FromInt32(5),
395415
},
396416
},
397417
expectedPatched: []int{2},
@@ -411,7 +431,7 @@ func TestDeploymentPatch(t *testing.T) {
411431
},
412432
Batches: []v1beta1.ReleaseBatch{
413433
{
414-
CanaryReplicas: intstr.FromInt(5),
434+
CanaryReplicas: intstr.FromInt32(5),
415435
},
416436
},
417437
expectedPatched: []int{5},
@@ -431,8 +451,8 @@ func TestDeploymentPatch(t *testing.T) {
431451
return ctx
432452
},
433453
Batches: []v1beta1.ReleaseBatch{
434-
{CanaryReplicas: intstr.FromInt(2)},
435-
{CanaryReplicas: intstr.FromInt(5)},
454+
{CanaryReplicas: intstr.FromInt32(2)},
455+
{CanaryReplicas: intstr.FromInt32(5)},
436456
},
437457
expectedPatched: []int{2, 3},
438458
},
@@ -451,8 +471,8 @@ func TestDeploymentPatch(t *testing.T) {
451471
return ctx
452472
},
453473
Batches: []v1beta1.ReleaseBatch{
454-
{CanaryReplicas: intstr.FromInt(2)},
455-
{CanaryReplicas: intstr.FromInt(5)},
474+
{CanaryReplicas: intstr.FromInt32(2)},
475+
{CanaryReplicas: intstr.FromInt32(5)},
456476
},
457477
expectedPatched: []int{2, 3},
458478
},
@@ -471,8 +491,8 @@ func TestDeploymentPatch(t *testing.T) {
471491
return ctx
472492
},
473493
Batches: []v1beta1.ReleaseBatch{
474-
{CanaryReplicas: intstr.FromInt(2)},
475-
{CanaryReplicas: intstr.FromInt(5)},
494+
{CanaryReplicas: intstr.FromInt32(2)},
495+
{CanaryReplicas: intstr.FromInt32(5)},
476496
},
477497
expectedPatched: []int{3, 2},
478498
},

‎pkg/controller/rollout/rollout_canary.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ func (m *canaryReleaseManager) doCanaryPaused(c *RolloutContext) (bool, error) {
317317
func doStepJump(rollout *v1beta1.Rollout, newStatus *v1beta1.RolloutStatus, steps []v1beta1.CanaryStep, workloadReplicas int) (jumped bool) {
318318
status := GetUnifiedStatus(newStatus)
319319
if status.IsNil() {
320-
klog.InfoS("doStepJump skipped: unified status is nil")
320+
klog.InfoS("doStepJump skipped: unified status is nil", "rollout", klog.KObj(rollout))
321321
return false
322322
}
323323
klog.InfoS("will do step jump", "steps", len(steps), "updatedReplicas", *status.UpdatedReplicas,
@@ -343,7 +343,7 @@ func doStepJump(rollout *v1beta1.Rollout, newStatus *v1beta1.RolloutStatus, step
343343
"oldCurrentStepState", currentStepStateBackup, "newCurrentStepState", status.CurrentStepState)
344344
return true
345345
}
346-
klog.InfoS("step not jumped")
346+
klog.InfoS("step not jumped", "rollout", klog.KObj(rollout))
347347
return false
348348
}
349349

0 commit comments

Comments
 (0)