sts-sponsors team mailing list archive

[Alexsander de Souza <mp+443301@xxxxxxxxxxxxxxxxxx>]

Alexsander de Souza has proposed merging ~alexsander-souza/maas:lp2016908_enable_apparmor into maas:master.

Commit message:
enable apparmor for Jammy and newer releases

the kernel bug that motivated MAAS to disable apparmor was fixed a while ago, 
so we don't need to do this for newer releases

fixes LP#2016908

Requested reviews:
  MAAS Lander (maas-lander): unittests
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #2016908 in MAAS: "udev fails to make prctl() syscall with apparmor=0 (as used by maas by default)"
  https://bugs.launchpad.net/maas/+bug/2016908

For more details, see:
https://code.launchpad.net/~alexsander-souza/maas/+git/maas/+merge/443301
-- 
Your team MAAS Committers is subscribed to branch maas:master.

diff --git a/src/provisioningserver/kernel_opts.py b/src/provisioningserver/kernel_opts.py
index 7ecbcf7..47335b8 100644
--- a/src/provisioningserver/kernel_opts.py
+++ b/src/provisioningserver/kernel_opts.py
@@ -8,6 +8,7 @@ from collections import namedtuple
 import os
 
 import curtin
+from distro_info import UbuntuDistroInfo
 from netaddr import IPAddress
 
 from provisioningserver.drivers import ArchitectureRegistry
@@ -111,13 +112,23 @@ def compose_purpose_opts(params):
         "cc:{'datasource_list': ['MAAS']}end_cc",
         # Read by cloud-init.
         "cloud-config-url=%s" % params.preseed_url,
-        # Disable apparmor in the ephemeral environment. This addresses
-        # MAAS bug LP: #1677336 due to LP: #1408106
-        "apparmor=0",
     ]
     return kernel_params
 
 
+def compose_apparmor_opts(params):
+    JAMMY_VER = 22.04
+    if params.osystem == "ubuntu":
+        di = UbuntuDistroInfo()
+        if di.valid(params.release) and (
+            float(di.version(params.release).removesuffix(" LTS")) < JAMMY_VER
+        ):
+            # Disable apparmor in the ephemeral environment. This addresses
+            # MAAS bug LP: #1677336 due to LP: #1408106
+            return ["apparmor=0"]
+    return []
+
+
 def compose_arch_opts(params):
     """Return any architecture-specific options required"""
     arch_subarch = f"{params.arch}/{params.subarch}"
@@ -141,10 +152,12 @@ def compose_kernel_command_line(params):
 
     :type params: `KernelParameters`.
     """
+    maaslog.info(f"compose_kernel_command_line {params}")
     options = []
     # nomodeset prevents video mode switching.
     options += ["nomodeset"]
     options += compose_purpose_opts(params)
+    options += compose_apparmor_opts(params)
     # Note: logging opts are not respected by ephemeral images, so
     #       these are actually "purpose_opts" but were left generic
     #       as it would be nice to have.
diff --git a/src/provisioningserver/tests/test_kernel_opts.py b/src/provisioningserver/tests/test_kernel_opts.py
index 1892a4f..396aa4e 100644
--- a/src/provisioningserver/tests/test_kernel_opts.py
+++ b/src/provisioningserver/tests/test_kernel_opts.py
@@ -310,19 +310,47 @@ class TestKernelOpts(MAASTestCase):
         # The result of compose_kernel_command_line includes the
         # options for apparmor. See LP: #1677336 and LP: #1408106
         params = self.make_kernel_parameters(
-            purpose="enlist", fs_host=factory.make_ipv4_address()
+            osystem="ubuntu",
+            release="focal",
+            purpose="enlist",
+            fs_host=factory.make_ipv4_address(),
         )
         cmdline = compose_kernel_command_line(params)
-        self.assertThat(cmdline, ContainsAll(["apparmor=0"]))
+        self.assertIn("apparmor=0", cmdline)
 
     def test_commissioning_compose_kernel_command_line_apparmor_disabled(self):
         # The result of compose_kernel_command_line includes the
         # options for apparmor. See LP: #1677336 and LP: #1408106
         params = self.make_kernel_parameters(
-            purpose="commissioning", fs_host=factory.make_ipv4_address()
+            osystem="ubuntu",
+            release="focal",
+            purpose="commissioning",
+            fs_host=factory.make_ipv4_address(),
+        )
+        cmdline = compose_kernel_command_line(params)
+        self.assertIn("apparmor=0", cmdline)
+
+    def test_enlist_compose_kernel_command_line_apparmor_default(self):
+        # For Jammy onwards, we should use the kernel default for apparmor
+        params = self.make_kernel_parameters(
+            osystem="ubuntu",
+            release="jammy",
+            purpose="enlist",
+            fs_host=factory.make_ipv4_address(),
+        )
+        cmdline = compose_kernel_command_line(params)
+        self.assertNotIn("apparmor=0", cmdline)
+
+    def test_commissioning_compose_kernel_command_line_apparmor_default(self):
+        # For Jammy onwards, we should use the kernel default for apparmor
+        params = self.make_kernel_parameters(
+            osystem="ubuntu",
+            release="jammy",
+            purpose="commissioning",
+            fs_host=factory.make_ipv4_address(),
         )
         cmdline = compose_kernel_command_line(params)
-        self.assertThat(cmdline, ContainsAll(["apparmor=0"]))
+        self.assertNotIn("apparmor=0", cmdline)
 
     def test_commissioning_compose_kernel_command_line_inc_extra_opts(self):
         mock_get_curtin_sep = self.patch(