curtin-dev team mailing list archive

[Olivier Gayot <mp+494047@xxxxxxxxxxxxxxxxxx>]

Olivier Gayot has proposed merging ~ogayot/curtin:nvme-o-tcp-drop-set-name into curtin:master.

Commit message:
do not squash

Requested reviews:
  Server Team CI bot (server-team-bot): continuous-integration
  curtin developers (curtin-dev)
Related bugs:
  Bug #2127072 in curtin: "NVMe/TCP on 25.10 fails to boot"
  https://bugs.launchpad.net/curtin/+bug/2127072

For more details, see:
https://code.launchpad.net/~ogayot/curtin/+git/curtin/+merge/494047

nvme-o-tcp: drop set-name directives from netplan config

If the nbft0 interface config contains a set-name directive, netplan will attempt to honor it during boot. This leads to a temporary network loss, which ends up fatal (i.e., it prevents booting) in a NVMe/TCP environment.

Make sure that we drop any set-name directive when configuring the critical interface for NVMe/TCP booting.

LP: #2127072
-- 
Your team curtin developers is requested to review the proposed merge of ~ogayot/curtin:nvme-o-tcp-drop-set-name into curtin:master.

diff --git a/curtin/nvme_tcp.py b/curtin/nvme_tcp.py
index 8cc0432..b7cfdc5 100644
--- a/curtin/nvme_tcp.py
+++ b/curtin/nvme_tcp.py
@@ -351,8 +351,10 @@ def dracut_adapt_netplan_config(cfg, *, target: pathlib.Path):
     '''Modify the netplan configuration (which has already been written to
     disk at this point) so that:
     * critical network interfaces (those handled by dracut) are not brought
-    down during boot.
-    * netplan does not panic if such an interface gets renamed.
+    down during boot. This can happen if they are not marked critical: true
+    (LP: #2084012) or if netplan is instructed to rename them (LP: #2127072).
+    * netplan does not panic if such an interface gets renamed by dracut (e.g.,
+    to nbft0).
     '''
     ifnames: Set[str] = set()
     modified = False
@@ -393,6 +395,8 @@ def dracut_adapt_netplan_config(cfg, *, target: pathlib.Path):
             continue
         # Ensure the interface is not brought down
         ifconfig['critical'] = True
+        with contextlib.suppress(KeyError):
+            del ifconfig['set-name']
         modified = True
         # Ensure we match the HW address and not the ifname.
         if 'match' not in ifconfig:
diff --git a/tests/unittests/test_nvme_tcp.py b/tests/unittests/test_nvme_tcp.py
index 064e5ec..227e317 100644
--- a/tests/unittests/test_nvme_tcp.py
+++ b/tests/unittests/test_nvme_tcp.py
@@ -459,6 +459,54 @@ network:
                 new_ens3_content['match']['macaddress'], 'aa:bb:cc:dd:ee:ff')
         self.assertTrue(new_ens3_content['critical'])
 
+    def test_dracut_adapt_netplan_config_25_10(self):
+        content = '''\
+# This is the network config written by 'subiquity'
+network:
+  ethernets:
+    enp1s0:
+      dhcp4: true
+      dhcp6: true
+      match:
+        macaddress: 52:54:00:6a:b9:8d
+      set-name: enp1s0
+  version: 2
+'''
+        cfg = {
+            'storage': {
+                'config': [{
+                    'type': 'nvme_controller',
+                    'id': 'nvme-controller-nvme0',
+                    'transport': 'tcp',
+                    'tcp_addr': '10.0.2.144',
+                    'tcp_port': 4420,
+                }],
+            }, 'write_files': {
+                'etc_netplan_installer': {
+                    'path': 'etc/netplan/installer.yaml'}
+            }
+        }
+
+        target = Path(self.tmp_dir())
+        netplan_conf_path = target / 'etc/netplan/installer.yaml'
+        netplan_conf_path.parent.mkdir(parents=True)
+        netplan_conf_path.write_text(content)
+
+        p_route_ifname = patch('curtin.nvme_tcp.get_route_dest_ifname',
+                               return_value='enp1s0')
+        p_hw_addr = patch('curtin.nvme_tcp.get_iface_hw_addr',
+                          return_value='52:54:00:6a:b9:8d')
+        with p_route_ifname, p_hw_addr:
+            nvme_tcp.dracut_adapt_netplan_config(cfg, target=target)
+
+        new_content = yaml.safe_load(netplan_conf_path.read_text())
+        new_enp1s0_content = new_content['network']['ethernets']['enp1s0']
+
+        self.assertEqual(
+                new_enp1s0_content['match']['macaddress'], '52:54:00:6a:b9:8d')
+        self.assertTrue(new_enp1s0_content['critical'])
+        self.assertNotIn('set-name', new_enp1s0_content)
+
     def test_dracut_adapt_netplan_config__no_config(self):
         content = '''\
 # This is the network config written by 'subiquity'