Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Automated install tests #226

Draft
wants to merge 76 commits into
base: master
Choose a base branch
from
Draft

Automated install tests #226

wants to merge 76 commits into from

Conversation

ydirson
Copy link
Contributor

@ydirson ydirson commented May 14, 2024 •
edited
Loading

Now based on top of preliminary PRs to separate generally-useful stuff:

Requires at least python 3.8 and pytest 8.2.

gduperrey
gduperrey reviewed May 14, 2024
View reviewed changes
scripts/install_xcpng.py Outdated
@@ -237,7 +198,7 @@ def main():
hdd = 'nvme0n1' if vm.is_uefi else 'sda'
generate_answerfile(tmp_local_path, installer, args.host, args.target_hostname, args.action, hdd,
netinstall_gpg_check)
generate_boot_conf(tmp_local_path, installer, args.action)
pxe.generate_boot_conf(tmp_local_path, installer)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this change. This option was used to have 3 actions: install, upgrade or restore.
I don't see how is it managed in the pxe file. It doesn't seems to exist anymore.

Copy link
Contributor Author

@ydirson ydirson May 14, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generate_boot_conf only uses the action to set rt=1 as a workaround in the restore case, I want to have a look at that to see if we can do better (e.g. an answerfile solution). Note it's all still very draft...

gduperrey
gduperrey reviewed May 14, 2024
View reviewed changes
data.py-dist
@@ -26,6 +26,15 @@ PXE_CONFIG_SERVER = 'pxe'
# Default VM images location
DEF_VM_URL = 'http://pxe/images/'

# Default shared ISO SR
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why using another ISO SR when we already have DEFAULT_NFS_ISO_DEVICE_CONFIG in the data.py?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I had missed that one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact those are different, DEFAULT_NFS_ISO_DEVICE_CONFIG is used in tests that create a new ISO SR from an existing NFS server, while what I'm doing is using an existing ISO SR.

Using an existing shared ISO SR is in fact not perfect because of the possibility of interference with other users (filename clashes), but turning an existing NFS share into an ISO SR does not help this, and does not seem really suitable when working with hosts that already have an ISO SR configured.

OTOH the test could provide its own ISO SR to workaround the filename clashes - or, more comfortably, make a clever use of mkstemp through ssh.

@ydirson
Copy link
Contributor Author

ydirson commented May 15, 2024 •
edited
Loading

Current status: install + firstboot work in a single test
Next:

  • xva export/import, split firstboot in separate test
  • include answerfile in install.img
  • answerfile generation

@ydirson ydirson force-pushed the install branch 3 times, most recently from 13282cc to 64d0e9c Compare May 23, 2024 08:16
@ydirson
Copy link
Contributor Author

ydirson commented May 23, 2024 •
edited
Loading

Current status: 2 separate tests for install and firstboot work without any manual intervention
Next steps:

  • use VM cache to gain time
  • write reinstall test
  • parametrize ISO remaster
  • write upgrade test

@ydirson ydirson force-pushed the install branch 4 times, most recently from 143c991 to 56abc4e Compare May 28, 2024 13:38
@ydirson
Copy link
Contributor Author

ydirson commented May 28, 2024 •
edited by gthvn1
Loading

Current status:

  • 3 separate tests for install, firstboot, "reinstall upgrade"
  • test dependencies declared (requires pytest-dependency, best used with pytest-order)
  • includes pytest_collection_modifyitems hook to automatically run the dependencies first

Next steps:

  • write a hook to not rerun dependencies whose output is in the vm cache, starting with Add support for a hook to override skip decision. RKrahl/pytest-dependency#77
  • start adding (explicit) parameters on tests
  • likely postoned
    • fix pytest_collection_modifyitems hook to properly register additional tests (some fixtures are missing)
    • improve hook to deal with test parameters
    • improve hook further by not ignoring dependents of tests explicitly requested (so in a->b->c->d chain with cached VMs pytest b d will also run c

@ydirson ydirson force-pushed the install branch 4 times, most recently from 1fda4a1 to 2dd8ec0 Compare June 14, 2024 08:04
@ydirson
Copy link
Contributor Author

ydirson commented Jun 14, 2024 •
edited
Loading

Current status:

  • cached VM descriptions now much less verbose, fitting in XO display
  • pytest_collection_modifyitems hook for auto-registring dependencies was moved out of scope, will be a later PR
  • start including some tests of the fixtures themselves
  • improvements to test parametrization well under way:
    • started to make fixtures more versatile, with ability to use callables taking test parameters, leading to less duplication
    • ... but bad interaction with pytest-order, to be investigated

Next steps:

  • deal with the regression on dependency ordering - may not be trivial or even doable
  • finish adding uefi/bios "firmware" parameter
  • add iso/net "package source" parameter
  • add ability to upgrade from XS
  • add ability to upgrade from 8.0-8.1 to 8.3
  • add ability to upgrade from 7.5-7.6 to 8.3
  • add "restore" test following "upgrade"

@ydirson ydirson force-pushed the install branch 3 times, most recently from b75d3ed to ebe6700 Compare June 19, 2024 16:03
@ydirson ydirson force-pushed the install branch 4 times, most recently from 358055b to 18e0ef8 Compare June 25, 2024 15:14
@ydirson ydirson force-pushed the install branch 10 times, most recently from 3383b51 to 795c185 Compare September 17, 2024 09:56
stormi and others added 3 commits September 18, 2024 10:22
@stormi
fixup: only one pool required for postinstall job
85cd765 
Signed-off-by: Samuel Verschelde <stormi-xcp@ylix.fr>
@ydirson
Merge branch 'stormi/post-install-test-job'
402d345
@ydirson
jobs.py: don't ignore --host-version when --hosts is passed
28412f7 
When using --hosts=cache://... it is much too early for such a check, so
we have to give the info manually, and the script would not allow this.

And honor it not just for "collect".
@ydirson ydirson force-pushed the install branch 5 times, most recently from 9a91ea3 to f167fac Compare September 18, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gduperrey gduperrey gduperrey left review comments

@gthvn1 gthvn1 gthvn1 approved these changes

@stormi stormi Awaiting requested review from stormi

@tescande tescande Awaiting requested review from tescande

@benjamreis benjamreis Awaiting requested review from benjamreis

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ydirson @gthvn1 @gduperrey @stormi