From dc6a3305eb06843ad87a5f2bbc1c3244fcd46da2 Mon Sep 17 00:00:00 2001
From: Tolga Ceylan <tolga.ceylan@gmail.com>
Date: Mon, 9 Apr 2018 10:47:00 -0700
Subject: [PATCH] fn: datastore dial retry should be configurable (#927)

* fn: datastore dial retry should be configurable

Setting this to high (60) in api and system tests.

* fn: netcat connect is not meaningful in wait for DB.
---
 api/datastore/sql/sql.go | 32 ++++++++++++++++++++++++++++----
 api_test.sh              |  3 +--
 helpers.sh               | 17 -----------------
 system_test.sh           |  5 ++---
 4 files changed, 31 insertions(+), 26 deletions(-)

diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go
index 9736550f9..8fb16b38a 100644
--- a/api/datastore/sql/sql.go
+++ b/api/datastore/sql/sql.go
@@ -10,6 +10,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"time"
 
@@ -90,6 +91,8 @@ const (
 	callSelector      = `SELECT id, created_at, started_at, completed_at, status, app_id, path, stats, error FROM calls`
 	appIDSelector     = `SELECT id, name, config, annotations, created_at, updated_at FROM apps WHERE id=?`
 	ensureAppSelector = `SELECT id FROM apps WHERE name=?`
+
+	EnvDBPingMaxRetries = "FN_DS_DB_PING_MAX_RETRIES"
 )
 
 type sqlStore struct {
@@ -136,8 +139,9 @@ func newDS(ctx context.Context, url *url.URL) (*sqlStore, error) {
 	}
 
 	db := sqlx.NewDb(sqldb, driver)
+
 	// force a connection and test that it worked
-	err = pingWithRetry(ctx, 10, time.Second*1, db)
+	err = pingWithRetry(ctx, db)
 	if err != nil {
 		log.WithFields(logrus.Fields{"url": uri}).WithError(err).Error("couldn't ping db")
 		return nil, err
@@ -169,13 +173,33 @@ func newDS(ctx context.Context, url *url.URL) (*sqlStore, error) {
 	return sdb, nil
 }
 
-func pingWithRetry(ctx context.Context, attempts int, sleep time.Duration, db *sqlx.DB) (err error) {
-	for i := 0; i < attempts; i++ {
+func pingWithRetry(ctx context.Context, db *sqlx.DB) (err error) {
+
+	attempts := int64(10)
+	if tmp := os.Getenv(EnvDBPingMaxRetries); tmp != "" {
+		attempts, err = strconv.ParseInt(tmp, 10, 64)
+		if err != nil {
+			return fmt.Errorf("cannot parse (%s) invalid %s=%s", err, EnvDBPingMaxRetries, tmp)
+		}
+		if attempts < 0 {
+			return fmt.Errorf("cannot parse invalid %s=%s", EnvDBPingMaxRetries, tmp)
+		}
+	}
+
+	if ctx.Err() != nil {
+		return ctx.Err()
+	}
+
+	for i := int64(0); i < attempts; i++ {
 		err = db.PingContext(ctx)
 		if err == nil {
 			return nil
 		}
-		time.Sleep(sleep)
+		select {
+		case <-ctx.Done():
+			return ctx.Err()
+		case <-time.After(time.Second * 1):
+		}
 	}
 	return err
 }
diff --git a/api_test.sh b/api_test.sh
index 66a59767e..cdf0faae5 100755
--- a/api_test.sh
+++ b/api_test.sh
@@ -19,7 +19,6 @@ case "$1" in
     MYSQL_HOST=`host ${DB_CONTAINER}`
     MYSQL_PORT=3306
     export FN_DB_URL="mysql://root:root@tcp(${MYSQL_HOST}:${MYSQL_PORT})/funcs"
-    wait_for_db ${MYSQL_HOST} ${MYSQL_PORT} 5
     ;;
 
     "postgres" )
@@ -29,7 +28,6 @@ case "$1" in
     POSTGRES_HOST=`host ${DB_CONTAINER}`
     POSTGRES_PORT=5432
     export FN_DB_URL="postgres://postgres:root@${POSTGRES_HOST}:${POSTGRES_PORT}/funcs?sslmode=disable"
-    wait_for_db ${POSTGRES_HOST} ${POSTGRES_PORT} 5
     ;;
 esac
 
@@ -51,6 +49,7 @@ esac
 #fi
 #pwd
 #./fn-api-tests.test -test.v  -test.parallel ${2:-1} ./...; cd ../../
+export FN_DS_DB_PING_MAX_RETRIES=60
 cd test/fn-api-tests && FN_API_URL="http://localhost:8080"  FN_DB_URL=${FN_DB_URL} go test -v  -parallel ${2:-1} ./...; cd ../../
 
 remove_containers
\ No newline at end of file
diff --git a/helpers.sh b/helpers.sh
index 923701f1d..23affb3e8 100644
--- a/helpers.sh
+++ b/helpers.sh
@@ -25,20 +25,3 @@ function remove_containers {
     docker rm -fv func-mysql-test 2>/dev/null || true
     docker rm -fv func-minio-test 2>/dev/null || true
 }
-
-function wait_for_db {
-  HOST="$1"
-  PORT="$2"
-  TIMEOUT="$3"
-  for i in `seq ${TIMEOUT}` ; do
-    ! nc -w 1 -z "${HOST}" "${PORT}" > /dev/null 2>&1
-    result=$?
-    if [ $result -ne 0 ] ; then
-      echo "DB listening on ${HOST}:${PORT}"
-      return
-    fi
-    sleep 1
-  done
-  echo "Failed to connect to DB on ${HOST}:${PORT}"
-  exit 1
-}
\ No newline at end of file
diff --git a/system_test.sh b/system_test.sh
index b7cfb366f..1c5a4e34b 100755
--- a/system_test.sh
+++ b/system_test.sh
@@ -24,7 +24,6 @@ case "$1" in
     MYSQL_HOST=`host ${DB_CONTAINER}`
     MYSQL_PORT=3307
     export FN_DB_URL="mysql://root:root@tcp(${MYSQL_HOST}:${MYSQL_PORT})/funcs"
-    wait_for_db ${MYSQL_HOST} ${MYSQL_PORT} 5
     ;;
 
     "postgres" )
@@ -34,12 +33,12 @@ case "$1" in
     POSTGRES_HOST=`host ${DB_CONTAINER}`
     POSTGRES_PORT=5433
     export FN_DB_URL="postgres://postgres:root@${POSTGRES_HOST}:${POSTGRES_PORT}/funcs?sslmode=disable"
-    wait_for_db ${POSTGRES_HOST} ${POSTGRES_PORT} 5
     ;;
 esac
 
 # avoid port conflicts with api_test.sh which are run in parallel
-FN_API_URL="http://localhost:8085"
+export FN_API_URL="http://localhost:8085"
+export FN_DS_DB_PING_MAX_RETRIES=60
 cd test/fn-system-tests && FN_DB_URL=${FN_DB_URL} FN_API_URL=${FN_API_URL} go test -v -parallel ${2:-1} ./...; cd ../../
 
 remove_system_containers