From 529f84553b11cca1e1bd71d016c6159dd4cd3490 Mon Sep 17 00:00:00 2001
From: Vincent Hou <shou@us.ibm.com>
Date: Wed, 1 Apr 2020 14:07:00 -0400
Subject: [PATCH] Fix the retry issue with deployment verification

---
 test/resources/knativeserving.go | 25 ++++++++++++-------------
 test/resources/verify.go         |  2 +-
 2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/test/resources/knativeserving.go b/test/resources/knativeserving.go
index ab5b5c7a..02fd3697 100644
--- a/test/resources/knativeserving.go
+++ b/test/resources/knativeserving.go
@@ -132,41 +132,40 @@ func getTestKSOperatorCRSpec() v1alpha1.KnativeServingSpec {
 
 // WaitForKnativeServingDeploymentState polls the status of the Knative deployments every `interval`
 // until `inState` returns `true` indicating the deployments match the desired deployments.
-func WaitForKnativeServingDeploymentState(clients *test.Clients, namespace string, expectedDeployments []string,
-	inState func(deps *v1.DeploymentList, expectedDeployments []string, err error) (bool, error)) (*v1alpha1.KnativeServing, error) {
+func WaitForKnativeServingDeploymentState(clients *test.Clients, namespace string, expectedDeployments []string, logf logging.FormatLogger,
+	inState func(deps *v1.DeploymentList, expectedDeployments []string, err error, logf logging.FormatLogger) (bool, error)) error {
 	span := logging.GetEmitableSpan(context.Background(), fmt.Sprintf("WaitForKnativeDeploymentState/%s/%s", expectedDeployments, "KnativeDeploymentIsReady"))
 	defer span.End()
 
-	var lastState *v1alpha1.KnativeServing
 	waitErr := wait.PollImmediate(Interval, Timeout, func() (bool, error) {
 		dpList, err := clients.KubeClient.Kube.AppsV1().Deployments(namespace).List(metav1.ListOptions{})
-		return inState(dpList, expectedDeployments, err)
+		return inState(dpList, expectedDeployments, err, logf)
 	})
 
 	if waitErr != nil {
-		return lastState, waitErr
+		return waitErr
 	}
-	return lastState, nil
+	return nil
 }
 
 // IsKnativeServingDeploymentReady will check the status conditions of the deployments and return true if the deployments meet the desired status.
-func IsKnativeServingDeploymentReady(dpList *v1.DeploymentList, expectedDeployments []string, err error) (bool, error) {
+func IsKnativeServingDeploymentReady(dpList *v1.DeploymentList, expectedDeployments []string, err error, logf logging.FormatLogger) (bool, error) {
 	if err != nil {
 		return false, err
 	}
 	if len(dpList.Items) != len(expectedDeployments) {
-		errMessage := fmt.Sprintf("The expected number of deployments is %v, and got %v.", len(expectedDeployments), len(dpList.Items))
-		return false, errors.New(errMessage)
+		logf("The expected number of deployments is %v, and got %v.", len(expectedDeployments), len(dpList.Items))
+		return false, nil
 	}
 	for _, deployment := range dpList.Items {
 		if !stringInList(deployment.Name, expectedDeployments) {
-			errMessage := fmt.Sprintf("The deployment %v is not found in the expected list of deployment.", deployment.Name)
-			return false, errors.New(errMessage)
+			logf("The deployment %v is not found in the expected list of deployment.", deployment.Name)
+			return false, nil
 		}
 		for _, c := range deployment.Status.Conditions {
 			if c.Type == v1.DeploymentAvailable && c.Status != corev1.ConditionTrue {
-				errMessage := fmt.Sprintf("The deployment %v is not ready.", deployment.Name)
-				return false, errors.New(errMessage)
+				logf("The deployment %v is not ready.", deployment.Name)
+				return false, nil
 			}
 		}
 	}
diff --git a/test/resources/verify.go b/test/resources/verify.go
index 08895e63..88a3aef8 100644
--- a/test/resources/verify.go
+++ b/test/resources/verify.go
@@ -237,7 +237,7 @@ func verifyNoKSOperatorCR(clients *test.Clients) error {
 
 // AssertKSOperatorDeploymentStatus verifies if the Knative deployments reach the READY status.
 func AssertKSOperatorDeploymentStatus(t *testing.T, clients *test.Clients, namespace string, expectedDeployments []string) {
-	if _, err := WaitForKnativeServingDeploymentState(clients, namespace, expectedDeployments,
+	if err := WaitForKnativeServingDeploymentState(clients, namespace, expectedDeployments, t.Logf,
 		IsKnativeServingDeploymentReady); err != nil {
 		t.Fatalf("Knative Serving deployments failed to meet the expected deployments: %v", err)
 	}