diff --git a/rollout/trafficrouting.go b/rollout/trafficrouting.go index 63ef0daadf..fd0c91decc 100644 --- a/rollout/trafficrouting.go +++ b/rollout/trafficrouting.go @@ -274,12 +274,6 @@ func (c *rolloutContext) reconcileTrafficRouting() error { c.recorder.Warnf(c.rollout, record.EventOptions{EventReason: conditions.WeightVerifyErrorReason}, conditions.WeightVerifyErrorMessage, err) return nil // return nil instead of error since we want to continue with normal reconciliation } - // At the end of the rollout we need to verify the weight is correct, and return an error if not because we don't want the rest of the - // reconcile process to continue. We don't need to do this if we are in the middle of the rollout because the rest of the reconcile - // process won't scale down the old replicasets yet due to being in the middle of some steps. - if weightVerified != nil && *weightVerified == false && desiredWeight == weightutil.MaxTrafficWeight(c.rollout) && len(c.rollout.Spec.Strategy.Canary.Steps) >= int(*c.rollout.Status.CurrentStepIndex) { - return fmt.Errorf("end of rollout, desired weight %d not yet verified", desiredWeight) - } var indexString string if index != nil { @@ -294,6 +288,12 @@ func (c *rolloutContext) reconcileTrafficRouting() error { } else { c.log.Infof("Desired weight (stepIdx: %s) %d not yet verified", indexString, desiredWeight) c.enqueueRolloutAfter(c.rollout, defaults.GetRolloutVerifyRetryInterval()) + // At the end of the rollout we need to verify the weight is correct, and return an error if not because we don't want the rest of the + // reconcile process to continue. We don't need to do this if we are in the middle of the rollout because the rest of the reconcile + // process won't scale down the old replicasets yet due to being in the middle of some steps. + if *weightVerified == false && desiredWeight == weightutil.MaxTrafficWeight(c.rollout) && len(c.rollout.Spec.Strategy.Canary.Steps) >= int(*c.rollout.Status.CurrentStepIndex) { + return fmt.Errorf("end of rollout, desired weight %d not yet verified", desiredWeight) + } } } } diff --git a/rollout/trafficrouting_test.go b/rollout/trafficrouting_test.go index f7dd459964..3c5eda4205 100644 --- a/rollout/trafficrouting_test.go +++ b/rollout/trafficrouting_test.go @@ -130,6 +130,53 @@ func TestReconcileTrafficRoutingVerifyWeightFalse(t *testing.T) { assert.True(t, enqueued) } +func TestReconcileTrafficRoutingVerifyWeightDesiredWeight100(t *testing.T) { + f := newFixture(t) + defer f.Close() + + steps := []v1alpha1.CanaryStep{ + { + SetWeight: pointer.Int32Ptr(10), + }, + { + Pause: &v1alpha1.RolloutPause{}, + }, + } + r1 := newCanaryRollout("foo", 10, nil, steps, pointer.Int32Ptr(2), intstr.FromInt(1), intstr.FromInt(0)) + r2 := bumpVersion(r1) + r2.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{} + r2.Spec.Strategy.Canary.CanaryService = "canary" + r2.Spec.Strategy.Canary.StableService = "stable" + + rs1 := newReplicaSetWithStatus(r1, 10, 10) + rs2 := newReplicaSetWithStatus(r2, 10, 10) + + rs1PodHash := rs1.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey] + canarySelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash} + stableSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs1PodHash} + canarySvc := newService("canary", 80, canarySelector, r2) + stableSvc := newService("stable", 80, stableSelector, r2) + + f.kubeobjects = append(f.kubeobjects, rs1, rs2, canarySvc, stableSvc) + f.replicaSetLister = append(f.replicaSetLister, rs1, rs2) + + r2 = updateCanaryRolloutStatus(r2, rs1PodHash, 10, 0, 10, false) + f.rolloutLister = append(f.rolloutLister, r2) + f.objects = append(f.objects, r2) + + f.fakeTrafficRouting = newUnmockedFakeTrafficRoutingReconciler() + f.fakeTrafficRouting.On("UpdateHash", mock.Anything, mock.Anything, mock.Anything).Return(nil) + f.fakeTrafficRouting.On("SetWeight", mock.Anything, mock.Anything).Return(func(desiredWeight int32, additionalDestinations ...v1alpha1.WeightDestination) error { + // make sure SetWeight was called with correct value + assert.Equal(t, int32(100), desiredWeight) + return nil + }) + f.fakeTrafficRouting.On("SetHeaderRoute", mock.Anything, mock.Anything).Return(nil) + f.fakeTrafficRouting.On("VerifyWeight", mock.Anything).Return(pointer.BoolPtr(false), nil) + f.runExpectError(getKey(r2, t), true) +} + func TestRolloutUseDesiredWeight(t *testing.T) { f := newFixture(t) defer f.Close()