Merge pull request #714 from diegolovison/10212-4

Reuse WaitFor function to ensure DSPA is ready

.github/scripts/tests/tests.sh

@@ -27,6 +27,7 @@ CONFIG_DIR="${GIT_WORKSPACE}/config"
 RESOURCES_DIR_CRD="${GIT_WORKSPACE}/.github/resources"
 OPENDATAHUB_NAMESPACE="opendatahub"
 RESOURCES_DIR_PYPI="${GIT_WORKSPACE}/.github/resources/pypiserver/base"
+ENDPOINT_TYPE="service"
 
 get_dspo_image() {
   if [ "$REGISTRY_ADDRESS" = "" ]; then
@@ -163,14 +164,14 @@ run_tests() {
   echo "---------------------------------"
   echo "Run tests"
   echo "---------------------------------"
-  ( cd $GIT_WORKSPACE && make integrationtest K8SAPISERVERHOST=${K8SAPISERVERHOST} DSPANAMESPACE=${DSPA_NAMESPACE} DSPAPATH=${DSPA_PATH} )
+  ( cd $GIT_WORKSPACE && make integrationtest K8SAPISERVERHOST=${K8SAPISERVERHOST} DSPANAMESPACE=${DSPA_NAMESPACE} DSPAPATH=${DSPA_PATH} ENDPOINT_TYPE=${ENDPOINT_TYPE} )
 }
 
 run_tests_dspa_external_connections() {
   echo "---------------------------------"
   echo "Run tests for DSPA with External Connections"
   echo "---------------------------------"
-  ( cd $GIT_WORKSPACE && make integrationtest K8SAPISERVERHOST=${K8SAPISERVERHOST} DSPANAMESPACE=${DSPA_EXTERNAL_NAMESPACE} DSPAPATH=${DSPA_EXTERNAL_PATH} )
+  ( cd $GIT_WORKSPACE && make integrationtest K8SAPISERVERHOST=${K8SAPISERVERHOST} DSPANAMESPACE=${DSPA_EXTERNAL_NAMESPACE} DSPAPATH=${DSPA_EXTERNAL_PATH} ENDPOINT_TYPE=${ENDPOINT_TYPE} )
 }
 
 undeploy_kind_resources() {
@@ -299,6 +300,16 @@ while [ "$#" -gt 0 ]; do
         exit 1
       fi
       ;;
+    --endpoint-type)
+      shift
+      if [[ -n "$1" ]]; then
+        ENDPOINT_TYPE="$1"
+        shift
+      else
+        echo "Error: --endpoint-type requires a value [service, route]"
+        exit 1
+      fi
+      ;;
     *)
       echo "Unknown command line switch: $1"
       exit 1

Makefile

@@ -62,6 +62,7 @@ KUBECONFIGPATH ?= $(HOME)/.kube/config
 K8SAPISERVERHOST ?= http://localhost:6443
 DSPANAMESPACE ?= default
 DSPAPATH ?= resources/dspa-lite.yaml
+ENDPOINT_TYPE ?= service
 
 # Get the currently used golang install path (in GOPATH/bin, unless GOBIN is set)
 ifeq (,$(shell go env GOBIN))
@@ -130,7 +131,7 @@ functest: manifests generate fmt vet envtest ## Run tests.
 .PHONY: integrationtest
 integrationtest: ## Run integration tests
 	cd tests && \
-	go test ./... --tags=test_integration -v -kubeconfig=${KUBECONFIGPATH} -k8sApiServerHost=${K8SAPISERVERHOST} -DSPANamespace=${DSPANAMESPACE} -DSPAPath=${DSPAPATH}
+	go test ./... --tags=test_integration -v -kubeconfig=${KUBECONFIGPATH} -k8sApiServerHost=${K8SAPISERVERHOST} -DSPANamespace=${DSPANAMESPACE} -DSPAPath=${DSPAPATH} -endpointType=${ENDPOINT_TYPE}
 
 ##@ Build
 

tests/suite_test.go

@@ -24,7 +24,6 @@ import (
 	"flag"
 	"fmt"
 	routev1 "github.com/openshift/api/route/v1"
-	"k8s.io/apimachinery/pkg/types"
 	"log"
 	"testing"
 	"time"
@@ -192,7 +191,7 @@ func (suite *IntegrationTestSuite) SetupSuite() {
 	}
 
 	err = testUtil.WaitForDSPAReady(suite.T(), ctx, clientmgr.k8sClient, DSPA.Name, DSPANamespace, DeployTimeout, PollInterval)
-	require.NoError(suite.T(), err, fmt.Sprintf("Error Deploying DSPA:\n%s", testUtil.PrintConditions(ctx, DSPA, DSPANamespace, clientmgr.k8sClient)))
+	require.NoError(suite.T(), err, fmt.Sprintf("Error waiting for DSPA being ready:\n%s", testUtil.PrintConditions(ctx, DSPA, DSPANamespace, clientmgr.k8sClient)))
 	loggr.Info("DSPA Deployed.")
 
 	if endpointType == "service" {
@@ -218,28 +217,9 @@ func (suite *IntegrationTestSuite) SetupSuite() {
 
 	} else if endpointType == "route" {
 
-		// Define the namespaced name
-		key := types.NamespacedName{
-			Namespace: DSPANamespace,
-			Name:      "ds-pipeline-" + suite.DSPA.Name,
-		}
-
-		route := &routev1.Route{}
-
-		// Retrieve the route
-		count := 1
-		ok := false
-		for count < 10 {
-			err := clientmgr.k8sClient.Get(context.TODO(), key, route)
-			if err == nil {
-				ok = true
-				break
-			}
-			count++
-			time.Sleep(1 * time.Second)
-		}
-		if !ok {
-			log.Fatal("failed to retrieve route")
+		route, err := testUtil.GetDSPARoute(clientmgr.k8sClient, DSPANamespace, suite.DSPA.Name)
+		if err != nil {
+			log.Fatal(err, "failed to retrieve route")
 		}
 
 		APIServerURL = fmt.Sprintf("https://%s", route.Status.Ingress[0].Host)
@@ -254,19 +234,15 @@ func (suite *IntegrationTestSuite) SetupSuite() {
 		}
 
 		// waiting for pods to sit down
-		count = 1
-		ok = false
-		for count < 10 {
-			response, _ := suite.Clientmgr.httpClient.Get(fmt.Sprintf("%s/apis/v2beta1/healthz", APIServerURL))
-			if response.StatusCode == 200 {
-				ok = true
-				break
+		err = testUtil.WaitFor(ctx, DeployTimeout, PollInterval, func() (bool, error) {
+			response, err := suite.Clientmgr.httpClient.Get(fmt.Sprintf("%s/apis/v2beta1/healthz", APIServerURL))
+			if response.StatusCode != 200 {
+				return false, err
 			}
-			count++
-			time.Sleep(1 * time.Second)
-		}
-		if !ok {
-			log.Fatal("Pods didn't started properly")
+			return true, nil
+		})
+		if err != nil {
+			log.Fatal(err, "healthz endpoint is not working properly")
 		}
 
 	} else {

tests/util/resources.go

@@ -16,6 +16,7 @@ package testUtil
 import (
 	"context"
 	"fmt"
+	routev1 "github.com/openshift/api/route/v1"
 	"testing"
 	"time"
 
@@ -41,7 +42,7 @@ func DeployDSPA(t *testing.T, ctx context.Context, client client.Client, deployD
 		Namespace: dspaNS,
 	}
 	fetchedDspa := &v1alpha1.DataSciencePipelinesApplication{}
-	return waitFor(ctx, timeout, interval, func() (bool, error) {
+	return WaitFor(ctx, timeout, interval, func() (bool, error) {
 		err := client.Get(ctx, nsn, fetchedDspa)
 		if err != nil {
 			return false, err
@@ -57,7 +58,7 @@ func WaitForDSPAReady(t *testing.T, ctx context.Context, client client.Client, d
 		Namespace: dspaNS,
 	}
 	dspa := &v1alpha1.DataSciencePipelinesApplication{}
-	return waitFor(ctx, timeout, interval, func() (bool, error) {
+	err := WaitFor(ctx, timeout, interval, func() (bool, error) {
 		err := client.Get(ctx, nsn, dspa)
 		if err != nil {
 			return false, err
@@ -69,6 +70,34 @@ func WaitForDSPAReady(t *testing.T, ctx context.Context, client client.Client, d
 		}
 		return false, nil
 	})
+	// fail fast
+	if err != nil {
+		return err
+	}
+	if dspa.Spec.EnableRoute {
+		err = WaitFor(ctx, timeout, interval, func() (bool, error) {
+			_, err := GetDSPARoute(client, dspaNS, dspa.ObjectMeta.Name)
+			if err != nil {
+				return false, err
+			}
+			return true, nil
+		})
+		// fail fast
+		if err != nil {
+			return err
+		}
+	}
+	return err
+}
+
+func GetDSPARoute(client client.Client, dspaNS, name string) (*routev1.Route, error) {
+	key := types.NamespacedName{
+		Namespace: dspaNS,
+		Name:      "ds-pipeline-" + name,
+	}
+	route := &routev1.Route{}
+	err := client.Get(context.TODO(), key, route)
+	return route, err
 }
 
 // DeleteDSPA will delete DSPA found in path by requesting
@@ -85,7 +114,7 @@ func DeleteDSPA(t *testing.T, ctx context.Context, client client.Client, dspaNam
 	}
 	err := client.Delete(ctx, dspa)
 	require.NoError(t, err)
-	return waitFor(ctx, timeout, interval, func() (bool, error) {
+	return WaitFor(ctx, timeout, interval, func() (bool, error) {
 		err := client.Get(ctx, nsn, dspa)
 		if apierrs.IsNotFound(err) {
 			return true, nil
@@ -122,17 +151,17 @@ func GetDSPAFromPath(t *testing.T, opts mf.Option, path string) *v1alpha1.DataSc
 	return dspa
 }
 
-// waitFor is a helper function
-func waitFor(ctx context.Context, timeout, interval time.Duration, conditionFunc func() (bool, error)) error {
+// WaitFor is a helper function
+func WaitFor(ctx context.Context, timeout, interval time.Duration, conditionFunc func() (bool, error)) error {
 	deadline := time.Now().Add(timeout)
 	for time.Now().Before(deadline) {
 		done, err := conditionFunc()
-		if done {
-			return nil
-		}
 		if err != nil {
 			return err
 		}
+		if done {
+			return nil
+		}
 		time.Sleep(interval)
 	}
 	return fmt.Errorf("timed out waiting for condition")