From bbdac24ed3d8d899ad0ec22b498b6793ccac5550 Mon Sep 17 00:00:00 2001 From: mathetake Date: Sat, 4 Apr 2020 10:48:29 +0900 Subject: [PATCH] pkg/canary: add unit test of isDeploymentReady --- pkg/canary/daemonset_ready_test.go | 20 +++---- pkg/canary/deployment_ready.go | 1 - pkg/canary/deployment_ready_test.go | 83 ++++++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 12 deletions(-) diff --git a/pkg/canary/daemonset_ready_test.go b/pkg/canary/daemonset_ready_test.go index 006c6046f..99cbda2b0 100644 --- a/pkg/canary/daemonset_ready_test.go +++ b/pkg/canary/daemonset_ready_test.go @@ -30,9 +30,9 @@ func TestDaemonSetController_isDaemonSetReady(t *testing.T) { // observed generation is less than desired generation ds := &appsv1.DaemonSet{Status: appsv1.DaemonSetStatus{}} ds.Status.ObservedGeneration-- - retyable, err := mocks.controller.isDaemonSetReady(cd, ds) + retryable, err := mocks.controller.isDaemonSetReady(cd, ds) require.Error(t, err) - require.True(t, retyable) + require.True(t, retryable) // succeeded ds = &appsv1.DaemonSet{Status: appsv1.DaemonSetStatus{ @@ -40,9 +40,9 @@ func TestDaemonSetController_isDaemonSetReady(t *testing.T) { DesiredNumberScheduled: 1, NumberAvailable: 1, }} - retyable, err = mocks.controller.isDaemonSetReady(cd, ds) + retryable, err = mocks.controller.isDaemonSetReady(cd, ds) require.NoError(t, err) - require.True(t, retyable) + require.True(t, retryable) // deadline exceeded ds = &appsv1.DaemonSet{Status: appsv1.DaemonSetStatus{ @@ -51,9 +51,9 @@ func TestDaemonSetController_isDaemonSetReady(t *testing.T) { }} cd.Status.LastTransitionTime = metav1.Now() cd.Spec.ProgressDeadlineSeconds = int32p(-1e6) - retyable, err = mocks.controller.isDaemonSetReady(cd, ds) + retryable, err = mocks.controller.isDaemonSetReady(cd, ds) require.Error(t, err) - require.False(t, retyable) + require.False(t, retryable) // only newCond not satisfied ds = &appsv1.DaemonSet{Status: appsv1.DaemonSetStatus{ @@ -62,9 +62,9 @@ func TestDaemonSetController_isDaemonSetReady(t *testing.T) { NumberAvailable: 1, }} cd.Spec.ProgressDeadlineSeconds = int32p(1e6) - retyable, err = mocks.controller.isDaemonSetReady(cd, ds) + retryable, err = mocks.controller.isDaemonSetReady(cd, ds) require.Error(t, err) - require.True(t, retyable) + require.True(t, retryable) require.True(t, strings.Contains(err.Error(), "new pods")) // only availableCond not satisfied @@ -73,8 +73,8 @@ func TestDaemonSetController_isDaemonSetReady(t *testing.T) { DesiredNumberScheduled: 1, NumberAvailable: 0, }} - retyable, err = mocks.controller.isDaemonSetReady(cd, ds) + retryable, err = mocks.controller.isDaemonSetReady(cd, ds) require.Error(t, err) - require.True(t, retyable) + require.True(t, retryable) require.True(t, strings.Contains(err.Error(), "available")) } diff --git a/pkg/canary/deployment_ready.go b/pkg/canary/deployment_ready.go index a16673f67..293d8f83a 100644 --- a/pkg/canary/deployment_ready.go +++ b/pkg/canary/deployment_ready.go @@ -81,7 +81,6 @@ func (c *DeploymentController) isDeploymentReady(deployment *appsv1.Deployment, return retriable, fmt.Errorf("waiting for rollout to finish: %d of %d updated replicas are available", deployment.Status.AvailableReplicas, deployment.Status.UpdatedReplicas) } - } else { return true, fmt.Errorf( "waiting for rollout to finish: observed deployment generation less then desired generation") diff --git a/pkg/canary/deployment_ready_test.go b/pkg/canary/deployment_ready_test.go index 8289f9827..e3d076782 100644 --- a/pkg/canary/deployment_ready_test.go +++ b/pkg/canary/deployment_ready_test.go @@ -1,9 +1,14 @@ package canary import ( + "strings" "testing" + "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + appsv1 "k8s.io/api/apps/v1" + v1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) func TestDeploymentController_IsReady(t *testing.T) { @@ -17,4 +22,80 @@ func TestDeploymentController_IsReady(t *testing.T) { require.NoError(t, err) } -// TODO: more detailed tests as daemonset +func TestDeploymentController_isDeploymentReady(t *testing.T) { + mocks := newDeploymentFixture() + + // observed generation is less than desired generation + dp := &appsv1.Deployment{Status: appsv1.DeploymentStatus{ObservedGeneration: -1}} + retryable, err := mocks.controller.isDeploymentReady(dp, 0) + assert.Error(t, err) + assert.True(t, retryable) + assert.True(t, strings.Contains(err.Error(), "generation")) + + // ok + dp = &appsv1.Deployment{Status: appsv1.DeploymentStatus{ + Replicas: 1, + UpdatedReplicas: 1, + ReadyReplicas: 1, + AvailableReplicas: 1, + }} + retryable, err = mocks.controller.isDeploymentReady(dp, 0) + assert.NoError(t, err) + assert.True(t, retryable) + + // old + dp = &appsv1.Deployment{Status: appsv1.DeploymentStatus{ + UpdatedReplicas: 1, + }, Spec: appsv1.DeploymentSpec{ + Replicas: int32p(2), + }} + _, err = mocks.controller.isDeploymentReady(dp, 0) + assert.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "new replicas")) + + // waiting for old replicas to be terminated + dp = &appsv1.Deployment{Status: appsv1.DeploymentStatus{ + UpdatedReplicas: 1, + Replicas: 2, + }} + _, err = mocks.controller.isDeploymentReady(dp, 0) + assert.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "termination")) + + // waiting for updated ones to be available + dp = &appsv1.Deployment{Status: appsv1.DeploymentStatus{ + UpdatedReplicas: 2, + AvailableReplicas: 1, + }} + _, err = mocks.controller.isDeploymentReady(dp, 0) + assert.Error(t, err) + assert.True(t, strings.Contains(err.Error(), "available")) + + // ProgressDeadlineExceeded + dp = &appsv1.Deployment{Status: appsv1.DeploymentStatus{ + Conditions: []appsv1.DeploymentCondition{ + {Type: appsv1.DeploymentProgressing, Reason: "ProgressDeadlineExceeded"}}, + }} + retryable, err = mocks.controller.isDeploymentReady(dp, 0) + assert.Error(t, err) + assert.False(t, retryable) + assert.True(t, strings.Contains(err.Error(), "exceeded")) + + // deadline exceeded + dp = &appsv1.Deployment{Status: appsv1.DeploymentStatus{ + Conditions: []appsv1.DeploymentCondition{ + {Type: appsv1.DeploymentProgressing}, + { + Type: appsv1.DeploymentAvailable, + Status: "False", + Reason: "MinimumReplicasUnavailable", + LastUpdateTime: v1.NewTime(time.Now().Add(-10 * time.Second)), + }, + }, + UpdatedReplicas: 2, + AvailableReplicas: 1, + }} + retryable, err = mocks.controller.isDeploymentReady(dp, 5) + assert.Error(t, err) + assert.False(t, retryable) +}