From fa0d4fb3d47a1d0f0e0317250417bfc39784fa2f Mon Sep 17 00:00:00 2001
From: Shizun Ge <shizunge@gmail.com>
Date: Mon, 12 Feb 2024 02:41:56 -0800
Subject: [PATCH] Misc cleanup

* Clarify uncessary commands.
* Update debug log on DOCKER_HOST.
* Do not set GANTRY_REGISTRY_HOST in tests.
* Load lib-common.sh st begining of tests.
---
 docs/migration.md                |  2 +-
 src/docker_hub_rate.sh           | 10 +++++-----
 src/entrypoint.sh                |  2 +-
 src/notification.sh              |  5 +----
 tests/gantry_login_spec.sh       |  3 ++-
 tests/gantry_notify_spec.sh      | 11 ++++-------
 tests/spec_gantry_test_helper.sh |  7 +++++--
 7 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/docs/migration.md b/docs/migration.md
index 3b2c4f9..98adc49 100644
--- a/docs/migration.md
+++ b/docs/migration.md
@@ -4,7 +4,7 @@
 
 * `docker manifest` CLI failed to get the image meta data for some registries.
 * High usage of docker hub rate. Getting manifest and then pulling the image double the usage.
-* Unnecessary `docker service update` commands slow down the overall process.
+* Running `docker service update` command when there is no new image slows down the overall process.
 * Removing images related
     * Failure of removing old images will exit and block subsequent updating.
     * `docker rmi` only works for the current host.
diff --git a/src/docker_hub_rate.sh b/src/docker_hub_rate.sh
index b40c177..d23efc9 100755
--- a/src/docker_hub_rate.sh
+++ b/src/docker_hub_rate.sh
@@ -21,10 +21,10 @@ _docker_hub_rate_token() {
   local TOKEN_URL="https://auth.docker.io/token?service=registry.docker.io&scope=repository:${IMAGE}:pull"
   if curl --version 1>/dev/null 2>&1; then
     if [ -n "${USER_AND_PASS}" ]; then
-      curl -s -S --user "${USER_AND_PASS}" "${TOKEN_URL}"
+      curl --silent --show-error --user "${USER_AND_PASS}" "${TOKEN_URL}"
       return $?
     fi
-    curl -s -S "${TOKEN_URL}"
+    curl --silent --show-error "${TOKEN_URL}"
     return $?
   fi
   [ -n "${USER_AND_PASS}" ] && log WARN "Cannot read docker hub rate for the given user because curl is not available."
@@ -38,7 +38,7 @@ _docker_hub_rate_read_rate() {
   local HEADER="Authorization: Bearer ${TOKEN}"
   local URL="https://registry-1.docker.io/v2/${IMAGE}/manifests/latest"
   if curl --version 1>/dev/null 2>&1; then
-    curl -s -S --head -H "${HEADER}" "${URL}" 2>&1
+    curl --silent --show-error --head -H "${HEADER}" "${URL}" 2>&1
     return $?
   fi
   # Add `--spider`` implies that you want to send a HEAD request (as opposed to GET or POST).
@@ -57,7 +57,7 @@ docker_hub_rate() {
     log_lines() { local LEVEL="${1}"; while read -r LINE; do [ -z "${LINE}" ] && continue; log "${LEVEL}" "${LINE}"; done; }
   fi
   local RESPONSE=
-  if ! RESPONSE=$(_docker_hub_rate_token "${IMAGE}" "${USER_AND_PASS}"); then
+  if ! RESPONSE=$(_docker_hub_rate_token "${IMAGE}" "${USER_AND_PASS}" 2>&1); then
     log DEBUG "Read docker hub token error: RESPONSE="
     echo "${RESPONSE}" | log_lines DEBUG
     echo "[GET TOKEN RESPONSE ERROR]"
@@ -71,7 +71,7 @@ docker_hub_rate() {
     echo "[PARSE TOKEN ERROR]"
     return 1
   fi
-  if ! RESPONSE=$(_docker_hub_rate_read_rate "${IMAGE}" "${TOKEN}"); then
+  if ! RESPONSE=$(_docker_hub_rate_read_rate "${IMAGE}" "${TOKEN}" 2>&1); then
     if echo "${RESPONSE}" | grep -q "Too Many Requests" ; then
       echo "0"
       return 0
diff --git a/src/entrypoint.sh b/src/entrypoint.sh
index 95a7270..cfa5c7b 100755
--- a/src/entrypoint.sh
+++ b/src/entrypoint.sh
@@ -82,6 +82,7 @@ gantry() {
   local START_TIME=
   START_TIME=$(date +%s)
 
+  [ -n "${DOCKER_HOST}" ] && log DEBUG "DOCKER_HOST=${DOCKER_HOST}"
   local RUN_ON_NODE=
   if ! RUN_ON_NODE=$(_run_on_node); then
     local HOST_STRING="${DOCKER_HOST:-"the current node"}"
@@ -91,7 +92,6 @@ gantry() {
     log DEBUG "Set NODE_NAME=${RUN_ON_NODE}"
     export NODE_NAME="${RUN_ON_NODE}"
   fi
-  [ -n "${DOCKER_HOST}" ] && log DEBUG "DOCKER_HOST=${DOCKER_HOST}"
   log INFO "Run on Docker host ${RUN_ON_NODE}."
 
   local ACCUMULATED_ERRORS=0
diff --git a/src/notification.sh b/src/notification.sh
index 4669d81..022a6dc 100755
--- a/src/notification.sh
+++ b/src/notification.sh
@@ -37,11 +37,8 @@ _notify_via_apprise() {
   [ -z "${BODY}" ] && BODY="${TITLE}"
   TITLE=$(_replace_newline "${TITLE}")
   BODY=$(_replace_newline "${BODY}")
-  local RETURN_VALUE=0
   local LOG=
-  LOG=$(curl --silent -X POST -H "Content-Type: application/json" --data "{\"title\": \"${TITLE}\", \"body\": \"${BODY}\", \"type\": \"${TYPE}\"}" "${URL}" 2>&1)
-  RETURN_VALUE="${?}"
-  if [ "${RETURN_VALUE}" = "0" ]; then
+  if LOG=$(curl --silent --show-error -X POST -H "Content-Type: application/json" --data "{\"title\": \"${TITLE}\", \"body\": \"${BODY}\", \"type\": \"${TYPE}\"}" "${URL}" 2>&1); then
     log INFO "Sent notification via Apprise:"
     echo "${LOG}" | log_lines INFO
   else
diff --git a/tests/gantry_login_spec.sh b/tests/gantry_login_spec.sh
index 240925a..097206e 100644
--- a/tests/gantry_login_spec.sh
+++ b/tests/gantry_login_spec.sh
@@ -108,7 +108,8 @@ Describe 'Login'
       # Since we pass credentials via the configs file, we can use other envs to login to docker hub and check the rate.
       # However we do not actually check whether we read rates correctly, in case password or usrename for docker hub is not set.
       # It seems there is no rate limit when running from the github actions, which also gives us a NaN error.
-      export GANTRY_REGISTRY_HOST="docker.io"
+      # Do not set GANTRY_REGISTRY_HOST to test the default config.
+      # export GANTRY_REGISTRY_HOST="docker.io"
       export GANTRY_REGISTRY_PASSWORD="${DOCKERHUB_PASSWORD:-""}"
       export GANTRY_REGISTRY_USER="${DOCKERHUB_USERNAME:-""}"
       local RETURN_VALUE=
diff --git a/tests/gantry_notify_spec.sh b/tests/gantry_notify_spec.sh
index 078bef8..fef7ef2 100644
--- a/tests/gantry_notify_spec.sh
+++ b/tests/gantry_notify_spec.sh
@@ -34,18 +34,15 @@ Describe 'Notify'
       local EMAIL_API_PORT=8025
       local SERVICE_NAME_APPRISE="${SERVICE_NAME}-apprise"
       local SERVICE_NAME_MAILPIT="${SERVICE_NAME}-mailpit"
-      local SCRIPT_DIR=
-      SCRIPT_DIR="$(get_script_dir)" || return 1
-      source "${SCRIPT_DIR}/../src/lib-common.sh"
       docker pull caronc/apprise
       docker pull axllent/mailpit
-      # Use docker_run to improve coverage on lib-common.sh.
+      # Use docker_run to improve coverage on lib-common.sh. `docker run` can do the same thing.
       docker_run -d --restart=on-failure:10 --name="${SERVICE_NAME_APPRISE}" --network=host \
         -e "APPRISE_STATELESS_URLS=mailto://localhost:${SMTP_PORT}?user=userid&pass=password" \
         caronc/apprise
       docker_run -d --restart=on-failure:10 --name="${SERVICE_NAME_MAILPIT}" --network=host \
         axllent/mailpit \
-        --smtp "0.0.0.0:${SMTP_PORT}" --listen "0.0.0.0:${EMAIL_API_PORT}" \
+        --smtp "localhost:${SMTP_PORT}" --listen "localhost:${EMAIL_API_PORT}" \
         --smtp-auth-accept-any --smtp-auth-allow-insecure
       export GANTRY_NOTIFICATION_APPRISE_URL="http://localhost:${APPRISE_PORT}/notify"
       export GANTRY_NOTIFICATION_TITLE="TEST_TITLE"
@@ -58,7 +55,7 @@ Describe 'Notify'
       docker logs "${SERVICE_NAME_APPRISE}" 2>&1
       echo "Print Mailpit log:"
       docker logs "${SERVICE_NAME_MAILPIT}" 2>&1
-      # Use docker_remove to improve coverage on lib-common.sh.
+      # Use docker_remove to improve coverage on lib-common.sh. `docker stop` and `docker rm` can do the same thing.
       docker_remove "${SERVICE_NAME_APPRISE}"
       docker_remove "${SERVICE_NAME_MAILPIT}"
       return "${RETURN_VALUE}"
@@ -135,4 +132,4 @@ Describe 'Notify'
       The stderr should satisfy spec_expect_message    "Failed to send notification via Apprise"
     End
   End
-End # Describe 'Notify'
\ No newline at end of file
+End # Describe 'Notify'
diff --git a/tests/spec_gantry_test_helper.sh b/tests/spec_gantry_test_helper.sh
index fee9d14..bc10baa 100644
--- a/tests/spec_gantry_test_helper.sh
+++ b/tests/spec_gantry_test_helper.sh
@@ -187,6 +187,9 @@ _stop_registry() {
 initialize_all_tests() {
   local SUITE_NAME="${1:-"gantry"}"
   SUITE_NAME=$(echo "${SUITE_NAME}" | tr ' ' '-')
+  local SCRIPT_DIR=
+  SCRIPT_DIR="$(_get_script_dir)" || return 1
+  source "${SCRIPT_DIR}/../src/lib-common.sh"
   echo "=============================="
   echo "== Starting tests in ${SUITE_NAME}"
   echo "=============================="
@@ -455,7 +458,7 @@ stop_service() {
   docker service rm "${SERVICE_NAME}"
 }
 
-get_script_dir() {
+_get_script_dir() {
   # SC2128: Expanding an array without an index only gives the first element.
   # SC3054 (warning): In POSIX sh, array references are undefined.
   # shellcheck disable=SC2128,SC3054
@@ -477,7 +480,7 @@ _get_entrypoint() {
     return 0
   fi
   local SCRIPT_DIR=
-  SCRIPT_DIR="$(get_script_dir)" || return 1
+  SCRIPT_DIR="$(_get_script_dir)" || return 1
   export STATIC_VAR_ENTRYPOINT="${SCRIPT_DIR}/../src/entrypoint.sh"
   echo "source ${STATIC_VAR_ENTRYPOINT}"
 }