improvements and add tests

Signed-off-by: Abhishek Kumar <abhishek.mrt22@gmail.com>

Author: shwstppr

engine/schema/pom.xml

@@ -153,6 +153,20 @@
                 <artifactId>exec-maven-plugin</artifactId>
                 <version>${cs.exec-maven-plugin.version}</version>
                 <executions>
+                    <execution>
+                        <id>test-templateConfig</id>
+                        <phase>test</phase>
+                        <goals>
+                            <goal>exec</goal>
+                        </goals>
+                        <configuration>
+                            <workingDirectory>${basedir}/</workingDirectory>
+                            <executable>bash</executable>
+                            <arguments>
+                                <argument>test_templateConfig.sh</argument>
+                            </arguments>
+                        </configuration>
+                    </execution>
                     <execution>
                         <id>systemvm-template-metadata</id>
                         <phase>package</phase>

engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java

@@ -515,8 +515,7 @@ protected void doUpgrades(GlobalLock lock) {
             String csVersion = parseSystemVmMetadata();
             final CloudStackVersion sysVmVersion = CloudStackVersion.parse(csVersion);
             final  CloudStackVersion currentVersion = CloudStackVersion.parse(currentVersionValue);
-            SystemVmTemplateRegistration.CS_MAJOR_VERSION  = String.format("%d.%d", sysVmVersion.getMajorRelease(), sysVmVersion.getMinorRelease());
-            SystemVmTemplateRegistration.CS_TINY_VERSION = String.valueOf(sysVmVersion.usesNewVersioning() ? sysVmVersion.getSecurityRelease() : sysVmVersion.getPatchRelease());
+            updateSystemVmTemplateVersion(sysVmVersion);
 
             LOGGER.info("DB version = {} Code Version = {}", dbVersion, currentVersion);
 
@@ -542,6 +541,18 @@ protected void doUpgrades(GlobalLock lock) {
         }
     }
 
+    /**
+     * Sets the system VM template major/tiny version used to look up the matching system VM template,
+     * from the version parsed out of the system VM template metadata file. Below the versioning cutover
+     * (major < 24) the tiny version is the legacy patch release; from the cutover onwards it is the
+     * security release, since the patch position is dropped in that scheme.
+     */
+    @VisibleForTesting
+    protected static void updateSystemVmTemplateVersion(CloudStackVersion sysVmVersion) {
+        SystemVmTemplateRegistration.CS_MAJOR_VERSION = String.format("%d.%d", sysVmVersion.getMajorRelease(), sysVmVersion.getMinorRelease());
+        SystemVmTemplateRegistration.CS_TINY_VERSION = String.valueOf(sysVmVersion.usesNewVersioning() ? sysVmVersion.getSecurityRelease() : sysVmVersion.getPatchRelease());
+    }
+
     /**
      * Hook that is called when an upgrade is required but the management server is clustered.
      * Default behavior is to exit the JVM, tests can override to throw instead.

engine/schema/src/test/java/com/cloud/upgrade/DatabaseUpgradeCheckerTest.java

@@ -421,4 +421,28 @@ public void testCalculateUpgradePath42010to42100() {
         assertTrue(upgrades[2] instanceof Upgrade42040to42100);
         assertEquals(currentVersion.toString(), upgrades[2].getUpgradedVersion());
     }
+
+    @Test
+    public void testUpdateSystemVmTemplateVersionBelowCutover() {
+        DatabaseUpgradeChecker.updateSystemVmTemplateVersion(CloudStackVersion.parse("4.22.1.0"));
+
+        assertEquals("4.22", SystemVmTemplateRegistration.CS_MAJOR_VERSION);
+        assertEquals("1", SystemVmTemplateRegistration.CS_TINY_VERSION);
+    }
+
+    @Test
+    public void testUpdateSystemVmTemplateVersionAtCutover() {
+        DatabaseUpgradeChecker.updateSystemVmTemplateVersion(CloudStackVersion.parse("24.0.1"));
+
+        assertEquals("24.0", SystemVmTemplateRegistration.CS_MAJOR_VERSION);
+        assertEquals("1", SystemVmTemplateRegistration.CS_TINY_VERSION);
+    }
+
+    @Test
+    public void testUpdateSystemVmTemplateVersionAfterCutover() {
+        DatabaseUpgradeChecker.updateSystemVmTemplateVersion(CloudStackVersion.parse("25.3.2"));
+
+        assertEquals("25.3", SystemVmTemplateRegistration.CS_MAJOR_VERSION);
+        assertEquals("2", SystemVmTemplateRegistration.CS_TINY_VERSION);
+    }
 }

engine/schema/src/test/java/com/cloud/upgrade/SystemVmTemplateRegistrationTest.java

@@ -1638,4 +1638,41 @@ public void updateTemplateUrlChecksumAndGuestOsId_ThrowsExceptionWhenUpdateFails
         assertTrue(exception.getMessage().contains("Exception while updating 'url' and 'checksum' for hypervisor type"));
         verify(vmTemplateDao).update(templateVO.getId(), templateVO);
     }
+
+    @Test
+    public void getSystemVmTemplateVersion_UsesExplicitVersionWhenSet() {
+        SystemVmTemplateRegistration registration = new SystemVmTemplateRegistration("4.22.0.0");
+
+        assertEquals("4.22.0.0", registration.getSystemVmTemplateVersion());
+    }
+
+    @Test
+    public void getSystemVmTemplateVersion_BuildsFromMajorAndTinyVersionBelowCutover() {
+        String previousMajor = SystemVmTemplateRegistration.CS_MAJOR_VERSION;
+        String previousTiny = SystemVmTemplateRegistration.CS_TINY_VERSION;
+        try {
+            SystemVmTemplateRegistration.CS_MAJOR_VERSION = "4.22";
+            SystemVmTemplateRegistration.CS_TINY_VERSION = "1";
+
+            assertEquals("4.22.1", systemVmTemplateRegistration.getSystemVmTemplateVersion());
+        } finally {
+            SystemVmTemplateRegistration.CS_MAJOR_VERSION = previousMajor;
+            SystemVmTemplateRegistration.CS_TINY_VERSION = previousTiny;
+        }
+    }
+
+    @Test
+    public void getSystemVmTemplateVersion_BuildsFromMajorAndTinyVersionAfterCutover() {
+        String previousMajor = SystemVmTemplateRegistration.CS_MAJOR_VERSION;
+        String previousTiny = SystemVmTemplateRegistration.CS_TINY_VERSION;
+        try {
+            SystemVmTemplateRegistration.CS_MAJOR_VERSION = "24.0";
+            SystemVmTemplateRegistration.CS_TINY_VERSION = "1";
+
+            assertEquals("24.0.1", systemVmTemplateRegistration.getSystemVmTemplateVersion());
+        } finally {
+            SystemVmTemplateRegistration.CS_MAJOR_VERSION = previousMajor;
+            SystemVmTemplateRegistration.CS_TINY_VERSION = previousTiny;
+        }
+    }
 }

engine/schema/templateConfig.sh

@@ -17,16 +17,26 @@
 # specific language governing permissions and limitations
 # under the License.
 
+NEW_VERSIONING_CUTOVER_MAJOR_VERSION=24
+
 function getTemplateVersion() {
   projVersion=$1
   version="$(cut -d'-' -f1 <<<"$projVersion")"
   subversion1="$(cut -d'.' -f1 <<<"$version")"
   subversion2="$(cut -d'.' -f2 <<<"$version")"
   minorversion="$(cut -d'.' -f3 <<<"$version")"
-  securityversion="$(cut -d'.' -f4 <<<"$version")"
   export CS_VERSION="${subversion1}"."${subversion2}"
   export CS_MINOR_VERSION="${minorversion}"
   export VERSION="${CS_VERSION}.${CS_MINOR_VERSION}"
+  if [[ "$subversion1" -ge "$NEW_VERSIONING_CUTOVER_MAJOR_VERSION" ]]; then
+    # New versioning (major.minor.security): the third component is the
+    # security release itself, there is no separate patch component.
+    export FULL_VERSION="${VERSION}"
+  else
+    # Legacy versioning (major.minor.patch.security)
+    securityversion="$(cut -d'.' -f4 <<<"$version")"
+    export FULL_VERSION="${VERSION}.${securityversion}"
+  fi
   export CS_SYSTEMTEMPLATE_REPO="https://download.cloudstack.org/systemvm/"
 }
 
@@ -64,7 +74,7 @@ function getChecksum() {
 
 function createMetadataFile() {
   local fileData=$(cat "$SOURCEFILE")
-  echo -e "["default"]\nversion = $VERSION.${securityversion}\ndownloadrepository = $CS_SYSTEMTEMPLATE_REPO\n" >> "$METADATAFILE"
+  echo -e "["default"]\nversion = $FULL_VERSION\ndownloadrepository = $CS_SYSTEMTEMPLATE_REPO\n" >> "$METADATAFILE"
   for template in "${templates[@]}"
   do
     section="${template%%:*}"
@@ -81,27 +91,30 @@ function createMetadataFile() {
   done
 }
 
-declare -a templates
-getTemplateVersion $1
-declare -A template_specs=(
-  [kvm-x86_64]="x86_64-kvm.qcow2.bz2"
-  [kvm-aarch64]="aarch64-kvm.qcow2.bz2"
-  [vmware]="x86_64-vmware.ova"
-  [xenserver]="x86_64-xen.vhd.bz2"
-  [hyperv]="x86_64-hyperv.vhd.zip"
-  [lxc]="x86_64-kvm.qcow2.bz2"
-  [ovm3]="x86_64-ovm.raw.bz2"
-)
+# Guard so the file can be sourced (e.g. by tests) without running the steps below.
+if [[ "${BASH_SOURCE[0]}" == "${0}" ]]; then
+  declare -a templates
+  getTemplateVersion $1
+  declare -A template_specs=(
+    [kvm-x86_64]="x86_64-kvm.qcow2.bz2"
+    [kvm-aarch64]="aarch64-kvm.qcow2.bz2"
+    [vmware]="x86_64-vmware.ova"
+    [xenserver]="x86_64-xen.vhd.bz2"
+    [hyperv]="x86_64-hyperv.vhd.zip"
+    [lxc]="x86_64-kvm.qcow2.bz2"
+    [ovm3]="x86_64-ovm.raw.bz2"
+  )
 
-templates=()
-for key in "${!template_specs[@]}"; do
-  url="${CS_SYSTEMTEMPLATE_REPO}/${CS_VERSION}/systemvmtemplate-$VERSION-${template_specs[$key]}"
-  templates+=("$key:$url")
-done
+  templates=()
+  for key in "${!template_specs[@]}"; do
+    url="${CS_SYSTEMTEMPLATE_REPO}/${CS_VERSION}/systemvmtemplate-$VERSION-${template_specs[$key]}"
+    templates+=("$key:$url")
+  done
 
-PARENTPATH="$( cd -- "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P )/dist/systemvm-templates/"
-mkdir -p "$PARENTPATH"
-METADATAFILE="${PARENTPATH}metadata.ini"
-echo > "$METADATAFILE"
-SOURCEFILE="${PARENTPATH}sha512sum.txt"
-createMetadataFile
+  PARENTPATH="$( cd -- "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P )/dist/systemvm-templates/"
+  mkdir -p "$PARENTPATH"
+  METADATAFILE="${PARENTPATH}metadata.ini"
+  echo > "$METADATAFILE"
+  SOURCEFILE="${PARENTPATH}sha512sum.txt"
+  createMetadataFile
+fi

engine/schema/test_templateConfig.sh

@@ -0,0 +1,100 @@
+#!/bin/bash
+
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements.  See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership.  The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License.  You may obtain a copy of the License at
+#
+#   http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied.  See the License for the
+# specific language governing permissions and limitations
+# under the License.
+#
+# Standalone smoke test for templateConfig.sh's version handling across the
+# 4.x (legacy) to 24.x (cutover) versioning schemes. Run directly:
+#   bash engine/schema/test_templateConfig.sh
+
+set -u
+
+SCRIPT_DIR="$( cd -- "$(dirname "${BASH_SOURCE[0]}")" >/dev/null 2>&1 ; pwd -P )"
+# shellcheck source=templateConfig.sh
+source "${SCRIPT_DIR}/templateConfig.sh"
+
+pass=0
+fail=0
+
+assert_eq() {
+  local description="$1"
+  local expected="$2"
+  local actual="$3"
+  if [[ "$expected" == "$actual" ]]; then
+    pass=$((pass + 1))
+    echo "PASS: $description"
+  else
+    fail=$((fail + 1))
+    echo "FAIL: $description (expected '$expected', got '$actual')"
+  fi
+}
+
+# --- getTemplateVersion: legacy versioning (major < 24) ---
+
+getTemplateVersion "4.22.0.0"
+assert_eq "legacy: CS_VERSION" "4.22" "$CS_VERSION"
+assert_eq "legacy: VERSION" "4.22.0" "$VERSION"
+assert_eq "legacy: FULL_VERSION keeps the 4th (security) component" "4.22.0.0" "$FULL_VERSION"
+
+getTemplateVersion "4.23.0.1-SNAPSHOT"
+assert_eq "legacy with -SNAPSHOT suffix: VERSION" "4.23.0" "$VERSION"
+assert_eq "legacy with -SNAPSHOT suffix: FULL_VERSION" "4.23.0.1" "$FULL_VERSION"
+
+# --- getTemplateVersion: new versioning (major >= 24, post-cutover) ---
+
+getTemplateVersion "24.0.0"
+assert_eq "cutover: CS_VERSION" "24.0" "$CS_VERSION"
+assert_eq "cutover: VERSION" "24.0.0" "$VERSION"
+assert_eq "cutover: FULL_VERSION has no trailing dot (patch dropped)" "24.0.0" "$FULL_VERSION"
+
+getTemplateVersion "24.1.2-SNAPSHOT"
+assert_eq "cutover with -SNAPSHOT suffix: VERSION" "24.1.2" "$VERSION"
+assert_eq "cutover with -SNAPSHOT suffix: FULL_VERSION" "24.1.2" "$FULL_VERSION"
+
+getTemplateVersion "99.9.9"
+assert_eq "future major: VERSION" "99.9.9" "$VERSION"
+assert_eq "future major: FULL_VERSION" "99.9.9" "$FULL_VERSION"
+
+# --- createMetadataFile: end-to-end metadata.ini "version" line ---
+
+run_create_metadata_file() {
+  local projVersion="$1"
+  local workdir
+  workdir="$(mktemp -d)"
+
+  getTemplateVersion "$projVersion"
+  METADATAFILE="${workdir}/metadata.ini"
+  SOURCEFILE="${workdir}/sha512sum.txt"
+  printf "abc123  systemvmtemplate-%s-x86_64-kvm.qcow2.bz2\n" "$VERSION" > "$SOURCEFILE"
+  templates=("kvm-x86_64:${CS_SYSTEMTEMPLATE_REPO}/${CS_VERSION}/systemvmtemplate-${VERSION}-x86_64-kvm.qcow2.bz2")
+  : > "$METADATAFILE"
+
+  createMetadataFile
+  grep '^version = ' "$METADATAFILE" | head -1 | cut -d' ' -f3
+
+  rm -rf "$workdir"
+}
+
+actual="$(run_create_metadata_file "4.22.0.0")"
+assert_eq "metadata.ini legacy 'version' line" "4.22.0.0" "$actual"
+
+actual="$(run_create_metadata_file "24.0.0")"
+assert_eq "metadata.ini cutover 'version' line has no trailing dot" "24.0.0" "$actual"
+
+echo ""
+echo "${pass} passed, ${fail} failed"
+[[ $fail -eq 0 ]]

utils/src/main/java/org/apache/cloudstack/utils/CloudStackVersion.java

@@ -75,7 +75,9 @@ private CloudStackVersion(final int majorRelease, final int minorRelease, final
      *     <li><code>&lt;major&gt;.&lt;minor&gt;.&lt;security release&gt;</code> (for versions &gt;= 24.0.0)</li>
      * </ul>
      *
-     * Legacy patch-based formats remain supported for backward compatibility.
+     * Legacy patch-based formats remain supported for backward compatibility, e.g. <code>4.23.0.1-SNAPSHOT</code>,
+     * but only below major version 24: a 4-position value whose major release is at or above 24, e.g.
+     * <code>24.0.0.1</code>, is rejected.
      *
      * If the string contains a suffix that begins with a "-" character, then the "-" and all characters following it
      * will be dropped.
@@ -208,13 +210,26 @@ public int compareTo(final CloudStackVersion thatVersion) {
     public static String trimRouterVersion(String version) {
         final String[] tokens = version.split(" ");
 
-        if (tokens.length >= 3 && FULL_VERSION_FORMAT.matcher(tokens[2]).matches()) {
+        if (tokens.length >= 3 && FULL_VERSION_FORMAT.matcher(tokens[2]).matches() && isParseableVersion(tokens[2])) {
             return tokens[2];
         }
 
         return "0";
     }
 
+    /**
+     * Whether {@link #parse(String)} would accept the given value, following the same cutover rule that
+     * rejects a 4-position <code>major.minor.patch.security</code> value once the major release reaches 24.
+     */
+    private static boolean isParseableVersion(final String value) {
+        try {
+            parse(value);
+            return true;
+        } catch (RuntimeException e) {
+            return false;
+        }
+    }
+
     private static ImmutableList<Integer> normalizeVersionValues(final ImmutableList<Integer> values) {
 
         checkArgument(values != null);

utils/src/test/java/org/apache/cloudstack/utils/CloudStackVersionTest.java

@@ -231,7 +231,9 @@ public void testGreaterThanAndLessThanCompareDirect(final String value, final St
         "Cloudstack Release 1.2.3.4-brnading-1519453362 Mon Jan  1 10:10:10 UTC 2018, 1.2.3.4-brnading-1519453362",
         "Cloudstack Release 1.2 Mon Jan  1 10:10:10 UTC 2018, 0",
         "Cloudstack Release 1.2-SNAPSHOT Mon Jan  1 10:10:10 UTC 2018, 0",
-        "Cloud stack Release 1.2.3.4 Mon Jan  1 10:10:10 UTC 2018, 0"
+        "Cloud stack Release 1.2.3.4 Mon Jan  1 10:10:10 UTC 2018, 0",
+        "Cloudstack Release 24.0.0 Mon Jan  1 10:10:10 UTC 2018, 24.0.0",
+        "Cloudstack Release 24.0.0.1 Mon Jan  1 10:10:10 UTC 2018, 0"
     })
     public void testTrimRouterVersion(final String value, final String expected) {