From 89a8e744f5fddcd4838e6c910ba0ab3381a395c7 Mon Sep 17 00:00:00 2001
From: Vincent Hou <shou@us.ibm.com>
Date: Tue, 31 Mar 2020 11:59:24 -0400
Subject: [PATCH] Improve the upgrade tests for better verification of the
 resources

---
 .../knativeeventing/knativeeventing.go        |  7 +++
 test/e2e-upgrade-tests.sh                     | 14 +++--
 test/resources/knativeeventing.go             | 40 ++++++++++++++
 test/resources/verify.go                      | 24 +++++++++
 .../upgrade/knativeeventingpreupgrade_test.go | 50 ++++++++++++++++++
 test/upgrade/knativeeventingupgrade_test.go   | 52 ++-----------------
 6 files changed, 134 insertions(+), 53 deletions(-)
 create mode 100644 test/upgrade/knativeeventingpreupgrade_test.go

diff --git a/pkg/reconciler/knativeeventing/knativeeventing.go b/pkg/reconciler/knativeeventing/knativeeventing.go
index 0b6dd998..32733910 100644
--- a/pkg/reconciler/knativeeventing/knativeeventing.go
+++ b/pkg/reconciler/knativeeventing/knativeeventing.go
@@ -361,5 +361,12 @@ func (r *Reconciler) deleteObsoleteResources(manifest *mf.Manifest, instance *ev
 		return err
 	}
 
+	// Remove the deployment sources-controller at 0.13
+	resource.SetAPIVersion("apps/v1")
+	resource.SetKind("deployment")
+	resource.SetName("sources-controller")
+	if err := manifest.Client.Delete(resource); err != nil {
+		return err
+	}
 	return nil
 }
diff --git a/test/e2e-upgrade-tests.sh b/test/e2e-upgrade-tests.sh
index f2ea9599..ce36aef9 100755
--- a/test/e2e-upgrade-tests.sh
+++ b/test/e2e-upgrade-tests.sh
@@ -30,7 +30,7 @@ source $(dirname $0)/../vendor/knative.dev/test-infra/scripts/e2e-tests.sh
 
 # Latest eventing operator release.
 readonly LATEST_EVENTING_OPERATOR_RELEASE_VERSION=$(git tag | sort -V | tail -1)
-readonly LATEST_EVENTING_RELEASE_VERSION="v0.13.3"
+readonly LATEST_EVENTING_RELEASE_VERSION="v0.13.4"
 
 OPERATOR_DIR=$(dirname $0)/..
 KNATIVE_EVENTING_DIR=${OPERATOR_DIR}/..
@@ -61,13 +61,13 @@ function install_latest_operator_release() {
       || fail_test "Unable to download latest Knative Eventing Operator release."
 
   kubectl apply -f "${release_yaml}" || fail_test "Knative Eventing Operator latest release installation failed"
+  wait_until_pods_running default || fail_test "Eventing Operator did not come up"
   create_custom_resource
-  wait_until_pods_running ${TEST_NAMESPACE}
 }
 
 function create_custom_resource() {
   echo ">> Creating the custom resource of Knative Eventing:"
-  cat <<EOF | kubectl apply -f -
+  cat <<-EOF | kubectl apply -f -
 apiVersion: operator.knative.dev/v1alpha1
 kind: KnativeEventing
 metadata:
@@ -128,10 +128,16 @@ initialize $@ --skip-istio-addon
 
 TIMEOUT=20m
 
+header "Running preupgrade tests"
+go_test_e2e -tags=preupgrade -timeout=${TIMEOUT} ./test/upgrade || fail_test
+
+header "Listing all the pods of the previous release"
+wait_until_pods_running ${TEST_NAMESPACE}
+
 install_head
 
 # If we got this far, the operator installed Knative Eventing
-header "Running tests for Knative Eventing Operator"
+header "Running postupgrade tests for Knative Eventing Operator"
 failed=0
 
 # Run the postupgrade tests
diff --git a/test/resources/knativeeventing.go b/test/resources/knativeeventing.go
index 8f66cd22..fab0d5f6 100644
--- a/test/resources/knativeeventing.go
+++ b/test/resources/knativeeventing.go
@@ -97,3 +97,43 @@ func getDeploymentStatus(d *v1.Deployment) corev1.ConditionStatus {
 	}
 	return "unknown"
 }
+
+// WaitForKnativeEventingDeploymentState polls the status of the Knative deployments every `interval`
+// until `inState` returns `true` indicating the deployments match the desired deployments.
+func WaitForKnativeEventingDeploymentState(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()
+
+	waitErr := wait.PollImmediate(Interval, Timeout, func() (bool, error) {
+		dpList, err := clients.KubeClient.Kube.AppsV1().Deployments(namespace).List(metav1.ListOptions{})
+		return inState(dpList, expectedDeployments, err, logf)
+	})
+
+	return waitErr
+}
+
+// IsKnativeEventingDeploymentReady will check the status conditions of the deployments and return true if the deployments meet the desired status.
+func IsKnativeEventingDeploymentReady(dpList *v1.DeploymentList, expectedDeployments []string, err error,
+	logf logging.FormatLogger) (bool, error) {
+	if err != nil {
+		return false, err
+	}
+	if len(dpList.Items) != len(expectedDeployments) {
+		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) {
+			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 {
+				logf("The deployment %v is not ready.", deployment.Name)
+				return false, nil
+			}
+		}
+	}
+	return true, nil
+}
diff --git a/test/resources/verify.go b/test/resources/verify.go
index af9eee77..8a2738b5 100644
--- a/test/resources/verify.go
+++ b/test/resources/verify.go
@@ -141,3 +141,27 @@ func verifyNoKnativeEventings(clients *test.Clients) error {
 	}
 	return nil
 }
+
+// AssertKEOperatorDeploymentStatus verifies if the Knative deployments reach the READY status.
+func AssertKEOperatorDeploymentStatus(t *testing.T, clients *test.Clients, namespace string, expectedDeployments []string) {
+	if err := WaitForKnativeEventingDeploymentState(clients, namespace, expectedDeployments, t.Logf,
+		IsKnativeEventingDeploymentReady); err != nil {
+		t.Fatalf("Knative Eventing deployments failed to meet the expected deployments: %v", err)
+	}
+}
+
+func assertEqual(t *testing.T, actual, expected interface{}) {
+	if actual == expected {
+		return
+	}
+	t.Fatalf("expected does not equal actual. \nExpected: %v\nActual: %v", expected, actual)
+}
+
+func stringInList(a string, list []string) bool {
+	for _, b := range list {
+		if b == a {
+			return true
+		}
+	}
+	return false
+}
diff --git a/test/upgrade/knativeeventingpreupgrade_test.go b/test/upgrade/knativeeventingpreupgrade_test.go
new file mode 100644
index 00000000..858d5896
--- /dev/null
+++ b/test/upgrade/knativeeventingpreupgrade_test.go
@@ -0,0 +1,50 @@
+// +build preupgrade
+
+/*
+Copyright 2020 The Knative Authors
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+    http://www.apache.org/licenses/LICENSE-2.0
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package e2e
+
+import (
+	"testing"
+
+	"knative.dev/eventing-operator/test"
+	"knative.dev/eventing-operator/test/client"
+	"knative.dev/eventing-operator/test/resources"
+	"knative.dev/pkg/test/logstream"
+)
+
+// TestKnativeEventingPreUpgrade verifies the KnativeEventing creation, before upgraded to the latest HEAD at master.
+func TestKnativeEventingPreUpgrade(t *testing.T) {
+	cancel := logstream.Start(t)
+	defer cancel()
+	clients := client.Setup(t)
+
+	names := test.ResourceNames{
+		KnativeEventing: test.EventingOperatorName,
+		Namespace:       test.EventingOperatorNamespace,
+	}
+
+	// Create a KnativeEventing
+	if _, err := resources.EnsureKnativeEventingExists(clients.KnativeEventing(), names); err != nil {
+		t.Fatalf("KnativeService %q failed to create: %v", names.KnativeEventing, err)
+	}
+
+	// Verify if resources match the requirement for the previous release before upgrade
+	t.Run("verify resources", func(t *testing.T) {
+		resources.AssertKEOperatorCRReadyStatus(t, clients, names)
+		expectedDeployments := []string{"eventing-controller", "eventing-webhook", "imc-controller",
+			"imc-dispatcher", "broker-controller", "sources-controller"}
+		resources.AssertKEOperatorDeploymentStatus(t, clients, names.Namespace, expectedDeployments)
+	})
+}
diff --git a/test/upgrade/knativeeventingupgrade_test.go b/test/upgrade/knativeeventingupgrade_test.go
index 6f5bd376..e217dad4 100644
--- a/test/upgrade/knativeeventingupgrade_test.go
+++ b/test/upgrade/knativeeventingupgrade_test.go
@@ -18,8 +18,6 @@ package e2e
 import (
 	"testing"
 
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
-
 	"knative.dev/eventing-operator/test"
 	"knative.dev/eventing-operator/test/client"
 	"knative.dev/eventing-operator/test/resources"
@@ -38,60 +36,16 @@ func TestKnativeEventingUpgrade(t *testing.T) {
 		Namespace:       test.EventingOperatorNamespace,
 	}
 
-	test.CleanupOnInterrupt(func() { test.TearDown(clients, names) })
-	defer test.TearDown(clients, names)
-
 	// Create a KnativeEventing
 	if _, err := resources.EnsureKnativeEventingExists(clients.KnativeEventing(), names); err != nil {
 		t.Fatalf("KnativeService %q failed to create: %v", names.KnativeEventing, err)
 	}
 
-	// Test if KnativeEventing can reach the READY status
-	t.Run("create", func(t *testing.T) {
-		resources.AssertKEOperatorCRReadyStatus(t, clients, names)
-	})
-
-	// Verify if resources match the latest requirement after upgrade
+	// Verify if resources match the requirement for the previous release before upgrade
 	t.Run("verify resources", func(t *testing.T) {
 		resources.AssertKEOperatorCRReadyStatus(t, clients, names)
-		// TODO: We only verify the deployment, but we need to add other resources as well, like ServiceAccount, ClusterRoleBinding, etc.
 		expectedDeployments := []string{"eventing-controller", "eventing-webhook", "imc-controller",
-			"imc-dispatcher", "broker-controller", "sources-controller"}
-		knativeEventingVerifyDeployment(t, clients, names, expectedDeployments)
-	})
-
-	// TODO: We will add one or sections here to run the tests tagged with postupgrade in knative evening.
-
-	// Delete the KnativeEventing to see if all resources will be removed
-	t.Run("delete", func(t *testing.T) {
-		resources.AssertKEOperatorCRReadyStatus(t, clients, names)
-		resources.KEOperatorCRDelete(t, clients, names)
+			"imc-dispatcher", "broker-controller"}
+		resources.AssertKEOperatorDeploymentStatus(t, clients, names.Namespace, expectedDeployments)
 	})
 }
-
-// knativeEventingVerifyDeployment verify whether the deployments have the correct number and names.
-func knativeEventingVerifyDeployment(t *testing.T, clients *test.Clients, names test.ResourceNames,
-	expectedDeployments []string) {
-	dpList, err := clients.KubeClient.Kube.AppsV1().Deployments(names.Namespace).List(metav1.ListOptions{})
-	assertEqual(t, err, nil)
-	assertEqual(t, len(dpList.Items), len(expectedDeployments))
-	for _, deployment := range dpList.Items {
-		assertEqual(t, stringInList(deployment.Name, expectedDeployments), true)
-	}
-}
-
-func assertEqual(t *testing.T, actual, expected interface{}) {
-	if actual == expected {
-		return
-	}
-	t.Fatalf("expected does not equal actual. \nExpected: %v\nActual: %v", expected, actual)
-}
-
-func stringInList(a string, list []string) bool {
-	for _, b := range list {
-		if b == a {
-			return true
-		}
-	}
-	return false
-}