curtin-dev team mailing list archive

[Dan Bungert <mp+466271@xxxxxxxxxxxxxxxxxx>]

Dan Bungert has proposed merging ~dbungert/curtin:kernel-cfg into curtin:master.

Commit message:
do not squash

create a KernelConfig attrs structure, refactor it in, and expand unit tests on the way
expands deserializer to handle key names with dashes, by unconditionally replacing dashes to underscores

Requested reviews:
  curtin developers (curtin-dev)

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

diff --git a/curtin/commands/curthooks.py b/curtin/commands/curthooks.py
index 34ff660..b278703 100644
--- a/curtin/commands/curthooks.py
+++ b/curtin/commands/curthooks.py
@@ -368,19 +368,15 @@ def install_kernel(cfg, target):
         env["FK_FORCE_CONTAINER"] = "yes"
         distro.install_packages([pkg], target=target, env=env)
 
-    kernel_cfg = cfg.get('kernel', {'package': None,
-                                    'fallback-package': "linux-generic",
-                                    'mapping': {}})
-
-    if kernel_cfg is None:
+    kernel_cfg_d = cfg.get('kernel', {})
+    if kernel_cfg_d is None:
         LOG.debug("Not installing any kernel since kernel: null was specified")
         return
 
-    kernel_package = kernel_cfg.get('package')
-    kernel_fallback = kernel_cfg.get('fallback-package')
+    kernel_cfg = config.fromdict(config.KernelConfig, kernel_cfg_d)
 
     mapping = copy.deepcopy(KERNEL_MAPPING)
-    config.merge_config(mapping, kernel_cfg.get('mapping', {}))
+    config.merge_config(mapping, kernel_cfg.mapping)
 
     # Machines using flash-kernel may need additional dependencies installed
     # before running. Run those checks in the ephemeral environment so the
@@ -389,8 +385,8 @@ def install_kernel(cfg, target):
     if fk_packages:
         distro.install_packages(fk_packages.split(), target=target)
 
-    if kernel_package:
-        install(kernel_package)
+    if kernel_cfg.package:
+        install(kernel_cfg.package)
         return
 
     # uname[2] is kernel name (ie: 3.16.0-7-generic)
@@ -406,8 +402,8 @@ def install_kernel(cfg, target):
     except KeyError:
         LOG.warn("Couldn't detect kernel package to install for %s."
                  % kernel)
-        if kernel_fallback is not None:
-            install(kernel_fallback)
+        if kernel_cfg.fallback_package is not None:
+            install(kernel_cfg.fallback_package)
         return
 
     package = "linux-{flavor}{map_suffix}".format(
@@ -420,11 +416,11 @@ def install_kernel(cfg, target):
             LOG.debug("installing kernel package '%s'", package)
             install(package)
     else:
-        if kernel_fallback is not None:
+        if kernel_cfg.fallback_package is not None:
             LOG.info("Kernel package '%s' not available.  "
                      "Installing fallback package '%s'.",
-                     package, kernel_fallback)
-            install(kernel_fallback)
+                     package, kernel_cfg.fallback_package)
+            install(kernel_cfg.fallback_package)
         else:
             LOG.warn("Kernel package '%s' not available and no fallback."
                      " System may not boot.", package)
diff --git a/curtin/config.py b/curtin/config.py
index b30ecc5..979bb09 100644
--- a/curtin/config.py
+++ b/curtin/config.py
@@ -154,6 +154,13 @@ class GrubConfig:
     update_nvram: bool = attr.ib(default=True, converter=value_as_boolean)
 
 
+@attr.s(auto_attribs=True)
+class KernelConfig:
+    package: typing.Optional[str] = None
+    fallback_package: str = "linux-generic"
+    mapping: dict = {}
+
+
 class SerializationError(Exception):
     def __init__(self, obj, path, message):
         self.obj = obj
@@ -232,6 +239,7 @@ class Deserializer:
             field.name: field for field in attr.fields(annotation)
             }
         for key, value in context.cur.items():
+            key = key.replace("-", "_")
             if key not in fields:
                 continue
             field = fields[key]
diff --git a/tests/unittests/test_config.py b/tests/unittests/test_config.py
index ae51744..603a5c9 100644
--- a/tests/unittests/test_config.py
+++ b/tests/unittests/test_config.py
@@ -195,5 +195,16 @@ class TestDeserializer(CiTestCase):
             WithConverter(val=True),
             deserializer.deserialize(WithConverter, {"val": "on"}))
 
+    def test_dash_to_underscore(self):
+        deserializer = config.Deserializer()
+
+        @attr.s(auto_attribs=True)
+        class DashToUnderscore:
+            a_b: bool = False
+
+        self.assertEqual(
+            DashToUnderscore(a_b=True),
+            deserializer.deserialize(DashToUnderscore, {"a-b": True}))
+
 
 # vi: ts=4 expandtab syntax=python
diff --git a/tests/unittests/test_curthooks.py b/tests/unittests/test_curthooks.py
index b73a19f..cbca425 100644
--- a/tests/unittests/test_curthooks.py
+++ b/tests/unittests/test_curthooks.py
@@ -55,15 +55,20 @@ class TestGetFlashKernelPkgs(CiTestCase):
 class TestCurthooksInstallKernel(CiTestCase):
     def setUp(self):
         super(TestCurthooksInstallKernel, self).setUp()
+        ccc = 'curtin.commands.curthooks'
         self.add_patch('curtin.distro.has_pkg_available', 'mock_haspkg')
         self.add_patch('curtin.distro.install_packages', 'mock_instpkg')
+        self.add_patch(ccc + '.os.uname', 'mock_uname')
+        self.add_patch(ccc + '.util.subp', 'mock_subp')
         self.add_patch(
-            'curtin.commands.curthooks.get_flash_kernel_pkgs',
+            ccc + '.get_flash_kernel_pkgs',
             'mock_get_flash_kernel_pkgs')
 
+        self.mock_get_flash_kernel_pkgs.return_value = None
         self.kernel_cfg = {'kernel': {'package': 'mock-linux-kernel',
                                       'fallback-package': 'mock-fallback',
                                       'mapping': {}}}
+        self.fk_env = {'FK_FORCE': 'yes', 'FK_FORCE_CONTAINER': 'yes'}
         # Tests don't actually install anything so we just need a name
         self.target = self.tmp_dir()
 
@@ -74,30 +79,51 @@ class TestCurthooksInstallKernel(CiTestCase):
         with patch.dict(os.environ, clear=True):
             curthooks.install_kernel(self.kernel_cfg, self.target)
 
-            env = {'FK_FORCE': 'yes', 'FK_FORCE_CONTAINER': 'yes'}
-
             inst_calls = [
                 call(['u-boot-tools'], target=self.target),
-                call([kernel_package], target=self.target, env=env)]
+                call([kernel_package], target=self.target, env=self.fk_env)]
 
             self.mock_instpkg.assert_has_calls(inst_calls)
 
     def test__installs_kernel_package(self):
         kernel_package = self.kernel_cfg.get('kernel', {}).get('package', {})
-        self.mock_get_flash_kernel_pkgs.return_value = None
 
         with patch.dict(os.environ, clear=True):
             curthooks.install_kernel(self.kernel_cfg, self.target)
 
-            env = {'FK_FORCE': 'yes', 'FK_FORCE_CONTAINER': 'yes'}
+            self.mock_instpkg.assert_called_with(
+                [kernel_package], target=self.target, env=self.fk_env)
+
+    def test__installs_kernel_fallback_package(self):
+        del self.kernel_cfg['kernel']['package']
+        fallback = self.kernel_cfg['kernel']['fallback-package']
+
+        self.mock_subp.return_value = ("warty", "")
+        self.mock_uname.return_value = (None, None, "1.2.3-4-flavor")
+
+        with patch.dict(os.environ, clear=True):
+            curthooks.install_kernel(self.kernel_cfg, self.target)
+
+            self.mock_instpkg.assert_called_with(
+                [fallback], target=self.target, env=self.fk_env)
+
+    def test__installs_kernel_from_mapping(self):
+        del self.kernel_cfg['kernel']['package']
+        self.kernel_cfg['kernel']['mapping'] = {
+            "warty": {"1.2.3": "-lts-dapper"}
+        }
+        self.mock_subp.return_value = ("warty", "")
+        self.mock_uname.return_value = (None, None, "1.2.3-4-flavor")
+
+        with patch.dict(os.environ, clear=True):
+            curthooks.install_kernel(self.kernel_cfg, self.target)
 
             self.mock_instpkg.assert_called_with(
-                [kernel_package], target=self.target, env=env)
+                ["linux-flavor-lts-dapper"],
+                target=self.target, env=self.fk_env)
 
     def test__installs_kernel_null(self):
         kernel_cfg = {'kernel': None}
-        self.mock_get_flash_kernel_pkgs.return_value = None
-
         with patch.dict(os.environ, clear=True):
             curthooks.install_kernel(kernel_cfg, self.target)