Refactor scripts/es-integration-test.sh (jaegertracing#4161)

## Which problem is this PR solving?
- OpenSearch integration tests very frequently fail
- This change adds retries, but does not fundamentally fix anything
else, except for making sure that we can get the container logs in case
of a future failure

## Short description of the changes
- Setup storage once before executing tests (even though token test does
not require a real ES, it could be just a unit test)
- Remove `-rm` from storage Docker commands so that the container is
still available to do `docker logs` after the timeout
- Change polling from 5s to 10s, for a total of 600sec
- If storage is not reachable in 600sec then kill the container and
retry two more times
- Use trap to tear down storage (useful for local tests as otherwise
container may remain if the tests fail)

Signed-off-by: Yuri Shkuro <github@ysh.us>

.github/workflows/ci-opensearch.yml

@@ -18,7 +18,7 @@ jobs:
       matrix:
         version:
         - major: 1.x
-          image: 1.0.0
+          image: 1.3.7
           distribution: opensearch
         - major: 2.x
           image: 2.3.0

plugin/storage/integration/token_propagation_test.go

@@ -88,23 +88,25 @@ func TestBearTokenPropagation(t *testing.T) {
 
 	// Run elastic search mocked server in background..
 	// is not a full server, just mocked the necessary stuff for this test.
-	srv := &http.Server{Addr: ":9200"}
+	srv := &http.Server{Addr: ":19200"}
 	defer srv.Shutdown(context.Background())
 
 	go createElasticSearchMock(srv, t)
 
 	// Test cases.
 	for _, testCase := range testCases {
-		// Ask for services query, this should return 200
-		req, err := http.NewRequest("GET", queryUrl, nil)
-		require.NoError(t, err)
-		req.Header.Add(testCase.headerName, testCase.headerValue)
-		client := &http.Client{}
-		resp, err := client.Do(req)
-		assert.NoError(t, err)
-		assert.NotNil(t, resp)
-		if err == nil && resp != nil {
-			assert.Equal(t, resp.StatusCode, http.StatusOK)
-		}
+		t.Run(testCase.name, func(t *testing.T) {
+			// Ask for services query, this should return 200
+			req, err := http.NewRequest("GET", queryUrl, nil)
+			require.NoError(t, err)
+			req.Header.Add(testCase.headerName, testCase.headerValue)
+			client := &http.Client{}
+			resp, err := client.Do(req)
+			assert.NoError(t, err)
+			assert.NotNil(t, resp)
+			if err == nil && resp != nil {
+				assert.Equal(t, resp.StatusCode, http.StatusOK)
+			}
+		})
 	}
 }

scripts/es-integration-test.sh

@@ -3,6 +3,21 @@
 PS4='T$(date "+%H:%M:%S") '
 set -euxf -o pipefail
 
+# use global variables to reflect status of db
+db_is_up=
+db_cid=
+
+exit_trap_command="echo executing exit traps"
+function cleanup {
+    eval "$exit_trap_command"
+}
+trap cleanup EXIT
+
+function add_exit_trap {
+    local to_add=$1
+    exit_trap_command="$exit_trap_command; $to_add"
+}
+
 usage() {
   echo $"Usage: $0 <elasticsearch|opensearch> <version>"
   exit 1
@@ -19,7 +34,6 @@ setup_es() {
   local tag=$1
   local image=docker.elastic.co/elasticsearch/elasticsearch
   local params=(
-    --rm
     --detach
     --publish 9200:9200
     --env "http.host=0.0.0.0"
@@ -35,7 +49,6 @@ setup_opensearch() {
   local image=opensearchproject/opensearch
   local tag=$1
   local params=(
-    --rm
     --detach
     --publish 9200:9200
     --env "http.host=0.0.0.0"
@@ -46,22 +59,24 @@ setup_opensearch() {
   echo ${cid}
 }
 
+# bearer token propagaton test uses real query service, but starts a fake DB at 19200
 setup_query() {
   local distro=$1
   local os=$(go env GOOS)
   local arch=$(go env GOARCH)
   local params=(
     --es.tls.enabled=false
     --es.version=7
-    --es.server-urls=http://127.0.0.1:9200
+    --es.server-urls=http://127.0.0.1:19200
     --query.bearer-token-propagation=true
   )
   SPAN_STORAGE_TYPE=${distro} ./cmd/query/query-${os}-${arch} ${params[@]}
 }
 
-wait_for_it() {
-  local url=$1
-  local cid=$2
+wait_for_storage() {
+  local distro=$1
+  local url=$2
+  local cid=$3
   local params=(
     --silent
     --output
@@ -72,18 +87,49 @@ wait_for_it() {
   local counter=0
   local max_counter=60
   while [[ "$(curl ${params[@]} ${url})" != "200" && ${counter} -le ${max_counter} ]]; do
-    sleep 5
-    counter=$((counter+1))
+    docker inspect ${cid} | jq '.[].State'
     echo "waiting for ${url} to be up..."
-    if [ ${counter} -eq ${max_counter} ]; then
-      echo "ERROR: elasticsearch/opensearch is down"
-      docker logs ${cid}
+    sleep 10
+    counter=$((counter+1))
+  done
+  # after the loop, do final verification and set status as global var
+  if [[ "$(curl ${params[@]} ${url})" != "200" ]]; then
+    echo "ERROR: ${distro} is not ready"
+    docker logs ${cid}
+    docker kill ${cid}
+    db_is_up=0
+  else
+    echo "SUCCESS: ${distro} is ready"
+    db_is_up=1
+  fi
+}
+
+bring_up_storage() {
+  local distro=$1
+  local version=$2
+  local cid
+  for retry in 1 2 3
+  do
+    if [ ${distro} = "elasticsearch" ]; then
+      cid=$(setup_es ${version})
+    elif [ ${distro} == "opensearch" ]; then
+      cid=$(setup_opensearch ${version})
+    else
+      echo "Unknown distribution $distro. Valid options are opensearch or elasticsearch"
+      usage
+    fi
+    wait_for_storage ${distro} "http://localhost:9200" ${cid}
+    if [ ${db_is_up} = "1" ]; then
+      break
+    else
+      echo "ERROR: unable to start ${distro}"
       exit 1
     fi
   done
+  db_cid=${cid}
 }
 
-teardown_es() {
+teardown_storage() {
   local cid=$1
   docker kill ${cid}
 }
@@ -100,37 +146,30 @@ build_query() {
 
 run_integration_test() {
   local distro=$1
-  local version=$2
-  local cid
-  if [ ${distro} = "elasticsearch" ]; then
-    cid=$(setup_es ${version})
-  elif [ ${distro} == "opensearch" ]; then
-    cid=$(setup_opensearch ${version})
-  else
-    echo "Unknown distribution $distro. Valid options are opensearch or elasticsearch"
-    usage
-  fi
-  wait_for_it "http://localhost:9200" ${cid}
   STORAGE=${distro} make storage-integration-test
   make index-cleaner-integration-test
   make index-rollover-integration-test
-  teardown_es ${cid}
 }
 
 run_token_propagation_test() {
   local distro=$1
   build_query
   setup_query ${distro} &
   local pid=$!
+  add_exit_trap "teardown_query ${pid}"
   make token-propagation-integration-test
-  teardown_query ${pid}
 }
 
 main() {
   check_arg "$@"
 
-  echo "Executing integration test for $1 $2"
-  run_integration_test "$1" "$2"
+  echo "Preparing $1 $2"
+  bring_up_storage "$1" "$2"
+  add_exit_trap "teardown_storage ${db_cid}"
+
+  echo "Executing main integration tests"
+  run_integration_test "$1"
+
   echo "Executing token propagation test"
   run_token_propagation_test "$1"
 }