Addressing comments

pkg/testframework/framework.go

@@ -9,9 +9,9 @@ import (
 	appv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	runtime "k8s.io/apimachinery/pkg/runtime"
-	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/wait"
 
 	consoleapi "github.com/openshift/console-operator/pkg/api"
@@ -49,19 +49,35 @@ func GetResource(client *Clientset, resource string) (runtime.Object, error) {
 	var err error
 	switch resource {
 	case "ConfigMap":
-		res, err = client.ConfigMaps(consoleapi.OpenShiftConsoleNamespace).Get(consoleapi.OpenShiftConsoleConfigMapName, metav1.GetOptions{})
+		res, err = GetConsoleConfigMap(client)
 	case "Service":
-		res, err = client.Services(consoleapi.OpenShiftConsoleNamespace).Get(consoleapi.OpenShiftConsoleServiceName, metav1.GetOptions{})
+		res, err = GetConsoleService(client)
 	case "Route":
-		res, err = client.Routes(consoleapi.OpenShiftConsoleNamespace).Get(consoleapi.OpenShiftConsoleRouteName, metav1.GetOptions{})
+		res, err = GetConsoleRoute(client)
 	case "Deployment":
 		fallthrough
 	default:
-		res, err = client.Deployments(consoleapi.OpenShiftConsoleNamespace).Get(consoleapi.OpenShiftConsoleDeploymentName, metav1.GetOptions{})
+		res, err = GetConsoleDeployment(client)
 	}
 	return res, err
 }
 
+func GetConsoleConfigMap(client *Clientset) (*corev1.ConfigMap, error) {
+	return client.ConfigMaps(consoleapi.OpenShiftConsoleNamespace).Get(consoleapi.OpenShiftConsoleConfigMapName, metav1.GetOptions{})
+}
+
+func GetConsoleService(client *Clientset) (*corev1.Service, error) {
+	return client.Services(consoleapi.OpenShiftConsoleNamespace).Get(consoleapi.OpenShiftConsoleServiceName, metav1.GetOptions{})
+}
+
+func GetConsoleRoute(client *Clientset) (*routev1.Route, error) {
+	return client.Routes(consoleapi.OpenShiftConsoleNamespace).Get(consoleapi.OpenShiftConsoleRouteName, metav1.GetOptions{})
+}
+
+func GetConsoleDeployment(client *Clientset) (*appv1.Deployment, error) {
+	return client.Deployments(consoleapi.OpenShiftConsoleNamespace).Get(consoleapi.OpenShiftConsoleDeploymentName, metav1.GetOptions{})
+}
+
 func deleteResource(client *Clientset, resource string) error {
 	var err error
 	switch resource {
@@ -90,7 +106,8 @@ func DeleteCompletely(getObject func() (runtime.Object, error), deleteObject fun
 		return err
 	}
 
-	uid := getUID(obj)
+	accessor, err := meta.Accessor(obj)
+	uid := accessor.GetUID()
 
 	policy := metav1.DeletePropagationForeground
 	if err := deleteObject(&metav1.DeleteOptions{
@@ -114,25 +131,10 @@ func DeleteCompletely(getObject func() (runtime.Object, error), deleteObject fun
 			return false, err
 		}
 
-		return getUID(obj) != uid, nil
-	})
-}
+		accessor, err := meta.Accessor(obj)
 
-func getUID(obj runtime.Object) types.UID {
-	configMap, ok := obj.(*corev1.ConfigMap)
-	if ok {
-		return configMap.ObjectMeta.GetUID()
-	}
-	service, ok := obj.(*corev1.Service)
-	if ok {
-		return service.ObjectMeta.GetUID()
-	}
-	route, ok := obj.(*routev1.Route)
-	if ok {
-		return route.ObjectMeta.GetUID()
-	}
-	deployment, _ := obj.(*appv1.Deployment)
-	return deployment.ObjectMeta.GetUID()
+		return accessor.GetUID() != uid, nil
+	})
 }
 
 // IsResourceAvailable checks if tested resource is available(recreated by console-operator),
@@ -156,25 +158,6 @@ func IsResourceAvailable(errChan chan error, client *Clientset, resource string)
 	errChan <- err
 }
 
-func IsResourceAvailable_(client *Clientset, resource string) error {
-	counter := 0
-	err := wait.Poll(1*time.Second, AsyncOperationTimeout, func() (stop bool, err error) {
-		_, err = GetResource(client, resource)
-		if err == nil {
-			return true, nil
-		}
-		if counter == 10 {
-			if err != nil {
-				return true, fmt.Errorf("deleted console %s was not recreated", resource)
-			}
-			return true, nil
-		}
-		counter++
-		return false, nil
-	})
-	return err
-}
-
 // IsResourceUnavailable checks if tested resource is unavailable(not recreated by console-operator),
 // during 10 second period. If not error will be returned.
 func IsResourceUnavailable(errChan chan error, client *Clientset, resource string) {

test/e2e/managed_test.go

@@ -34,8 +34,9 @@ func TestEditManagedConfigMap(t *testing.T) {
 	defer testframework.MustManageConsole(t, client)
 	testframework.MustManageConsole(t, client)
 
-	if !patchAndCheckConfigMap(t, client) {
-		t.Fatalf("console ConfigMap data are not set to default value")
+	err := patchAndCheckConfigMap(t, client, true)
+	if err != nil {
+		t.Fatalf("error: %s", err)
 	}
 }
 
@@ -44,8 +45,9 @@ func TestEditManagedService(t *testing.T) {
 	defer testframework.MustManageConsole(t, client)
 	testframework.MustManageConsole(t, client)
 
-	if !patchAndCheckService(t, client) {
-		t.Fatalf("console Service data are not set to default value")
+	err := patchAndCheckService(t, client, true)
+	if err != nil {
+		t.Fatalf("error: %s", err)
 	}
 }
 
@@ -54,7 +56,8 @@ func TestEditManagedRoute(t *testing.T) {
 	defer testframework.MustManageConsole(t, client)
 	testframework.MustManageConsole(t, client)
 
-	if !patchAndCheckRoute(t, client) {
-		t.Fatalf("console Route data are not set to default value")
+	err := patchAndCheckRoute(t, client, true)
+	if err != nil {
+		t.Fatalf("error: %s", err)
 	}
 }

test/e2e/unmanaged_test.go

@@ -35,8 +35,9 @@ func TestEditUnmanagedConfigMap(t *testing.T) {
 	defer testframework.MustManageConsole(t, client)
 	testframework.MustUnmanageConsole(t, client)
 
-	if patchAndCheckConfigMap(t, client) {
-		t.Fatalf("console ConfigMap data are set to default value")
+	err := patchAndCheckConfigMap(t, client, false)
+	if err != nil {
+		t.Fatalf("error: %s", err)
 	}
 }
 
@@ -45,8 +46,9 @@ func TestEditUnmanagedService(t *testing.T) {
 	defer testframework.MustManageConsole(t, client)
 	testframework.MustUnmanageConsole(t, client)
 
-	if patchAndCheckService(t, client) {
-		t.Fatalf("console Service status are set to default value")
+	err := patchAndCheckService(t, client, false)
+	if err != nil {
+		t.Fatalf("error: %s", err)
 	}
 }
 
@@ -55,7 +57,8 @@ func TestEditUnmanagedRoute(t *testing.T) {
 	defer testframework.MustManageConsole(t, client)
 	testframework.MustUnmanageConsole(t, client)
 
-	if patchAndCheckRoute(t, client) {
-		t.Fatalf("console Route status are set to default value")
+	err := patchAndCheckRoute(t, client, false)
+	if err != nil {
+		t.Fatalf("error: %s", err)
 	}
 }

test/e2e/util.go

@@ -5,99 +5,88 @@ import (
 	"testing"
 	"time"
 
-	routev1 "github.com/openshift/api/route/v1"
 	"github.com/openshift/console-operator/pkg/testframework"
-	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/wait"
 
 	consoleapi "github.com/openshift/console-operator/pkg/api"
 )
 
-// Basically each of these tests helpers are similar, they only vary in the
-// resource they are GETting, PATCHing and patch itself.
-// Since after the patch is done, the test needs to wait, till it can GET
-// patched object(or not, if the operator status is set to Managed).
+// Each of these tests helpers are similar, they only vary in the
+// resource they are GETting and PATCHing.
+// After the patch is done the test will poll the given resource.
+// In case the console-operator is Managed state the patched data should
+// not be equal to the one obtained after patch is applied.
+// In case the console-operator is Unmanaged state the patched data should
+// be equal to the one obtained after patch is applied.
 
-func patchAndCheckConfigMap(t *testing.T, client *testframework.Clientset) bool {
-	res, err := testframework.GetResource(client, "ConfigMap")
-	errorCheck(t, err)
-	configMap, ok := res.(*corev1.ConfigMap)
-	if !ok {
-		t.Fatalf("unable to type received object as ConfigMap")
-	}
-	originalData := configMap.Data
+var pollTimeout = 10 * time.Second
 
+func patchAndCheckConfigMap(t *testing.T, client *testframework.Clientset, isOperatorManaged bool) error {
 	t.Logf("patching Data on the console ConfigMap")
-	_, err = client.ConfigMaps(consoleapi.OpenShiftConsoleOperatorNamespace).Patch(consoleapi.OpenShiftConsoleConfigMapName, types.MergePatchType, []byte(`{"data": {"console-config.yaml": "test"}}`))
-	errorCheck(t, err)
-
-	time.Sleep(5 * time.Second)
-
-	res, err = testframework.GetResource(client, "ConfigMap")
-	errorCheck(t, err)
-	configMap, ok = res.(*corev1.ConfigMap)
-	if !ok {
-		t.Fatalf("unable to type received object as ConfigMap")
+	configMap, err := client.ConfigMaps(consoleapi.OpenShiftConsoleNamespace).Patch(consoleapi.OpenShiftConsoleConfigMapName, types.MergePatchType, []byte(`{"data": {"console-config.yaml": "test"}}`))
+	if err != nil {
+		return err
 	}
-	newData := configMap.Data
-
-	return reflect.DeepEqual(originalData, newData)
+	patchedData := configMap.Data
+
+	t.Logf("polling for patched Data on the console ConfigMap")
+	err = wait.Poll(1*time.Second, pollTimeout, func() (stop bool, err error) {
+		configMap, err = testframework.GetConsoleConfigMap(client)
+		if err != nil {
+			return true, err
+		}
+		newData := configMap.Data
+		if isOperatorManaged {
+			return !reflect.DeepEqual(patchedData, newData), nil
+		}
+		return reflect.DeepEqual(patchedData, newData), nil
+	})
+	return err
 }
 
-func patchAndCheckService(t *testing.T, client *testframework.Clientset) bool {
-	res, err := testframework.GetResource(client, "Service")
-	errorCheck(t, err)
-	service, ok := res.(*corev1.Service)
-	if !ok {
-		t.Fatalf("unable to type received object as Service")
-	}
-	originalData := service.GetAnnotations()
-
-	t.Logf("patching Annotations on the console Service")
-	_, err = client.Services(consoleapi.OpenShiftConsoleOperatorNamespace).Patch(consoleapi.OpenShiftConsoleServiceName, types.MergePatchType, []byte(`{"metadata": {"annotations": {"service.alpha.openshift.io/serving-cert-secret-name": "test"}}}`))
-	errorCheck(t, err)
-
-	time.Sleep(5 * time.Second)
-
-	res, err = testframework.GetResource(client, "Service")
-	errorCheck(t, err)
-	service, ok = res.(*corev1.Service)
-	if !ok {
-		t.Fatalf("unable to type received object as Service")
+func patchAndCheckService(t *testing.T, client *testframework.Clientset, isOperatorManaged bool) error {
+	t.Logf("patching Annotation on the console Service")
+	service, err := client.Services(consoleapi.OpenShiftConsoleNamespace).Patch(consoleapi.OpenShiftConsoleServiceName, types.MergePatchType, []byte(`{"metadata": {"annotations": {"service.alpha.openshift.io/serving-cert-secret-name": "test"}}}`))
+	if err != nil {
+		return err
 	}
-	newData := service.GetAnnotations()
-
-	return reflect.DeepEqual(originalData, newData)
+	patchedData := service.GetAnnotations()
+
+	t.Logf("polling for patched Annotation on the console Service")
+	err = wait.Poll(1*time.Second, pollTimeout, func() (stop bool, err error) {
+		service, err = testframework.GetConsoleService(client)
+		if err != nil {
+			return true, err
+		}
+		newData := service.GetAnnotations()
+		if isOperatorManaged {
+			return !reflect.DeepEqual(patchedData, newData), nil
+		}
+		return reflect.DeepEqual(patchedData, newData), nil
+	})
+	return err
 }
 
-func patchAndCheckRoute(t *testing.T, client *testframework.Clientset) bool {
-	res, err := testframework.GetResource(client, "Route")
-	errorCheck(t, err)
-	route, ok := res.(*routev1.Route)
-	if !ok {
-		t.Fatalf("unable to type received object as Route")
-	}
-	originalData := route.Spec.Port.TargetPort
-
+func patchAndCheckRoute(t *testing.T, client *testframework.Clientset, isOperatorManaged bool) error {
 	t.Logf("patching TargetPort on the console Route")
-	_, err = client.Routes(consoleapi.OpenShiftConsoleOperatorNamespace).Patch(consoleapi.OpenShiftConsoleRouteName, types.MergePatchType, []byte(`{"spec": {"port": {"targetPort": "http"}}}`))
-	errorCheck(t, err)
-
-	time.Sleep(5 * time.Second)
-
-	res, err = testframework.GetResource(client, "Route")
-	errorCheck(t, err)
-	route, ok = res.(*routev1.Route)
-	if !ok {
-		t.Fatalf("unable to type received object as Route")
-	}
-	newData := route.Spec.Port.TargetPort
-
-	return reflect.DeepEqual(originalData, newData)
-}
-
-func errorCheck(t *testing.T, err error) {
+	route, err := client.Routes(consoleapi.OpenShiftConsoleNamespace).Patch(consoleapi.OpenShiftConsoleRouteName, types.MergePatchType, []byte(`{"spec": {"port": {"targetPort": "http"}}}`))
 	if err != nil {
-		t.Fatalf("Fatal error: %s", err)
+		return err
 	}
+	patchedData := route.Spec.Port.TargetPort
+
+	t.Logf("polling for patched TargetPort on the console Route")
+	err = wait.Poll(1*time.Second, pollTimeout, func() (stop bool, err error) {
+		route, err = testframework.GetConsoleRoute(client)
+		if err != nil {
+			return true, err
+		}
+		newData := route.Spec.Port.TargetPort
+		if isOperatorManaged {
+			return !reflect.DeepEqual(patchedData, newData), nil
+		}
+		return reflect.DeepEqual(patchedData, newData), nil
+	})
+	return err
 }