Copilot commented on code in PR #14033:
URL: https://github.com/apache/cloudstack/pull/14033#discussion_r4012068426
##########
engine/schema/src/main/java/com/cloud/upgrade/DatabaseUpgradeChecker.java:
##########
@@ -513,8 +515,13 @@ protected void doUpgrades(GlobalLock lock) {
String csVersion = parseSystemVmMetadata();
final CloudStackVersion sysVmVersion =
CloudStackVersion.parse(csVersion);
final CloudStackVersion currentVersion =
CloudStackVersion.parse(currentVersionValue);
- SystemVmTemplateRegistration.CS_MAJOR_VERSION =
sysVmVersion.getMajorRelease() + "." + sysVmVersion.getMinorRelease();
- SystemVmTemplateRegistration.CS_TINY_VERSION =
String.valueOf(sysVmVersion.getPatchRelease());
+ if (sysVmVersion.usesNewVersioning()) {
+ SystemVmTemplateRegistration.CS_MAJOR_VERSION =
String.valueOf(sysVmVersion.getMajorRelease());
+ SystemVmTemplateRegistration.CS_TINY_VERSION =
String.valueOf(sysVmVersion.getMajorRelease());
Review Comment:
For a new-format metadata value such as `24.0.1`, these assignments make
`getSystemVmTemplateVersion()` return `24.24` because it concatenates
`CS_MAJOR_VERSION` and `CS_TINY_VERSION`. That two-component value cannot be
parsed by `CloudStackVersion` and does not represent the system VM release;
keep the major/minor components in `CS_MAJOR_VERSION` and use the security
release as the tiny component.
##########
utils/src/main/java/org/apache/cloudstack/utils/CloudStackVersion.java:
##########
@@ -39,22 +39,23 @@
*/
public final class CloudStackVersion implements Comparable<CloudStackVersion> {
- private final static Pattern NUMBER_VERSION_FORMAT =
Pattern.compile("(\\d+\\.){2}(\\d+\\.)?\\d+");
- private final static Pattern FULL_VERSION_FORMAT =
Pattern.compile("(\\d+\\.){2}(\\d+\\.)?\\d+(-[a-zA-Z]+)?(-\\d+)?(-SNAPSHOT)?");
+ private final static Pattern NUMBER_VERSION_FORMAT =
Pattern.compile("\\d+\\.\\d+\\.\\d+(?:\\.\\d+)?");
+ private final static Pattern FULL_VERSION_FORMAT =
Pattern.compile("\\d+\\.\\d+\\.\\d+(?:\\.\\d+)?(?:-[a-zA-Z]+)?(?:-\\d+)?(?:-SNAPSHOT)?");
Review Comment:
This matcher still accepts four-component values with a major version of 24
or later, while `parse()` now rejects those values. `trimRouterVersion()`
therefore returns inputs such as `24.0.0.1` that its callers pass to
`CloudStackVersion.compare()`, which throws instead of handling the router
version as invalid; make this validation follow the same cutover rule as
`parse()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
