From 13c130090297cecd139e00e3d03eeda31b959774 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Sun, 28 Apr 2024 21:53:22 -0600 Subject: [PATCH] bug(package_update): avoid snap refresh in images without snap command When package_update or package_upgrade: true are provided in cloud-config do not call snap refresh on systems that do not have the snap command. This was intended as fixed in cbe5f3a119 but the fix only avoided snap refresh during package install, not the generic package update operation. Fixes: GH-5143 LP: #2064132 --- cloudinit/distros/__init__.py | 6 ++ tests/unittests/distros/test_init.py | 134 +++++++++++++++++++++------ 2 files changed, 114 insertions(+), 26 deletions(-) diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index 9c4d6b2333c..e3239daaedd 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -396,6 +396,12 @@ def package_command(self, command, args=None, pkgs=None): def update_package_sources(self): for manager in self.package_managers: + if not manager.available(): + LOG.debug( + "Skipping update for package manager '%s': not available.", + manager.name, + ) + continue try: manager.update_package_sources() except Exception as e: diff --git a/tests/unittests/distros/test_init.py b/tests/unittests/distros/test_init.py index 986ccafcbca..7809899ff85 100644 --- a/tests/unittests/distros/test_init.py +++ b/tests/unittests/distros/test_init.py @@ -33,6 +33,8 @@ # unable to parse URLs ("[", "]"). INVALID_URL_CHARS.remove(separator) +M_PATH = "cloudinit.distros.package_management." + class TestGetPackageMirrorInfo: """ @@ -253,31 +255,120 @@ def test_valid_substitution( assert {"primary": expected} == ret -class TestInstall: - """Tests for cloudinit.distros.Distro.install_packages.""" +class TestUpdatePackageSources: + """Tests for cloudinit.distros.Distro.update_package_sources.""" - @pytest.fixture(autouse=True) - def ensure_available(self, mocker): + @pytest.mark.parametrize( + "apt_error,snap_error,expected_logs", + [ + pytest.param( + RuntimeError("fail to find 'apt' command"), + None, + [ + "Failed to update package using apt: fail to find 'apt'" + " command" + ], + ), + pytest.param( + None, + RuntimeError("fail to find 'snap' command"), + [ + "Failed to update package using snap: fail to find 'snap'" + " command" + ], + ), + ], + ) + def test_log_errors_with_updating_package_source( + self, apt_error, snap_error, expected_logs, mocker, caplog + ): + """Log error raised from any package_manager.update_package_sources.""" + mocker.patch(M_PATH + "apt.Apt.available", return_value=True) + mocker.patch(M_PATH + "snap.Snap.available", return_value=True) + mocker.patch( + M_PATH + "apt.Apt.update_package_sources", + side_effect=apt_error, + ) mocker.patch( - "cloudinit.distros.package_management.apt.Apt.available", - return_value=True, + M_PATH + "snap.Snap.update_package_sources", + side_effect=snap_error, ) + _get_distro("ubuntu").update_package_sources() + for log in expected_logs: + assert log in caplog.text + + @pytest.mark.parametrize( + "apt_available,snap_available,expected_logs", + [ + pytest.param( + True, + False, + ["Skipping update for package manager 'snap': not available."], + ), + pytest.param( + False, + True, + ["Skipping update for package manager 'apt': not available."], + ), + pytest.param( + False, + False, + [ + "Skipping update for package manager 'apt': not" + " available.", + "Skipping update for package manager 'snap': not" + " available.", + ], + ), + ], + ) + def test_run_available_package_managers( + self, apt_available, snap_available, expected_logs, mocker, caplog + ): + """Avoid update_package_sources on unavailable package managers""" + + mocker.patch(M_PATH + "apt.Apt.available", return_value=apt_available) mocker.patch( - "cloudinit.distros.package_management.snap.Snap.available", - return_value=True, + M_PATH + "snap.Snap.available", + return_value=snap_available, ) + m_apt_update = mocker.patch(M_PATH + "apt.Apt.update_package_sources") + m_snap_update = mocker.patch( + M_PATH + "snap.Snap.update_package_sources" + ) + _get_distro("ubuntu").update_package_sources() + if not snap_available: + m_snap_update.assert_not_called() + else: + m_snap_update.assert_called_once() + if not apt_available: + m_apt_update.assert_not_called() + else: + m_apt_update.assert_called_once() + for log in expected_logs: + assert log in caplog.text + + +class TestInstall: + """Tests for cloudinit.distros.Distro.install_packages.""" + + @pytest.fixture(autouse=True) + def ensure_available(self, mocker): + mocker.patch(M_PATH + "apt.Apt.available", return_value=True) + mocker.patch(M_PATH + "snap.Snap.available", return_value=True) + @pytest.fixture def m_apt_install(self, mocker): return mocker.patch( - "cloudinit.distros.package_management.apt.Apt.install_packages", + M_PATH + "apt.Apt.install_packages", return_value=[], ) @pytest.fixture def m_snap_install(self, mocker): return mocker.patch( - "cloudinit.distros.package_management.snap.Snap.install_packages", + M_PATH + "snap.Snap.install_packages", return_value=[], ) @@ -324,7 +415,7 @@ def test_non_default_package_manager_fail( ): """Test fail from package manager not supported by distro.""" m_snap_install = mocker.patch( - "cloudinit.distros.package_management.snap.Snap.install_packages", + M_PATH + "snap.Snap.install_packages", return_value=["pkg3"], ) with pytest.raises( @@ -356,7 +447,7 @@ def test_specific_package_manager_fail_doesnt_retry( ): """Test fail from package manager doesn't retry as generic.""" m_apt_install = mocker.patch( - "cloudinit.distros.package_management.apt.Apt.install_packages", + M_PATH + "apt.Apt.install_packages", return_value=["pkg1"], ) with pytest.raises(PackageInstallerError): @@ -369,14 +460,8 @@ def test_no_attempt_if_no_package_manager( self, mocker, m_apt_install, m_snap_install, caplog ): """Test that no attempt is made if there are no package manager.""" - mocker.patch( - "cloudinit.distros.package_management.apt.Apt.available", - return_value=False, - ) - mocker.patch( - "cloudinit.distros.package_management.snap.Snap.available", - return_value=False, - ) + mocker.patch(M_PATH + "apt.Apt.available", return_value=False) + mocker.patch(M_PATH + "snap.Snap.available", return_value=False) with pytest.raises(PackageInstallerError): _get_distro("ubuntu").install_packages( ["pkg1", "pkg2", {"other": "pkg3"}] @@ -449,16 +534,13 @@ def test_failed( So test various combinations of these scenarios. """ + mocker.patch(M_PATH + "apt.Apt.available", return_value=apt_available) mocker.patch( - "cloudinit.distros.package_management.apt.Apt.available", - return_value=apt_available, - ) - mocker.patch( - "cloudinit.distros.package_management.apt.Apt.install_packages", + M_PATH + "apt.Apt.install_packages", return_value=apt_failed, ) mocker.patch( - "cloudinit.distros.package_management.snap.Snap.install_packages", + M_PATH + "snap.Snap.install_packages", return_value=snap_failed, ) with pytest.raises(PackageInstallerError) as exc: