Igor Brovtsin has proposed merging 
~igor-brovtsin/maas:dgx-platform-kernels-lookup into maas:master.

Commit message:
Updated kernel lookup mechanisms to use platforms

Requested reviews:
  MAAS Lander (maas-lander): unittests
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~igor-brovtsin/maas/+git/maas/+merge/441824

This MP introduces multiple changes that allow better platform-optimised kernel 
support. Namely, it changes the order of kernels returned by 
`get_working_kernel`: when no specific kernel was requested, platform-optimised 
(machine platform == kernel platform) kernels take precedence over 
platform-supporting (machine platform in kernel supported platforms) and 
generic (kernel platform == "generic"  kernels. This way, machines with a 
platform selected will get the platform-optimised kernel by default, if 
possible.

MERGING THIS WILL BREAK MAAS UNTIL WE UPDATE THE IMAGES.MAAS.IO TO INCLUDE 
PLATFORM FIELDS!
-- 
Your team MAAS Maintainers is requested to review the proposed merge of 
~igor-brovtsin/maas:dgx-platform-kernels-lookup into maas:master.
diff --git a/src/maasserver/models/bootresource.py b/src/maasserver/models/bootresource.py
index be9dc9e..ac7abd1 100644
--- a/src/maasserver/models/bootresource.py
+++ b/src/maasserver/models/bootresource.py
@@ -261,6 +261,7 @@ class BootResourceManager(Manager):
         platform=None,
         kflavor=None,
         include_subarches=False,
+        strict_platform_match=False,
     ):
         """Return the set of kernels.
 
@@ -318,9 +319,9 @@ class BootResourceManager(Manager):
                 resource_supported_platforms = resource.extra.get(
                     "supported_platforms", ""
                 ).split(",")
-                if (
-                    resource_platform != platform
-                    and platform not in resource_supported_platforms
+                if resource_platform != platform and (
+                    strict_platform_match
+                    or platform not in resource_supported_platforms
                 ):
                     continue
 
@@ -355,12 +356,15 @@ class BootResourceManager(Manager):
         # Make sure kernels named with a version come after the kernels named
         # with the first letter of release. This switched in Xenial so this
         # preserves the chronological order of the kernels.
-        return sorted(
-            kernels, key=lambda k: get_release_version_from_string(k)
-        )
+        return sorted(kernels, key=get_release_version_from_string)
 
     def get_kernels(
-        self, name=None, architecture=None, platform=None, kflavor=None
+        self,
+        name=None,
+        architecture=None,
+        platform=None,
+        kflavor=None,
+        strict_platform_match=False,
     ):
         """Return the set of usable kernels for the given name, arch,
         platform, and kflavor.
@@ -375,6 +379,7 @@ class BootResourceManager(Manager):
             platform=platform,
             kflavor=kflavor,
             include_subarches=False,
+            strict_platform_match=strict_platform_match,
         )
 
     def get_supported_kernel_compatibility_levels(
diff --git a/src/maasserver/rpc/tests/test_boot.py b/src/maasserver/rpc/tests/test_boot.py
index 21e69a2..19ab64d 100644
--- a/src/maasserver/rpc/tests/test_boot.py
+++ b/src/maasserver/rpc/tests/test_boot.py
@@ -47,7 +47,7 @@ from provisioningserver.rpc.exceptions import BootConfigNoResponse
 from provisioningserver.utils.network import get_source_address
 
 
-def get_config(*args, query_count=36, **kwargs):
+def get_config(*args, query_count=42, **kwargs):
     count, result = count_queries(orig_get_config, *args, **kwargs)
     assert (
         count <= query_count
diff --git a/src/maasserver/testing/architecture.py b/src/maasserver/testing/architecture.py
index 5141328..fcd667e 100644
--- a/src/maasserver/testing/architecture.py
+++ b/src/maasserver/testing/architecture.py
@@ -20,13 +20,20 @@ def make_arch(
     """
     if arch_name is None:
         arch_name = factory.make_name("arch")
-    factory.make_default_ubuntu_release_bootable(arch_name, extra=extra)
     if with_subarch:
         if subarch_name is None:
             subarch_name = factory.make_name("sub")
-        return f"{arch_name}/{subarch_name}"
+        result = f"{arch_name}/{subarch_name}"
     else:
-        return arch_name
+        result = arch_name
+
+    if not extra:
+        extra = {}
+    extra.setdefault("platform", "generic")
+    extra.setdefault("supported_platforms", subarch_name)
+    factory.make_default_ubuntu_release_bootable(arch_name, extra=extra)
+
+    return result
 
 
 def patch_usable_architectures(testcase, architectures=None):
diff --git a/src/maasserver/testing/factory.py b/src/maasserver/testing/factory.py
index e78b84b..508fb7f 100644
--- a/src/maasserver/testing/factory.py
+++ b/src/maasserver/testing/factory.py
@@ -2362,6 +2362,8 @@ class Factory(maastesting.factory.Factory):
         bootloader_type=None,
         rolling=False,
         base_image="",
+        platform="generic",
+        supported_platforms="generic",
     ):
         if rtype is None:
             if base_image:
@@ -2375,6 +2377,7 @@ class Factory(maastesting.factory.Factory):
                 os = self.make_name("os")
                 series = self.make_name("series")
                 name = f"{os}/{series}"
+        subarch = None
         if architecture is None:
             arch = self.make_name("arch")
             subarch = self.make_name("subarch")
@@ -2384,7 +2387,16 @@ class Factory(maastesting.factory.Factory):
                 self.make_name("key"): self.make_name("value")
                 for _ in range(3)
             }
-        return BootResource.objects.create(
+
+        if "platform" not in extra:
+            extra["platform"] = platform
+        if "supported_platforms" not in extra:
+            extra["supported_platforms"] = supported_platforms
+
+        if subarch:
+            extra["supported_platforms"] += ',' + subarch
+
+        result = BootResource.objects.create(
             rtype=rtype,
             name=name,
             architecture=architecture,
@@ -2394,6 +2406,7 @@ class Factory(maastesting.factory.Factory):
             rolling=rolling,
             base_image=base_image,
         )
+        return result
 
     def make_BootResourceSet(self, resource, version=None, label=None):
         if version is None:
diff --git a/src/maasserver/utils/osystems.py b/src/maasserver/utils/osystems.py
index 6ee8a0d..69db4bb 100644
--- a/src/maasserver/utils/osystems.py
+++ b/src/maasserver/utils/osystems.py
@@ -665,13 +665,19 @@ def get_working_kernel(
         )
 
     os_release = osystem + "/" + distro_series
+    kernel_str_valid = requested_kernel and validate_kernel_str(
+        requested_kernel
+    )
+    min_compat_lvl_valid = min_compatibility_level and validate_kernel_str(
+        min_compatibility_level
+    )
 
-    if requested_kernel and validate_kernel_str(requested_kernel):
+    if kernel_str_valid:
         # Specific kernel was requested -- check whether it will work
-        usable_kernels = BootResource.objects.get_kernels(
-            os_release, architecture=arch
+        available_kernels = get_available_kernels_prioritising_platform(
+            arch, os_release, platform
         )
-        if requested_kernel not in usable_kernels:
+        if requested_kernel not in available_kernels:
             raise ValidationError(
                 "%s is not available for %s on %s."
                 % (requested_kernel, os_release, architecture)
@@ -680,10 +686,7 @@ def get_working_kernel(
             raise ValidationError(
                 f"{requested_kernel} is too old to use on {os_release}."
             )
-        if (
-            min_compatibility_level
-            and validate_kernel_str(min_compatibility_level)
-        ) and (
+        if min_compat_lvl_valid and (
             not release_a_newer_than_b(
                 requested_kernel, min_compatibility_level
             )
@@ -693,9 +696,7 @@ def get_working_kernel(
                 % (requested_kernel, min_compatibility_level)
             )
         return requested_kernel
-    elif min_compatibility_level and validate_kernel_str(
-        min_compatibility_level
-    ):
+    elif min_compat_lvl_valid:
         # No specific kernel was requested, but there is a minimal
         # compatibility level restriction. Look for kernels that could
         # fit the description.
@@ -705,13 +706,12 @@ def get_working_kernel(
         valid_kflavors = {
             br.kflavor for br in BootResource.objects.exclude(kflavor=None)
         }
-        kflavor = "generic"
-        for kernel_part in min_compatibility_level.split("-"):
-            if kernel_part in valid_kflavors:
-                kflavor = kernel_part
-                break
-        usable_kernels = BootResource.objects.get_kernels(
-            os_release, architecture=arch, kflavor=kflavor
+        _, _, _, kflavor = parse_subarch_kernel_string(min_compatibility_level)
+        if not kflavor or kflavor not in valid_kflavors:
+            kflavor = "generic"
+
+        usable_kernels = get_available_kernels_prioritising_platform(
+            arch, os_release, platform, kflavor=kflavor
         )
         for i in usable_kernels:
             if release_a_newer_than_b(
@@ -722,14 +722,87 @@ def get_working_kernel(
             "%s has no kernels available which meet min_hwe_kernel(%s)."
             % (distro_series, min_compatibility_level)
         )
-    for kernel in BootResource.objects.get_kernels(
-        os_release, architecture=arch, kflavor="generic"
-    ):
+
+    # No specific kernel, no requirements. Pick the first kernel suitable
+    # for the distro.
+    available_kernels = get_available_kernels_prioritising_platform(
+        arch, os_release, platform
+    )
+    for kernel in available_kernels:
         if release_a_newer_than_b(kernel, distro_series):
             return kernel
     raise ValidationError("%s has no kernels available." % distro_series)
 
 
+def get_available_kernels_prioritising_platform(
+    arch, os_release, platform, kflavor=None
+):
+    """Wrapper around `get_kernels` that prioritises platform-exact
+    kernels over platform-generic and generic kernels
+
+    This way we may have both "platform-exact" and "platform-generic"
+    kernels (e.g. `linux-raspi` generic and `linux-raspi-zero` exact)
+    in a way that allows MAAS to choose the best platform-supporting
+    kernel that is available to it.
+    """
+
+    # We cannot always use the generic kernels, because some platforms
+    # won't boot with them. However, we still need to fetch them because
+    # we want them at the end of the kernel list.
+    generic_kernels = BootResource.objects.get_kernels(
+        os_release,
+        architecture=arch,
+        platform="generic",
+        kflavor=kflavor,
+    )
+
+    # Save DB queries for the vast majority of the machines
+    if platform == "generic":
+        return generic_kernels
+
+    # Kernels are sorted by the rules of `get_release_version_from_string`,
+    # meaning that platform-optimised kernels will end up being the last
+    # on the list. While in other contexts this is reasonable, here we
+    # want them to have priority over the generic ones. The idea is
+    # simple: we fetch the kernels that match the platform exactly,
+    # then we fetch the ones that *support* the platform,
+    # "platform-generic" ones. The latter might also contain some
+    # simply-generic kernels that we want to end up the last, so we
+    # filter them out by using the simply-generic kernel list we fetch
+    # earlier.
+    #
+    # TODO This part adds 6 extra queries and we might want to fix it
+    platform_exact_kernels = BootResource.objects.get_kernels(
+        os_release,
+        architecture=arch,
+        platform=platform,
+        kflavor=kflavor,
+        strict_platform_match=True,
+    )
+    platform_generic_kernels = BootResource.objects.get_kernels(
+        os_release,
+        architecture=arch,
+        platform=platform,
+        kflavor=kflavor,
+        strict_platform_match=False,
+    )
+
+    # Generic kernel filtering, see above
+    platform_kernels = list(platform_exact_kernels)
+    generic_supporting_kernels = []
+    for k in platform_generic_kernels:
+        if k in generic_kernels:
+            generic_supporting_kernels.append(k)
+        else:
+            platform_kernels.append(k)
+
+    # Make [*platform-exact, *platform-generic, *generic]
+    available_kernels = (
+        platform_kernels + generic_supporting_kernels
+    )
+    return available_kernels
+
+
 def validate_min_hwe_kernel(min_hwe_kernel):
     """Check that the min_hwe_kernel is avalible."""
     if not min_hwe_kernel or min_hwe_kernel == "":
diff --git a/src/maasserver/utils/tests/test_osystems.py b/src/maasserver/utils/tests/test_osystems.py
index f580ddb..ccf05c2 100644
--- a/src/maasserver/utils/tests/test_osystems.py
+++ b/src/maasserver/utils/tests/test_osystems.py
@@ -768,29 +768,29 @@ class TestReleaseANewerThanB(MAASServerTestCase):
 
 class TestGetWorkingKernel(MAASServerTestCase):
     def test_get_working_kernel_returns_default_kernel(self):
-        self.patch(BootResource.objects, "get_kernels").return_value = (
+        self.patch(BootResource.objects, "get_kernels").return_value = [
             "hwe-t",
             "hwe-u",
-        )
+        ]
         hwe_kernel = get_working_kernel(
             None, None, "amd64/generic", "ubuntu", "trusty"
         )
         self.assertEqual(hwe_kernel, "hwe-t")
 
     def test_get_working_kernel_set_kernel(self):
-        self.patch(BootResource.objects, "get_kernels").return_value = (
+        self.patch(BootResource.objects, "get_kernels").return_value = [
             "hwe-t",
             "hwe-v",
-        )
+        ]
         hwe_kernel = get_working_kernel(
             "hwe-v", None, "amd64/generic", "ubuntu", "trusty"
         )
         self.assertEqual(hwe_kernel, "hwe-v")
 
     def test_get_working_kernel_accepts_ga_kernel(self):
-        self.patch(BootResource.objects, "get_kernels").return_value = (
+        self.patch(BootResource.objects, "get_kernels").return_value = [
             "ga-16.04",
-        )
+        ]
         hwe_kernel = get_working_kernel(
             "ga-16.04", None, "amd64/generic", "ubuntu", "xenial"
         )
@@ -813,10 +813,10 @@ class TestGetWorkingKernel(MAASServerTestCase):
 
     def test_get_working_kernel_fails_with_missing_hwe_kernel(self):
         exception_raised = False
-        self.patch(BootResource.objects, "get_kernels").return_value = (
+        self.patch(BootResource.objects, "get_kernels").return_value = [
             "hwe-t",
             "hwe-u",
-        )
+        ]
         try:
             get_working_kernel(
                 "hwe-v", None, "amd64/generic", "ubuntu", "trusty"
@@ -831,10 +831,10 @@ class TestGetWorkingKernel(MAASServerTestCase):
 
     def test_get_working_kernel_fails_with_old_kernel_and_newer_release(self):
         exception_raised = False
-        self.patch(BootResource.objects, "get_kernels").return_value = (
+        self.patch(BootResource.objects, "get_kernels").return_value = [
             "hwe-t",
             "hwe-v",
-        )
+        ]
         try:
             get_working_kernel(
                 "hwe-t", None, "amd64/generic", "ubuntu", "vivid"
@@ -848,10 +848,10 @@ class TestGetWorkingKernel(MAASServerTestCase):
 
     def test_get_working_kernel_fails_with_old_kern_and_new_min_hwe_kern(self):
         exception_raised = False
-        self.patch(BootResource.objects, "get_kernels").return_value = (
+        self.patch(BootResource.objects, "get_kernels").return_value = [
             "hwe-t",
             "hwe-v",
-        )
+        ]
         try:
             get_working_kernel(
                 "hwe-t", "hwe-v", "amd64/generic", "ubuntu", "precise"
@@ -866,10 +866,10 @@ class TestGetWorkingKernel(MAASServerTestCase):
 
     def test_get_working_kernel_fails_with_no_avalible_kernels(self):
         exception_raised = False
-        self.patch(BootResource.objects, "get_kernels").return_value = (
+        self.patch(BootResource.objects, "get_kernels").return_value = [
             "hwe-t",
             "hwe-v",
-        )
+        ]
         try:
             get_working_kernel(
                 "hwe-t", "hwe-v", "amd64/generic", "ubuntu", "precise"
@@ -900,10 +900,10 @@ class TestGetWorkingKernel(MAASServerTestCase):
         self.assertTrue(exception_raised)
 
     def test_get_working_kernel_always_sets_kern_with_commissionable_os(self):
-        self.patch(BootResource.objects, "get_kernels").return_value = (
+        self.patch(BootResource.objects, "get_kernels").return_value = [
             "hwe-t",
             "hwe-v",
-        )
+        ]
         mock_get_config = self.patch(Config.objects, "get_config")
         mock_get_config.return_value = "trusty"
         kernel = get_working_kernel(
@@ -921,11 +921,15 @@ class TestGetWorkingKernel(MAASServerTestCase):
 
     def test_get_working_kernel_sets_hwe_kern_to_min_hwe_kern_for_edge(self):
         # Regression test for LP:1654412
-        mock_get_kernels = self.patch(BootResource.objects, "get_kernels")
-        mock_get_kernels.return_value = (
+        import maasserver.utils.osystems as osystems
+        mock_get_kernels = self.patch(
+            osystems,
+            "get_available_kernels_prioritising_platform"
+        )
+        mock_get_kernels.return_value = [
             "hwe-16.04",
             "hwe-16.04-edge",
-        )
+        ]
         arch = factory.make_name("arch")
 
         kernel = get_working_kernel(
@@ -933,11 +937,8 @@ class TestGetWorkingKernel(MAASServerTestCase):
         )
 
         self.assertEqual("hwe-16.04-edge", kernel)
-        self.assertThat(
-            mock_get_kernels,
-            MockCalledOnceWith(
-                "ubuntu/xenial", architecture=arch, kflavor="generic"
-            ),
+        mock_get_kernels.assert_called_with(
+            arch, "ubuntu/xenial", "generic", kflavor="generic"
         )
 
     def test_get_working_kernel_uses_base_image_for_lookup_with_custom_images(
-- 
Mailing list: https://launchpad.net/~sts-sponsors
Post to     : sts-sponsors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~sts-sponsors
More help   : https://help.launchpad.net/ListHelp

Reply via email to