add tests.sh, shellcheck and fix compare_versions (#70)

* add tests.sh shellcheck and fix compare_versions

This commits adds the tests.sh file with unit tests for helper functions in upgrade.sh, also fix some shellcheck warnings

Signed-off-by: Ian Cardoso <osodracnai@gmail.com>

.github/workflows/tests.yml

@@ -0,0 +1,33 @@
+name: Unit Tests
+
+on:
+  push:
+  pull_request:
+
+jobs:
+  shellcheck:
+    name: Shellcheck
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v3
+      - name: Set up shell environment
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y shellcheck
+        shell: bash
+      - name: Run shellcheck
+        run: shellcheck --severity warning scripts/upgrade.sh scripts/tests.sh
+
+  test:
+    name: Run Unit Tests
+    runs-on: ubuntu-latest
+    container:
+      image: busybox
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@v1
+      - name: Run unit tests
+        run: |
+          cd scripts
+          ./tests.sh

scripts/tests.sh

@@ -0,0 +1,72 @@
+#!/bin/sh -x
+
+# shellcheck disable=SC1091
+. ./upgrade.sh
+
+compare_versions_test() {
+    failed_cases=""
+    while [ $# -ge 3 ]; do
+        version1="$1"
+        version2="$2"
+        expected_result="$3"
+
+        compare_versions "$version1" "$version2"
+        result=$?
+        if [ "$result" -ne "$expected_result" ]; then
+            failed_cases="${failed_cases}compare_versions_test test case failed: ${version1}, ${version2}"
+        fi
+
+        # Shift the positional parameters to move to the next test case
+        shift 3
+    done
+
+    if [ -n "$failed_cases" ]; then
+        echo "$failed_cases"
+        exit 1
+    fi
+
+    echo "All compare_versions_test test cases passed."
+}
+
+# Function to compare semantic versions
+# Returns 0 if version1 <= version2, 1 otherwise
+compare_versions_test \
+    "1.0.0" "1.0.0" "0" \
+    "1.0.0" "1.5.0" "0" \
+    "1.2.3" "1.0.0" "1" \
+    "1.0.0" "1.2.3" "0" \
+    "1.1.0" "1.0.10" "1" \
+    "2.0.0" "2.0.0-rc1" "1"
+
+build_date_tests() {
+    # Test cases in the format: build_date1, build_date2, expected_result
+    # Example: "2023-06-23T14:58:45Z" "2023-06-20T12:30:15Z" 1 (means build_date1 is more recent)
+    while [ $# -ge 3 ]; do
+        build_date1="$1"
+        build_date2="$2"
+        expected_result="$3"
+
+        compare_build_dates "$build_date1" "$build_date2"
+        result=$?
+        if [ "$result" -ne "$expected_result" ]; then
+            echo "Build date test case failed: $build_date1, $build_date2"
+            exit 1
+        fi
+
+        # Shift the positional parameters to move to the next set of test cases
+        shift 3
+    done
+   echo "All build_date_tests test cases passed."
+}
+
+# Function to compare build dates
+# Returns 0 if build_date1 <= build_date2, 1 otherwise
+build_date_tests \
+    "2023-06-23T14:58:45Z" "2023-06-23T14:58:45Z" 0 \
+    "2023-06-20T12:30:15Z" "2023-06-23T14:58:45Z" 0 \
+    "2023-06-23T14:58:45Z" "2023-06-20T12:30:15Z" 1 \
+    "2023-07-01T08:30:00Z" "2023-07-30T20:00:00Z" 0 \
+    "2023-01-01T00:00:00Z" "2022-12-31T23:59:59Z" 1 \
+    "2023-06-01T00:00:00Z" "2023-05-31T23:59:59Z" 1 \
+    "2023-06-15T12:00:00Z" "2023-06-15T11:59:59Z" 1
+

scripts/upgrade.sh

@@ -16,8 +16,8 @@ get_k3s_process_info() {
 
   # If we found multiple pids, and the kernel exposes pid namespaces through procfs, filter out any pids
   # not running in the init pid namespace. This will exclude copies of k3s running in containers.
-  if [ "$(echo $K3S_PID | wc -w)" != "1" ] && [ -e /proc/1/ns/pid ]; then
-    K3S_PID=$(for PID in $K3S_PID; do if [ $(readlink /proc/1/ns/pid) == $(readlink /proc/$PID/ns/pid) ]; then echo $PID; fi; done)
+  if [ "$(echo "$K3S_PID" | wc -w)" != "1" ] && [ -e /proc/1/ns/pid ]; then
+    K3S_PID=$(for PID in $K3S_PID; do if [ "$(readlink /proc/1/ns/pid)" = "$(readlink /proc/"$PID"/ns/pid)" ]; then echo "$PID"; fi; done)
   fi
 
   # Check to see if we have any pids left
@@ -27,29 +27,29 @@ get_k3s_process_info() {
 
   # If we still have multiple pids, print out the matching process info for troubleshooting purposes,
   # and exit with a fatal error.
-  if [ "$(echo $K3S_PID | wc -w)" != "1" ]; then
+   if [ "$(echo "$K3S_PID" | wc -w)" != "1" ]; then
     for PID in $K3S_PID; do
-      ps -fp $PID || true
+      ps -fp "$PID" || true
     done
     fatal "Found multiple K3s pids"
   fi
 
-  K3S_PPID=$(ps -p $K3S_PID -o ppid= | awk '{print $1}')
+  K3S_PPID=$(ps -p "$K3S_PID" -o ppid= | awk '{print $1}')
   info "K3S binary is running with pid $K3S_PID, parent pid $K3S_PPID"
 
   # When running with the --log flag, the 'k3s server|agent' process is nested under a 'k3s init' process.
   # If the parent pid is not 1 (init/systemd) then we are nested and need to operate against that 'k3s init' pid instead.
   # Make sure that the parent pid is actually k3s though, as openrc systems may run k3s under supervise-daemon instead of
   # as a child process of init.
-  if [ "$K3S_PPID" != "1" ] && grep -a k3s /host/proc/${K3S_PPID}/cmdline | grep -q -v supervise-daemon; then
+  if [ "$K3S_PPID" != "1" ] && grep -a k3s "/host/proc/${K3S_PPID}/cmdline" | grep -q -v supervise-daemon; then
     K3S_PID="${K3S_PPID}"
   fi
 
   # When running in k3d, k3s will be pid 1 and is always at /bin/k3s
-  if [ "$K3S_PID" == "1" ]; then
+  if [ "$K3S_PID" = "1" ]; then
     K3S_BIN_PATH="/bin/k3s"
   else
-    K3S_BIN_PATH=$(awk 'NR==1 {print $1}' /host/proc/${K3S_PID}/cmdline)
+    K3S_BIN_PATH=$(awk 'NR==1 {print $1}' "/host/proc/${K3S_PID}/cmdline")
   fi
 
   if [ -z "$K3S_BIN_PATH" ]; then
@@ -67,21 +67,24 @@ replace_binary() {
   fi
 
   info "Comparing old and new binaries"
-  BIN_CHECKSUMS="$(sha256sum $NEW_BINARY $FULL_BIN_PATH)"
+  BIN_CHECKSUMS="$(sha256sum "$NEW_BINARY" "$FULL_BIN_PATH")"
 
   if [ "$?" != "0" ]; then
     fatal "Failed to calculate binary checksums"
   fi
 
   BIN_COUNT="$(echo "${BIN_CHECKSUMS}" | awk '{print $1}' | uniq | wc -l)"
-  if [ "$BIN_COUNT" == "1" ]; then
+  if [ "$BIN_COUNT" = "1" ]; then
     info "Binary already been replaced"
     exit 0
   fi
 
+  set +e
+
   NEW_BIN_SEMVER="$($NEW_BINARY -v | grep -Eo 'v[0-9]+\.[0-9]+\.[0-9]+')"
   FULL_BIN_SEMVER="$($FULL_BIN_PATH -v | grep -Eo 'v[0-9]+\.[0-9]+\.[0-9]+')"
 
+  # Returns 0 if version1 <= version2, 1 otherwise
   compare_versions "$FULL_BIN_SEMVER" "$NEW_BIN_SEMVER"
 
   if [ $? -eq 1 ]; then
@@ -90,22 +93,25 @@ replace_binary() {
   fi
 
   NEW_BIN_RELEASE_DATE="$($NEW_BINARY kubectl version --client=true -o yaml | grep -Eo 'buildDate:[[:space:]]+"([^"]+)' | cut -d'"' -f2)"
-  FULL_NEW_BIN_RELEASE_DATE="$($FULL_BIN_PATH kubectl version --client=true -o yaml | grep -Eo 'buildDate:[[:space:]]+"([^"]+)' | cut -d'"' -f2)"
+  FULL_BIN_RELEASE_DATE="$($FULL_BIN_PATH kubectl version --client=true -o yaml | grep -Eo 'buildDate:[[:space:]]+"([^"]+)' | cut -d'"' -f2)"
 
-  compare_build_dates "$NEW_BIN_RELEASE_DATE" "$FULL_NEW_BIN_RELEASE_DATE"
+  # Returns 0 if build_date1 <= build_date2, 1 otherwise
+  compare_build_dates "$FULL_BIN_RELEASE_DATE" "$NEW_BIN_RELEASE_DATE"
 
   if [ $? -eq 1 ]; then
     echo "Error: Current build date ${FULL_BIN_RELEASE_DATE} is more recent than ${NEW_BIN_RELEASE_DATE}"
     exit 1
   fi
 
-  K3S_CONTEXT=$(getfilecon $FULL_BIN_PATH 2>/dev/null | awk '{print $2}' || true)
+  set -e
+
+  K3S_CONTEXT=$(getfilecon "$FULL_BIN_PATH" 2>/dev/null | awk '{print $2}' || true)
   info "Deploying new k3s binary to $K3S_BIN_PATH"
-  cp $NEW_BINARY $FULL_BIN_PATH
+  cp "$NEW_BINARY" "$FULL_BIN_PATH"
 
   if [ -n "${K3S_CONTEXT}" ]; then
     info 'Restoring k3s bin context'
-    setfilecon "${K3S_CONTEXT}" $FULL_BIN_PATH
+    setfilecon "${K3S_CONTEXT}" "$FULL_BIN_PATH"
   fi
 
   info "K3s binary has been replaced successfully"
@@ -115,7 +121,7 @@ replace_binary() {
 kill_k3s_process() {
     # the script sends SIGTERM to the process and let the supervisor
     # to automatically restart k3s with the new version
-    kill -SIGTERM $K3S_PID
+    kill -SIGTERM "$K3S_PID"
     info "Successfully killed old k3s pid $K3S_PID"
 }
 
@@ -131,14 +137,14 @@ prepare() {
   NAMESPACE=$(cat /var/run/secrets/kubernetes.io/serviceaccount/namespace)
   while true; do
     # make sure control-plane plan does exist
-    PLAN=$(${KUBECTL_BIN} get plan $CONTROLPLANE_PLAN -o jsonpath='{.metadata.name}' -n $NAMESPACE 2>/dev/null)
+    PLAN=$(${KUBECTL_BIN} get plan "$CONTROLPLANE_PLAN" -o jsonpath='{.metadata.name}' -n "$NAMESPACE" 2>/dev/null)
     if [ -z "$PLAN" ]; then
 	    info "Waiting for control-plane Plan $CONTROLPLANE_PLAN to be created"
 	    sleep 5
 	    continue
     fi
-    NUM_NODES=$(${KUBECTL_BIN} get plan $CONTROLPLANE_PLAN -n $NAMESPACE -o json | jq '.status.applying | length')
-    if [ "$NUM_NODES" == "0" ]; then
+    NUM_NODES=$(${KUBECTL_BIN} get plan "$CONTROLPLANE_PLAN" -n "$NAMESPACE" -o json | jq '.status.applying | length')
+    if [ "$NUM_NODES" = "0" ]; then
       break
     fi
     info "Waiting for all control-plane nodes to be upgraded"
@@ -154,7 +160,7 @@ verify_controlplane_versions() {
       sleep 5
       continue
     fi
-    if [ "$CONTROLPLANE_NODE_VERSION" == "$SYSTEM_UPGRADE_PLAN_LATEST_VERSION" ]; then
+    if [ "$CONTROLPLANE_NODE_VERSION" = "$SYSTEM_UPGRADE_PLAN_LATEST_VERSION" ]; then
         info "All control-plane nodes have been upgraded to version to $CONTROLPLANE_NODE_VERSION"
 		    break
 		fi
@@ -167,8 +173,8 @@ verify_controlplane_versions() {
 # Function to compare semantic versions
 # Returns 0 if version1 <= version2, 1 otherwise
 compare_versions() {
-    version1=$1
-    version2=$2
+    version1="$1"
+    version2="$2"
 
     if [ "$version1" = "$version2" ]; then
         return 0
@@ -193,9 +199,11 @@ compare_versions() {
     set -- $version2
     version2_parts=$#
 
-    for i in $(seq 1 $version1_parts); do
-        num1=$(eval echo \$"$i")
-        num2=$(eval echo \$"$i")
+    for _ in $(seq 1 $version1_parts); do
+        num1=$1
+        shift
+        num2=$1
+        shift
 
         # Remove leading zeros
         num1=$(echo "$num1" | sed 's/^0*//')
@@ -209,24 +217,41 @@ compare_versions() {
             num2=0
         fi
 
-        if [ $num1 -lt $num2 ]; then
+        if [ "$num1" -lt "$num2" ]; then
             return 0
-        elif [ $num1 -gt $num2 ]; then
+        elif [ "$num1" -gt "$num2" ]; then
             return 1
         fi
     done
 
     return 0
 }
 
+# Function to convert "2023-06-20T12:30:15Z" format to "2023-06-20 12:30:15"
+convert_date_format() {
+    date_str=$1
+    echo "$date_str" | sed 's/T/ /; s/Z//'
+}
+
 # Function to compare build dates
 # Returns 0 if build_date1 <= build_date2, 1 otherwise
 compare_build_dates() {
-    build_date1=$1
-    build_date2=$2
+    build_date1=$(convert_date_format "$1")
+    build_date2=$(convert_date_format "$2")
+
+    # Convert build_date1 to seconds since the epoch
+    timestamp1=$(date -u -d "$build_date1" "+%s" 2>/dev/null)
+    if [ -z "$timestamp1" ]; then
+        echo "Error: Invalid date format for build_date1."
+        return 2
+    fi
 
-    timestamp1=$(date -u -d "$build_date1" +%s)
-    timestamp2=$(date -u -d "$build_date2" +%s)
+    # Convert build_date2 to seconds since the epoch
+    timestamp2=$(date -u -d "$build_date2" "+%s" 2>/dev/null)
+    if [ -z "$timestamp2" ]; then
+        echo "Error: Invalid date format for build_date2."
+        return 2
+    fi
 
     if [ "$timestamp1" -le "$timestamp2" ]; then
         return 0