From 07cd6bb955c2864f1143701557cc154981be558a Mon Sep 17 00:00:00 2001
From: Julian Nguyen <109386615+juliannguyen4@users.noreply.github.com>
Date: Mon, 8 Jul 2024 14:34:25 -0700
Subject: [PATCH] [CLIENT-3017] CI/CD: Wait for Aerospike server Docker
 container to be ready using asinfo instead of a time delay (#642)

- Fix dev-workflow-p1.yml not skipping server release tests when running manually and setting input to false
---
 .github/actions/run-ee-server/action.yml      | 26 +++++++++++--
 .../wait-for-as-server-to-start/action.yml    | 25 +++++++++++++
 .github/workflows/dev-workflow-p1.yml         |  2 +-
 .github/workflows/stage-tests.yml             |  3 --
 .github/workflows/test-server-rc.yml          | 13 ++-----
 .github/workflows/tests.yml                   | 37 ++++++++-----------
 .github/workflows/valgrind.yml                |  4 --
 .../wait-for-as-server-to-start.bash          | 29 +++++++++++++++
 8 files changed, 98 insertions(+), 41 deletions(-)
 create mode 100644 .github/actions/wait-for-as-server-to-start/action.yml
 create mode 100755 .github/workflows/wait-for-as-server-to-start.bash

diff --git a/.github/actions/run-ee-server/action.yml b/.github/actions/run-ee-server/action.yml
index 0e5ae5e3a..f08bc76b1 100644
--- a/.github/actions/run-ee-server/action.yml
+++ b/.github/actions/run-ee-server/action.yml
@@ -1,5 +1,5 @@
 name: 'Run EE Server'
-description: 'Run EE server'
+description: 'Run EE server. Returns once server is ready. Only tested on Linux and macOS'
 inputs:
   # All inputs in composite actions are strings
   use-server-rc:
@@ -56,10 +56,17 @@ runs:
     run: docker login --username ${{ inputs.docker-hub-username }} --password ${{ inputs.docker-hub-password }}
     shell: bash
 
+  - run: docker run -d --name aerospike -p 3000:3000 $SERVER_IMAGE:${{ inputs.server-tag }}
+    shell: bash
+
+  - uses: ./.github/actions/wait-for-as-server-to-start
+    id: wait-for-server1
+    with:
+      container-name: aerospike
+      is-security-enabled: false
+
   - name: Get default aerospike.conf from Docker server EE container
     run: |
-      docker run -d --name aerospike -p 3000-3002:3000-3002 $SERVER_IMAGE:${{ inputs.server-tag }}
-      sleep 5
       docker cp aerospike:/etc/aerospike/aerospike.conf ./configs/aerospike.conf
       docker container stop aerospike
       docker container rm aerospike
@@ -75,6 +82,19 @@ runs:
     run: docker run -tid -v $(pwd)/configs:/opt/aerospike/etc -p 3000:3000 --name aerospike $SERVER_IMAGE:${{ inputs.server-tag }} asd --config-file /opt/aerospike/etc/aerospike.conf
     shell: bash
 
+  - uses: ./.github/actions/wait-for-as-server-to-start
+    id: wait-for-server2
+    with:
+      container-name: aerospike
+      is-security-enabled: true
+
+  # Enabling debug logging for workflow runs doesn't show container logs
+  # So we need this step for now
+  - if: ${{ !cancelled() && (steps.wait-for-server1.outcome == 'failure' || steps.wait-for-server2.outcome == 'failure') }}
+    name: Print logs to help debug why the server failed to start up
+    run: docker container logs aerospike
+    shell: bash
+
   - name: Create user in database for tests
     # Use default admin user to create another user for testing
     run: docker exec aerospike asadm --user admin --password admin --enable -e "manage acl create user superuser password superuser roles read-write-udf sys-admin user-admin data-admin"
diff --git a/.github/actions/wait-for-as-server-to-start/action.yml b/.github/actions/wait-for-as-server-to-start/action.yml
new file mode 100644
index 000000000..119fbcc3a
--- /dev/null
+++ b/.github/actions/wait-for-as-server-to-start/action.yml
@@ -0,0 +1,25 @@
+name: 'Wait for Aerospike server to start'
+description: Only tested on Linux and macOS
+inputs:
+  container-name:
+    required: true
+  is-security-enabled:
+    required: false
+    default: 'false'
+
+runs:
+  using: "composite"
+  steps:
+  - name: 'macOS: install timeout command'
+    if: ${{ runner.os == 'macOS' }}
+    run: brew install coreutils
+    shell: bash
+
+  # Composite actions doesn't support step-level timeout-minutes
+  # Use timeout command and store polling logic in file to make it easier to read
+  # Call bash shell explicitly since timeout uses "sh" shell by default, for some reason
+  # Also, we don't want to fail if we timeout in case the server *did* finish starting up but the script couldn't detect it due to a bug
+  # Effectively, this composite action is like calling "sleep" that is optimized to exit early when it detects an ok from the server
+  - name: Wait for EE server to start
+    run: timeout 5 bash ./.github/workflows/wait-for-as-server-to-start.bash ${{ inputs.container-name }} ${{ inputs.is-security-enabled }} || true
+    shell: bash
diff --git a/.github/workflows/dev-workflow-p1.yml b/.github/workflows/dev-workflow-p1.yml
index 7d2c5cda7..6acfd96f9 100644
--- a/.github/workflows/dev-workflow-p1.yml
+++ b/.github/workflows/dev-workflow-p1.yml
@@ -26,7 +26,7 @@ jobs:
   test-with-server-release:
     uses: ./.github/workflows/build-wheels.yml
     with:
-      run_tests: ${{ github.event_name == 'workflow_dispatch' && inputs.run_server_release_tests || true }}
+      run_tests: ${{ github.event_name == 'pull_request' && true || inputs.run_server_release_tests }}
     secrets: inherit
 
   test-with-server-rc:
diff --git a/.github/workflows/stage-tests.yml b/.github/workflows/stage-tests.yml
index be8fc7874..e73f2ab40 100644
--- a/.github/workflows/stage-tests.yml
+++ b/.github/workflows/stage-tests.yml
@@ -208,9 +208,6 @@ jobs:
         docker-hub-username: ${{ secrets.DOCKER_HUB_BOT_USERNAME }}
         docker-hub-password: ${{ secrets.DOCKER_HUB_BOT_PW }}
 
-    - name: Wait for server to be ready
-      run: sleep 5
-
     - name: Install wheel
       run: python3 -m pip install *.whl
 
diff --git a/.github/workflows/test-server-rc.yml b/.github/workflows/test-server-rc.yml
index b9ac5d95a..33a642f1c 100644
--- a/.github/workflows/test-server-rc.yml
+++ b/.github/workflows/test-server-rc.yml
@@ -48,9 +48,6 @@ jobs:
         docker-hub-username: ${{ secrets.DOCKER_HUB_BOT_USERNAME }}
         docker-hub-password: ${{ secrets.DOCKER_HUB_BOT_PW }}
 
-    - name: Wait for server to be ready
-      run: sleep 5
-
     - uses: actions/download-artifact@v4
       with:
         name: ${{ matrix.python[1] }}-manylinux_${{ matrix.platform }}.build
@@ -102,9 +99,6 @@ jobs:
         docker-hub-username: ${{ secrets.DOCKER_HUB_BOT_USERNAME }}
         docker-hub-password: ${{ secrets.DOCKER_HUB_BOT_PW }}
 
-    - name: Wait for server to be ready
-      run: sleep 5
-
     - uses: actions/download-artifact@v4
       with:
         name: ${{ matrix.python[1] }}-macosx_x86_64.build
@@ -171,15 +165,16 @@ jobs:
       run: cp config.conf.template config.conf
       working-directory: test
 
-    - name: Wait for server to be ready
-      run: sleep 5
-
     - name: Install wheel
       run: python${{ matrix.python-version[0] }} -m pip install --force-reinstall --break-system-packages *.whl
 
     - run: python${{ matrix.python-version[0] }} -m pip install --force-reinstall --break-system-packages -r requirements.txt
       working-directory: test
 
+    - uses: ./.github/actions/wait-for-as-server-to-start
+      with:
+        container-name: aerospike
+
     - run: python${{ matrix.python-version[0] }} -m pytest new_tests/
       id: test
       working-directory: test
diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
index 9ac6e835e..5aa87602d 100644
--- a/.github/workflows/tests.yml
+++ b/.github/workflows/tests.yml
@@ -99,14 +99,14 @@ jobs:
     - name: Run Aerospike server
       run: docker run -d --name aerospike -p 3000-3002:3000-3002 aerospike/aerospike-server
 
-    - name: Wait for database to be ready
-      # Should be ready after 3 seconds
-      run: sleep 3
-
     - name: Create config.conf
       run: cp config.conf.template config.conf
       working-directory: test
 
+    - uses: ./.github/actions/wait-for-as-server-to-start
+      with:
+        container-name: aerospike
+
     - name: Get number of tests
       run: echo "NUM_TESTS=$(python3 -m pytest new_tests/ --collect-only -q | tail -n 1 | awk '{print $1;}')" >> $GITHUB_ENV
       working-directory: test
@@ -152,8 +152,6 @@ jobs:
         docker-hub-username: ${{ secrets.DOCKER_HUB_BOT_USERNAME }}
         docker-hub-password: ${{ secrets.DOCKER_HUB_BOT_PW }}
 
-    - run: sleep 5
-
     - run: python3 -m pytest --cov=aerospike_helpers --cov-report xml:coverage.xml ./new_tests
       working-directory: test
 
@@ -277,14 +275,14 @@ jobs:
     - name: Run Aerospike server
       run: docker run -d --name aerospike -p 3000-3002:3000-3002 aerospike/aerospike-server
 
-    - name: Wait for database to be ready
-      # Should be ready after 3 seconds
-      run: sleep 3
-
     - name: Create config.conf
       run: cp config.conf.template config.conf
       working-directory: test
 
+    - uses: ./.github/actions/wait-for-as-server-to-start
+      with:
+        container-name: aerospike
+
     - name: Run tests
       run: python -m pytest ./new_tests
       working-directory: test
@@ -337,14 +335,14 @@ jobs:
       if: ${{ !contains(github.event.pull_request.labels.*.name, 'new-server-features') }}
       run: docker run -d --name aerospike -p 3000-3002:3000-3002 aerospike/aerospike-server
 
-    - name: Wait for database to be ready
-      # Should be ready after 3 seconds
-      run: sleep 3
-
     - name: Create config.conf
       run: cp config.conf.template config.conf
       working-directory: test
 
+    - uses: ./.github/actions/wait-for-as-server-to-start
+      with:
+        container-name: aerospike
+
     - name: Run tests
       run: python -m pytest ./new_tests -vv
       working-directory: test
@@ -375,14 +373,14 @@ jobs:
     - name: Run lowest supported server
       run: docker run -d --name aerospike -p 3000-3002:3000-3002 aerospike/aerospike-server:${{ vars.LOWEST_SUPPORTED_SERVER_VERSION }}
 
-    - name: Wait for database to be ready
-      # Should be ready after 3 seconds
-      run: sleep 3
-
     - name: Create config.conf
       run: cp config.conf.template config.conf
       working-directory: test
 
+    - uses: ./.github/actions/wait-for-as-server-to-start
+      with:
+        container-name: aerospike
+
     - name: Run tests
       run: python -m pytest ./new_tests
       working-directory: test
@@ -416,9 +414,6 @@ jobs:
         docker-hub-username: ${{ secrets.DOCKER_HUB_BOT_USERNAME }}
         docker-hub-password: ${{ secrets.DOCKER_HUB_BOT_PW }}
 
-    - name: Wait for server to start
-      run: sleep 5
-
     - name: Run tests
       run: python -m pytest ./new_tests/test_admin_*.py
       working-directory: test
diff --git a/.github/workflows/valgrind.yml b/.github/workflows/valgrind.yml
index 254d9d56c..011539d2c 100644
--- a/.github/workflows/valgrind.yml
+++ b/.github/workflows/valgrind.yml
@@ -48,10 +48,6 @@ jobs:
         docker-hub-username: ${{ secrets.DOCKER_HUB_BOT_USERNAME }}
         docker-hub-password: ${{ secrets.DOCKER_HUB_BOT_PW }}
 
-    - name: Wait for database to be ready
-      # Should be ready after 3 seconds
-      run: sleep 3
-
     - run: sudo apt update
     - run: sudo apt install valgrind -y
     - run: PYTHONMALLOC=malloc valgrind --leak-check=full --error-exitcode=1 python3 -m pytest -v new_tests/${{ github.event.inputs.test-file }}
diff --git a/.github/workflows/wait-for-as-server-to-start.bash b/.github/workflows/wait-for-as-server-to-start.bash
new file mode 100755
index 000000000..4f20ba4f7
--- /dev/null
+++ b/.github/workflows/wait-for-as-server-to-start.bash
@@ -0,0 +1,29 @@
+#!/bin/bash
+
+set -x
+# Makes sure that if the "docker exec" command fails, it is not ignored
+set -o pipefail
+
+container_name=$1
+is_security_enabled=$2
+
+while true; do
+    if [[ $is_security_enabled == true ]]; then
+        # We need to pass credentials to asinfo if server requires it
+        # TODO: passing in hardcoded credentials since I can't figure out how to use --instance with global astools.conf
+        user_credentials="--user=admin --password=admin"
+    fi
+
+    # An unset variable will have a default empty value
+    # Intermediate step is to print docker exec command's output in case it fails
+    # Sometimes, errors only appear in stdout and not stderr, like if asinfo throws an error because of no credentials
+    # (This is a bug in asinfo since all error messages should be sent to stderr)
+    # But piping and passing stdin to grep will hide the first command's stdout.
+    # grep doesn't have a way to print all lines passed as input.
+    # ack does have an option but it doesn't come installed by default
+    # shellcheck disable=SC2086 # The flags in user credentials should be separate anyways. Not one string
+    if docker exec "$container_name" asinfo $user_credentials -v status | tee >(cat) | grep -qE "^ok"; then
+        # Server is ready when asinfo returns ok
+        break
+    fi
+done