purge: check for installed packages in place when removing/reinstalling

Using the apt_pkg.Cache to check for the installation information leads to outdated data and wrong behavior. Signed-off-by: Renan Rodrigo <[email protected]>
canonical · Oct 27, 2023 · 6ba7c37 · 6ba7c37
1 parent 84e23e2
commit 6ba7c37
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 6 deletions.
diff --git a/uaclient/entitlements/repo.py b/uaclient/entitlements/repo.py
@@ -239,13 +239,15 @@ def prompt_for_purge(self, packages_to_remove, packages_to_reinstall):
         return True
 
     def execute_removal(self, packages_to_remove):
-        # We need to check for package.current_ver again, because there is an
-        # intermediate step between listing the packages and acting on them.
+        # We need to check again if the package is installed, because there is
+        # an intermediate step between listing the packages and acting on them.
+        # Some reinstalls may also uninstall dependencies.
         # Packages may be removed between those operations.
+        installed_packages = apt.get_installed_packages_names()
         to_remove = [
             package.name
             for package in packages_to_remove
-            if package.current_ver
+            if package.name in installed_packages
         ]
         if to_remove:
             apt.purge_packages(
@@ -256,13 +258,14 @@ def execute_removal(self, packages_to_remove):
             )
 
     def execute_reinstall(self, packages_to_reinstall):
-        # We need to check for package.current_ver again, because there is an
-        # intermediate step between listing the packages and acting on them.
+        # We need to check again if the package is installed, because there is
+        # an intermediate step between listing the packages and acting on them.
         # Packages may be removed between those operations.
+        installed_packages = apt.get_installed_packages_names()
         to_reinstall = [
             "{}={}".format(package.name, version.ver_str)
             for (package, version) in packages_to_reinstall
-            if package.current_ver
+            if package.name in installed_packages
         ]
         if to_reinstall:
             apt.reinstall_packages(to_reinstall)

diff --git a/uaclient/entitlements/tests/test_repo.py b/uaclient/entitlements/tests/test_repo.py
@@ -864,13 +864,16 @@ def test_prompt_for_purge(
         ),
     )
     @mock.patch(M_PATH + "apt.purge_packages")
+    @mock.patch(M_PATH + "apt.get_installed_packages_names")
     def test_execute_removal(
         self,
+        m_installed_packages,
         m_apt_purge,
         remove,
         expected_remove,
         entitlement_factory,
     ):
+        m_installed_packages.return_value = ["remove1", "remove2"]
         entitlement = entitlement_factory(
             RepoTestEntitlement,
             affordances={"series": ["xenial"]},
@@ -903,13 +906,16 @@ def test_execute_removal(
         ),
     )
     @mock.patch(M_PATH + "apt.run_apt_install_command")
+    @mock.patch(M_PATH + "apt.get_installed_packages_names")
     def test_execute_reinstall(
         self,
+        m_installed_packages,
         m_apt_install,
         reinstall,
         expected_install,
         entitlement_factory,
     ):
+        m_installed_packages.return_value = ["reinstall1", "reinstall2"]
         entitlement = entitlement_factory(
             RepoTestEntitlement,
             affordances={"series": ["xenial"]},