Merge ~ltrager/maas:fix_architecture into maas:master

[Lee Trager]

Automatically fix an incorrect architecture.

When adding a new machine if a user selects the wrong architecture booting will fail. Every boot architecture except PXE only supports one Debian
architecture. Automatically swap the user selected architecture for the
one detected during boot. Commissioning will properly set the
architecture.

Commissioning may still fail due to the archive being set to
ports.ubuntu.com however 50-maas-01-commissioning will still run which
will fix the machine architecture. When a user retries commissioning will
work. This is preferable to failing to load the kernel from the bootloader
which can give a cryptic error message.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b fix_architecture lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: a529235ba36266901d37684cb615e0a2ec744bcf

review: Approve

[Alberto Donato (ack)]

LGTM

review: Approve

[MAAS Lander (maas-lander)]

LANDING
-b fix_architecture lp:~ltrager/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: FAILED BUILD
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/9846/consoleText

Unmerged commits

a529235... by Lee Trager on 2021-04-21

Automatically fix an incorrect architecture.

When adding a new machine if a user selects the wrong architecture booting will
fail. Every boot architecture except PXE only supports one Debian
architecture. Automatically swap the user selected architecture for the one
detected during boot. Commissioning will properly set the architecture.

Commissioning may still fail due to the archive being set to ports.ubuntu.com
however 50-maas-01-commissioning will still run which will fix the
machine architecture. When a user retries commissioning will work. This is
preferable to failing to load the kernel from the bootloader which can give
a cryptic error message.

a06daa6... by Lee Trager on 2021-04-15

Reenable AMD64 UEFI HTTP booting and enable ARM64 UEFI booting.

MAAS will now respond to AMD64 and ARM64 UEFI HTTP boot requests. When booting
over HTTP MAAS will now send a GRUB configuration which will continue to use
the UEFI firmware's internal HTTP stack instead of GRUB's. While various bugs
have been fixed in GRUB's HTTP stack this should avoid any future ones entirely.

diff --git a/src/provisioningserver/boot/__init__.py b/src/provisioningserver/boot/__init__.py
index 9a38f11..994901f 100644
--- a/src/provisioningserver/boot/__init__.py
+++ b/src/provisioningserver/boot/__init__.py
@@ -352,15 +352,27 @@ class BootMethod(metaclass=ABCMeta):
         """
         dtb_subarchs = ["xgene-uboot-mustang"]
 
+        def get_arch(params):
+            # Most boot loader arches are for a specific architecture. The
+            # exception is 00:00(PXE) which is for i386 and AMD64. Use the
+            # defined architecture incase the user selected the wrong one.
+            # Subarches are identical across architectures except for armhf
+            # which additonaly has generic-lpae.
+            if len(self.bootloader_arches) == 1:
+                return self.bootloader_arches[0]
+            else:
+                return params.arch
+
         def fs_host(params):
             return "http://%s:5248/images/" % (
                 convert_host_to_uri_str(params.fs_host)
             )
 
         def image_dir(params):
+
             return compose_image_path(
                 params.osystem,
-                params.arch,
+                get_arch(params),
                 params.subarch,
                 params.release,
                 params.label,
@@ -403,7 +415,7 @@ class BootMethod(metaclass=ABCMeta):
                 return None
 
         def kernel_command(params):
-            return compose_kernel_command_line(params)
+            return compose_kernel_command_line(params, get_arch(params))
 
         namespace = {
             "fs_host": fs_host,
diff --git a/src/provisioningserver/boot/grub.py b/src/provisioningserver/boot/grub.py
index 286e350..56436ac 100644
--- a/src/provisioningserver/boot/grub.py
+++ b/src/provisioningserver/boot/grub.py
@@ -119,7 +119,7 @@ class UEFIAMD64BootMethod(BootMethod):
             return re.sub(
                 r"cc:{(?P<inner>[^}]*)}end_cc",
                 r"cc:\{\g<inner>\}end_cc",
-                compose_kernel_command_line(params),
+                compose_kernel_command_line(params, self.bootloader_arches[0]),
             )
 
         template = self.get_template(
@@ -225,7 +225,7 @@ class OpenFirmwarePPC64ELBootMethod(UEFIAMD64BootMethod):
 
     name = "open-firmware_ppc64el"
     bios_boot_method = "open-firmware"
-    bootloader_arches = ["ppc64el", "ppc64"]
+    bootloader_arches = ["ppc64el"]
     bootloader_path = "bootppc64.bin"
     bootloader_files = ["bootppc64.bin"]
     arch_octet = "00:0C"
diff --git a/src/provisioningserver/boot/powernv.py b/src/provisioningserver/boot/powernv.py
index e1e870e..b81aaba 100644
--- a/src/provisioningserver/boot/powernv.py
+++ b/src/provisioningserver/boot/powernv.py
@@ -73,6 +73,7 @@ class PowerNVBootMethod(BootMethod):
     name = "powernv"
     bios_boot_method = "powernv"
     template_subdir = "pxe"
+    bootloader_arches = ["ppc64el"]
     bootloader_path = "pxelinux.0"
     arch_octet = "00:0E"
     user_class = None
@@ -138,7 +139,9 @@ class PowerNVBootMethod(BootMethod):
         # Modify the kernel_command to inject the BOOTIF. PowerNV fails to
         # support the IPAPPEND pxelinux flag.
         def kernel_command(params):
-            cmd_line = compose_kernel_command_line(params)
+            cmd_line = compose_kernel_command_line(
+                params, self.bootloader_arches[0]
+            )
             if mac is not None:
                 return "%s BOOTIF=%s" % (cmd_line, format_bootif(mac))
             return cmd_line
diff --git a/src/provisioningserver/boot/s390x.py b/src/provisioningserver/boot/s390x.py
index 310cf0a..947c287 100644
--- a/src/provisioningserver/boot/s390x.py
+++ b/src/provisioningserver/boot/s390x.py
@@ -72,6 +72,7 @@ class S390XBootMethod(BootMethod):
     name = "s390x"
     bios_boot_method = "s390x"
     template_subdir = "pxe"
+    bootloader_arches = ["s390x"]
     # boots390x.bin is a place holder to allow the path_prefix to be set.
     # s390x KVM uses a bootloader shipped with KVM.
     bootloader_path = "boots390x.bin"
@@ -139,7 +140,9 @@ class S390XBootMethod(BootMethod):
         # Modify the kernel_command to inject the BOOTIF. S390x fails to
         # support the IPAPPEND pxelinux flag.
         def kernel_command(params):
-            cmd_line = compose_kernel_command_line(params)
+            cmd_line = compose_kernel_command_line(
+                params, self.bootloader_arches[0]
+            )
             if mac is not None:
                 return "%s BOOTIF=%s" % (cmd_line, format_bootif(mac))
             return cmd_line
diff --git a/src/provisioningserver/boot/s390x_partition.py b/src/provisioningserver/boot/s390x_partition.py
index c45e348..cb43e0d 100644
--- a/src/provisioningserver/boot/s390x_partition.py
+++ b/src/provisioningserver/boot/s390x_partition.py
@@ -15,6 +15,7 @@ class S390XPartitionBootMethod(BootMethod):
     name = "s390x_partition"
     bios_boot_method = "s390x_partition"
     template_subdir = "s390x_partition"
+    bootloader_arches = ["s390x"]
     # S390X partitions has its bootloader built into the firmware. The
     # "bootloader" provided must be the bootloader configuration file. The
     # file format is similar to pxelinux.cfg but supports limited options
@@ -56,7 +57,9 @@ class S390XPartitionBootMethod(BootMethod):
         namespace = self.compose_template_namespace(kernel_params)
 
         def kernel_command(params):
-            cmd_line = compose_kernel_command_line(params)
+            cmd_line = compose_kernel_command_line(
+                params, self.bootloader_arches[0]
+            )
             # Modify the kernel_command to inject the BOOTIF. S390X doesn't
             # support the IPAPPEND pxelinux flag.
             if mac is not None:
diff --git a/src/provisioningserver/boot/tests/test_boot.py b/src/provisioningserver/boot/tests/test_boot.py
index eb61cab..5eae9b2 100644
--- a/src/provisioningserver/boot/tests/test_boot.py
+++ b/src/provisioningserver/boot/tests/test_boot.py
@@ -255,7 +255,37 @@ class TestBootMethod(MAASTestCase):
             template_namespace["initrd_path"](kernel_params),
         )
         self.assertEqual(
-            compose_kernel_command_line(kernel_params),
+            compose_kernel_command_line(kernel_params, kernel_params.arch),
+            template_namespace["kernel_command"](kernel_params),
+        )
+        self.assertEqual(kernel_params, template_namespace["kernel_params"])
+        self.assertEqual(
+            "%s/%s" % (image_dir, kernel_params.kernel),
+            template_namespace["kernel_path"](kernel_params),
+        )
+        self.assertIsNone(template_namespace["dtb_path"](kernel_params))
+
+    def test_compose_template_namespace_fixes_arch(self):
+        kernel_params = make_kernel_parameters()
+        method = FakeBootMethod()
+        arch = factory.make_name("arch")
+        method.bootloader_arches = [arch]
+        image_dir = compose_image_path(
+            kernel_params.osystem,
+            arch,
+            kernel_params.subarch,
+            kernel_params.release,
+            kernel_params.label,
+        )
+
+        template_namespace = method.compose_template_namespace(kernel_params)
+
+        self.assertEqual(
+            "%s/%s" % (image_dir, kernel_params.initrd),
+            template_namespace["initrd_path"](kernel_params),
+        )
+        self.assertEqual(
+            compose_kernel_command_line(kernel_params, arch),
             template_namespace["kernel_command"](kernel_params),
         )
         self.assertEqual(kernel_params, template_namespace["kernel_params"])
@@ -288,7 +318,7 @@ class TestBootMethod(MAASTestCase):
             template_namespace["initrd_path"](kernel_params),
         )
         self.assertEqual(
-            compose_kernel_command_line(kernel_params),
+            compose_kernel_command_line(kernel_params, kernel_params.arch),
             template_namespace["kernel_command"](kernel_params),
         )
         self.assertEqual(kernel_params, template_namespace["kernel_params"])
@@ -319,7 +349,7 @@ class TestBootMethod(MAASTestCase):
             template_namespace["initrd_path"](kernel_params),
         )
         self.assertEqual(
-            compose_kernel_command_line(kernel_params),
+            compose_kernel_command_line(kernel_params, kernel_params.arch),
             template_namespace["kernel_command"](kernel_params),
         )
         self.assertEqual(kernel_params, template_namespace["kernel_params"])
@@ -334,6 +364,8 @@ class TestBootMethod(MAASTestCase):
 
     def test_compose_template_namespace_include_debug(self):
         debug = factory.pick_bool()
+        boot.debug_enabled.cache_clear()
+        self.addCleanup(boot.debug_enabled.cache_clear)
         self.useFixture(ClusterConfigurationFixture(debug=debug))
         kernel_params = make_kernel_parameters()
         method = FakeBootMethod()
diff --git a/src/provisioningserver/kernel_opts.py b/src/provisioningserver/kernel_opts.py
index 47fd85c..f7082d7 100644
--- a/src/provisioningserver/kernel_opts.py
+++ b/src/provisioningserver/kernel_opts.py
@@ -1,4 +1,4 @@
-# Copyright 2012-2017 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Generate kernel command-line options for inclusion in PXE configs."""
@@ -77,8 +77,9 @@ def get_last_directory(root):
     return max(dirs)
 
 
-def compose_purpose_opts(params):
+def compose_purpose_opts(params, bootloader_arch=None):
     """Return the list of the purpose-specific kernel options."""
+    arch = bootloader_arch if bootloader_arch else params.arch
     kernel_params = [
         "ro",
         "root=squash:http://%s:5248/images/%s/%s/%s/%s/%s/squashfs"
@@ -89,7 +90,7 @@ def compose_purpose_opts(params):
                 else params.fs_host
             ),
             params.osystem,
-            params.arch,
+            arch,
             params.subarch,
             params.release,
             params.label,
@@ -137,7 +138,7 @@ def get_curtin_kernel_cmdline_sep():
     return getattr(curtin, CURTIN_KERNEL_CMDLINE_NAME, "--")
 
 
-def compose_kernel_command_line(params):
+def compose_kernel_command_line(params, bootloader_arch):
     """Generate a line of kernel options for booting `node`.
 
     :type params: `KernelParameters`.
@@ -145,7 +146,7 @@ def compose_kernel_command_line(params):
     options = []
     # nomodeset prevents video mode switching.
     options += ["nomodeset"]
-    options += compose_purpose_opts(params)
+    options += compose_purpose_opts(params, bootloader_arch)
     # 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 0985ed6..53450ed 100644
--- a/src/provisioningserver/tests/test_kernel_opts.py
+++ b/src/provisioningserver/tests/test_kernel_opts.py
@@ -1,4 +1,4 @@
-# Copyright 2012-2016 Canonical Ltd.  This software is licensed under the
+# Copyright 2012-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test composition of kernel command lines."""
@@ -121,7 +121,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose=random.choice(["commissioning", "xinstall", "enlist"])
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(cmdline, Contains("overlayroot_cfgdisk=disabled"))
 
     def test_xinstall_compose_kernel_command_line_inc_purpose_opts4(self):
@@ -130,7 +130,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="xinstall", fs_host=factory.make_ipv4_address()
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(
             cmdline,
             ContainsAll(
@@ -149,7 +149,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="xinstall", fs_host=factory.make_ipv6_address()
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(
             cmdline,
             ContainsAll(
@@ -168,7 +168,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="xinstall", fs_host=factory.make_ipv4_address()
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(
             cmdline,
             ContainsAll(
@@ -185,7 +185,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="ephemeral", fs_host=factory.make_ipv4_address()
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(
             cmdline,
             ContainsAll(
@@ -202,7 +202,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="commissioning", fs_host=factory.make_ipv4_address()
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(
             cmdline,
             ContainsAll(
@@ -221,7 +221,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="commissioning", fs_host=factory.make_ipv6_address()
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(
             cmdline,
             ContainsAll(
@@ -240,7 +240,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="commissioning", fs_host=factory.make_ipv4_address()
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(
             cmdline,
             ContainsAll(
@@ -257,7 +257,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="enlist", fs_host=factory.make_ipv4_address()
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(
             cmdline,
             ContainsAll(
@@ -276,7 +276,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="enlist", fs_host=factory.make_ipv6_address()
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(
             cmdline,
             ContainsAll(
@@ -295,7 +295,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="enlist", fs_host=factory.make_ipv4_address()
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(
             cmdline,
             ContainsAll(
@@ -312,7 +312,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="enlist", fs_host=factory.make_ipv4_address()
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(cmdline, ContainsAll(["apparmor=0"]))
 
     def test_commissioning_compose_kernel_command_line_apparmor_disabled(self):
@@ -321,7 +321,7 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="commissioning", fs_host=factory.make_ipv4_address()
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(cmdline, ContainsAll(["apparmor=0"]))
 
     def test_commissioning_compose_kernel_command_line_inc_extra_opts(self):
@@ -332,14 +332,14 @@ class TestKernelOpts(MAASTestCase):
         mock_get_curtin_sep.return_value = sep
         extra_opts = "special console=ABCD -- options to pass"
         params = self.make_kernel_parameters(extra_opts=extra_opts)
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         # There should be KERNEL_CMDLINE_COPY_TO_INSTALL_SEP surrounded by
         # spaces before the options, but otherwise added verbatim.
         self.assertThat(cmdline, Contains(" %s " % sep + extra_opts))
 
     def test_commissioning_compose_kernel_handles_extra_opts_None(self):
         params = self.make_kernel_parameters(extra_opts=None)
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertNotIn(cmdline, "None")
 
     def test_compose_kernel_command_line_inc_common_opts(self):
@@ -350,15 +350,15 @@ class TestKernelOpts(MAASTestCase):
         params = self.make_kernel_parameters(
             purpose="commissioning", arch="i386"
         )
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(cmdline, ContainsAll(expected))
 
         params = self.make_kernel_parameters(purpose="xinstall", arch="i386")
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(cmdline, ContainsAll(expected))
 
         params = self.make_kernel_parameters(purpose="install", arch="i386")
-        cmdline = compose_kernel_command_line(params)
+        cmdline = compose_kernel_command_line(params, params.arch)
         self.assertThat(cmdline, ContainsAll(expected))
 
     def test_compose_kernel_command_line_inc_purpose_opts_xinstall_node(self):
@@ -366,7 +366,7 @@ class TestKernelOpts(MAASTestCase):
         # options for a "xinstall" node.
         params = self.make_kernel_parameters(purpose="xinstall")
         self.assertThat(
-            compose_kernel_command_line(params),
+            compose_kernel_command_line(params, params.arch),
             ContainsAll(["root=squash:http://"]),
         )
 
@@ -375,20 +375,21 @@ class TestKernelOpts(MAASTestCase):
         # options for a "commissioning" node.
         params = self.make_kernel_parameters(purpose="commissioning")
         self.assertThat(
-            compose_kernel_command_line(params),
+            compose_kernel_command_line(params, params.arch),
             ContainsAll(["root=squash:http://"]),
         )
 
     def test_compose_kernel_command_line_inc_arm_specific_option(self):
         params = self.make_kernel_parameters(arch="armhf", subarch="highbank")
         self.assertThat(
-            compose_kernel_command_line(params), Contains("console=ttyAMA0")
+            compose_kernel_command_line(params, params.arch),
+            Contains("console=ttyAMA0"),
         )
 
     def test_compose_kernel_command_line_not_inc_arm_specific_option(self):
         params = self.make_kernel_parameters(arch="i386")
         self.assertThat(
-            compose_kernel_command_line(params),
+            compose_kernel_command_line(params, params.arch),
             Not(Contains("console=ttyAMA0")),
         )
 
@@ -405,7 +406,7 @@ class TestKernelOpts(MAASTestCase):
     def test_compose_rootfs_over_http_ipv4(self):
         params = make_kernel_parameters(fs_host=factory.make_ipv4_address())
         self.assertThat(
-            compose_kernel_command_line(params),
+            compose_kernel_command_line(params, params.arch),
             ContainsAll(
                 [
                     "ro",
@@ -425,7 +426,7 @@ class TestKernelOpts(MAASTestCase):
     def test_compose_rootfs_over_http_ipv6(self):
         params = make_kernel_parameters(fs_host=factory.make_ipv6_address())
         self.assertThat(
-            compose_kernel_command_line(params),
+            compose_kernel_command_line(params, params.arch),
             ContainsAll(
                 [
                     "ro",