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.

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

feat: Add RolloutCompleted condition #1074

Merged
merged 13 commits into from
Apr 20, 2021
2 changes: 2 additions & 0 deletions pkg/apis/rollouts/v1alpha1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,8 @@ const (
RolloutReplicaFailure RolloutConditionType = "ReplicaFailure"
// RolloutPaused means that rollout is in a paused state. It is still progressing at this point.
RolloutPaused RolloutConditionType = "Paused"
// RolloutCompleted means that rollout is in a completed state. It is still progressing at this point.
RolloutCompleted RolloutConditionType = "Completed"
)

// RolloutCondition describes the state of a rollout at a certain point.
Expand Down
2 changes: 1 addition & 1 deletion rollout/analysis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1628,7 +1628,7 @@ func TestDoNotCreatePrePromotionAnalysisAfterPromotionRollout(t *testing.T) {

f.run(getKey(r2, t))

newConditions := generateConditionsPatch(true, conditions.NewRSAvailableReason, rs2, true, "")
newConditions := generateConditionsPatchWithComplete(true, conditions.NewRSAvailableReason, rs2, true, "", true)
expectedPatch := fmt.Sprintf(`{
"status":{
"conditions":%s
Expand Down
46 changes: 45 additions & 1 deletion rollout/bluegreen_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package rollout

import (
"encoding/json"
"fmt"
"strconv"
"testing"
Expand Down Expand Up @@ -1012,7 +1013,7 @@ func TestBlueGreenRolloutCompleted(t *testing.T) {

f.run(getKey(r2, t))

newConditions := generateConditionsPatch(true, conditions.NewRSAvailableReason, rs2, true, "")
newConditions := generateConditionsPatchWithComplete(true, conditions.NewRSAvailableReason, rs2, true, "", true)
expectedPatch := fmt.Sprintf(`{
"status":{
"conditions":%s
Expand All @@ -1022,6 +1023,49 @@ func TestBlueGreenRolloutCompleted(t *testing.T) {
assert.Equal(t, cleanPatch(expectedPatch), patch)
}

func TestBlueGreenRolloutCompletedFalse(t *testing.T) {
f := newFixture(t)
defer f.Close()

r1 := newBlueGreenRollout("foo", 1, nil, "bar", "")
completedCondition, _ := newCompletedCondition(true)
conditions.SetRolloutCondition(&r1.Status, completedCondition)

r2 := bumpVersion(r1)
rs2 := newReplicaSetWithStatus(r2, 1, 1)
progressingCondition, _ := newProgressingCondition(conditions.PausedRolloutReason, rs2, "")
conditions.SetRolloutCondition(&r2.Status, progressingCondition)
pausedCondition, _ := newPausedCondition(true)
conditions.SetRolloutCondition(&r2.Status, pausedCondition)

f.kubeobjects = append(f.kubeobjects, rs2)
f.replicaSetLister = append(f.replicaSetLister, rs2)
rs2PodHash := rs2.Labels[v1alpha1.DefaultRolloutUniqueLabelKey]

serviceSelector := map[string]string{v1alpha1.DefaultRolloutUniqueLabelKey: rs2PodHash}
s := newService("bar", 80, serviceSelector, r2)
f.kubeobjects = append(f.kubeobjects, s)

r2 = updateBlueGreenRolloutStatus(r2, "", rs2PodHash, rs2PodHash, 1, 1, 1, 1, true, false)
r2.Status.ObservedGeneration = strconv.Itoa(int(r2.Generation))

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

patchIndex := f.expectPatchRolloutAction(r1)
f.run(getKey(r2, t))

patch := f.getPatchedRollout(patchIndex)
rolloutPatch := v1alpha1.Rollout{}
err := json.Unmarshal([]byte(patch), &rolloutPatch)
assert.NoError(t, err)

index := len(rolloutPatch.Status.Conditions) - 1
assert.Equal(t, v1alpha1.RolloutCompleted, rolloutPatch.Status.Conditions[index].Type)
assert.Equal(t, corev1.ConditionFalse, rolloutPatch.Status.Conditions[index].Status)
}

func TestBlueGreenUnableToReadScaleDownAt(t *testing.T) {
f := newFixture(t)
defer f.Close()
Expand Down
30 changes: 30 additions & 0 deletions rollout/controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,26 @@ func newPausedCondition(isPaused bool) (v1alpha1.RolloutCondition, string) {
return condition, string(conditionBytes)
}

func newCompletedCondition(isCompleted bool) (v1alpha1.RolloutCondition, string) {
status := corev1.ConditionTrue
if !isCompleted {
status = corev1.ConditionFalse
}
condition := v1alpha1.RolloutCondition{
LastTransitionTime: metav1.Now(),
LastUpdateTime: metav1.Now(),
Message: conditions.RolloutCompletedReason,
Reason: conditions.RolloutCompletedReason,
Status: status,
Type: v1alpha1.RolloutCompleted,
}
conditionBytes, err := json.Marshal(condition)
if err != nil {
panic(err)
}
return condition, string(conditionBytes)
}

func newProgressingCondition(reason string, resourceObj runtime.Object, optionalMessage string) (v1alpha1.RolloutCondition, string) {
status := corev1.ConditionTrue
msg := ""
Expand Down Expand Up @@ -307,6 +327,16 @@ func generateConditionsPatchWithPause(available bool, progressingReason string,
return fmt.Sprintf("[%s, %s, %s]", progressingCondition, pauseCondition, availableCondition)
}

func generateConditionsPatchWithComplete(available bool, progressingReason string, progressingResource runtime.Object, availableConditionFirst bool, progressingMessage string, isCompleted bool) string {
_, availableCondition := newAvailableCondition(available)
_, progressingCondition := newProgressingCondition(progressingReason, progressingResource, progressingMessage)
_, completeCondition := newCompletedCondition(isCompleted)
if availableConditionFirst {
return fmt.Sprintf("[%s, %s, %s]", availableCondition, completeCondition, progressingCondition)
}
return fmt.Sprintf("[%s, %s, %s]", completeCondition, progressingCondition, availableCondition)
}

// func updateBlueGreenRolloutStatus(r *v1alpha1.Rollout, preview, active string, availableReplicas, updatedReplicas, hpaReplicas int32, pause bool, available bool, progressingStatus string) *v1alpha1.Rollout {
func updateBlueGreenRolloutStatus(r *v1alpha1.Rollout, preview, active, stable string, availableReplicas, updatedReplicas, totalReplicas, hpaReplicas int32, pause bool, available bool) *v1alpha1.Rollout {
newRollout := updateBaseRolloutStatus(r, availableReplicas, updatedReplicas, totalReplicas, hpaReplicas)
Expand Down
20 changes: 17 additions & 3 deletions rollout/sync.go
Original file line number Diff line number Diff line change
Expand Up @@ -563,14 +563,28 @@ func isIndefiniteStep(r *v1alpha1.Rollout) bool {
}

func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutStatus) v1alpha1.RolloutStatus {
if len(c.rollout.Status.PauseConditions) > 0 || c.rollout.Spec.Paused {
isPaused := len(c.rollout.Status.PauseConditions) > 0 || c.rollout.Spec.Paused

completeCond := conditions.GetRolloutCondition(c.rollout.Status, v1alpha1.RolloutCompleted)
if !isPaused && conditions.RolloutComplete(c.rollout, &newStatus) {
updateCompletedCond := conditions.NewRolloutCondition(v1alpha1.RolloutCompleted, corev1.ConditionTrue, conditions.RolloutCompletedReason, conditions.RolloutCompletedReason)
conditions.SetRolloutCondition(&newStatus, *updateCompletedCond)
} else {
if completeCond != nil {
updateCompletedCond := conditions.NewRolloutCondition(v1alpha1.RolloutCompleted, corev1.ConditionFalse, conditions.RolloutCompletedReason, conditions.RolloutCompletedReason)
conditions.SetRolloutCondition(&newStatus, *updateCompletedCond)
}
}

if isPaused {
return newStatus
}

// If there is only one replica set that is active then that means we are not running
// a new rollout and this is a resync where we don't need to estimate any progress.
// In such a case, we should simply not estimate any progress for this rollout.
currentCond := conditions.GetRolloutCondition(c.rollout.Status, v1alpha1.RolloutProgressing)

isCompleteRollout := newStatus.Replicas == newStatus.AvailableReplicas && currentCond != nil && currentCond.Reason == conditions.NewRSAvailableReason
// Check for progress only if the latest rollout hasn't completed yet.
if !isCompleteRollout {
Expand All @@ -590,8 +604,8 @@ func (c *rolloutContext) calculateRolloutConditions(newStatus v1alpha1.RolloutSt
if c.newRS != nil {
msg = fmt.Sprintf(conditions.ReplicaSetCompletedMessage, c.newRS.Name)
}
condition := conditions.NewRolloutCondition(v1alpha1.RolloutProgressing, corev1.ConditionTrue, conditions.NewRSAvailableReason, msg)
conditions.SetRolloutCondition(&newStatus, *condition)
progressingCondition := conditions.NewRolloutCondition(v1alpha1.RolloutProgressing, corev1.ConditionTrue, conditions.NewRSAvailableReason, msg)
conditions.SetRolloutCondition(&newStatus, *progressingCondition)
case conditions.RolloutProgressing(c.rollout, &newStatus):
// If there is any progress made, continue by not checking if the rollout failed. This
// behavior emulates the rolling updater progressDeadline check.
Expand Down
61 changes: 61 additions & 0 deletions test/e2e/functional_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -904,3 +904,64 @@ spec:
}).
ExpectActiveRevision("2")
}

func (s *FunctionalSuite) TestKubectlWaitForCompleted() {
s.Given().
RolloutObjects(`
kind: Service
apiVersion: v1
metadata:
name: rollout-bluegreen-active
spec:
selector:
app: rollout-bluegreen
ports:
- protocol: TCP
port: 80
targetPort: 8080
---
apiVersion: argoproj.io/v1alpha1
kind: Rollout
metadata:
name: rollout-bluegreen
spec:
replicas: 1
revisionHistoryLimit: 2
selector:
matchLabels:
app: rollout-bluegreen
template:
metadata:
labels:
app: rollout-bluegreen
spec:
containers:
- name: rollouts-demo
image: argoproj/rollouts-demo:blue
imagePullPolicy: Always
ports:
- containerPort: 8080
strategy:
blueGreen:
activeService: rollout-bluegreen-active
autoPromotionEnabled: true
`).
When().
ApplyManifests().
WaitForRolloutReplicas(1).
WaitForRolloutStatus("Healthy").
UpdateSpec().
Then().
ExpectRollout("Completed", func(r *v1alpha1.Rollout) bool {
cmd := exec.Command("kubectl", "wait", "--for=condition=Completed=False", fmt.Sprintf("rollout/%s", r.Name))
return cmd.Run() == nil
}).
When().
PromoteRollout().
Then().
ExpectRollout("Completed", func(r *v1alpha1.Rollout) bool {
cmd := exec.Command("kubectl", "wait", "--for=condition=Completed=True", fmt.Sprintf("rollout/%s", r.Name))
return cmd.Run() == nil
}).
ExpectActiveRevision("2")
}
2 changes: 2 additions & 0 deletions utils/conditions/conditions.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ const (
// within the given deadline (progressDeadlineSeconds).
ReplicaSetTimeOutMessage = "ReplicaSet %q has timed out progressing."

// RolloutCompletedReason is added in a rollout when it is completed.
RolloutCompletedReason = "RolloutCompleted"
// RolloutCompletedMessage is added when the rollout is completed
RolloutCompletedMessage = "Rollout %q has successfully progressed."
// ReplicaSetCompletedMessage is added when the rollout is completed
Expand Down