curtin-dev team mailing list archive

[Dan Bungert <mp+469605@xxxxxxxxxxxxxxxxxx>]

Dan Bungert has proposed merging ~dbungert/curtin:skip-if-installed into curtin:master.

Commit message:
do not squash

attempt to skip kernel installs

For the OEM case, it helps if we don't redundantly try to install
kernels already installed, as we may not have that repo in our list
right this second.



Requested reviews:
  curtin developers (curtin-dev)

For more details, see:
https://code.launchpad.net/~dbungert/curtin/+git/curtin/+merge/469605
-- 
Your team curtin developers is requested to review the proposed merge of ~dbungert/curtin:skip-if-installed into curtin:master.

diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index 51b2a08..b7931ff 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -363,7 +363,8 @@ def install_kernel(cfg, target):
         # is mostly or always doing chroot installs.  LP: #1992990
         env["FK_FORCE"] = "yes"
         env["FK_FORCE_CONTAINER"] = "yes"
-        distro.install_packages([pkg], target=target, env=env)
+        distro.install_packages([pkg], target=target, env=env,
+                                skip_if_installed=True)
 
     kernel_cfg_d = cfg.get('kernel', {})
     if kernel_cfg_d is None:
@@ -394,7 +395,8 @@ def install_kernel(cfg, target):
         # See LP: #1640519
         fk_packages = get_flash_kernel_pkgs()
         if fk_packages:
-            distro.install_packages(fk_packages.split(), target=target)
+            distro.install_packages(fk_packages.split(), target=target,
+                                    skip_if_installed=True)
 
         if kernel_cfg.package:
             install(kernel_cfg.package)
diff --git a/curtin/distro.py b/curtin/distro.py
index 56a80bb..fa30956 100644
--- a/curtin/distro.py
+++ b/curtin/distro.py
@@ -452,6 +452,7 @@ def system_upgrade(opts=None, target=None, env=None, allow_daemons=False,
 
 def install_packages(pkglist, osfamily=None, opts=None, target=None, env=None,
                      allow_daemons=False,
+                     skip_if_installed=False,
                      download_retries: Optional[Sequence[int]] = None,
                      download_only=False, assume_downloaded=False):
     if isinstance(pkglist, str):
@@ -471,6 +472,15 @@ def install_packages(pkglist, osfamily=None, opts=None, target=None, env=None,
         raise ValueError('No package install command for distro: %s' %
                          osfamily)
 
+    if skip_if_installed:
+        if not (pkglist := filter_installed_packages(pkglist)):
+            # If a package is already installed, we need not attempt to install
+            # it.  In some cases it's safe to try, but if a package was
+            # installed from a special repo, such as are used to deliver OEM
+            # kernels for Ubuntu, then install operations without that special
+            # repo will not be successful.
+            return True
+
     return install_cmd('install', args=pkglist, opts=opts, target=target,
                        env=env, allow_daemons=allow_daemons,
                        download_retries=download_retries,
@@ -478,7 +488,7 @@ def install_packages(pkglist, osfamily=None, opts=None, target=None, env=None,
                        assume_downloaded=assume_downloaded)
 
 
-def dpkg_query_list_kernels(target=None):
+def dpkg_query_list_packages(target=None):
     target = target_path(target)
     cmd = [
         "dpkg-query",
@@ -487,8 +497,12 @@ def dpkg_query_list_kernels(target=None):
     ]
 
     out, _ = subp(cmd, capture=True, target=target)
+    return out.splitlines()
+
+
+def dpkg_query_list_kernels(target=None):
     results = []
-    for line in out.splitlines():
+    for line in dpkg_query_list_packages(target):
         package, status, provides = line.strip().split("/")
         # status abbreviation is 3 columns, 3rd is error state
         if status != "ii ":
@@ -499,6 +513,19 @@ def dpkg_query_list_kernels(target=None):
     return results
 
 
+def dpkg_query_filter_installed_packages(pkglist, target=None):
+    results = []
+    for line in dpkg_query_list_packages(target=target):
+        package, status, provides = line.strip().split("/")
+        if package not in pkglist:
+            continue
+        # status abbreviation is 3 columns, 3rd is error state
+        if status == "ii ":
+            continue
+        results.append(package)
+    return results
+
+
 def list_kernels(osfamily=None, target=None):
     if not osfamily:
         osfamily = get_osfamily(target=target)
@@ -509,12 +536,30 @@ def list_kernels(osfamily=None, target=None):
 
     list_kernels_cmd = distro_cfg.get(osfamily)
     if list_kernels_cmd is None:
-        raise ValueError('No package purge command for distro: %s' %
+        raise ValueError('No list kernels command for distro: %s' %
                          osfamily)
 
     return list_kernels_cmd(target=target)
 
 
+def filter_installed_packages(pkglist, osfamily=None, target=None):
+    """ given a list of packages, remove from that list the ones already
+        installed """
+    if not osfamily:
+        osfamily = get_osfamily(target=target)
+
+    distro_cfg = {
+        DISTROS.debian: dpkg_query_filter_installed_packages
+    }
+
+    filter_installed_packages_cmd = distro_cfg.get(osfamily)
+    if filter_installed_packages_cmd is None:
+        raise ValueError('No filter installed packages command for distro: %s'
+                         % osfamily)
+
+    return filter_installed_packages_cmd(pkglist, target=target)
+
+
 @contextmanager
 def ensure_one_kernel(osfamily=None, target=None):
     """ensure_one_kernel is a context manager that evalutates the state of
diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
index 47df8bf..7df0959 100644
--- a/tests/unittests/test_curthooks.py
+++ b/tests/unittests/test_curthooks.py
@@ -85,8 +85,11 @@ class TestCurthooksInstallKernel(CiTestCase):
             curthooks.install_kernel(kernel_cfg, self.target)
 
             inst_calls = [
-                call(['u-boot-tools'], target=self.target),
-                call([kernel_package], target=self.target, env=self.fk_env)]
+                call(['u-boot-tools'], skip_if_installed=True,
+                     target=self.target),
+                call([kernel_package], skip_if_installed=True,
+                     target=self.target, env=self.fk_env),
+            ]
 
             self.mock_instpkg.assert_has_calls(inst_calls)
 
@@ -97,7 +100,8 @@ class TestCurthooksInstallKernel(CiTestCase):
             curthooks.install_kernel(kernel_cfg, self.target)
 
             self.mock_instpkg.assert_called_with(
-                [kernel_package], target=self.target, env=self.fk_env)
+                [kernel_package], skip_if_installed=True, target=self.target,
+                env=self.fk_env)
 
     def test__installs_kernel_fallback_package(self):
         fallback_package = "mock-linux-kernel-fallback"
@@ -110,7 +114,8 @@ class TestCurthooksInstallKernel(CiTestCase):
             curthooks.install_kernel(kernel_cfg, self.target)
 
             self.mock_instpkg.assert_called_with(
-                [fallback_package], target=self.target, env=self.fk_env)
+                [fallback_package], skip_if_installed=True, target=self.target,
+                env=self.fk_env)
 
     def test__installs_kernel_from_mapping(self):
         kernel_cfg = {
@@ -130,6 +135,7 @@ class TestCurthooksInstallKernel(CiTestCase):
 
             self.mock_instpkg.assert_called_with(
                 ["linux-flavor-lts-dapper"],
+                skip_if_installed=True,
                 target=self.target, env=self.fk_env)
 
     @parameterized.expand((
@@ -163,6 +169,7 @@ class TestCurthooksInstallKernel(CiTestCase):
 
             self.mock_instpkg.assert_called_with(
                 [to_install_kernel_package],
+                skip_if_installed=True,
                 target=self.target,
                 env=self.fk_env,
             )
@@ -187,6 +194,7 @@ class TestCurthooksInstallKernel(CiTestCase):
 
             self.mock_instpkg.assert_called_with(
                 [to_install_kernel_package],
+                skip_if_installed=True,
                 target=self.target,
                 env=self.fk_env,
             )
@@ -212,6 +220,7 @@ class TestCurthooksInstallKernel(CiTestCase):
             # apt shouldn't have to do very much
             self.mock_instpkg.assert_called_with(
                 [to_install_kernel_package],
+                skip_if_installed=True,
                 target=self.target,
                 env=self.fk_env,
             )