check the xapi version in the installer by glehmann · Pull Request #48 · xcp-ng/host-installer

glehmann · 2025-06-06T09:50:51Z

No description provided.

ydirson · 2025-06-06T15:13:18Z

backend.py

+        available_xapi_versions = repository.listPackagesFromRepos(
+            main_repositories, 'xapi-core', '%{evr}')
+        if not available_xapi_versions:
+            raise RuntimeError("No xapi package found")


could be good to be more specific, like "... found in package source" (to avoid confusion with installed package)

To me, the message should be either about XAPI, as the component name, or xapi-core, as the package name that we actually looked for.

ydirson · 2025-06-06T15:24:53Z

backend.py

+        if not available_xapi_versions:
+            raise RuntimeError("No xapi package found")
+        available_xapi_version = available_xapi_versions[0]
+        for v in available_xapi_versions[1:]:


This [1:] leads me to think some comments would be useful here.

Note that listPackagesFromRepos is written with the assumption that repoquery will return all available versions (as dnf repoquery does) but quick tests right now show only a single version of each package in CentOS7 (like --latest-limit=1 with dnf) - that makes me even more puzzled about that slice.

that's just to select the greatest available version, in case there would be more than one returned by repoquery.

Oh I only now realize that's a "latest version in list" here.
IMO we'll rather want to make sure repoquery does that on its own, and ensure it returns a single xapi-core version.

The fact listPackagesFromRepos only returns a single now today is a bug (assuming the CentOS7 repoquery was acting more like dnf repoquery).
I'd go with a flag for to return only the latest, add --show-duplicates for when it isn't set, and an assert len()==1 when set to make we adjust flags properly when we'll switch to dnf for v9.

So you would use repoquery with latest_only=True (for example) in the case that we're commenting here, right?

I think something is wrong for linstor: with repoquery returning a single value by default, how can we select the exact version for linstor if it's not the last one from the available packages?

The linstor case would use latest_only=False. But today we only support upgrading it from an ISO, on which the only available version will be the latest one.

ydirson · 2025-06-06T15:28:07Z

backend.py

+        available_xapi_version = available_xapi_versions[0]
+        for v in available_xapi_versions[1:]:
+            if rpm.labelCompare(('pkg', 'arch', v), ('pkg', 'arch', available_xapi_version)) >= 0:
+                available_xapi_version = v


behaviour when/if several versions are available is "latest wins", we may prefer to assert that we have a single version and avoid looping

Indeed: repoquery needs --show-duplicates if you want more than one version. At least in the version that we're using.

ydirson · 2025-06-06T15:34:35Z

backend.py

+        if rpm.labelCompare(('pkg', 'arch', available_xapi_version), ('pkg', 'arch', answers['xapi-version'])) < 0:
+            raise RuntimeError("Cannot upgrade the host: the xapi version on the host (%s) "
+                               "is newer than the xapi version to install (%s). "
+                               "Please make sure to use the latest ISO." %


"latest ISO" may not be sufficient if user has upgraded his XS to a newly-released XAPI

we likely won't release new ISO for every XAPI update, so we'll likely direct users to installing from HTTP to get the latest version

even then, the message(s) to use will need to be carefully crafted

Maybe use a list of suggestions like those? @stormi surely has more ideas

(don't show when already installing from official URL) "try installing package from the network for a more recent version"

wait for a new xapi package and refrain from updating in on your XS host in the meantime

Regarding the Latest ISO, I agree with Yann.

Regarding installing from HTTP, at the moment, we don't support installing from the base+updates repos, and only update netinstall repos at the same time that we release new ISOs. We'd like to do better in the future, but that requires CI improvements, so that the installer is always tested with the latest packages, and possibly installer improvements too (Maybe there's a way to make it work as is, but it's untested).

As there can be various situations in which this message is displayed, let's just state what we know for sure and tell users to refer to the official upgrade documentation.

(Which means that we have a documentation update to prepare so that this error is documented in it...)

Suggested change

"Please make sure to use the latest ISO." %

"Please refer to the XCP-ng upgrade documentation." %

Regarding installing from HTTP, at the moment, we don't support installing from the base+updates repos

That would be easily solved by generating self-sufficient repos instead of having them stacked on one another

ydirson · 2025-06-06T15:37:16Z

upgrade.py


    prepUpgradeArgs = []
-    prepStateChanges = ['installation-uuid', 'control-domain-uuid', 'management-address-type', 'linstor-version']
+    prepStateChanges = ['installation-uuid', 'control-domain-uuid', 'management-address-type', 'linstor-version', 'xapi-version']


I would rather add xapi-version (which could be interesting upstream) before linstor-version (which I'm less sure we'll manage to upstream easily)

good point, but as we need both on the same line, and for now this PR is based on the version with all the branches merged. I'll take care of that later when I'll make this PR based on upstream only :-)

stormi · 2025-06-06T18:09:09Z

backend.py

+        available_xapi_versions = repository.listPackagesFromRepos(
+            main_repositories, 'xapi-core', '%{evr}')
+        if not available_xapi_versions:
+            raise RuntimeError("No xapi package found")


To me, the message should be either about XAPI, as the component name, or xapi-core, as the package name that we actually looked for.

stormi · 2025-06-06T18:17:23Z

backend.py

+        available_xapi_version = available_xapi_versions[0]
+        for v in available_xapi_versions[1:]:
+            if rpm.labelCompare(('pkg', 'arch', v), ('pkg', 'arch', available_xapi_version)) >= 0:
+                available_xapi_version = v


Indeed: repoquery needs --show-duplicates if you want more than one version. At least in the version that we're using.

stormi · 2025-06-06T18:24:35Z

backend.py

+            if rpm.labelCompare(('pkg', 'arch', v), ('pkg', 'arch', available_xapi_version)) >= 0:
+                available_xapi_version = v
+        if rpm.labelCompare(('pkg', 'arch', available_xapi_version), ('pkg', 'arch', answers['xapi-version'])) < 0:
+            raise RuntimeError("Cannot upgrade the host: the xapi version on the host (%s) "


Suggested change

raise RuntimeError("Cannot upgrade the host: the xapi version on the host (%s) "

raise RuntimeError("Cannot upgrade the host: the XAPI version on the host (%s) "

stormi · 2025-06-06T18:24:45Z

backend.py

+                available_xapi_version = v
+        if rpm.labelCompare(('pkg', 'arch', available_xapi_version), ('pkg', 'arch', answers['xapi-version'])) < 0:
+            raise RuntimeError("Cannot upgrade the host: the xapi version on the host (%s) "
+                               "is newer than the xapi version to install (%s). "


Suggested change

"is newer than the xapi version to install (%s). "

"is newer than the XAPI version to install (%s). "

stormi · 2025-06-06T18:28:48Z

backend.py

+        if rpm.labelCompare(('pkg', 'arch', available_xapi_version), ('pkg', 'arch', answers['xapi-version'])) < 0:
+            raise RuntimeError("Cannot upgrade the host: the xapi version on the host (%s) "
+                               "is newer than the xapi version to install (%s). "
+                               "Please make sure to use the latest ISO." %


Regarding the Latest ISO, I agree with Yann.

Regarding installing from HTTP, at the moment, we don't support installing from the base+updates repos, and only update netinstall repos at the same time that we release new ISOs. We'd like to do better in the future, but that requires CI improvements, so that the installer is always tested with the latest packages, and possibly installer improvements too (Maybe there's a way to make it work as is, but it's untested).

As there can be various situations in which this message is displayed, let's just state what we know for sure and tell users to refer to the official upgrade documentation.

(Which means that we have a documentation update to prepare so that this error is documented in it...)

Suggested change

"Please make sure to use the latest ISO." %

"Please refer to the XCP-ng upgrade documentation." %

backend.py

Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>

ydirson

lgtm

stormi · 2025-06-11T14:55:35Z

backend.py

+        if rpm.labelCompare(('pkg', 'arch', available_xapi_versions[0]), ('pkg', 'arch', answers['xapi-version'])) < 0:
+            raise RuntimeError("Cannot upgrade the host: the XAPI version on the host (%s) "
+                               "is newer than the XAPI version to install (%s). "
+                               "Please refer to the XCP-ng upgrade documentation." %


Let's not forget to add said doc. We can discuss what we should tell users. I know it's already covered at least for migration from XS, but I'm thinking about users who'd go check the docs after they saw the error. Something that explicitly gives them the error message their search for and explains their options.

stormi · 2025-06-11T14:56:27Z

backend.py

+            raise RuntimeError("No XAPI package found in package source")
+        assert len(available_xapi_versions) == 1
+        if rpm.labelCompare(('pkg', 'arch', available_xapi_versions[0]), ('pkg', 'arch', answers['xapi-version'])) < 0:
+            raise RuntimeError("Cannot upgrade the host: the XAPI version on the host (%s) "


Didn't we just say that it may be better to make it a warning screen rather than a fatal error?

glehmann requested review from stormi and ydirson June 6, 2025 09:50

ydirson requested changes Jun 6, 2025

View reviewed changes

stormi reviewed Jun 6, 2025

View reviewed changes

glehmann added 2 commits June 10, 2025 23:21

allow to query multiple versions of a package

9a903f2

Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>

check the xapi version in the installer

2b7b38c

Signed-off-by: Gaëtan Lehmann <gaetan.lehmann@vates.tech>

glehmann force-pushed the gln/xapi-version-check-lppx branch from 2d25a78 to 2b7b38c Compare June 10, 2025 21:25

ydirson approved these changes Jun 11, 2025

View reviewed changes

stormi reviewed Jun 11, 2025

View reviewed changes

	"Please make sure to use the latest ISO." %
	"Please refer to the XCP-ng upgrade documentation." %

	raise RuntimeError("Cannot upgrade the host: the xapi version on the host (%s) "
	raise RuntimeError("Cannot upgrade the host: the XAPI version on the host (%s) "

	"is newer than the xapi version to install (%s). "
	"is newer than the XAPI version to install (%s). "

Conversation

glehmann commented Jun 6, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ydirson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants