Improve tests

There were many instances where `unstructured.Nested*` methods could be
used for better safety (preventing panics if types failed to be
converted).

Additionally, this removes the 5 minute wait for the new config-policy
rollout, in favor of an `Eventually` section that doesn't wait for the
pod to become fully Ready. We really only care how it's configured,
which we can check without that wait.

Signed-off-by: Justin Kulikauskas <[email protected]>

Author: JustinKuli

test/e2e/case1_framework_deployment_test.go

@@ -92,6 +92,8 @@ var _ = Describe("Test framework deployment", func() {
 
 		checkContainersAndAvailability(cluster, 0)
 
+		// Adding this annotation and later verifying the cluster namespace is not removed checks
+		// that the helm values annotation and the logging level annotation are stackable.
 		By(logPrefix + "annotating the managedclusteraddon with the " + loggingLevelAnnotation + " annotation")
 		Kubectl("annotate", "-n", cluster.clusterName, "-f", case1ManagedClusterAddOnCR, loggingLevelAnnotation)
 
@@ -125,6 +127,8 @@ var _ = Describe("Test framework deployment", func() {
 
 		checkContainersAndAvailability(cluster, 0)
 
+		// Adding this annotation and later verifying the cluster namespace is not removed checks
+		// that the multiclusterhub annotation and the logging level annotation are stackable.
 		By(logPrefix + "annotating the managedclusteraddon with the " + loggingLevelAnnotation + " annotation")
 		Kubectl("annotate", "-n", cluster.clusterName, "-f", case1ManagedClusterAddOnCR, loggingLevelAnnotation)
 
@@ -319,10 +323,10 @@ func checkContainersAndAvailability(cluster managedClusterConfig, clusterIdx int
 	Eventually(func() int {
 		deploy := GetWithTimeout(cluster.clusterClient, gvrDeployment,
 			case1DeploymentName, addonNamespace, true, 30)
-		spec := deploy.Object["spec"].(map[string]interface{})["template"].(map[string]interface{})["spec"]
-		containers := spec.(map[string]interface{})["containers"]
 
-		return len(containers.([]interface{}))
+		containers, _, _ := unstructured.NestedSlice(deploy.Object, "spec", "template", "spec", "containers")
+
+		return len(containers)
 	}, 60, 1).Should(Equal(desiredContainerCount))
 
 	if startupProbeInCluster(clusterIdx) {
@@ -331,11 +335,18 @@ func checkContainersAndAvailability(cluster managedClusterConfig, clusterIdx int
 			deploy := GetWithTimeout(
 				cluster.clusterClient, gvrDeployment, case1DeploymentName, addonNamespace, true, 30,
 			)
-			status := deploy.Object["status"]
-			replicas := status.(map[string]interface{})["replicas"]
-			availableReplicas := status.(map[string]interface{})["availableReplicas"]
 
-			return (availableReplicas != nil) && replicas.(int64) == availableReplicas.(int64)
+			replicas, found, err := unstructured.NestedInt64(deploy.Object, "status", "replicas")
+			if !found || err != nil {
+				return false
+			}
+
+			available, found, err := unstructured.NestedInt64(deploy.Object, "status", "availableReplicas")
+			if !found || err != nil {
+				return false
+			}
+
+			return available == replicas
 		}, 240, 1).Should(Equal(true))
 	}
 
@@ -345,9 +356,10 @@ func checkContainersAndAvailability(cluster managedClusterConfig, clusterIdx int
 			LabelSelector: case1PodSelector,
 		}
 		pods := ListWithTimeoutByNamespace(cluster.clusterClient, gvrPod, opts, addonNamespace, 1, true, 30)
-		phase := pods.Items[0].Object["status"].(map[string]interface{})["phase"]
 
-		return phase.(string) == "Running"
+		phase, _, _ := unstructured.NestedString(pods.Items[0].Object, "status", "phase")
+
+		return phase == "Running"
 	}, 60, 1).Should(Equal(true))
 
 	By(logPrefix + "showing the framework managedclusteraddon as available")

test/e2e/case2_config_deployment_test.go

@@ -3,8 +3,6 @@
 package e2e
 
 import (
-	"fmt"
-
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -33,10 +31,9 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 				deploy = GetWithTimeout(
 					cluster.clusterClient, gvrDeployment, case2DeploymentName, addonNamespace, true, 30,
 				)
-				spec := deploy.Object["spec"].(map[string]interface{})["template"].(map[string]interface{})["spec"]
-				containers := spec.(map[string]interface{})["containers"]
+				containers, _, _ := unstructured.NestedSlice(deploy.Object, "spec", "template", "spec", "containers")
 
-				return len(containers.([]interface{}))
+				return len(containers)
 			}, 60, 1).Should(Equal(1))
 
 			if startupProbeInCluster(i) {
@@ -45,11 +42,18 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 					deploy = GetWithTimeout(
 						cluster.clusterClient, gvrDeployment, case2DeploymentName, addonNamespace, true, 30,
 					)
-					status := deploy.Object["status"]
-					replicas := status.(map[string]interface{})["replicas"]
-					availableReplicas := status.(map[string]interface{})["availableReplicas"]
 
-					return (availableReplicas != nil) && replicas.(int64) == availableReplicas.(int64)
+					replicas, found, err := unstructured.NestedInt64(deploy.Object, "status", "replicas")
+					if !found || err != nil {
+						return false
+					}
+
+					available, found, err := unstructured.NestedInt64(deploy.Object, "status", "availableReplicas")
+					if !found || err != nil {
+						return false
+					}
+
+					return available == replicas
 				}, 240, 1).Should(Equal(true))
 			}
 
@@ -59,9 +63,10 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 					LabelSelector: case2PodSelector,
 				}
 				pods := ListWithTimeoutByNamespace(cluster.clusterClient, gvrPod, opts, addonNamespace, 1, true, 30)
-				phase := pods.Items[0].Object["status"].(map[string]interface{})["phase"]
 
-				return phase.(string) == "Running"
+				phase, _, _ := unstructured.NestedString(pods.Items[0].Object, "status", "phase")
+
+				return phase == "Running"
 			}, 60, 1).Should(Equal(true))
 
 			By(logPrefix + "showing the config-policy-controller managedclusteraddon as available")
@@ -84,7 +89,7 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 	})
 
 	It("should create a config-policy-controller deployment with custom logging levels and concurrency", func() {
-		for i, cluster := range managedClusterList {
+		for _, cluster := range managedClusterList {
 			logPrefix := cluster.clusterType + " " + cluster.clusterName + ": "
 			By(logPrefix + "deploying the default config-policy-controller managedclusteraddon")
 			Kubectl("apply", "-n", cluster.clusterName, "-f", case2ManagedClusterAddOnCR)
@@ -117,54 +122,33 @@ var _ = Describe("Test config-policy-controller deployment", func() {
 				evaluationConcurrencyAnnotation,
 			)
 
-			By(logPrefix + "restarting the config-policy-controller deployment")
-			Kubectl(
-				"-n",
-				addonNamespace,
-				"rollout",
-				"restart",
-				"deployments/config-policy-controller",
-				fmt.Sprintf("--kubeconfig=%s%d.kubeconfig", kubeconfigFilename, i+1),
-			)
-			Kubectl(
-				"-n",
-				addonNamespace,
-				"rollout",
-				"status",
-				"deployments/config-policy-controller",
-				"--watch",
-				"--timeout=360s", // to allow for the 5 minute delay on old k8s
-				fmt.Sprintf("--kubeconfig=%s%d.kubeconfig", kubeconfigFilename, i+1),
-			)
-
 			By(logPrefix + "verifying the pod has been deployed with a new logging level and concurrency")
-			opts := metav1.ListOptions{
-				LabelSelector: case2PodSelector,
-			}
-			pods := ListWithTimeoutByNamespace(cluster.clusterClient, gvrPod, opts, addonNamespace, 1, true, 60)
-			phase := pods.Items[0].Object["status"].(map[string]interface{})["phase"]
+			Eventually(func(g Gomega) {
+				opts := metav1.ListOptions{
+					LabelSelector: case2PodSelector,
+				}
+				pods := ListWithTimeoutByNamespace(cluster.clusterClient, gvrPod, opts, addonNamespace, 1, true, 60)
+				phase := pods.Items[0].Object["status"].(map[string]interface{})["phase"]
 
-			Expect(phase.(string)).To(Equal("Running"))
-			containerList, _, err := unstructured.NestedSlice(pods.Items[0].Object, "spec", "containers")
-			if err != nil {
-				panic(err)
-			}
-			for _, container := range containerList {
-				if containerObj, ok := container.(map[string]interface{}); ok {
-					if Expect(containerObj).To(HaveKey("name")) && containerObj["name"] != case2DeploymentName {
+				g.Expect(phase.(string)).To(Equal("Running"))
+				containerList, _, err := unstructured.NestedSlice(pods.Items[0].Object, "spec", "containers")
+				g.Expect(err).To(BeNil())
+				for _, container := range containerList {
+					containerObj, ok := container.(map[string]interface{})
+					g.Expect(ok).To(BeTrue())
+
+					if g.Expect(containerObj).To(HaveKey("name")) && containerObj["name"] != case2DeploymentName {
 						continue
 					}
-					if Expect(containerObj).To(HaveKey("args")) {
+					if g.Expect(containerObj).To(HaveKey("args")) {
 						args := containerObj["args"]
-						Expect(args).To(ContainElement("--log-encoder=console"))
-						Expect(args).To(ContainElement("--log-level=8"))
-						Expect(args).To(ContainElement("--v=6"))
-						Expect(args).To(ContainElement("--evaluation-concurrency=5"))
+						g.Expect(args).To(ContainElement("--log-encoder=console"))
+						g.Expect(args).To(ContainElement("--log-level=8"))
+						g.Expect(args).To(ContainElement("--v=6"))
+						g.Expect(args).To(ContainElement("--evaluation-concurrency=5"))
 					}
-				} else {
-					panic(fmt.Errorf("containerObj type assertion failed"))
 				}
-			}
+			}, 180, 10).Should(Succeed())
 
 			By(logPrefix +
 				"removing the config-policy-controller deployment when the ManagedClusterAddOn CR is removed")