diff --git a/controller/bluegreen.go b/controller/bluegreen.go index 93592fd725..45b3142ac7 100644 --- a/controller/bluegreen.go +++ b/controller/bluegreen.go @@ -178,7 +178,7 @@ func (c *Controller) syncRolloutStatusBlueGreen(oldRSs []*appsv1.ReplicaSet, new } newStatus.BlueGreen.ActiveSelector = activeSelector - activeRS := GetActiveReplicaSet(r, allRSs) + activeRS := replicasetutil.GetActiveReplicaSet(allRSs, newStatus.BlueGreen.ActiveSelector) if activeRS != nil { newStatus.HPAReplicas = replicasetutil.GetActualReplicaCountForReplicaSets([]*appsv1.ReplicaSet{activeRS}) newStatus.Selector = metav1.FormatLabelSelector(activeRS.Spec.Selector) @@ -224,7 +224,7 @@ func (c *Controller) scaleBlueGreen(rollout *v1alpha1.Rollout, newRS *appsv1.Rep } allRS := append([]*appsv1.ReplicaSet{newRS}, oldRSs...) - activeRS := GetActiveReplicaSet(rollout, allRS) + activeRS := replicasetutil.GetActiveReplicaSet(allRS, rollout.Status.BlueGreen.ActiveSelector) if activeRS != nil { if *(activeRS.Spec.Replicas) != rolloutReplicas { _, _, err := c.scaleReplicaSetAndRecordEvent(activeRS, rolloutReplicas, rollout) diff --git a/controller/bluegreen_test.go b/controller/bluegreen_test.go index fc89ab0e71..b05b44ba42 100644 --- a/controller/bluegreen_test.go +++ b/controller/bluegreen_test.go @@ -228,16 +228,18 @@ func TestBlueGreenHandlePause(t *testing.T) { generatedConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs2.Name, true) now := metav1.Now().UTC().Format(time.RFC3339) + newSelector := metav1.FormatLabelSelector(rs2.Spec.Selector) expectedPatchWithoutSubs := `{ "status": { "blueGreen": { "activeSelector": "%s", "scaleDownDelayStartTime": "%s" }, - "conditions": %s + "conditions": %s, + "selector": "%s" } }` - expectedPatch := calculatePatch(r2, fmt.Sprintf(expectedPatchWithoutSubs, rs2PodHash, now, generatedConditions)) + expectedPatch := calculatePatch(r2, fmt.Sprintf(expectedPatchWithoutSubs, rs2PodHash, now, generatedConditions, newSelector)) patchIndex := f.expectPatchRolloutActionWithPatch(r2, expectedPatch) f.run(getKey(r2, t)) @@ -273,12 +275,14 @@ func TestBlueGreenHandlePause(t *testing.T) { "blueGreen": { "activeSelector": "%s" }, - "conditions": %s + "conditions": %s, + "selector": "%s" } }` generateConditions := generateConditionsPatch(true, conditions.ReplicaSetUpdatedReason, rs1.Name, false) - expectedPatch := calculatePatch(r1, fmt.Sprintf(expectedPatchWithoutSubs, rs1PodHash, generateConditions)) + newSelector := metav1.FormatLabelSelector(rs1.Spec.Selector) + expectedPatch := calculatePatch(r1, fmt.Sprintf(expectedPatchWithoutSubs, rs1PodHash, generateConditions, newSelector)) patchRolloutIndex := f.expectPatchRolloutActionWithPatch(r1, expectedPatch) f.run(getKey(r1, t)) @@ -341,10 +345,12 @@ func TestBlueGreenHandlePause(t *testing.T) { "scaleDownDelayStartTime": "%s" }, "pauseStartTime": null, - "conditions": %s + "conditions": %s, + "selector": "%s" } }` - expected2ndPatch := calculatePatch(r2, fmt.Sprintf(expected2ndPatchWithoutSubs, rs2PodHash, now.UTC().Format(time.RFC3339), generatedConditions)) + newSelector := metav1.FormatLabelSelector(rs2.Spec.Selector) + expected2ndPatch := calculatePatch(r2, fmt.Sprintf(expected2ndPatchWithoutSubs, rs2PodHash, now.UTC().Format(time.RFC3339), generatedConditions, newSelector)) rollout2ndPatch := f.getPatchedRollout(patchRolloutIndex) assert.Equal(t, expected2ndPatch, rollout2ndPatch) }) diff --git a/controller/controller_test.go b/controller/controller_test.go index 3f7c5902ff..8f4fb2a4d2 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -155,10 +155,10 @@ func newProgressingCondition(reason, resourceName string) (v1alpha1.RolloutCondi } func newAvailableCondition(available bool) (v1alpha1.RolloutCondition, string) { - message := "Rollout is not serving traffic from the active service." + message := conditions.NotAvailableMessage status := corev1.ConditionFalse if available { - message = "Rollout is serving traffic from the active service." + message = conditions.AvailableMessage status = corev1.ConditionTrue } @@ -166,7 +166,7 @@ func newAvailableCondition(available bool) (v1alpha1.RolloutCondition, string) { LastTransitionTime: metav1.Now(), LastUpdateTime: metav1.Now(), Message: message, - Reason: "Available", + Reason: conditions.AvailableReason, Status: status, Type: v1alpha1.RolloutAvailable, } diff --git a/controller/service.go b/controller/service.go index 3b7bf52048..2363d349d9 100644 --- a/controller/service.go +++ b/controller/service.go @@ -157,21 +157,3 @@ func (c *Controller) getRolloutSelectorLabel(svc *corev1.Service) (string, bool) currentSelectorValue, ok := svc.Spec.Selector[v1alpha1.DefaultRolloutUniqueLabelKey] return currentSelectorValue, ok } - -// GetActiveReplicaSet finds the replicaset that is serving traffic from the active service or returns nil -func GetActiveReplicaSet(rollout *v1alpha1.Rollout, allRS []*appsv1.ReplicaSet) *appsv1.ReplicaSet { - if rollout.Status.BlueGreen.ActiveSelector == "" { - return nil - } - for _, rs := range allRS { - if rs == nil { - continue - } - if podHash, ok := rs.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]; ok { - if podHash == rollout.Status.BlueGreen.ActiveSelector { - return rs - } - } - } - return nil -} diff --git a/controller/service_test.go b/controller/service_test.go index db62f3f43d..94c650731e 100644 --- a/controller/service_test.go +++ b/controller/service_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -146,33 +145,6 @@ func TestReconcileActiveService(t *testing.T) { } } -func TestGetActiveReplicaSet(t *testing.T) { - rolloutNoActiveSelector := &v1alpha1.Rollout{} - assert.Nil(t, GetActiveReplicaSet(rolloutNoActiveSelector, nil)) - - rollout := &v1alpha1.Rollout{ - Status: v1alpha1.RolloutStatus{ - BlueGreen: v1alpha1.BlueGreenStatus{ - ActiveSelector: "1234", - }, - }, - } - rs1 := &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: "abcd"}, - }, - } - assert.Nil(t, GetActiveReplicaSet(rollout, []*appsv1.ReplicaSet{rs1})) - - rs2 := &appsv1.ReplicaSet{ - ObjectMeta: metav1.ObjectMeta{ - Labels: map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: "1234"}, - }, - } - assert.Equal(t, rs2, GetActiveReplicaSet(rollout, []*appsv1.ReplicaSet{nil, rs1, rs2})) - -} - func TestGetPreviewAndActiveServices(t *testing.T) { f := newFixture(t) @@ -271,4 +243,4 @@ func TestPreviewServiceNotFound(t *testing.T) { }` _, pausedCondition := newProgressingCondition(conditions.ServiceNotFoundReason, notUsedPreviewSvc.Name) assert.Equal(t, calculatePatch(r, fmt.Sprintf(expectedPatch, pausedCondition)), patch) -} \ No newline at end of file +} diff --git a/controller/sync.go b/controller/sync.go index b6ff6a0271..922ce782ec 100644 --- a/controller/sync.go +++ b/controller/sync.go @@ -456,11 +456,15 @@ func (c *Controller) calculateRolloutConditions(r *v1alpha1.Rollout, newStatus v } } - if newRS != nil && annotations.IsSaturated(r, newRS) { - availability := conditions.NewRolloutCondition(v1alpha1.RolloutAvailable, corev1.ConditionTrue, conditions.Available, "Rollout is serving traffic from the active service.") + activeRS := replicasetutil.GetActiveReplicaSet(allRSs, newStatus.BlueGreen.ActiveSelector) + if r.Spec.Strategy.BlueGreenStrategy != nil && activeRS != nil && annotations.IsSaturated(r, activeRS) { + availability := conditions.NewRolloutCondition(v1alpha1.RolloutAvailable, corev1.ConditionTrue, conditions.AvailableReason, conditions.AvailableMessage) + conditions.SetRolloutCondition(&newStatus, *availability) + } else if r.Spec.Strategy.CanaryStrategy != nil && replicasetutil.GetAvailableReplicaCountForReplicaSets(allRSs) > defaults.GetRolloutReplicasOrDefault(r) { + availability := conditions.NewRolloutCondition(v1alpha1.RolloutAvailable, corev1.ConditionTrue, conditions.AvailableReason, conditions.AvailableMessage) conditions.SetRolloutCondition(&newStatus, *availability) } else { - noAvailability := conditions.NewRolloutCondition(v1alpha1.RolloutAvailable, corev1.ConditionFalse, conditions.Available, "Rollout is not serving traffic from the active service.") + noAvailability := conditions.NewRolloutCondition(v1alpha1.RolloutAvailable, corev1.ConditionFalse, conditions.AvailableReason, conditions.NotAvailableMessage) conditions.SetRolloutCondition(&newStatus, *noAvailability) } diff --git a/utils/conditions/conditions.go b/utils/conditions/conditions.go index 7e3df9cce4..98a2ffe49f 100644 --- a/utils/conditions/conditions.go +++ b/utils/conditions/conditions.go @@ -40,8 +40,12 @@ const ( InvalidStrategyMessage = "Multiple Strategies can not be listed" // DuplicatedServicesMessage the message to indicate that the rollout uses the same service for the active and preview services DuplicatedServicesMessage = "This rollout uses the same service for the active and preview services, but two different services are required." - // Available the reason to indicate that the rollout is serving traffic from the active service - Available = "Available" + // AvailableReason the reason to indicate that the rollout is serving traffic from the active service + AvailableReason = "AvailableReason" + // NotAvailableMessage the message to indicate that the Rollout does not have min availability + NotAvailableMessage = "Rollout does not have minimum availability" + // AvailableMessage the message to indicate that the Rollout does have min availability + AvailableMessage = "Rollout has minimum availability" // Reasons and Messages for rollout Progressing Condition diff --git a/utils/replicaset/replicaset.go b/utils/replicaset/replicaset.go index c996822064..b046456f04 100644 --- a/utils/replicaset/replicaset.go +++ b/utils/replicaset/replicaset.go @@ -238,3 +238,21 @@ func ResetCurrentStepIndex(rollout *v1alpha1.Rollout) *int32 { } return nil } + +// GetActiveReplicaSet finds the replicaset that is serving traffic from the active service or returns nil +func GetActiveReplicaSet(allRS []*appsv1.ReplicaSet, activeSelector string) *appsv1.ReplicaSet { + if activeSelector == "" { + return nil + } + for _, rs := range allRS { + if rs == nil { + continue + } + if podHash, ok := rs.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]; ok { + if podHash == activeSelector { + return rs + } + } + } + return nil +} diff --git a/utils/replicaset/replicaset_test.go b/utils/replicaset/replicaset_test.go index 48fed5f06d..a75fc17294 100644 --- a/utils/replicaset/replicaset_test.go +++ b/utils/replicaset/replicaset_test.go @@ -569,3 +569,21 @@ func TestResetCurrentStepIndex(t *testing.T) { assert.Nil(t, newStepIndex) } + +func TestGetActiveReplicaSet(t *testing.T) { + assert.Nil(t, GetActiveReplicaSet(nil, "")) + rs1 := &appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: "abcd"}, + }, + } + assert.Nil(t, GetActiveReplicaSet([]*appsv1.ReplicaSet{rs1}, "1234")) + + rs2 := &appsv1.ReplicaSet{ + ObjectMeta: metav1.ObjectMeta{ + Labels: map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: "1234"}, + }, + } + assert.Equal(t, rs2, GetActiveReplicaSet([]*appsv1.ReplicaSet{nil, rs1, rs2}, "1234")) + +} \ No newline at end of file