fix broken compare_versions

Rework date and version comparisons functions; use actual input values
for tests.

Signed-off-by: Ian Cardoso <osodracnai@gmail.com>
Signed-off-by: Brad Davidson <brad.davidson@rancher.com>

.github/workflows/tests.yml

@@ -17,7 +17,7 @@ jobs:
           sudo apt-get install -y shellcheck
         shell: bash
       - name: Run shellcheck
-        run: shellcheck --severity warning scripts/upgrade.sh scripts/tests.sh
+        run: shellcheck scripts/upgrade.sh scripts/tests.sh
 
   test:
     name: Run Unit Tests
@@ -30,4 +30,4 @@ jobs:
       - name: Run unit tests
         run: |
           cd scripts
-          ./tests.sh
+          ./tests.sh

scripts/tests.sh

@@ -13,32 +13,40 @@ compare_versions_test() {
         compare_versions "$version1" "$version2"
         result=$?
         if [ "$result" -ne "$expected_result" ]; then
-            failed_cases="${failed_cases}compare_versions_test test case failed: ${version1}, ${version2}"
+            failed_cases=1
+            echo "compare_versions_test test case failed: ${version1}, ${version2}: expected $expected_result, got $result"
         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
+# Returns 0 if version2 >= version1, 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"
+    "v1.0.0+k3s1" "v1.25.5+k3s1" "0" \
+    "v1.25.5+k3s1" "v1.0.0+k3s1" "1" \
+    "v1.25.5+k3s1" "v1.25.5+k3s1" "0" \
+    "v1.25.5+k3s1" "v1.25.6+k3s1" "0" \
+    "v1.26.1+k3s1" "v1.25.6+k3s1" "1" \
+    "v1.25.5+k3s1" "v1.26.1+k3s1" "0" \
+    "v1.25.5+k3s1" "v1.25.4+k3s1" "1" \
+    "v1.25.4+k3s1" "v1.25.5-rc1+k3s1" "0" \
+    "v1.25.5-rc1+k3s1" "v1.25.5-rc2+k3s1" "0" \
+    "v1.25.5-rc2+k3s1" "v1.25.5-rc1+k3s1" "0" \
+    "v1.25.5-rc2+k3s1" "v1.25.5+k3s1" "0" \
+    "v1.25.5-rc1+k3s1" "v1.25.4+k3s1" "1" \
+
 
 build_date_tests() {
+    failed_cases=""
     # 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
@@ -49,25 +57,32 @@ build_date_tests() {
         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
+            failed_cases=1
+            echo "Build date test case failed: $build_date1, $build_date2: expected $expected_result, got $result"
         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."
+
+    if [ -n "$failed_cases" ]; then
+        exit 1
+    fi
+
+    echo "All build_date_tests test cases passed."
 }
 
 # Function to compare build dates
-# Returns 0 if build_date1 <= build_date2, 1 otherwise
+# Returns 0 if year+month of build_date2 >= build_date1, 1 otherwise
 build_date_tests \
+    "1999-01-01T12:45:56Z" "2100-12-31T01:23:45Z" 0 \
+    "2100-12-31T01:23:45Z" "1999-01-01T12:45:56Z" 1 \
     "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" 0 \
     "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" 0 \
-    "2023-07-25T22:46:58Z" "2023-07-25T22:00:15Z" 0
+    "2023-07-25T22:46:58Z" "2023-07-25T22:00:15Z" 0 \
 

scripts/upgrade.sh

@@ -12,6 +12,7 @@ fatal()
 }
 
 get_k3s_process_info() {
+  # shellcheck disable=SC2009
   K3S_PID=$(ps -ef | grep -E "( |/)k3s .*(server|agent)" | grep -E -v "(init|grep|channelserver|supervise-daemon)" | awk '{print $2}')
 
   # If we found multiple pids, and the kernel exposes pid namespaces through procfs, filter out any pids
@@ -41,7 +42,7 @@ get_k3s_process_info() {
   # 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" ] && cat "/host/proc/${K3S_PPID}/cmdline" | tr "\0" " " | grep k3s | grep -q -v supervise-daemon; then
+  if [ "$K3S_PPID" != "1" ] && tr "\0" " " < "/host/proc/${K3S_PPID}/cmdline" | grep k3s | grep -q -v supervise-daemon; then
     K3S_PID="${K3S_PPID}"
   fi
 
@@ -69,6 +70,7 @@ replace_binary() {
   info "Comparing old and new binaries"
   BIN_CHECKSUMS="$(sha256sum "$NEW_BINARY" "$FULL_BIN_PATH")"
 
+  # shellcheck disable=SC2181
   if [ "$?" != "0" ]; then
     fatal "Failed to calculate binary checksums"
   fi
@@ -81,8 +83,8 @@ replace_binary() {
 
   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]+')"
+  NEW_BIN_SEMVER="$($NEW_BINARY -v | head -1)"
+  FULL_BIN_SEMVER="$($FULL_BIN_PATH -v | head -1)"
 
   # Returns 0 if version1 <= version2, 1 otherwise
   compare_versions "$FULL_BIN_SEMVER" "$NEW_BIN_SEMVER"
@@ -170,61 +172,28 @@ verify_controlplane_versions() {
   done
 }
 
-# Function to compare semantic versions
+# Function to compare semantic versions.
+# Compares only major.minor.patch, ignoring any leading characters and trailing pre-release or build metadata.
 # Returns 0 if version1 <= version2, 1 otherwise
 compare_versions() {
-    version1="$1"
-    version2="$2"
+    version1=$(echo "$1" | grep -Eo '[0-9]+\.[0-9]+\.[0-9]+')
+    version2=$(echo "$2" | grep -Eo '[0-9]+\.[0-9]+\.[0-9]+')
 
     if [ "$version1" = "$version2" ]; then
         return 0
     fi
 
     IFS=.
-    set -- $version1
-    version1_parts=$#
-    set -- $version2
-    version2_parts=$#
 
-    # Append additional parts to the version with fewer parts
-    if [ $version1_parts -lt $version2_parts ]; then
-        version1="${version1}.0"
-    elif [ $version2_parts -lt $version1_parts ]; then
-        version2="${version2}.0"
-    fi
-
-    IFS=.
+    # shellcheck disable=SC2086
     set -- $version1
-    version1_parts=$#
+    version1=$(printf "%03d%03d%03d" "$@")
+
+    # shellcheck disable=SC2086
     set -- $version2
-    version2_parts=$#
-
-    for _ in $(seq 1 $version1_parts); do
-        num1=$1
-        shift
-        num2=$1
-        shift
-
-        # Remove leading zeros
-        num1=$(echo "$num1" | sed 's/^0*//')
-        num2=$(echo "$num2" | sed 's/^0*//')
-
-        if [ -z "$num1" ]; then
-            num1=0
-        fi
-
-        if [ -z "$num2" ]; then
-            num2=0
-        fi
-
-        if [ "$num1" -lt "$num2" ]; then
-            return 0
-        elif [ "$num1" -gt "$num2" ]; then
-            return 1
-        fi
-    done
+    version2=$(printf "%03d%03d%03d" "$@")
 
-    return 0
+    test "$version2" -ge "$version1"
 }
 
 # Function to convert "2023-06-20T12:30:15Z" format to "2023-06-20 12:30:15"
@@ -255,11 +224,7 @@ compare_build_dates() {
         return 2
     fi
 
-    if [ "$timestamp1" -le "$timestamp2" ]; then
-        return 0
-    else
-        return 1
-    fi
+    test "$timestamp2" -ge "$timestamp1"
 }
 
 upgrade() {