ohu/xcpng/001 - Tools - adds update scenario by olivierh-pro · Pull Request #415 · xcp-ng/xcp-ng-tests

olivierh-pro · 2026-03-06T08:37:55Z

xcpng-539

This PR adds new sub package inside the shared packages lib. The new sub package provides update actions.

update_host: Updates one host with basic actions (clean metadata, yum update and reboot)
update_all: Describes a scenario with all logic to perform for updating a set of hosts

Note

The code for this PR is based on branch ohu/xcpng-539/000.

Please, consider #413 before this one.

ydirson

We don't use the "feat(...)" convention in this project. Flexible prefix is much more useful, here that could be something like:

adds sub package tools for automation scripts
tools: adds tasks sub packages
tools/tasks: adds update task
tools/tasks: adds an update scenario for all targets

ydirson · 2026-03-09T12:30:23Z

lib/tools/tasks/update.py

+def update_host(host: Host, enablerepos: List[str] = []):
+    """Updates the target.
+
+    Updating a remote host target has the following workflow:
+
+    * Cleans cached metadata
+    * Updates packages (with optional repositories enabled or not)
+    * Reboots (verifies whether the host is up or not)
+
+    :param :py:class:`lib.host.Host` host: Target host to update.
+    :param List[str] enablerepos: Extra repo(s) to enable (default: []).
+    """
+    logger.info(f"[{host}] Begin updating target host")
+
+    host.yum_clean_metadata()
+    host.yum_update(enablerepos=enablerepos)
+    # Everything's ok, just reboot
+    host.reboot(verify=True)
+
+    logger.info(f"[{host}] Updated!")


This function being essentially a series of calls o Host methods, wouldn't it make sense to be a Host method itself?

You are completely right, it makes sense, especially with OOP mindset. To be honest I didn't dare to modify Host class 😅 .

The real reason is: I wanted to encapsulate "an update scenario" in a sub package because it is not an "atomic task" as it is suggested by host methods.

Not sure what to do next, anyone else has an opinion here ? @xcp-ng/os-platform-release

TODO: Quick explanation about role of this func, a wrapper

lib/tools/tasks/update.py

ydirson · 2026-03-09T12:50:09Z

lib/tools/tasks/update.py

+    try:
+        pools = [Pool(h) for h in hosts]
+        logger.info("Preparing Pools...")
+    except AssertionError as ae:
+        logger.critical(ae)
+        sys.exit(1)


What does this try/exit bring us over default behavior?

I really prefer to explicitely exit with code error. By reading this code, I expect to receive error from Pool and then quit the program.

In my opnion, the line that throws AssertionError doesn't completely describe the case. I would have written the code below instead:

if not host.is_maste(): raise EmptyPoolError(f"{master_hostname_or_ip} is not a master host. Required to instantiate a Pool.")

I usually don't like catching AssertionError (or SystemExit). If I need to, I try to keep it in the root function.
I would prefer the Pool constructor sending a more meaningful exception, but I'm fine with the assertion too—this is a test lib after all

TODO: Remove error handling, let the AssertionError raises anyway

lib/tools/tasks/update.py

glehmann · 2026-03-10T08:34:42Z

lib/tools/tasks/update.py

+    try:
+        pools = [Pool(h) for h in hosts]
+        logger.info("Preparing Pools...")
+    except AssertionError as ae:
+        logger.critical(ae)
+        sys.exit(1)


I usually don't like catching AssertionError (or SystemExit). If I need to, I try to keep it in the root function.
I would prefer the Pool constructor sending a more meaningful exception, but I'm fine with the assertion too—this is a test lib after all

lib/tools/tasks/update.py

* instantiates an object logger in `__init__` * logfmt and datefmt match with pytest config found in file `./pytest.ini` at root level Signed-off-by: Olivier Hoareau <olivier.hoareau@vates.tech>

Signed-off-by: Olivier Hoareau <olivier.hoareau@vates.tech>

This task performs a basic update and takes only one host. It is intended for being used against a list of hosts. Signed-off-by: Olivier Hoareau <olivier.hoareau@vates.tech>

This change adds a new function that describes the main update scenario. It handles multiple hosts and then runs an update on each of them. Basically, it is the kind of function that can be called directly from a CLI. Signed-off-by: Olivier Hoareau <olivier.hoareau@vates.tech>

olivierh-pro changed the title ~~Tools - adds update scenario~~ ohu/xcpng-539/001 - Tools - adds update scenario Mar 6, 2026

olivierh-pro force-pushed the ohu/xcpng-539/001 branch 2 times, most recently from d11a9e0 to 0738378 Compare March 6, 2026 15:08

olivierh-pro changed the title ~~ohu/xcpng-539/001 - Tools - adds update scenario~~ ohu/xcpng/001 - Tools - adds update scenario Mar 6, 2026

olivierh-pro marked this pull request as ready for review March 6, 2026 15:17

olivierh-pro requested a review from a team as a code owner March 6, 2026 15:17

olivierh-pro mentioned this pull request Mar 6, 2026

Ohu/xcpng/002 - Tools - Adds CLI for using update scripts #417

Draft

glehmann approved these changes Mar 9, 2026

View reviewed changes

glehmann requested a review from a team March 9, 2026 08:43

ydirson self-requested a review March 9, 2026 11:40

ydirson requested changes Mar 9, 2026

View reviewed changes

olivierh-pro force-pushed the ohu/xcpng-539/001 branch 3 times, most recently from a3dfcb9 to f64c154 Compare March 9, 2026 16:15

glehmann reviewed Mar 10, 2026

View reviewed changes

lib/tools/tasks/update.py Outdated Show resolved Hide resolved

olivierh-pro force-pushed the ohu/xcpng-539/001 branch 2 times, most recently from 65ea2a4 to c8c5013 Compare March 10, 2026 15:54

olivierh-pro requested review from glehmann and ydirson March 10, 2026 15:55

glehmann approved these changes Mar 10, 2026

View reviewed changes

olivierh-pro added 4 commits March 12, 2026 10:16

feat(tools): adds sub package tools for automation scripts

54ce53a

* instantiates an object logger in `__init__` * logfmt and datefmt match with pytest config found in file `./pytest.ini` at root level Signed-off-by: Olivier Hoareau <olivier.hoareau@vates.tech>

feat(tools): adds tasks sub packages

a7e4e14

Signed-off-by: Olivier Hoareau <olivier.hoareau@vates.tech>

feat(tools): adds update task

c739495

This task performs a basic update and takes only one host. It is intended for being used against a list of hosts. Signed-off-by: Olivier Hoareau <olivier.hoareau@vates.tech>

olivierh-pro force-pushed the ohu/xcpng-539/001 branch from c8c5013 to a8ab52e Compare March 12, 2026 09:17

Conversation

olivierh-pro commented Mar 6, 2026

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants