sts-sponsors team mailing list archive

[Alberto Donato <mp+436320@xxxxxxxxxxxxxxxxxx>]

Alberto Donato has proposed merging ~ack/maas:bootif-from-cmdline into maas:master.

Commit message:
set boot interface for machines when generating sampledata

This also fixes getting the boot interface from kernel commandline during commissioning



Requested reviews:
  MAAS Maintainers (maas-maintainers)

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

diff --git a/pyproject.toml b/pyproject.toml
index c2d17f5..1748cdd 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -51,6 +51,7 @@ testpaths = [
   "src/maasserver/management/commands/pytest_tests",
   "src/maasserver/models/pytest_tests",
   "src/maasserver/pytest_tests",
+  "src/metadataserver/builtin_scripts/pytest_tests",
   "src/provisioningserver/utils/pytest_tests",
   # [[[end]]]
 ]
diff --git a/src/maasserver/testing/commissioning.py b/src/maasserver/testing/commissioning.py
index a209d84..4657009 100644
--- a/src/maasserver/testing/commissioning.py
+++ b/src/maasserver/testing/commissioning.py
@@ -197,6 +197,7 @@ class FakeCommissioningData:
         self._usb_devices = []
         self.networks = {}
         self._network_cards = []
+        self._boot_interface_mac = None
         if disks is None:
             disks = [LXDDisk("sda")]
         self._disks = list(disks)
@@ -339,6 +340,8 @@ class FakeCommissioningData:
                 network.name, len(card.ports), address=network.hwaddr
             )
         card.ports.append(port)
+        if self._boot_interface_mac is None:
+            self._boot_interface_mac = network.hwaddr
         return network
 
     def create_physical_network_without_nic(
@@ -525,6 +528,13 @@ class FakeCommissioningData:
             data["storage-extra"] = deepcopy(self.storage_extra)
         return data
 
+    def render_kernel_cmdline(self):
+        """Return the kernel command line, including the BOOTIF."""
+        cmdline = "console=tty1 console=ttyS0"
+        if self._boot_interface_mac:
+            cmdline += f"{cmdline} BOOTIF=01-{self._boot_interface_mac}"
+        return cmdline
+
     def _generate_interfaces(self):
         # XXX: It would be good if this method could basically call
         # get_all_interfaces_definition(), passing in information it
diff --git a/src/maasserver/testing/sampledata/machine.py b/src/maasserver/testing/sampledata/machine.py
index 393437f..7c69ca4 100644
--- a/src/maasserver/testing/sampledata/machine.py
+++ b/src/maasserver/testing/sampledata/machine.py
@@ -8,7 +8,10 @@ from django.contrib.auth.models import User
 from maasserver.enum import NODE_STATUS
 from maasserver.models import BMC, Machine, Pod, Tag
 from maasserver.testing.commissioning import FakeCommissioningData
-from metadataserver.builtin_scripts.hooks import process_lxd_results
+from metadataserver.builtin_scripts.hooks import (
+    process_lxd_results,
+    update_boot_interface,
+)
 
 from . import LOGGER
 from .common import range_one
@@ -87,6 +90,9 @@ def make_machines(
         machine.tags.add(*random.choices(tags, k=10))
         lxd_info = json.dumps(machine_info.render()).encode()
         process_lxd_results(machine, lxd_info, 0)
+        update_boot_interface(
+            machine, machine_info.render_kernel_cmdline().encode(), 0
+        )
         make_scripts(machine, lxd_info)
         machines.append(machine)
         if n % 10 == 0:
diff --git a/src/maastesting/pytest.dirs b/src/maastesting/pytest.dirs
index 8093455..2c155af 100644
--- a/src/maastesting/pytest.dirs
+++ b/src/maastesting/pytest.dirs
@@ -8,5 +8,6 @@ src/maasserver/djangosettings/pytest_tests
 src/maasserver/management/commands/pytest_tests
 src/maasserver/models/pytest_tests
 src/maasserver/pytest_tests
+src/metadataserver/builtin_scripts/pytest_tests
 src/provisioningserver/utils/pytest_tests
 # [[[end]]]
diff --git a/src/metadataserver/builtin_scripts/hooks.py b/src/metadataserver/builtin_scripts/hooks.py
index 8d1dbc9..b269747 100644
--- a/src/metadataserver/builtin_scripts/hooks.py
+++ b/src/metadataserver/builtin_scripts/hooks.py
@@ -175,14 +175,12 @@ def update_interface_details(interface, details):
         interface.save(update_fields=["updated", *update_fields])
 
 
-BOOTIF_RE = re.compile(r"BOOTIF=\d\d-([0-9a-f]{2}(?:-[0-9a-f]{2}){5})")
+BOOTIF_RE = re.compile(r"BOOTIF=\d\d-([0-9a-f]{2}([:-][0-9a-f]{2}){5})")
 
 
 def parse_bootif_cmdline(cmdline):
     match = BOOTIF_RE.search(cmdline)
-    if match:
-        return match.group(1).replace("-", ":").lower()
-    return None
+    return match[1].replace("-", ":").lower() if match else None
 
 
 def update_boot_interface(node, output, exit_status):
diff --git a/src/metadataserver/builtin_scripts/pytest_tests/__init__.py b/src/metadataserver/builtin_scripts/pytest_tests/__init__.py
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/src/metadataserver/builtin_scripts/pytest_tests/__init__.py
diff --git a/src/metadataserver/builtin_scripts/pytest_tests/test_hooks.py b/src/metadataserver/builtin_scripts/pytest_tests/test_hooks.py
new file mode 100644
index 0000000..3055fb2
--- /dev/null
+++ b/src/metadataserver/builtin_scripts/pytest_tests/test_hooks.py
@@ -0,0 +1,17 @@
+import pytest
+
+from metadataserver.builtin_scripts.hooks import parse_bootif_cmdline
+
+
+class TestParseBootifCmdline:
+    @pytest.mark.parametrize(
+        "cmdline,expected",
+        [
+            ("BOOTIF=01-aa:bb:cc:dd:ee:ff", "aa:bb:cc:dd:ee:ff"),
+            ("BOOTIF=01-aa-bb-cc-dd-ee-ff", "aa:bb:cc:dd:ee:ff"),
+            ("BOOTIF=garbage", None),
+            ("BOOTIF=01-aa-bb-cc-dd-ee", None),
+        ],
+    )
+    def test_parse(self, cmdline, expected):
+        assert parse_bootif_cmdline(cmdline) == expected
diff --git a/src/metadataserver/builtin_scripts/tests/test_hooks.py b/src/metadataserver/builtin_scripts/tests/test_hooks.py
index 595ffad..8ee17cc 100644
--- a/src/metadataserver/builtin_scripts/tests/test_hooks.py
+++ b/src/metadataserver/builtin_scripts/tests/test_hooks.py
@@ -59,7 +59,6 @@ from metadataserver.builtin_scripts.hooks import (
     filter_modaliases,
     get_dmi_data,
     NODE_INFO_SCRIPTS,
-    parse_bootif_cmdline,
     parse_interfaces,
     process_lxd_results,
     retag_node_for_hardware_by_modalias,
@@ -4769,28 +4768,6 @@ class TestUpdateBootInterface(MAASServerTestCase):
         self.assertIn("kernel-cmdline failed with status: 1", logger.output)
 
 
-class TestParseBootifCmdline(MAASTestCase):
-    def test_normal(self):
-        mac_address = "01:02:03:04:05:06"
-        self.assertEqual(
-            mac_address,
-            parse_bootif_cmdline(
-                KERNEL_CMDLINE_OUTPUT.format(
-                    mac_address=mac_address.replace(":", "-")
-                )
-            ),
-        )
-
-    def test_garbage(self):
-        self.assertIsNone(parse_bootif_cmdline("garbage"))
-
-    def test_garbage_bootif(self):
-        self.assertIsNone(parse_bootif_cmdline("BOOTIF=garbage"))
-
-    def test_bootif_too_few(self):
-        self.assertIsNone(parse_bootif_cmdline("BOOTIF=01-02-03-04-05-06"))
-
-
 class TestHardwareSyncNotify(MAASServerTestCase):
     def setup(self):
         details = EVENT_DETAILS[EVENT_TYPES.NODE_HARDWARE_SYNC_BLOCK_DEVICE]