ci/ipsec: Fix downgrade version retrieval

Figuring out the right "previous patch release version number" to
downgrade to in print-downgrade-version.sh turns out to be more complex
than expected [0][1][2][3].

This commit is an attempt to 1) fix issues with the current script and
2) overall make the script clearer, so we can avoid repeating these
mistakes.

As for the fixes, there are two things that are not correct with the
current version. First, we're trying to validate the existence of the
tag to downgrade to, in case the script runs on top of a release
preparation commit for which file VERSION has been updated to a value
that does not yet contains a corresponding tag. This part of the script
is actually OK, but not the way we call it in the IPsec workflow: we use
"fetch-tags: true" but "fetch-depth: 0" (the default), and the two are
not compatible, a shallow clone results in no tags being fetched.

To address this, we change our strategy: we fetch two commits in the CI
workflow, the latest commit and the rest of the history compressed into
a first, giant commit. This makes it possible to check whether the
VERSION file has been modified in the latest commit, in which case we
consider this is a release preparation commit and we decrement the
patch version number before returning it. Just make sure the VERSION
update comes in the last commit of your PRs.

The second issue is that we would return no value from the script if the
patch release is zero. This is to avoid any attempt to find a previous
patch release when working on a development branch. However, this logics
is incorrect (it comes from a previous version of the script where we
would always decrement the patch number). After the first release of a
new minor version, it's fine to have a patch number at 0. What we should
check instead is whether the version ends with "-dev".

This commit brings additional changes for clarity: more comments, and a
better separation between the "get latest patch release" and "get
previous stable branch" cases, moving the relevant code to independent
functions. We also edit the IPsec workflow to add a (otherwise
pointless) call to the script, so that we can check its logs, which was
previously difficult in CI given that the script runs in a subshell.

Sample output from the script:

    VERSION         Previous        Previous        Previous patch
                    stable          patch release   release (on top of
                    branch          (on a random    a commit updating
                                    commit)         VERSION)

    1.14.3          v1.13           v1.14.3         v1.14.2
    1.14.1          v1.13           v1.14.1         v1.14.0
    1.14.0          v1.13           v1.14.0         <error>
    1.14.1-dev      v1.13           <error>         <error>
    1.15.0-dev      v1.14           <error>         <error>
    1.13.90         v1.12           v1.13.90        v1.13.89
    2.0.1           <error>         v2.0.1          v2.0.0

[0] 56dfec2 ("contrib/scripts: Support patch releases in print-downgrade-version.sh")
[1] 4d7902f ("contrib/scripts: Remove special handling for patch release number 90")
[2] 5581963 ("ci/ipsec: Fix version retrieval for downgrades to closest patch release")
[3] 3803f53 ("ci/ipsec: Fix downgrade version for release preparation commits")

Fixes: 3803f53 ("ci/ipsec: Fix downgrade version for release preparation commits")
Signed-off-by: Quentin Monnet <quentin@isovalent.com>

.github/workflows/tests-ipsec-upgrade.yaml

@@ -153,12 +153,12 @@ jobs:
         uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
         with:
           ref: ${{ inputs.context-ref || github.sha }}
-          # We need to be able to check the existence of the tag we want to
-          # downgrade to, in case we're on a release preparation commit and the
-          # value in VERSION does not correspond to an existing tag yet. In
-          # that case, print-downgrade-version.sh needs to adjust the patch
-          # release number to downgrade to.
-          fetch-tags: true
+          # We need some context on the last commit to determine whether we're
+          # on a release preparation commit, in which case the value in VERSION
+          # does not correspond to an existing tag yet and the script
+          # print-downgrade-version.sh needs to adjust the patch release number
+          # to downgrade to.
+          fetch-depth: 2
           persist-credentials: false
 
       - name: Set Environment Variables
@@ -174,13 +174,15 @@ jobs:
           fi
           echo sha=${SHA} >> $GITHUB_OUTPUT
           if [ "${{ matrix.mode }}" = "minor" ]; then
+            contrib/scripts/print-downgrade-version.sh || true
             CILIUM_DOWNGRADE_VERSION=$(contrib/scripts/print-downgrade-version.sh)
             IMAGE_TAG=${CILIUM_DOWNGRADE_VERSION}
           else
             # Upgrade from / downgrade to patch release.
             # In some cases we expect to fail to get the version number, do not
             # fail the workflow in such case. This is typically the case on
             # main branch where we don't have preceeding patch releases.
+            contrib/scripts/print-downgrade-version.sh patch || true
             CILIUM_DOWNGRADE_VERSION=$(contrib/scripts/print-downgrade-version.sh patch || true)
             # Pass an empty tag to the cilium-config action to fall back to the
             # default release image, without crafting an image path with the

contrib/scripts/print-downgrade-version.sh

@@ -2,54 +2,80 @@
 #
 # A utility script to print the branch name of the previous stable or patch
 # release.
+#
+# The script returns the estimated branch or tag name for the version to
+# downgrade to. If it cannot determine this value, it returns nothing (and
+# prints a message to stderr). It belongs to the calling workflow to determine
+# whether an empty value should lead to an error or to skipping parts of a CI
+# job.
 
 set -o errexit
 set -o nounset
 
-SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
-VERSION=${VERSION-"$(cat "$SCRIPT_DIR/../../VERSION")"}
-if [[ $VERSION =~ ([0-9^]+)\.([0-9^]+)\.([0-9^]+).* ]] ; then
-    major=${BASH_REMATCH[1]}
-    minor=${BASH_REMATCH[2]}
-    patch=${BASH_REMATCH[3]}
+SCRIPT_DIR="$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )"
+VERSION="${VERSION-"$(cat "${SCRIPT_DIR}/../../VERSION")"}"
+if [[ "${VERSION}" =~ ([0-9^]+)\.([0-9^]+)\.([0-9^]+).* ]] ; then
+    major="${BASH_REMATCH[1]}"
+    minor="${BASH_REMATCH[2]}"
+    patch="${BASH_REMATCH[3]}"
 else
-  >&2 echo "ERROR: failed to parse version '$VERSION'"
+  >&2 echo "ERROR: failed to parse version '${VERSION}'"
   exit 1
 fi
 
-if [[ ${1-} == "patch" ]] ; then
-    # If user passed "patch" as first argument, print the latest patch version
-
-    if [[ "${patch}" -le "0" ]] ; then
-        >&2 echo "ERROR: failed to deduce patch release previous to version '$VERSION'"
+print_prev_patch() {
+    # If we're on the development branch, there is no previous patch release to
+    # downgrade to. Calling workflow should typically skip the job.
+    if [[ "${VERSION}" =~ -dev$ ]] ; then
+        >&2 echo "ERROR: no previous patch release for development version '${VERSION}'"
         exit 1
     fi
 
-    tag="v${major}.${minor}.${patch}${TAG_SUFFIX:-}"
+    # In most cases, the previous patch release is in fact the same as
+    # indicated in VERSION and we just need to return it (with TAG_SUFFIX as
+    # required).
 
     # Hack: When working on a patch release preparation commit, file VERSION
     # contains the new value for the release that is yet to be tagged and
-    # published. So if the tag does not exist, we want to downgrade to the
-    # previous patch release.
+    # published. In this case, we want to downgrade to the previous patch
+    # release.
     #
-    # Only run this step if we're in a Git repository, and we have tag v1.0.0
-    # in the repo (otherwise, we're likely on a shallow clone, with no tags
-    # fetched).
+    # Only run this step if we're in a Git repository, and we have more than
+    # one commit in the repo (otherwise, we're likely on a shallow clone).
+    # This means CI workflows relying on a previous patch release and calling
+    # this script should download at least part of the history, with
+    # "fetch-depth: 2".
     if git rev-parse --is-inside-work-tree &> /dev/null && \
-        git rev-parse --verify --end-of-options v1.0.0 &> /dev/null && \
-        ! git rev-parse --verify --end-of-options "${tag}" &> /dev/null; then
-        >&2 echo "INFO: tag ${tag} not found, decrementing patch release number"
+        git rev-parse --verify HEAD^ &> /dev/null && \
+        git diff --name-only HEAD^..HEAD | grep -q "^VERSION$"; then
+        # If the patch version is 0, we cannot decrement it further.
+        if [[ "${patch}" -le "0" ]] ; then
+            >&2 echo "ERROR: failed to deduce patch release previous to version '${VERSION}'"
+            exit 1
+        fi
         patch=$((patch - 1))
-        tag="v${major}.${minor}.${patch}${TAG_SUFFIX:-}"
     fi
-    echo "${tag}"
-else
+
+    echo "v${major}.${minor}.${patch}${TAG_SUFFIX:-}"
+}
+
+print_prev_branch() {
+    # If the minor version is 0, we cannot decrement it further.
     if [[ "${minor}" == "0" ]] ; then
-        >&2 echo "ERROR: failed to deduce release previous to version '$VERSION'"
+        >&2 echo "ERROR: failed to deduce release previous to version '${VERSION}'"
         exit 1
     fi
-    # Else print the previous stable version by decrementing the minor version
-    # and trimming the patch version.
-    ((minor--))
+
+    # Print the previous stable version by decrementing the minor version and
+    # trimming the patch version.
+    minor=$((minor - 1))
     echo "v${major}.${minor}${BRANCH_SUFFIX:-}"
+}
+
+# If user passed "patch" as first argument, print the latest patch version.
+# Otherwise, print the latest stable version.
+if [[ ${1-} == "patch" ]] ; then
+    print_prev_patch
+else
+    print_prev_branch
 fi