pkg/{canary,controller}: remove unused skipLivenessChecks

pkg/canary/config_tracker_test.go

@@ -15,8 +15,7 @@ func TestConfigTracker_ConfigMaps(t *testing.T) {
 		configMap := newDeploymentControllerTestConfigMap()
 		configMapProjected := newDeploymentControllerTestConfigProjected()
 
-		err := mocks.controller.Initialize(mocks.canary, true)
-		require.NoError(t, err)
+		mocks.initializeCanary(t)
 
 		depPrimary, err := mocks.kubeClient.AppsV1().Deployments("default").Get("podinfo-primary", metav1.GetOptions{})
 		require.NoError(t, err)
@@ -53,7 +52,7 @@ func TestConfigTracker_ConfigMaps(t *testing.T) {
 		configMap := newDaemonSetControllerTestConfigMap()
 		configMapProjected := newDaemonSetControllerTestConfigProjected()
 
-		err := mocks.controller.Initialize(mocks.canary, true)
+		err := mocks.controller.Initialize(mocks.canary)
 		require.NoError(t, err)
 
 		depPrimary, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get("podinfo-primary", metav1.GetOptions{})
@@ -93,8 +92,7 @@ func TestConfigTracker_Secrets(t *testing.T) {
 		secret := newDeploymentControllerTestSecret()
 		secretProjected := newDeploymentControllerTestSecretProjected()
 
-		err := mocks.controller.Initialize(mocks.canary, true)
-		require.NoError(t, err)
+		mocks.initializeCanary(t)
 
 		depPrimary, err := mocks.kubeClient.AppsV1().Deployments("default").Get("podinfo-primary", metav1.GetOptions{})
 		if assert.NoError(t, err) {
@@ -131,8 +129,7 @@ func TestConfigTracker_Secrets(t *testing.T) {
 		secret := newDaemonSetControllerTestSecret()
 		secretProjected := newDaemonSetControllerTestSecretProjected()
 
-		err := mocks.controller.Initialize(mocks.canary, true)
-		require.NoError(t, err)
+		mocks.controller.Initialize(mocks.canary)
 
 		daePrimary, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get("podinfo-primary", metav1.GetOptions{})
 		if assert.NoError(t, err) {

pkg/canary/controller.go

@@ -13,7 +13,7 @@ type Controller interface {
 	SetStatusWeight(canary *flaggerv1.Canary, val int) error
 	SetStatusIterations(canary *flaggerv1.Canary, val int) error
 	SetStatusPhase(canary *flaggerv1.Canary, phase flaggerv1.CanaryPhase) error
-	Initialize(canary *flaggerv1.Canary, skipLivenessChecks bool) error
+	Initialize(canary *flaggerv1.Canary) error
 	Promote(canary *flaggerv1.Canary) error
 	HasTargetChanged(canary *flaggerv1.Canary) (bool, error)
 	HaveDependenciesChanged(canary *flaggerv1.Canary) (bool, error)

pkg/canary/daemonset_controller.go

@@ -74,17 +74,15 @@ func (c *DaemonSetController) ScaleFromZero(cd *flaggerv1.Canary) error {
 
 // Initialize creates the primary DaemonSet, scales down the canary DaemonSet,
 // and returns the pod selector label and container ports
-func (c *DaemonSetController) Initialize(cd *flaggerv1.Canary, skipLivenessChecks bool) (err error) {
+func (c *DaemonSetController) Initialize(cd *flaggerv1.Canary) (err error) {
 	err = c.createPrimaryDaemonSet(cd)
 	if err != nil {
 		return fmt.Errorf("createPrimaryDaemonSet failed: %w", err)
 	}
 
 	if cd.Status.Phase == "" || cd.Status.Phase == flaggerv1.CanaryPhaseInitializing {
-		if !skipLivenessChecks && !cd.SkipAnalysis() {
-			if err := c.IsPrimaryReady(cd); err != nil {
-				return fmt.Errorf("IsPrimaryReady failed: %w", err)
-			}
+		if err := c.IsPrimaryReady(cd); err != nil {
+			return fmt.Errorf("IsPrimaryReady failed: %w", err)
 		}
 
 		c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).

pkg/canary/daemonset_controller_test.go

@@ -15,7 +15,7 @@ import (
 
 func TestDaemonSetController_Sync(t *testing.T) {
 	mocks := newDaemonSetFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
+	err := mocks.controller.Initialize(mocks.canary)
 	require.NoError(t, err)
 
 	daePrimary, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get("podinfo-primary", metav1.GetOptions{})
@@ -29,7 +29,7 @@ func TestDaemonSetController_Sync(t *testing.T) {
 
 func TestDaemonSetController_Promote(t *testing.T) {
 	mocks := newDaemonSetFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
+	err := mocks.controller.Initialize(mocks.canary)
 	require.NoError(t, err)
 
 	dae2 := newDaemonSetControllerTestPodInfoV2()
@@ -60,7 +60,7 @@ func TestDaemonSetController_NoConfigTracking(t *testing.T) {
 	mocks := newDaemonSetFixture()
 	mocks.controller.configTracker = &NopTracker{}
 
-	err := mocks.controller.Initialize(mocks.canary, true)
+	err := mocks.controller.Initialize(mocks.canary)
 	require.NoError(t, err)
 
 	daePrimary, err := mocks.kubeClient.AppsV1().DaemonSets("default").Get("podinfo-primary", metav1.GetOptions{})
@@ -75,7 +75,7 @@ func TestDaemonSetController_NoConfigTracking(t *testing.T) {
 
 func TestDaemonSetController_HasTargetChanged(t *testing.T) {
 	mocks := newDaemonSetFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
+	err := mocks.controller.Initialize(mocks.canary)
 	require.NoError(t, err)
 
 	// save last applied hash
@@ -163,7 +163,7 @@ func TestDaemonSetController_HasTargetChanged(t *testing.T) {
 func TestDaemonSetController_Scale(t *testing.T) {
 	t.Run("ScaleToZero", func(t *testing.T) {
 		mocks := newDaemonSetFixture()
-		err := mocks.controller.Initialize(mocks.canary, true)
+		err := mocks.controller.Initialize(mocks.canary)
 		require.NoError(t, err)
 
 		err = mocks.controller.ScaleToZero(mocks.canary)
@@ -179,7 +179,7 @@ func TestDaemonSetController_Scale(t *testing.T) {
 	})
 	t.Run("ScaleFromZeo", func(t *testing.T) {
 		mocks := newDaemonSetFixture()
-		err := mocks.controller.Initialize(mocks.canary, true)
+		err := mocks.controller.Initialize(mocks.canary)
 		require.NoError(t, err)
 
 		err = mocks.controller.ScaleFromZero(mocks.canary)
@@ -197,7 +197,7 @@ func TestDaemonSetController_Scale(t *testing.T) {
 
 func TestDaemonSetController_Finalize(t *testing.T) {
 	mocks := newDaemonSetFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
+	err := mocks.controller.Initialize(mocks.canary)
 	require.NoError(t, err)
 
 	err = mocks.controller.Finalize(mocks.canary)

pkg/canary/daemonset_ready_test.go

@@ -4,7 +4,6 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	appsv1 "k8s.io/api/apps/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -14,8 +13,8 @@ import (
 
 func TestDaemonSetController_IsReady(t *testing.T) {
 	mocks := newDaemonSetFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
-	assert.NoError(t, err, "Expected primary readiness check to fail")
+	err := mocks.controller.Initialize(mocks.canary)
+	require.NoError(t, err)
 
 	err = mocks.controller.IsPrimaryReady(mocks.canary)
 	require.NoError(t, err)

pkg/canary/daemonset_status_test.go

@@ -12,7 +12,7 @@ import (
 
 func TestDaemonSetController_SyncStatus(t *testing.T) {
 	mocks := newDaemonSetFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
+	err := mocks.controller.Initialize(mocks.canary)
 	require.NoError(t, err)
 
 	status := flaggerv1.CanaryStatus{
@@ -36,7 +36,7 @@ func TestDaemonSetController_SyncStatus(t *testing.T) {
 
 func TestDaemonSetController_SetFailedChecks(t *testing.T) {
 	mocks := newDaemonSetFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
+	err := mocks.controller.Initialize(mocks.canary)
 	require.NoError(t, err)
 
 	err = mocks.controller.SetStatusFailedChecks(mocks.canary, 1)
@@ -49,7 +49,7 @@ func TestDaemonSetController_SetFailedChecks(t *testing.T) {
 
 func TestDaemonSetController_SetState(t *testing.T) {
 	mocks := newDaemonSetFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
+	err := mocks.controller.Initialize(mocks.canary)
 	require.NoError(t, err)
 
 	err = mocks.controller.SetStatusPhase(mocks.canary, flaggerv1.CanaryPhaseProgressing)

pkg/canary/deployment_controller.go

@@ -28,17 +28,15 @@ type DeploymentController struct {
 
 // Initialize creates the primary deployment, hpa,
 // scales to zero the canary deployment and returns the pod selector label and container ports
-func (c *DeploymentController) Initialize(cd *flaggerv1.Canary, skipLivenessChecks bool) (err error) {
+func (c *DeploymentController) Initialize(cd *flaggerv1.Canary) (err error) {
 	primaryName := fmt.Sprintf("%s-primary", cd.Spec.TargetRef.Name)
 	if err := c.createPrimaryDeployment(cd); err != nil {
 		return fmt.Errorf("createPrimaryDeployment failed: %w", err)
 	}
 
 	if cd.Status.Phase == "" || cd.Status.Phase == flaggerv1.CanaryPhaseInitializing {
-		if !skipLivenessChecks && !cd.SkipAnalysis() {
-			if err := c.IsPrimaryReady(cd); err != nil {
-				return fmt.Errorf("IsPrimaryReady failed: %w", err)
-			}
+		if err := c.IsPrimaryReady(cd); err != nil {
+			return fmt.Errorf("IsPrimaryReady failed: %w", err)
 		}
 
 		c.logger.With("canary", fmt.Sprintf("%s.%s", cd.Name, cd.Namespace)).

pkg/canary/deployment_controller_test.go

@@ -15,8 +15,7 @@ import (
 
 func TestDeploymentController_Sync(t *testing.T) {
 	mocks := newDeploymentFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
-	require.NoError(t, err)
+	mocks.initializeCanary(t)
 
 	depPrimary, err := mocks.kubeClient.AppsV1().Deployments("default").Get("podinfo-primary", metav1.GetOptions{})
 	require.NoError(t, err)
@@ -33,11 +32,10 @@ func TestDeploymentController_Sync(t *testing.T) {
 
 func TestDeploymentController_Promote(t *testing.T) {
 	mocks := newDeploymentFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
-	require.NoError(t, err)
+	mocks.initializeCanary(t)
 
 	dep2 := newDeploymentControllerTestV2()
-	_, err = mocks.kubeClient.AppsV1().Deployments("default").Update(dep2)
+	_, err := mocks.kubeClient.AppsV1().Deployments("default").Update(dep2)
 	require.NoError(t, err)
 
 	config2 := newDeploymentControllerTestConfigMapV2()
@@ -74,10 +72,9 @@ func TestDeploymentController_Promote(t *testing.T) {
 
 func TestDeploymentController_ScaleToZero(t *testing.T) {
 	mocks := newDeploymentFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
-	require.NoError(t, err)
+	mocks.initializeCanary(t)
 
-	err = mocks.controller.ScaleToZero(mocks.canary)
+	err := mocks.controller.ScaleToZero(mocks.canary)
 	require.NoError(t, err)
 
 	c, err := mocks.kubeClient.AppsV1().Deployments("default").Get("podinfo", metav1.GetOptions{})
@@ -88,9 +85,7 @@ func TestDeploymentController_ScaleToZero(t *testing.T) {
 func TestDeploymentController_NoConfigTracking(t *testing.T) {
 	mocks := newDeploymentFixture()
 	mocks.controller.configTracker = &NopTracker{}
-
-	err := mocks.controller.Initialize(mocks.canary, true)
-	require.NoError(t, err)
+	mocks.initializeCanary(t)
 
 	depPrimary, err := mocks.kubeClient.AppsV1().Deployments("default").Get("podinfo-primary", metav1.GetOptions{})
 	require.NoError(t, err)
@@ -104,8 +99,7 @@ func TestDeploymentController_NoConfigTracking(t *testing.T) {
 
 func TestDeploymentController_HasTargetChanged(t *testing.T) {
 	mocks := newDeploymentFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
-	require.NoError(t, err)
+	mocks.initializeCanary(t)
 
 	// save last applied hash
 	canary, err := mocks.flaggerClient.FlaggerV1beta1().Canaries("default").Get("podinfo", metav1.GetOptions{})
@@ -190,46 +184,35 @@ func TestDeploymentController_HasTargetChanged(t *testing.T) {
 }
 
 func TestDeploymentController_Finalize(t *testing.T) {
-
 	mocks := newDeploymentFixture()
 
-	tables := []struct {
+	for _, tc := range []struct {
 		mocks            deploymentControllerFixture
 		callInitialize   bool
 		shouldError      bool
 		expectedReplicas int32
 		canary           *flaggerv1.Canary
 	}{
-		//Primary not found returns error
+		// primary not found returns error
 		{mocks, false, false, 1, mocks.canary},
-		//Happy path
+		// happy path
 		{mocks, true, false, 1, mocks.canary},
-	}
-
-	for _, table := range tables {
-		if table.callInitialize {
-			err := mocks.controller.Initialize(table.canary, true)
-			if err != nil {
-				t.Fatal(err.Error())
-			}
+	} {
+		if tc.callInitialize {
+			mocks.initializeCanary(t)
 		}
 
-		err := mocks.controller.Finalize(table.canary)
-
-		if table.shouldError && err == nil {
-			t.Error("Expected error while calling Finalize, but none was returned")
-		} else if !table.shouldError && err != nil {
-			t.Errorf("Expected no error would be returned while calling Finalize, but returned %s", err)
+		err := mocks.controller.Finalize(tc.canary)
+		if tc.shouldError {
+			require.Error(t, err)
+		} else {
+			require.NoError(t, err)
 		}
 
-		if table.expectedReplicas > 0 {
+		if tc.expectedReplicas > 0 {
 			c, err := mocks.kubeClient.AppsV1().Deployments(mocks.canary.Namespace).Get(mocks.canary.Name, metav1.GetOptions{})
-			if err != nil {
-				t.Fatal(err.Error())
-			}
-			if int32Default(c.Spec.Replicas) != table.expectedReplicas {
-				t.Errorf("Expected replicas %d recieved replicas %d", table.expectedReplicas, c.Spec.Replicas)
-			}
+			require.NoError(t, err)
+			require.Equal(t, tc.expectedReplicas, *c.Spec.Replicas)
 		}
 	}
 }

pkg/canary/deployment_fixture_test.go

@@ -1,6 +1,10 @@
 package canary
 
 import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
 	appsv1 "k8s.io/api/apps/v1"
 	hpav2 "k8s.io/api/autoscaling/v2beta1"
@@ -24,6 +28,28 @@ type deploymentControllerFixture struct {
 	logger        *zap.SugaredLogger
 }
 
+func (d deploymentControllerFixture) initializeCanary(t *testing.T) {
+	err := d.controller.Initialize(d.canary)
+	require.Error(t, err) // not ready yet
+
+	primaryName := fmt.Sprintf("%s-primary", d.canary.Spec.TargetRef.Name)
+	p, err := d.controller.kubeClient.AppsV1().
+		Deployments(d.canary.Namespace).Get(primaryName, metav1.GetOptions{})
+	require.NoError(t, err)
+
+	p.Status = appsv1.DeploymentStatus{
+		Replicas:          1,
+		UpdatedReplicas:   1,
+		ReadyReplicas:     1,
+		AvailableReplicas: 1,
+	}
+
+	_, err = d.controller.kubeClient.AppsV1().Deployments(d.canary.Namespace).Update(p)
+	require.NoError(t, err)
+
+	require.NoError(t, d.controller.Initialize(d.canary))
+}
+
 func newDeploymentFixture() deploymentControllerFixture {
 	// init canary
 	canary := newDeploymentControllerTestCanary()

pkg/canary/deployment_ready_test.go

@@ -8,12 +8,13 @@ import (
 
 func TestDeploymentController_IsReady(t *testing.T) {
 	mocks := newDeploymentFixture()
-	err := mocks.controller.Initialize(mocks.canary, true)
-	require.NoError(t, err, "Expected primary readiness check to fail")
+	mocks.controller.Initialize(mocks.canary)
 
-	err = mocks.controller.IsPrimaryReady(mocks.canary)
+	err := mocks.controller.IsPrimaryReady(mocks.canary)
 	require.Error(t, err)
 
 	_, err = mocks.controller.IsCanaryReady(mocks.canary)
 	require.NoError(t, err)
 }
+
+// TODO: more detailed tests as daemonset