Fix startup scripts to POSIX standards / add timeout for cwagent wait…

…ing (#2016)

* Added timeout to cwagents waiting + shellcheck addition

* Setting +x permission on run-shellcheck.sh

* Fix Shellcheck errors by doing as little work as possible (#2022)

# Summary | Résumé

These fixes a bunch of shellcheck errors and ignores a few I didn't test, actually I didn't test any of this, most of the fixes were pretty straightforward ones, there was one I fixed with co-pilot that should be tested and the others I didn't fix I just assumed they worked as is since they were in the existing codebase.

- **Fixes**: https://www.shellcheck.net/wiki/SC1091 Couldn't find the file that was being referenced (`environment_test.sh`) in `scripts/run_single_test.sh` so I just the source to /dev/null
- **Fixes**: https://www.shellcheck.net/wiki/SC2028 Replaces some echo's where a control character was passed "\n" with a printf so it should work the same and not complain
- **Fixes the read -p error**: https://www.shellcheck.net/wiki/SC3045 by asking co-pilot to fix it, I can't guarantee this one will work, **YOU SHOULD TEST THIS FILE: scripts/run_celery_purge.sh**
- **Ignores**: https://www.shellcheck.net/wiki/SC2009 I'm assuming it worked and I didn't want to find the pgrep way of doing it, also it's not posix
- **Ignores**: https://www.shellcheck.net/wiki/SC2219 I'm assuming it worked and I didn't want to test if the following works
```bash 
((wait_time++)) || true
```
- **Fixes**: https://www.shellcheck.net/wiki/SC2105 By removing the break from the case, they don't need it.
- **Fixes**: https://www.shellcheck.net/wiki/SC3046 By replacing `source` with `.`
- **Fixes**: https://www.shellcheck.net/wiki/SC2068 by wrapping array Expansion with double quotes
- **Fixes**: https://www.shellcheck.net/wiki/SC2086 By wrapping the variable in double quotes

---------

Co-authored-by: Calvin Rodo <[email protected]>

.github/workflows/scripts/run-shellcheck.sh

@@ -0,0 +1,3 @@
+#!/bin/bash
+
+docker run --rm -v "$PWD:/mnt" koalaman/shellcheck:v0.9.0 -P ./bin/ -x ./scripts/*.sh

.github/workflows/shellcheck.yml

@@ -0,0 +1,14 @@
+name: Shellcheck
+on:
+  push:
+    paths:
+      - "**/*.sh"
+
+jobs:
+  shellcheck:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - name: Shellcheck
+      run: |
+        .github/workflows/scripts/run-shellcheck.sh

scripts/run_celery.sh

@@ -1,25 +1,30 @@
 #!/bin/sh
 
-# runs celery with all celery queues except the throtted sms queue
-
 set -e
 
-# Check and see if this is running in K8s and if so, wait for cloudwatch agent
-if [[ ! -z "${STATSD_HOST}" ]]; then
+# Runs celery with all celery queues except the throtted sms queue.
 
-    echo "Initializing... Waiting for CWAgent to become ready."
-    while :
-    do
-        if  nc -vz $STATSD_HOST 25888; then
+# Check and see if this is running in K8s and if so, wait for cloudwatch agent
+if [ -n "${STATSD_HOST}" ]; then
+    echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds."
+    timeout=30
+    while [ $timeout -gt 0 ]; do
+        if nc -vz "$STATSD_HOST" 25888; then
             echo "CWAgent is Ready."
-            break;
+            break
         else
             echo "Waiting for CWAgent to become ready."
             sleep 1
+            timeout=$((timeout - 1))
         fi
     done
+
+    if [ $timeout -eq 0 ]; then
+        echo "Timeout reached. CWAgent did not become ready in 30 seconds."
+        exit 1
+    fi
 fi
 
 echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"
 
-celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-email-tasks,service-callbacks,delivery-receipts
+celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-email-tasks,service-callbacks,delivery-receipts

scripts/run_celery_beat.sh

@@ -1,23 +1,28 @@
 #!/bin/sh
 
-# runs the celery beat process. This runs the periodic tasks
-
 set -e
 
-# Check and see if this is running in K8s and if so, wait for cloudwatch agent
-if [[ ! -z "${STATSD_HOST}" ]]; then
+# Runs the celery beat process, i.e the Celery periodic tasks.
 
-    echo "Initializing... Waiting for CWAgent to become ready."
-    while :
-    do
-        if  nc -vz $STATSD_HOST 25888; then
+# Check and see if this is running in K8s and if so, wait for cloudwatch agent
+if [ -n "${STATSD_HOST}" ]; then
+    echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds."
+    timeout=30
+    while [ $timeout -gt 0 ]; do
+        if nc -vz "$STATSD_HOST" 25888; then
             echo "CWAgent is Ready."
-            break;
+            break
         else
             echo "Waiting for CWAgent to become ready."
             sleep 1
+            timeout=$((timeout - 1))
         fi
     done
+
+    if [ $timeout -eq 0 ]; then
+        echo "Timeout reached. CWAgent did not become ready in 30 seconds."
+        exit 1
+    fi
 fi
 
 celery -A run_celery.notify_celery beat --loglevel=INFO

scripts/run_celery_core_tasks.sh

@@ -1,25 +1,31 @@
 #!/bin/sh
 
-# runs celery with all celery queues except send-throttled-sms-tasks, send-sms-* and send-email-*
-
 set -e
 
-# Check and see if this is running in K8s and if so, wait for cloudwatch agent
-if [[ ! -z "${STATSD_HOST}" ]]; then
+# Runs celery with all celery queues except send-throttled-sms-tasks, 
+# send-sms-* and send-email-*.
 
-    echo "Initializing... Waiting for CWAgent to become ready."
-    while :
-    do
-        if  nc -vz $STATSD_HOST 25888; then
+# Check and see if this is running in K8s and if so, wait for cloudwatch agent
+if [ -n "${STATSD_HOST}" ]; then
+    echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds."
+    timeout=30
+    while [ $timeout -gt 0 ]; do
+        if nc -vz "$STATSD_HOST" 25888; then
             echo "CWAgent is Ready."
-            break;
+            break
         else
             echo "Waiting for CWAgent to become ready."
             sleep 1
+            timeout=$((timeout - 1))
         fi
     done
+
+    if [ $timeout -eq 0 ]; then
+        echo "Timeout reached. CWAgent did not become ready in 30 seconds."
+        exit 1
+    fi
 fi
 
 echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"
 
-celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,service-callbacks,delivery-receipts
+celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,service-callbacks,delivery-receipts

scripts/run_celery_exit.sh

@@ -10,6 +10,7 @@ function get_celery_pids {
   # and keep only these PIDs
 
   set +o pipefail # so grep returning no matches does not premature fail pipe
+  # shellcheck disable=SC2009 # We don't want to bother re-writing this to use pgrep
   APP_PIDS=$(ps aux --sort=start_time | grep 'celery worker' | grep 'bin/celery' | head -1 | awk '{print $2}')
   set -o pipefail # pipefail should be set everywhere else
 }
@@ -18,12 +19,12 @@ function send_signal_to_celery_processes {
   # refresh pids to account for the case that some workers may have terminated but others not
   get_celery_pids
   # send signal to all remaining apps
-  echo ${APP_PIDS} | tr -d '\n' | tr -s ' ' | xargs echo "Sending signal ${1} to processes with pids: " >> /proc/1/fd/1
+  echo "${APP_PIDS}" | tr -d '\n' | tr -s ' ' | xargs echo "Sending signal ${1} to processes with pids: " >> /proc/1/fd/1
   echo "We will send ${1} signal" >> /proc/1/fd/1
   for value in ${APP_PIDS}
   do
-    echo kill -s ${1} $value
-    kill -s ${1} $value
+    echo kill -s "${1}" "$value"
+    kill -s "${1}" "$value"
   done
   #echo ${APP_PIDS} | xargs kill -s ${1}
 }
@@ -59,6 +60,7 @@ function on_exit {
     echo "exit function is running with wait time of 9s" >> /proc/1/fd/1
     get_celery_pids
     ensure_celery_is_running
+    # shellcheck disable=SC2219 # We could probably rewrite it as `((wait_time++)) || true` but I haven't tested and I assume this works as is
     let wait_time=wait_time+1
     sleep 1
   done

scripts/run_celery_local.sh

@@ -7,4 +7,4 @@ set -e
 
 echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"
 
-celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-throttled-sms-tasks,send-email-high,send-email-medium,send-email-low,send-email-tasks,service-callbacks,delivery-receipts
+celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low,send-throttled-sms-tasks,send-email-high,send-email-medium,send-email-low,send-email-tasks,service-callbacks,delivery-receipts

scripts/run_celery_no_sms_sending.sh

@@ -1,26 +1,31 @@
 #!/bin/sh
 
-# runs celery with only the throttled sms sending queue
-
-# runs celery with all celery queues except send-throttled-sms-tasks, send-sms-tasks, send-sms-high, send-sms-medium, or send-sms-low
-
 set -e
 
+# Runs celery with all celery queues except send-throttled-sms-tasks, 
+# send-sms-tasks, send-sms-high, send-sms-medium, or send-sms-low.
+
 # Check and see if this is running in K8s and if so, wait for cloudwatch agent
-if [[ ! -z "${STATSD_HOST}" ]]; then
-    echo "Initializing... Waiting for CWAgent to become ready."
-    while :
-    do
-        if  nc -vz $STATSD_HOST 25888; then
+if [ -n "${STATSD_HOST}" ]; then
+    echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds."
+    timeout=30
+    while [ $timeout -gt 0 ]; do
+        if nc -vz "$STATSD_HOST" 25888; then
             echo "CWAgent is Ready."
-            break;
+            break
         else
             echo "Waiting for CWAgent to become ready."
             sleep 1
+            timeout=$((timeout - 1))
         fi
     done
+
+    if [ $timeout -eq 0 ]; then
+        echo "Timeout reached. CWAgent did not become ready in 30 seconds."
+        exit 1
+    fi
 fi
 
 echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"
 
-celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-email-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,delivery-receipts
+celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q database-tasks,-priority-database-tasks.fifo,-normal-database-tasks,-bulk-database-tasks,job-tasks,notify-internal-tasks,periodic-tasks,priority-tasks,normal-tasks,bulk-tasks,reporting-tasks,research-mode-tasks,retry-tasks,send-email-tasks,send-email-high,send-email-medium,send-email-low,service-callbacks,delivery-receipts

scripts/run_celery_purge.sh

@@ -2,15 +2,16 @@
 
 set -e
 
-echo "\n--------------------------------------------------\n"
-echo "                   WARNING!!!!\n"
-echo "    This script is for local development only!\n"
-echo "  It will delete everything in the celery queues.\n"
-echo "--------------------------------------------------\n"
-echo "Are you sure you want to continue?"
-read -p "If so, type 'purge'> " check
+printf "\n--------------------------------------------------\n"
+printf "                   WARNING!!!!\n"
+printf "    This script is for local development only!\n"
+printf "  It will delete everything in the celery queues.\n"
+printf "\n--------------------------------------------------\n"
+printf "Are you sure you want to continue?"
+echo "If so, type 'purge'> \c"
+read -r check
     case $check in
-        purge ) echo "purging!"; celery -A run_celery.notify_celery purge -f; break;;
-        * ) echo "\nNot purging\n";;
+        purge ) echo "purging!"; celery -A run_celery.notify_celery purge -f;;
+        * ) printf "\nNot purging\n";;
     esac
 

scripts/run_celery_send_email.sh

@@ -1,26 +1,31 @@
 #!/bin/sh
 
-# runs celery with only the send-email-* queues
-
 set -e
 
-# Check and see if this is running in K8s and if so, wait for cloudwatch agent
-if [[ ! -z "${STATSD_HOST}" ]]; then
+# Runs celery with only the send-email-* queues.
 
-    echo "Initializing... Waiting for CWAgent to become ready."
-    while :
-    do
-        if  nc -vz $STATSD_HOST 25888; then
+# Check and see if this is running in K8s and if so, wait for cloudwatch agent
+if [ -n "${STATSD_HOST}" ]; then
+    echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds."
+    timeout=30
+    while [ $timeout -gt 0 ]; do
+        if nc -vz "$STATSD_HOST" 25888; then
             echo "CWAgent is Ready."
-            break;
+            break
         else
             echo "Waiting for CWAgent to become ready."
             sleep 1
+            timeout=$((timeout - 1))
         fi
     done
+
+    if [ $timeout -eq 0 ]; then
+        echo "Timeout reached. CWAgent did not become ready in 30 seconds."
+        exit 1
+    fi
 fi
 
 echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"
 
-# TODO: we shouldn't be using the send-email-tasks queue anymore - once we verify this we can remove it
-celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q send-email-tasks,send-email-high,send-email-medium,send-email-low
+# TODO: we shouldn't be using the send-email-tasks queue anymore, once we verify this we can remove it
+celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q send-email-tasks,send-email-high,send-email-medium,send-email-low

scripts/run_celery_send_sms.sh

@@ -1,23 +1,31 @@
 #!/bin/sh
 
-# runs celery with only the send-sms-* queues
 set -e
 
-if [[ ! -z "${STATSD_HOST}" ]]; then
-    echo "Initializing... Waiting for CWAgent to become ready."
-    while :
-    do
-        if  nc -vz $STATSD_HOST 25888; then
+# Runs celery with only the send-sms-* queues.
+
+# Check and see if this is running in K8s and if so, wait for cloudwatch agent
+if [ -n "${STATSD_HOST}" ]; then
+    echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds."
+    timeout=30
+    while [ $timeout -gt 0 ]; do
+        if nc -vz "$STATSD_HOST" 25888; then
             echo "CWAgent is Ready."
-            break;
+            break
         else
             echo "Waiting for CWAgent to become ready."
             sleep 1
+            timeout=$((timeout - 1))
         fi
     done
+
+    if [ $timeout -eq 0 ]; then
+        echo "Timeout reached. CWAgent did not become ready in 30 seconds."
+        exit 1
+    fi
 fi
 
 echo "Start celery, concurrency: ${CELERY_CONCURRENCY-4}"
 
 # TODO: we shouldn't be using the send-sms-tasks queue anymore - once we verify this we can remove it
-celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency=${CELERY_CONCURRENCY-4} -Q send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low
+celery -A run_celery.notify_celery worker --pidfile="/tmp/celery.pid" --loglevel=INFO --concurrency="${CELERY_CONCURRENCY-4}" -Q send-sms-tasks,send-sms-high,send-sms-medium,send-sms-low

scripts/run_celery_sms.sh

@@ -1,22 +1,28 @@
 #!/bin/sh
 
-# runs celery with only the throttled sms sending queue
-
 set -e
 
+# Runs celery with only the throttled sms sending queue.
 
-if [[ ! -z "${STATSD_HOST}" ]]; then
-    echo "Initializing... Waiting for CWAgent to become ready."
-    while :
-    do
-        if  nc -vz $STATSD_HOST 25888; then
+# Check and see if this is running in K8s and if so, wait for cloudwatch agent
+if [ -n "${STATSD_HOST}" ]; then
+    echo "Initializing... Waiting for CWAgent to become ready within the next 30 seconds."
+    timeout=30
+    while [ $timeout -gt 0 ]; do
+        if nc -vz "$STATSD_HOST" 25888; then
             echo "CWAgent is Ready."
-            break;
+            break
         else
             echo "Waiting for CWAgent to become ready."
             sleep 1
+            timeout=$((timeout - 1))
         fi
     done
+
+    if [ $timeout -eq 0 ]; then
+        echo "Timeout reached. CWAgent did not become ready in 30 seconds."
+        exit 1
+    fi
 fi
 
 celery -A run_celery.notify_celery worker --loglevel=INFO --concurrency=1 -Q send-throttled-sms-tasks

scripts/run_single_test.sh

@@ -1,4 +1,5 @@
 #!/bin/sh
 # run a single unit test, pass in the unit test name for example: tests/app/service/test_rest.py::test_get_template_list
-source environment_test.sh
-py.test $@
+# shellcheck source=/dev/null # Not finding this file in code base
+. environment_test.sh
+py.test "$@"

scripts/run_tests.sh

@@ -12,9 +12,9 @@ function display_result {
   EXIT_STATUS=$2
   TEST=$3
 
-  if [ $RESULT -ne 0 ]; then
+  if [ "$RESULT" -ne 0 ]; then
     echo -e "\033[31m$TEST failed\033[0m"
-    exit $EXIT_STATUS
+    exit "$EXIT_STATUS"
   else
     echo -e "\033[32m$TEST passed\033[0m"
   fi