From 56936df7a53dc5a6054659191dc03416fb1fbc45 Mon Sep 17 00:00:00 2001 From: Adam Williamson Date: Wed, 22 Feb 2023 14:54:18 -0800 Subject: [PATCH] Update live install tests: handle awkward install ordering There's this awkward path for the live image install tests on updates. We run the 'are the correct versions of all the packages installed' check on these tests to ensure the right versions actually made it onto the live image. So we don't run `dnf -y update` at the end of repo_setup_updates on that path, because if we did that, even if the packages on the live image were old, we'd update them there and hide the problem. However, this causes a bit of an ordering issue, because in order to set up the advisory repo, we need to install a few packages. What if the update under test includes one of those packages, or a dependency that wasn't already installed? In that case, we wind up with the older stable version of the package (because obviously we can't install the newer version from the advisory repo *before we've set up the advisory repo*), don't update it later, and so the 'correct version' check at the end of the test fails. See: https://openqa.fedoraproject.org/tests/1778707 for a case of this happening with a python-cryptography update. Up till now I was trying to handle this by just updating the specific packages we install, but that doesn't account for *dependencies* of them. I looked down the path of trying to generate a list of all those dependencies and update all of them but it looks a bit mad. So instead let's try this. On that specific path, we'll generate the "all installed packages" list *before* we run repo_setup, so it just doesn't include anything that gets installed during repo_setup. The implementation is a bit icky but not too horrible. We *could* just *always* generate the all installed packages list earlier, but then that would mean we *wouldn't* catch dep issues in this kind of package on the other test paths, whereas currently we do. I don't want to lose that. Signed-off-by: Adam Williamson --- lib/utils.pm | 17 +++++++++-------- tests/_advisory_post.pm | 12 ++++++++++++ 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/lib/utils.pm b/lib/utils.pm index b439e6e6..c611875d 100644 --- a/lib/utils.pm +++ b/lib/utils.pm @@ -7,7 +7,7 @@ use Exporter; use lockapi; use testapi; -our @EXPORT = qw/run_with_error_check type_safely type_very_safely desktop_vt boot_to_login_screen console_login console_switch_layout desktop_switch_layout console_loadkeys_us do_bootloader boot_decrypt check_release menu_launch_type repo_setup setup_workaround_repo disable_updates_repos cleanup_workaround_repo console_initial_setup handle_welcome_screen gnome_initial_setup anaconda_create_user check_desktop download_modularity_tests quit_firefox advisory_get_installed_packages advisory_check_nonmatching_packages start_with_launcher quit_with_shortcut disable_firefox_studies select_rescue_mode copy_devcdrom_as_isofile get_release_number check_left_bar check_top_bar check_prerelease check_version spell_version_number _assert_and_click is_branched rec_log click_unwanted_notifications repos_mirrorlist register_application get_registered_applications solidify_wallpaper check_and_install_git download_testdata make_serial_writable/; +our @EXPORT = qw/run_with_error_check type_safely type_very_safely desktop_vt boot_to_login_screen console_login console_switch_layout desktop_switch_layout console_loadkeys_us do_bootloader boot_decrypt check_release menu_launch_type repo_setup setup_workaround_repo disable_updates_repos cleanup_workaround_repo console_initial_setup handle_welcome_screen gnome_initial_setup anaconda_create_user check_desktop download_modularity_tests quit_firefox advisory_get_all_packages advisory_get_installed_packages advisory_check_nonmatching_packages start_with_launcher quit_with_shortcut disable_firefox_studies select_rescue_mode copy_devcdrom_as_isofile get_release_number check_left_bar check_top_bar check_prerelease check_version spell_version_number _assert_and_click is_branched rec_log click_unwanted_notifications repos_mirrorlist register_application get_registered_applications solidify_wallpaper check_and_install_git download_testdata make_serial_writable/; # We introduce this global variable to hold the list of applications that have # registered during the apps_startstop_test when they have sucessfully run. @@ -466,7 +466,7 @@ sub setup_workaround_repo { # for any release, the hash can be empty and this will do nothing my $version = shift || get_var("VERSION"); cleanup_workaround_repo; - script_run "dnf -y install bodhi-client createrepo koji", 300; + script_run "dnf -y install bodhi-client createrepo_c koji", 300; # write a repo config file, unless this is the support_server test # and it is running on a different release than the update is for # (in this case we need the repo to exist but do not want to use @@ -677,11 +677,6 @@ sub _repo_setup_updates { # already and we want to fail if they weren't, or CANNED # tests, there's no point updating the toolbox script_run "dnf -y update", 1200 unless (get_var("UPGRADE") || get_var("INSTALL") || get_var("CANNED")); - # however, on liveinst tests, we need to update the packages - # we installed above, just in case they're in the update - # under test; otherwise we get a bogus failure for the package - # not being updated - script_run "dnf -y update bodhi-client createrepo koji", 600 if (get_var("INSTALL") && !get_var("CANNED")); } # exit the toolbox on CANNED if (get_var("CANNED")) { @@ -1120,6 +1115,12 @@ sub quit_with_shortcut { } +sub advisory_get_all_packages { + # skip if it's already been done + return unless script_run "test -f /tmp/allpkgs.txt"; + assert_script_run 'rpm -qa --qf "%{SOURCERPM} %{NAME} %{EPOCHNUM} %{VERSION} %{RELEASE}\n" | sort -u > /tmp/allpkgs.txt', timeout => 90; +} + # For update tests (this only works if we've been through # _repo_setup_updates), figure out which packages from the update # are currently installed. This is here so we can do it both in @@ -1128,7 +1129,7 @@ sub advisory_get_installed_packages { # bail out if the file doesn't exist: this is in case we get # here in the post-fail hook but we failed before creating it return if script_run "test -f /mnt/updatepkgs.txt"; - assert_script_run 'rpm -qa --qf "%{SOURCERPM} %{NAME} %{EPOCHNUM} %{VERSION} %{RELEASE}\n" | sort -u > /tmp/allpkgs.txt', timeout => 90; + advisory_get_all_packages; # this finds lines which appear in both files # http://www.unix.com/unix-for-dummies-questions-and-answers/34549-find-matching-lines-between-2-files.html if (script_run 'comm -12 /tmp/allpkgs.txt /mnt/updatepkgs.txt > /mnt/testedpkgs.txt') { diff --git a/tests/_advisory_post.pm b/tests/_advisory_post.pm index 76642caf..e1b2b76e 100644 --- a/tests/_advisory_post.pm +++ b/tests/_advisory_post.pm @@ -6,6 +6,18 @@ use utils; sub run { my $self = shift; $self->root_console(tty => 3); + # on the install_default_update test path, we want to generate + # the all-installed-package list *before* we do repo_setup, so + # it doesn't include packages installed as part of repo_setup. + # this is because we don't do a dnf update at the end of repo_ + # setup on this path (in order to make sure that the updated + # packages were actually on the live image), so if any package + # from the update under test gets installed as part of repo_ + # setup, it won't be the version from the update, but the older + # version from the stable repos, and we won't then update it. + # if we include it in the allpkgs list, we'll fail later because + # it's "too old" + advisory_get_all_packages if (get_var("INSTALL") && !get_var("CANNED")); # do repo_setup if it's not been done already - this is for the # install_default_update tests repo_setup;