nixops-hetzner icon indicating copy to clipboard operation
nixops-hetzner copied to clipboard

The interface detection/renaming logic is racy

Open nh2 opened this issue 3 years ago • 1 comments

As I reported in https://github.com/NixOS/nixpkgs/issues/153134#issuecomment-1003641748, the fact that nixops generates a udev rule to rename an interface based on a MAC address, to a name like eth0, will race with the interface detection of the kernel (which may change across kernel versions) and naming schemes of systemd (which may change across systemd versions).

This is done here:

https://github.com/NixOS/nixops-hetzner/blob/84f4eebb89b049c4f86aa779349397c3dedc0c43/nixops_hetzner/backends/server.py#L519-L530

As outlined there, I think nixops-hetzner could address it by:

  • Not using names eth0 as the target names as suggested, but something like internet0.

And further, given that udev-rule based renaming is now apparently deprecated:

  • Switching from the generated udev extraRules for renaming to a .link file.

nh2 avatar Jan 02 '22 00:01 nh2

Not using names eth0 as the target names as suggested, but something like internet0.

I'm using this patch on NixOps 1.6 to address it:

diff --git a/nixops/backends/hetzner.py b/nixops/backends/hetzner.py
index 0ddf377..20faa8d 100644
--- a/nixops/backends/hetzner.py
+++ b/nixops/backends/hetzner.py
@@ -512,7 +512,7 @@ class HetznerState(MachineState):
         cmd = "ip addr show | sed -n -e 's/^[0-9]*: *//p' | cut -d: -f1"
         return self.run_command(cmd, capture_stdout=True).splitlines()
 
-    def _get_udev_rule_for(self, interface):
+    def _get_udev_rule_for(self, interface, stable_interface_name):
         """
         Get lines suitable for services.udev.extraRules for 'interface',
         and thus essentially map the device name to a hardware address.
@@ -524,7 +524,7 @@ class HetznerState(MachineState):
 
         rule = 'ACTION=="add", SUBSYSTEM=="net", ATTR{{address}}=="{0}", '
         rule += 'NAME="{1}"'
-        return rule.format(mac_addr, interface)
+        return rule.format(mac_addr, stable_interface_name)
 
     def _get_ipv4_addr_and_prefix_for(self, interface):
         """
@@ -586,11 +586,19 @@ class HetznerState(MachineState):
 
         server = self._get_server_by_ip(self.main_ipv4)
 
+        # Stable interface names:
+        # Based on the MAC address, we give each interface
+        # a stable name "net0", "net1", and so on.
+        # The "net" prefix does not collide with names that the kernel
+        # or systemd would assign.
+
         # Global networking options
         defgw_ip, defgw_dev = self._get_default_gw()
+        defgw_stable_iface_name = None  # assgined in loop below
         v6defgw = None
 
-        # Interface-specific networking options
+        # Interface-specific networking options.
+        stable_iface_index = 1
         for iface in self._get_ethernet_interfaces():
             if iface == "lo":
                 continue
@@ -599,10 +607,13 @@ class HetznerState(MachineState):
             if result is None:
                 continue
 
-            udev_rules.append(self._get_udev_rule_for(iface))
+            stable_iface_name = "net" + str(stable_iface_index)
+            stable_iface_index += 1
+
+            udev_rules.append(self._get_udev_rule_for(iface, stable_iface_name))
 
             ipv4addr, prefix = result
-            iface_attrs[iface] = {
+            iface_attrs[stable_iface_name] = {
                 'ipv4': {
                     'addresses': [
                         {'address': ipv4addr, 'prefixLength': int(prefix)},
@@ -622,13 +633,15 @@ class HetznerState(MachineState):
             #   https://github.com/NixOS/nixops/pull/1032#issuecomment-433741624
             if iface != defgw_dev:
                 net = self._calculate_ipv4_subnet(ipv4addr, int(prefix))
-                iface_attrs[iface]['ipv4'] = {
+                iface_attrs[stable_iface_name]['ipv4'] = {
                     'routes': [{
                         'address': net,
                         'prefixLength': int(prefix),
                         'via': defgw_ip,
                     }],
                 }
+            else:
+                defgw_stable_iface_name = stable_iface_name
 
             # IPv6 subnets only for eth0
             v6subnets = []
@@ -644,9 +657,9 @@ class HetznerState(MachineState):
                         v6defgw.get('address') == subnet.gateway)
                 v6defgw = {
                     'address': subnet.gateway,
-                    'interface': defgw_dev,
+                    'interface': defgw_stable_iface_name,
                 }
-            iface_attrs[iface]['ipv6'] = { 'addresses': v6subnets }
+            iface_attrs[stable_iface_name]['ipv6'] = { 'addresses': v6subnets }
 
         self.net_info = {
             'services': {
@@ -656,7 +669,7 @@ class HetznerState(MachineState):
                 'interfaces': iface_attrs,
                 'defaultGateway': {
                     'address': defgw_ip,
-                    'interface': defgw_dev,
+                    'interface': defgw_stable_iface_name,
                 },
                 'defaultGateway6': v6defgw,
                 'nameservers': self._get_nameservers(),

nh2 avatar Jan 02 '22 06:01 nh2