Merge ~alexsander-souza/maas:lp1914812_add_centos_quirks into maas:master

[Alexsander de Souza]

add support to quirks in the ephemeral kernel during xinstall

Some older OSes (like Centos/7) require that we tune the ephemeral OS kernel to match the capabilities of the one being deployed.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b lp1914812_add_centos_quirks lp:~alexsander-souza/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 2e07343225f6028d20d53628d94d4475280c190c

review: Approve

[Christian Grabowski (cgrabowski)]

+1

review: Approve

diff --git a/src/maasserver/rpc/boot.py b/src/maasserver/rpc/boot.py
index c850aa7..931a7e0 100644
--- a/src/maasserver/rpc/boot.py
+++ b/src/maasserver/rpc/boot.py
@@ -223,11 +223,13 @@ def get_boot_config_for_machine(machine, configs, purpose):
             # LP:2013529, machine HWE kernel might not exist for
             # commissioning osystem/series
             use_machine_hwe_kernel = False
+        final_osystem, final_series = osystem, series
     else:
         osystem = machine.get_osystem(default=configs["default_osystem"])
         series = machine.get_distro_series(
             default=configs["default_distro_series"]
         )
+        final_osystem, final_series = osystem, series
         # XXX: roaksoax LP: #1739761 - Since the switch to squashfs (and
         # drop of iscsi), precise is no longer deployable. To address a
         # squashfs image is made available allowing it to be deployed in
@@ -317,7 +319,7 @@ def get_boot_config_for_machine(machine, configs, purpose):
         else:
             subarch = "no-such-kernel"
 
-    return osystem, series, subarch
+    return osystem, series, subarch, final_osystem, final_series
 
 
 def get_base_url_for_local_ip(local_ip, internal_domain):
@@ -352,6 +354,15 @@ def get_final_boot_purpose(machine, arch, purpose):
         return purpose
 
 
+def get_quirks_kernel_opts(
+    final_osystem: str, final_series: str
+) -> str | None:
+    if final_osystem == "centos":
+        return "nvme-core.multipath=0"
+
+    return None
+
+
 @synchronous
 @transactional
 def get_config(
@@ -418,6 +429,7 @@ def get_config(
         if log_port is None:
             log_port = 514  # Fallback to default UDP syslog port.
 
+    ephemeral_opts: str | None = None
     # XXX: Instead of updating the machine directly, we should store the
     # information and update the machine later. The current code doesn't
     # work when you first boot a machine that has IPMI configured, since
@@ -542,6 +554,7 @@ def get_config(
                 "log_port": log_port,
                 "extra_opts": "",
                 "http_boot": True,
+                "ephemeral_opts": ephemeral_opts or "",
             }
 
         # Log the request into the event log for that machine.
@@ -554,9 +567,13 @@ def get_config(
         else:
             event_log_pxe_request(machine, purpose)
 
-        osystem, series, subarch = get_boot_config_for_machine(
-            machine, configs, purpose
-        )
+        (
+            osystem,
+            series,
+            subarch,
+            final_osystem,
+            final_series,
+        ) = get_boot_config_for_machine(machine, configs, purpose)
 
         extra_kernel_opts = machine.get_effective_kernel_options(
             default_kernel_opts=configs["kernel_opts"]
@@ -580,6 +597,10 @@ def get_config(
         extra_kernel_opts = merge_kparams_with_extra(
             kparams, extra_kernel_opts
         )
+        if final_osystem != "ubuntu":
+            ephemeral_opts = get_quirks_kernel_opts(
+                final_osystem, final_series
+            )
     else:
         purpose = "commissioning"  # enlistment
         if configs["use_rack_proxy"]:
@@ -665,6 +686,7 @@ def get_config(
         # As of MAAS 2.4 only HTTP boot is supported. This ensures MAAS 2.3
         # rack controllers use HTTP boot as well.
         "http_boot": True,
+        "ephemeral_opts": ephemeral_opts or "",
     }
     if machine is not None:
         params["system_id"] = machine.system_id
diff --git a/src/maasserver/rpc/tests/test_boot.py b/src/maasserver/rpc/tests/test_boot.py
index 9e1f972..6928a67 100644
--- a/src/maasserver/rpc/tests/test_boot.py
+++ b/src/maasserver/rpc/tests/test_boot.py
@@ -235,6 +235,7 @@ class TestGetConfig(MAASServerTestCase):
                 "log_port": 5247,
                 "extra_opts": "",
                 "http_boot": True,
+                "ephemeral_opts": "",
             },
             config,
         )
@@ -280,6 +281,7 @@ class TestGetConfig(MAASServerTestCase):
                 "log_port": syslog_port,
                 "extra_opts": "",
                 "http_boot": True,
+                "ephemeral_opts": "",
             },
             config,
         )
@@ -324,6 +326,7 @@ class TestGetConfig(MAASServerTestCase):
                 "log_port": 5247,
                 "extra_opts": "",
                 "http_boot": True,
+                "ephemeral_opts": "",
             },
             config,
         )
@@ -1356,6 +1359,7 @@ class TestGetConfig(MAASServerTestCase):
             rack_controller.system_id, local_ip, remote_ip, mac=mac
         )
         self.assertEqual(distro_series, observed_config["release"])
+        self.assertEqual("", observed_config["ephemeral_opts"])
 
     def test_returns_base_image_for_custom_ubuntu_image_xinstall(self):
         self.patch(boot_module, "get_boot_filenames").return_value = (
@@ -1390,6 +1394,32 @@ class TestGetConfig(MAASServerTestCase):
         expected_osystem, expected_series = custom_image.base_image.split("/")
         self.assertEqual(expected_osystem, observed_config["osystem"])
         self.assertEqual(expected_series, observed_config["release"])
+        self.assertEqual("", observed_config["ephemeral_opts"])
+
+    def test_returns_quirks_for_centos_image_xinstall(self):
+        osystem = Config.objects.get_config("default_osystem")
+        release = Config.objects.get_config("default_distro_series")
+        rack_controller = factory.make_RackController()
+        local_ip = factory.make_ip_address()
+        remote_ip = factory.make_ip_address()
+        self.make_node(arch_name="amd64")
+        node = factory.make_Node_with_Interface_on_Subnet(
+            status=NODE_STATUS.DEPLOYING,
+            osystem="centos",
+            distro_series="centos71",
+            architecture="amd64/generic",
+            primary_rack=rack_controller,
+        )
+        mac = node.get_boot_interface().mac_address
+        self.patch_autospec(boot_module, "event_log_pxe_request")
+        observed_config = get_config(
+            rack_controller.system_id, local_ip, remote_ip, mac=mac
+        )
+        self.assertEqual(osystem, observed_config["osystem"])
+        self.assertEqual(release, observed_config["release"])
+        self.assertEqual(
+            "nvme-core.multipath=0", observed_config["ephemeral_opts"]
+        )
 
     # XXX: roaksoax LP: #1739761 - Deploying precise is now done using
     # the commissioning ephemeral environment.
@@ -1593,7 +1623,7 @@ class TestGetBootConfigForMachine(MAASServerTestCase):
             ]
         )
 
-        osystem, series, config_arch = get_boot_config_for_machine(
+        osystem, series, config_arch, _, _ = get_boot_config_for_machine(
             machine, configs, "xinstall"
         )
 
@@ -1634,7 +1664,7 @@ class TestGetBootConfigForMachine(MAASServerTestCase):
             ]
         )
 
-        osystem, series, config_arch = get_boot_config_for_machine(
+        osystem, series, config_arch, _, _ = get_boot_config_for_machine(
             machine, configs, "xinstall"
         )
 
@@ -1672,7 +1702,7 @@ class TestGetBootConfigForMachine(MAASServerTestCase):
             ]
         )
 
-        osystem, series, config_arch = get_boot_config_for_machine(
+        osystem, series, config_arch, _, _ = get_boot_config_for_machine(
             machine, configs, "xinstall"
         )
 
@@ -1737,7 +1767,7 @@ class TestGetBootConfigForMachine(MAASServerTestCase):
             architecture=f"amd64/{legacy_subarch}",
         )
 
-        osystem, series, config_arch = get_boot_config_for_machine(
+        osystem, series, config_arch, _, _ = get_boot_config_for_machine(
             modern_machine, configs, "xinstall"
         )
         self.assertEqual("ubuntu", osystem)
@@ -1745,7 +1775,7 @@ class TestGetBootConfigForMachine(MAASServerTestCase):
         self.assertEqual(subarch, config_arch)
         self.assertNotEqual(machine_hwe_kernel, config_arch)
 
-        osystem, series, config_arch = get_boot_config_for_machine(
+        osystem, series, config_arch, _, _ = get_boot_config_for_machine(
             legacy_machine, configs, "xinstall"
         )
         self.assertEqual("ubuntu", osystem)
@@ -1796,7 +1826,7 @@ class TestGetBootConfigForMachine(MAASServerTestCase):
             architecture=f"amd64/{legacy_subarch}",
         )
 
-        osystem, series, config_arch = get_boot_config_for_machine(
+        osystem, series, config_arch, _, _ = get_boot_config_for_machine(
             machine, configs, "commissioning"
         )
         self.assertEqual(configs["commissioning_osystem"], osystem)
@@ -1804,6 +1834,51 @@ class TestGetBootConfigForMachine(MAASServerTestCase):
         self.assertEqual(legacy_subarch, config_arch)
         self.assertNotEqual(machine_hwe_kernel, config_arch)
 
+    def test_get_boot_config_for_machine_centos(self):
+        subarch = "ga-20.04"
+        # We need a base image to be present. In fact, we always
+        # needed that for the new image to boot.
+        factory.make_default_ubuntu_release_bootable(arch=f"amd64/{subarch}")
+        factory.make_usable_boot_resource(
+            name="centos/7",
+            architecture="amd64/generic",
+        )
+        machine = factory.make_Machine(
+            architecture="amd64/generic",
+            status=NODE_STATUS.DEPLOYING,
+            osystem="centos",
+            distro_series="7",
+        )
+        configs = Config.objects.get_configs(
+            [
+                "commissioning_osystem",
+                "commissioning_distro_series",
+                "enable_third_party_drivers",
+                "default_min_hwe_kernel",
+                "default_osystem",
+                "default_distro_series",
+                "kernel_opts",
+                "use_rack_proxy",
+                "maas_internal_domain",
+                "remote_syslog",
+                "maas_syslog_port",
+            ]
+        )
+
+        (
+            osystem,
+            series,
+            config_arch,
+            final_osystem,
+            final_series,
+        ) = get_boot_config_for_machine(machine, configs, "xinstall")
+
+        self.assertEqual(configs["commissioning_osystem"], osystem)
+        self.assertEqual(configs["commissioning_distro_series"], series)
+        self.assertEqual(subarch, config_arch)
+        self.assertEqual("centos", final_osystem)
+        self.assertEqual("7", final_series)
+
 
 class TestGetNodeFromMacOrHardwareUUID(MAASServerTestCase):
     def test_get_node_from_mac_or_hardware_uuid_with_regular_mac(self):
diff --git a/src/maasserver/rpc/tests/test_regionservice_calls.py b/src/maasserver/rpc/tests/test_regionservice_calls.py
index dbc756a..5a22f1b 100644
--- a/src/maasserver/rpc/tests/test_regionservice_calls.py
+++ b/src/maasserver/rpc/tests/test_regionservice_calls.py
@@ -371,6 +371,7 @@ class TestRegionProtocol_GetBootConfig(MAASTransactionServerTestCase):
                     "fs_host",
                     "log_host",
                     "extra_opts",
+                    "ephemeral_opts",
                 ]
             ),
         )
diff --git a/src/provisioningserver/kernel_opts.py b/src/provisioningserver/kernel_opts.py
index b0da4a0..56f3add 100644
--- a/src/provisioningserver/kernel_opts.py
+++ b/src/provisioningserver/kernel_opts.py
@@ -44,6 +44,7 @@ KernelParametersBase = namedtuple(
         # verbatim to the kernel command line
         "http_boot",  # Used to make sure a MAAS 2.3 rack controller uses
         # http_boot.
+        "ephemeral_opts",  # Same as 'extra_opts' but used only in the ephemeral OS
     ),
 )
 
@@ -162,6 +163,8 @@ def compose_kernel_command_line(params):
     #       as it would be nice to have.
     options += compose_logging_opts(params)
     options += compose_arch_opts(params)
+    if params.ephemeral_opts:
+        options.append(params.ephemeral_opts)
     cmdline_sep = get_curtin_kernel_cmdline_sep()
     if params.extra_opts:
         # Using --- before extra opts makes both d-i and Curtin install
diff --git a/src/provisioningserver/rpc/region.py b/src/provisioningserver/rpc/region.py
index 6c8ba93..779949b 100644
--- a/src/provisioningserver/rpc/region.py
+++ b/src/provisioningserver/rpc/region.py
@@ -154,6 +154,7 @@ class GetBootConfig(amp.Command):
         # will try to use TGT as Twisted sets optional parameters to False when
         # not defined.
         (b"http_boot", amp.Boolean(optional=True)),
+        (b"ephemeral_opts", amp.Unicode(optional=True)),
     ]
     errors = {BootConfigNoResponse: b"BootConfigNoResponse"}
 
diff --git a/src/provisioningserver/tests/test_kernel_opts.py b/src/provisioningserver/tests/test_kernel_opts.py
index 396aa4e..7e62449 100644
--- a/src/provisioningserver/tests/test_kernel_opts.py
+++ b/src/provisioningserver/tests/test_kernel_opts.py
@@ -467,3 +467,20 @@ class TestKernelOpts(MAASTestCase):
                 ]
             ),
         )
+
+    def test_xinstall_compose_kernel_command_line_ephemeral_opts(self):
+        # ephemeral opts MUST appear before the separator
+        mock_get_curtin_sep = self.patch(
+            kernel_opts, "get_curtin_kernel_cmdline_sep"
+        )
+        sep = factory.make_name("sep")
+        mock_get_curtin_sep.return_value = sep
+
+        ephem_opt = "EPHEMERAL_OPT=1"
+        params = self.make_kernel_parameters(
+            purpose="xinstall",
+            ephemeral_opts=ephem_opt,
+        )
+        cmdline = compose_kernel_command_line(params)
+        self.assertIn(ephem_opt, cmdline)
+        self.assertTrue(cmdline.find(sep) > cmdline.find(ephem_opt))