Add proper multi-bond support to debianish distros (#83)

We've long supported configuring multiple bonds for RedHat based distros, this is the same but for Debians. Still left is Alpine.

Author: mmlb

packetnetworking/builder.py

@@ -96,7 +96,6 @@ def build_bonding(self):
         )
 
     def build_interfaces(self):
-        self.interfaces = utils.WhereList()
         physical_ifaces = utils.get_interfaces()
         matched_ifaces = utils.get_matched_interfaces(
             self.nw_metadata.interfaces, physical_ifaces
@@ -110,7 +109,7 @@ def build_interfaces(self):
 
     def build_bonds(self):
         self.bonds = utils.RecursiveDictAttributes({})
-        for iface in self.nw_metadata.interfaces:
+        for iface in self.interfaces:
             if "bond" in iface and iface.bond:
                 if iface.bond not in self.bonds:
                     self.bonds[iface.bond] = [iface]

packetnetworking/distros/debian/templates/bonded/etc_network_interfaces.j2

@@ -3,17 +3,21 @@ iface lo inet loopback
 
 {% if osinfo.distro == 'ubuntu' %}
 {% for iface in interfaces %}
+
 auto {{ iface.name }}
 iface {{ iface.name }} inet manual
 {% if iface.name != interfaces[0].name %}
     pre-up sleep 4
 {% endif %}
     bond-master bond0
-
 {% endfor %}
 {% endif %}
-auto bond0
-iface bond0 inet static
+
+{% for bond in bonds %}
+
+auto {{ bond }}
+iface {{ bond }} inet static
+    {% if bond == "bond0" %}
     {% if ip4pub %}
     address {{ ip4pub.address }}
     netmask {{ ip4pub.netmask }}
@@ -25,6 +29,7 @@ iface bond0 inet static
     {% endif %}
     dns-nameservers {{ resolvers | join(" ") }}
 
+    {% endif %}
     bond-downdelay 200
     bond-miimon 100
     bond-mode {{ net.bonding.mode }}
@@ -33,7 +38,8 @@ iface bond0 inet static
     {% if osinfo.distro == 'ubuntu' and net.bonding.mode == 4 %}
     bond-lacp-rate 1
     {% endif %}
-    bond-slaves {{ interfaces | selectattr('bond', 'in', '["bond0"]') | map(attribute='name') | join(' ')}}
+    bond-slaves {{ bonds[bond] | map(attribute='name') | sort | join(' ') }}
+{% if bond == "bond0" %}
 {% if ip6pub %}
 
 iface bond0 inet6 static
@@ -52,3 +58,5 @@ iface bond0:0 inet static
     post-down route del -net {{ subnet }} gw {{ ip4priv.gateway }}
     {% endfor %}
 {% endif %}
+{% endif %}
+{% endfor %}

packetnetworking/distros/debian/test_bonded.py

@@ -64,7 +64,8 @@ def test_public_bonded_task_etc_network_interfaces(
     result += dedent(partial)
     if distro == "ubuntu":
         result += "    bond-lacp-rate 1\n"
-    result += f"""    bond-slaves {' '.join([iface.name for iface in builder.network.interfaces if iface.bond == "bond0"])}\n"""
+    result += f"""    bond-slaves {' '.join(sorted(nic.name for nic in builder.network.bonds["bond0"]))}\n"""
+
     partial = f"""
         iface bond0 inet6 static
             address {ipv6pub.address}
@@ -79,6 +80,24 @@ def test_public_bonded_task_etc_network_interfaces(
             post-down route del -net 10.0.0.0/8 gw {ipv4priv.gateway}
         """
     result += dedent(partial)
+
+    for bond, members in builder.network.bonds.items():
+        if bond == "bond0":
+            continue
+
+        partial = f"""
+            auto {bond}
+            iface {bond} inet static
+                bond-downdelay 200
+                bond-miimon 100
+                bond-mode {bonding_mode}
+                bond-updelay 200
+                bond-xmit_hash_policy layer3+4
+            """
+        result += dedent(partial)
+        if distro == "ubuntu":
+            result += "    bond-lacp-rate 1\n"
+        result += f"    bond-slaves {' '.join(sorted(nic.name for nic in members))}\n"
     assert tasks["etc/network/interfaces"] == result
 
 
@@ -135,8 +154,25 @@ def test_private_bonded_task_etc_network_interfaces(
     result += dedent(partial)
     if distro == "ubuntu":
         result += "    bond-lacp-rate 1\n"
-    result += f"""    bond-slaves {' '.join([iface.name for iface in builder.network.interfaces if iface.bond == "bond0"])}\n"""
+    result += f"    bond-slaves {' '.join(sorted(nic.name for nic in builder.network.bonds['bond0']))}\n"
+
+    for bond, members in builder.network.bonds.items():
+        if bond == "bond0":
+            continue
 
+        partial = f"""
+            auto {bond}
+            iface {bond} inet static
+                bond-downdelay 200
+                bond-miimon 100
+                bond-mode {bonding_mode}
+                bond-updelay 200
+                bond-xmit_hash_policy layer3+4
+            """
+        result += dedent(partial)
+        if distro == "ubuntu":
+            result += "    bond-lacp-rate 1\n"
+        result += f"    bond-slaves {' '.join(sorted(nic.name for nic in members))}\n"
     assert tasks["etc/network/interfaces"] == result
 
 
@@ -194,7 +230,7 @@ def test_public_bonded_task_etc_network_interfaces_with_custom_private_ip_space(
     if distro == "ubuntu":
         result += "    bond-lacp-rate 1\n"
 
-    result += f"""    bond-slaves {' '.join([iface.name for iface in builder.network.interfaces if iface.bond == "bond0"])}\n"""
+    result += f"""    bond-slaves {' '.join(sorted(nic.name for nic in builder.network.bonds["bond0"]))}\n"""
     partial = f"""
         iface bond0 inet6 static
             address {ipv6pub.address}
@@ -211,6 +247,24 @@ def test_public_bonded_task_etc_network_interfaces_with_custom_private_ip_space(
             post-down route del -net 172.16.0.0/12 gw {ipv4priv.gateway}
         """
     result += dedent(partial)
+
+    for bond, members in builder.network.bonds.items():
+        if bond == "bond0":
+            continue
+
+        partial = f"""
+            auto {bond}
+            iface {bond} inet static
+                bond-downdelay 200
+                bond-miimon 100
+                bond-mode {bonding_mode}
+                bond-updelay 200
+                bond-xmit_hash_policy layer3+4
+            """
+        result += dedent(partial)
+        if distro == "ubuntu":
+            result += "    bond-lacp-rate 1\n"
+        result += f"    bond-slaves {' '.join(sorted(nic.name for nic in members))}\n"
     assert tasks["etc/network/interfaces"] == result
 
 
@@ -269,7 +323,25 @@ def test_private_bonded_task_etc_network_interfaces_with_custom_private_ip_space
     if distro == "ubuntu":
         result += "    bond-lacp-rate 1\n"
 
-    result += f"""    bond-slaves {' '.join([iface.name for iface in builder.network.interfaces if iface.bond == "bond0"])}\n"""
+    result += f"""    bond-slaves {' '.join(sorted(nic.name for nic in builder.network.bonds["bond0"]))}\n"""
+
+    for bond, members in builder.network.bonds.items():
+        if bond == "bond0":
+            continue
+
+        partial = f"""
+            auto {bond}
+            iface {bond} inet static
+                bond-downdelay 200
+                bond-miimon 100
+                bond-mode {bonding_mode}
+                bond-updelay 200
+                bond-xmit_hash_policy layer3+4
+            """
+        result += dedent(partial)
+        if distro == "ubuntu":
+            result += "    bond-lacp-rate 1\n"
+        result += f"    bond-slaves {' '.join(sorted(nic.name for nic in members))}\n"
     assert tasks["etc/network/interfaces"] == result
 
 

packetnetworking/distros/distro_builder.py

@@ -1,4 +1,5 @@
 import os
+import re
 import sys
 import logging
 import json
@@ -73,6 +74,7 @@ def build_tasks(self):
 
     def context(self):
         return {
+            "bonds": self.network.bonds,
             "hostname": self.metadata.hostname,
             "iface0": self.network.interfaces[0],
             "interfaces": self.network.interfaces,
@@ -153,14 +155,17 @@ def render(self):
             tmpl.environment.globals.update(generated_header=utils.generated_header)
 
             try:
+                content = tmpl.render(context)
+                # replace multiple empty lines with just one
+                content = re.sub(r"\n+(?=\n)", "\n", content)
                 if file_mode or mode:
                     rendered_tasks[path] = {
                         "file_mode": file_mode,
                         "mode": mode,
-                        "content": tmpl.render(context),
+                        "content": content,
                     }
                 else:
-                    rendered_tasks[path] = tmpl.render(context)
+                    rendered_tasks[path] = content
             except UndefinedError:
                 # having to use print as log.* isn't printing out the json
                 print(

packetnetworking/distros/test_distro_builder.py

@@ -175,6 +175,7 @@ def test_distro_builder_context_as_expected(fake_distro_builder_with_metadata):
     fake_distro = fake_distro_builder_with_metadata()
     context = fake_distro.context()
     wanted_context = {
+        "bonds": fake_distro.network.bonds,
         "hostname": fake_distro.metadata.hostname,
         "iface0": fake_distro.network.interfaces[0],
         "interfaces": fake_distro.network.interfaces,