Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

chore: revert service selector switches base 1.7.2 (do not merge) #3790

Closed
wants to merge 1 commit into from
Closed

chore: revert service selector switches base 1.7.2 (do not merge) #3790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 0 additions & 118 deletions rollout/canary_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1432,124 +1432,6 @@ func TestCanarySVCSelectors(t *testing.T) {
}
}

func TestCanarySVCSelectorsBasicCanaryAbortServiceSwitchBack(t *testing.T) {
for _, tc := range []struct {
canaryReplicas int32
canaryAvailReplicas int32
shouldAbortRollout bool
shouldTargetNewRS bool
}{
{2, 2, false, true}, // Rollout, canaryService should point at the canary RS
{2, 2, true, false}, // Rollout aborted, canaryService should point at the stable RS
} {
namespace := "namespace"
selectorLabel := "selector-labels-test"
selectorNewRSVal := "new-rs-xxx"
selectorStableRSVal := "stable-rs-xxx"
stableService := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "stable",
Namespace: namespace,
Annotations: map[string]string{v1alpha1.ManagedByRolloutsKey: selectorLabel},
Labels: map[string]string{
v1alpha1.DefaultRolloutUniqueLabelKey: selectorStableRSVal,
},
},
}
canaryService := &corev1.Service{
ObjectMeta: metav1.ObjectMeta{
Name: "canary",
Namespace: namespace,
Annotations: map[string]string{v1alpha1.ManagedByRolloutsKey: selectorLabel},
},
}
kubeclient := k8sfake.NewSimpleClientset(stableService, canaryService)
informers := k8sinformers.NewSharedInformerFactory(kubeclient, 0)
servicesLister := informers.Core().V1().Services().Lister()

rollout := &v1alpha1.Rollout{
ObjectMeta: metav1.ObjectMeta{
Name: selectorLabel,
Namespace: namespace,
},
Spec: v1alpha1.RolloutSpec{
Strategy: v1alpha1.RolloutStrategy{
Canary: &v1alpha1.CanaryStrategy{
StableService: stableService.Name,
CanaryService: canaryService.Name,
},
},
},
}

pc := pauseContext{
rollout: rollout,
}
if tc.shouldAbortRollout {
pc.AddAbort("Add Abort")
}

rc := rolloutContext{
log: logutil.WithRollout(rollout),
pauseContext: &pc,
reconcilerBase: reconcilerBase{
servicesLister: servicesLister,
kubeclientset: kubeclient,
recorder: record.NewFakeEventRecorder(),
},
rollout: rollout,
newRS: &v1.ReplicaSet{
ObjectMeta: metav1.ObjectMeta{
Name: "canary",
Namespace: namespace,
Labels: map[string]string{
v1alpha1.DefaultRolloutUniqueLabelKey: selectorNewRSVal,
},
},
Spec: v1.ReplicaSetSpec{
Replicas: pointer.Int32Ptr(tc.canaryReplicas),
},
Status: v1.ReplicaSetStatus{
AvailableReplicas: tc.canaryAvailReplicas,
},
},
stableRS: &v1.ReplicaSet{
ObjectMeta: metav1.ObjectMeta{
Name: "stable",
Namespace: namespace,
Labels: map[string]string{
v1alpha1.DefaultRolloutUniqueLabelKey: selectorStableRSVal,
},
},
Spec: v1.ReplicaSetSpec{
Replicas: pointer.Int32Ptr(tc.canaryReplicas),
},
Status: v1.ReplicaSetStatus{
AvailableReplicas: tc.canaryAvailReplicas,
},
},
}

stopchan := make(chan struct{})
defer close(stopchan)
informers.Start(stopchan)
informers.WaitForCacheSync(stopchan)
err := rc.reconcileStableAndCanaryService()
assert.NoError(t, err, "unable to reconcileStableAndCanaryService")
updatedCanarySVC, err := servicesLister.Services(rc.rollout.Namespace).Get(canaryService.Name)
assert.NoError(t, err, "unable to get updated canary service")
if tc.shouldTargetNewRS {
assert.Equal(t, selectorNewRSVal, updatedCanarySVC.Spec.Selector[v1alpha1.DefaultRolloutUniqueLabelKey],
"canary SVC should have newRS selector label when newRS has %d replicas and %d AvailableReplicas",
tc.canaryReplicas, tc.canaryAvailReplicas)
} else {
assert.Equal(t, selectorStableRSVal, updatedCanarySVC.Spec.Selector[v1alpha1.DefaultRolloutUniqueLabelKey],
"canary SVC should have stableRS selector label when newRS has %d replicas and %d AvailableReplicas",
tc.canaryReplicas, tc.canaryAvailReplicas)
}
}
}

func TestCanaryRolloutWithInvalidCanaryServiceName(t *testing.T) {
f := newFixture(t)
defer f.Close()
Expand Down
14 changes: 7 additions & 7 deletions rollout/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,13 +258,13 @@ func (c *rolloutContext) reconcileStableAndCanaryService() error {
return err
}

if c.pauseContext != nil && c.pauseContext.IsAborted() && c.rollout.Spec.Strategy.Canary.TrafficRouting == nil {
err = c.ensureSVCTargets(c.rollout.Spec.Strategy.Canary.CanaryService, c.stableRS, true)
if err != nil {
return err
}
return nil
}
//if c.pauseContext != nil && c.pauseContext.IsAborted() && c.rollout.Spec.Strategy.Canary.TrafficRouting == nil {
// err = c.ensureSVCTargets(c.rollout.Spec.Strategy.Canary.CanaryService, c.stableRS, true)
// if err != nil {
// return err
// }
// return nil
//}

if dynamicallyRollingBackToStable, currSelector := isDynamicallyRollingBackToStable(c.rollout, c.newRS); dynamicallyRollingBackToStable {
// User may have interrupted an update in order go back to stableRS, and is using dynamic
Expand Down
8 changes: 4 additions & 4 deletions rollout/trafficrouting.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,10 @@ func (c *rolloutContext) reconcileTrafficRouting() error {
// fully aborted and stable pods will be there, if we check for availability it causes issues with ALB readiness gates if all stable pods
// have the desired readiness gate on them during an abort we get stuck in a loop because all the stable go unready and rollouts won't be able
// to switch the desired services because there is no ready pods which causes pods to get stuck progressing forever waiting for readiness.
err = c.ensureSVCTargets(c.rollout.Spec.Strategy.Canary.CanaryService, c.stableRS, false)
if err != nil {
return err
}
//err = c.ensureSVCTargets(c.rollout.Spec.Strategy.Canary.CanaryService, c.stableRS, false)
//if err != nil {
// return err
//}
}
err := reconciler.RemoveManagedRoutes()
if err != nil {
Expand Down
75 changes: 1 addition & 74 deletions rollout/trafficrouting_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -946,7 +946,7 @@ func TestDynamicScalingDontIncreaseWeightWhenAborted(t *testing.T) {
f.objects = append(f.objects, r2)

f.expectPatchRolloutAction(r2)
f.expectPatchServiceAction(canarySvc, rs1PodHash)
//f.expectPatchServiceAction(canarySvc, rs1PodHash)

f.fakeTrafficRouting = newUnmockedFakeTrafficRoutingReconciler()
f.fakeTrafficRouting.On("UpdateHash", mock.Anything, mock.Anything, mock.Anything).Return(nil)
Expand Down Expand Up @@ -1032,79 +1032,6 @@ func TestDynamicScalingDecreaseWeightAccordingToStableAvailabilityWhenAborted(t
f.run(getKey(r1, t))
}

// TestDynamicScalingDecreaseWeightAccordingToStableAvailabilityWhenAbortedAndResetService verifies we decrease the weight
// to the canary depending on the availability of the stable ReplicaSet when aborting and also that at the end of the abort
// we reset the canary service selectors back to the stable service
func TestDynamicScalingDecreaseWeightAccordingToStableAvailabilityWhenAbortedAndResetService(t *testing.T) {
f := newFixture(t)
defer f.Close()

steps := []v1alpha1.CanaryStep{
{
SetWeight: pointer.Int32Ptr(50),
},
{
Pause: &v1alpha1.RolloutPause{},
},
}
r1 := newCanaryRollout("foo", 5, nil, steps, pointer.Int32Ptr(1), intstr.FromInt(1), intstr.FromInt(1))
r1.Spec.Strategy.Canary.DynamicStableScale = true
r1.Spec.Strategy.Canary.TrafficRouting = &v1alpha1.RolloutTrafficRouting{
SMI: &v1alpha1.SMITrafficRouting{},
}
r1.Spec.Strategy.Canary.CanaryService = "canary"
r1.Spec.Strategy.Canary.StableService = "stable"
r1.Status.ReadyReplicas = 5
r1.Status.AvailableReplicas = 5
r1.Status.Abort = true
r1.Status.AbortedAt = &metav1.Time{Time: time.Now().Add(-1 * time.Minute)}
r2 := bumpVersion(r1)

rs1 := newReplicaSetWithStatus(r1, 5, 5)
rs2 := newReplicaSetWithStatus(r2, 0, 0)

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, r1)
stableSvc := newService("stable", 80, stableSelector, r1)
r2.Status.StableRS = rs1PodHash
r2.Status.Canary.Weights = &v1alpha1.TrafficWeights{
Canary: v1alpha1.WeightDestination{
Weight: 20,
ServiceName: "canary",
PodTemplateHash: rs2PodHash,
},
Stable: v1alpha1.WeightDestination{
Weight: 80,
ServiceName: "stable",
PodTemplateHash: rs1PodHash,
},
}

f.kubeobjects = append(f.kubeobjects, rs1, rs2, canarySvc, stableSvc)
f.replicaSetLister = append(f.replicaSetLister, rs1, rs2)

f.rolloutLister = append(f.rolloutLister, r2)
f.objects = append(f.objects, r2)

f.expectPatchRolloutAction(r2)
f.expectPatchServiceAction(canarySvc, rs1PodHash)

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(0), desiredWeight)
return nil
})
f.fakeTrafficRouting.On("SetHeaderRoute", mock.Anything, mock.Anything).Return(nil)
f.fakeTrafficRouting.On("RemoveManagedRoutes", mock.Anything, mock.Anything).Return(nil)
f.fakeTrafficRouting.On("VerifyWeight", mock.Anything).Return(pointer.BoolPtr(true), nil)
f.run(getKey(r1, t))
}

func TestRolloutReplicaIsAvailableAndGenerationNotBeModifiedShouldModifyVirtualServiceSHeaderRoute(t *testing.T) {
f := newFixture(t)
defer f.Close()
Expand Down
10 changes: 0 additions & 10 deletions test/e2e/canary_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -567,10 +567,6 @@ func (s *CanarySuite) TestCanaryScaleDownOnAbort() {
WaitForRolloutStatus("Paused").
AbortRollout().
WaitForRolloutStatus("Degraded").
Then().
// Expect that the canary service selector has been moved back to stable
ExpectServiceSelector("canary-scaledowndelay-canary", map[string]string{"app": "canary-scaledowndelay", "rollouts-pod-template-hash": "66597877b7"}, false).
When().
Sleep(3*time.Second).
Then().
ExpectRevisionPodCount("2", 0)
Expand All @@ -584,10 +580,6 @@ func (s *CanarySuite) TestCanaryScaleDownOnAbortNoTrafficRouting() {
WaitForRolloutStatus("Paused").
AbortRollout().
WaitForRolloutStatus("Degraded").
Then().
// Expect that the canary service selector has been moved back to stable
ExpectServiceSelector("canary-scaledowndelay-canary", map[string]string{"app": "canary-scaledowndelay", "rollouts-pod-template-hash": "66597877b7"}, false).
When().
Sleep(3*time.Second).
Then().
ExpectRevisionPodCount("2", 0)
Expand Down Expand Up @@ -663,8 +655,6 @@ func (s *CanarySuite) TestCanaryDynamicStableScale() {
WaitForRevisionPodCount("2", 0).
Sleep(2*time.Second). //WaitForRevisionPodCount does not wait for terminating pods and so ExpectServiceSelector fails sleep a bit for the terminating pods to be deleted
Then().
// Expect that the canary service selector is now set to stable because of dynamic stable scale is over and we have all pods up on stable rs
ExpectServiceSelector("dynamic-stable-scale-canary", map[string]string{"app": "dynamic-stable-scale", "rollouts-pod-template-hash": "868d98995b"}, false).
ExpectRevisionPodCount("1", 4)
}

Expand Down
Loading