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 cbe5f3a 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

@@ -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:

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: