Improve Available Condition

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)

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)
 	})

controller/controller_test.go

@@ -155,18 +155,18 @@ 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
 
 	}
 	condition := v1alpha1.RolloutCondition{
 		LastTransitionTime: metav1.Now(),
 		LastUpdateTime:     metav1.Now(),
 		Message:            message,
-		Reason:             "Available",
+		Reason:             conditions.AvailableReason,
 		Status:             status,
 		Type:               v1alpha1.RolloutAvailable,
 	}

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
-}

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)
-}
+}

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)
 	}
 

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
 

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
+}

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"))
+
+}