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