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.

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

in_place_upgrade_guest: automates the case that verifies in place upgrade on a guest #6195

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rh-jugraham
Copy link
Contributor

@rh-jugraham rh-jugraham commented Feb 19, 2025

Case ID: VIRT-293757

Automates the case that verifies that an in place upgrade can be successfully completed on a guest. This case has already been automated by /tp-qemu/qemu/tests/in_place_upgrade.py - this specific test in tp-libvirt exists such that an in place guest upgrade test can be accomplished without depending on 'kar'.

This test specifically supports upgrading from rhel X-1 -> rhel X on a rhel X host.

Tests passing:

(.libvirt-ci-venv-ci-runtest-ZbmaDy) [root@ampere-mtsnow-altra-09 ~]# avocado run --vt-type libvirt --vt-omit-data-loss --vt-machine-type arm64-mmio in_place_upgrade_guest --vt-connect-uri qemu:///system
JOB ID     : 3947dcc69ad9e030e06a3828ae35e8470aa508dc
JOB LOG    : /var/log/avocado/job-results/job-2025-02-24T15.09-3947dcc/job.log
 (1/1) type_specific.io-github-autotest-libvirt.in_place_upgrade_guest.rhel9_to_rhel10.0: STARTED
 (1/1) type_specific.io-github-autotest-libvirt.in_place_upgrade_guest.rhel9_to_rhel10.0: PASS (1233.92 s)
RESULTS    : PASS 1 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB HTML   : /var/log/avocado/job-results/job-2025-02-24T15.09-3947dcc/results.html
JOB TIME   : 1235.61 s

@rh-jugraham rh-jugraham changed the title in place upgrade guest in_place_upgrade_guest Feb 24, 2025
@rh-jugraham rh-jugraham changed the title in_place_upgrade_guest in_place_upgrade_guest: automates the case that verifies in place upgrade on a guest Feb 24, 2025
@rh-jugraham rh-jugraham force-pushed the ipu_guest branch 2 times, most recently from 6060ddd to 65b6498 Compare February 24, 2025 20:10
@rh-jugraham
Copy link
Contributor Author

@dzhengfy @Yingshun Could you help review this PR?

A few notes:

  1. I added a 64k kernel workaround for now that can be removed when/if there is a change
  2. I was considering adding a new test type as this is not technically a libvirt specific test; however, I wasn't sure if it made sense to create a new test type for 1 test
  3. I wasn't sure how to best add in version specific information - I added it in the cfg under the rhel9_to_rhel10.0 variant (similar to the tp-qemu ipu test) but unsure if there is an alternative

@rh-jugraham rh-jugraham marked this pull request as ready for review February 25, 2025 11:43
@rh-jugraham rh-jugraham force-pushed the ipu_guest branch 3 times, most recently from 1d124cb to 5acd880 Compare February 25, 2025 22:30
list_kernel_cmd = "grubby --info=ALL | grep ^kernel"
default_kernel_cmd = "grub2-set-default"
variants:
- to_rhelX.0:
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, Julia

I am Okay with it. Just to mention that, we may need to do some settings in CI. We need to ask CI to provision a guest with latest RHEL(X-1) OS. We need to make sure CI knows the minor release of RHEL(X-1).

Copy link
Contributor

Choose a reason for hiding this comment

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

We have various guest in different versions in US/PEK2 storage server. So I think we just fetch it as we want.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dzhengfy This does make more sense to me (we have a rhelX.z machine and we are testing upgrading from rhelX-1.y --> X.z). However, how do we make sure the CI knows the minor release of rhelX-1.y - how do we know which guest is the latest to fetch? And what is the best way to fetch it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Update: understood - the test will assume that it already has a guest at version X-1.y on a host with version X.z.

Copy link
Contributor

@dzhengfy dzhengfy left a comment

Choose a reason for hiding this comment

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

Others LGTM

release_check_cmd = "cat /etc/redhat-release"
kernel_check_cmd = "uname -r"
pagesize_check_cmd = "getconf PAGESIZE"
compose_url = "http://download.eng.bos.redhat.com"
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember to replace this internal URL with a pattern and we will pass the real value in Libvirt-CI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dzhengfy Done - created a PR in ci-shared-datas. One question: is it an issue that the test has baseurl={compose_url}/rhel-{target_major_release}/nightly...? I'm not sure if the full path like this belongs in the code but I had struggled defining it completely in the cfg (especially getting vm_arch_name).

list_kernel_cmd = "grubby --info=ALL | grep ^kernel"
default_kernel_cmd = "grub2-set-default"
variants:
- to_rhelX.0:
Copy link
Contributor

Choose a reason for hiding this comment

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

We have various guest in different versions in US/PEK2 storage server. So I think we just fetch it as we want.

@rh-jugraham rh-jugraham force-pushed the ipu_guest branch 3 times, most recently from 8f8f59c to d6d4962 Compare February 27, 2025 18:43
leapp_preupgrade_cmd = "leapp preupgrade --debug --no-rhsm"
leapp_upgrade_cmd = "leapp upgrade --debug --no-rhsm"
list_kernel_cmd = "grubby --info=ALL | grep ^kernel"
default_kernel_cmd = "grub2-set-default"
Copy link
Contributor

@smitterl smitterl Feb 28, 2025

Choose a reason for hiding this comment

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

On s390x there's no grub. grubby --set-default is more agnostic (though it still requires running zipl on s390x after using it).
Generally, I think setting the default kernel to boot is useful to live in its own helper function, it would make sense to add it here: https://github.com/avocado-framework/avocado-vt/blob/master/virttest/utils_test/__init__.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.

Good call - I created a PR for that!

enabled=0
gpgcheck=0
""")
session.cmd("cat <<EOF > %s %s\nEOF" % (upgrade_repos_path, upgrade_repos_content))
Copy link
Contributor

@smitterl smitterl Feb 28, 2025

Choose a reason for hiding this comment

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

As mentioned above, it's usually better to use the python library instead of bash in this case:

with(upgrade_repos_path...) as f:
     f.write(f"""
[APPSTREAM]
...
gpgcheck=0
""")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@smitterl How would I do so on the vm since with open() as f tries to open the file on the host - I haven't been able to find an alternative for the guest.

Copy link
Contributor

@smitterl smitterl left a comment

Choose a reason for hiding this comment

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

Thanks for this. I think this can be improved to be more generally useful, esp. on more architectures.

@rh-jugraham
Copy link
Contributor Author

Thanks for this. I think this can be improved to be more generally useful, esp. on more architectures.

@smitterl Thank you for the detailed feedback! I believe I have addressed all comments or added a question if confused.

@rh-jugraham rh-jugraham requested review from dzhengfy and smitterl March 4, 2025 21:59
@rh-jugraham rh-jugraham requested a review from iccaszhulili March 4, 2025 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants