Merge ~darren-birkett/cloud-init:fix-infiniband-1847114 into cloud-init:master

[Darren Birkett]

net: enable infiniband support in eni and sysconfig renderers

Commit e7b0e5f72 added support for configuring infiniband devices by
adding a new infiniband 'type'. This commit updates eni and sysconfig
renderers to consume this new type and configure infiniband devices
correctly.

LP: #1847114

[Ryan Harper (raharper)]

Thanks for looking at this. I'd like to get this plumbed into the unittests for network generation ( tests/unittests/test_net.py ); I suggest adding this to the 'all' scenario, update the yaml to include the IB interface:

@@ -1456,6 +1476,12 @@ pre-down route del -net 10.0.0.0/8 gw 11.0.0.1 metric 3 || true
                   vlan_id: 200
                   subnets:
                       - type: dhcp4
+ # An Infiniband
+ - type: infiniband
+ name: ib0
+ mac_address: 00:11:22:33:44:56
+ subnets:
+ - type: dhcp4

And then fix up the expected eni and sysconfig output; lastly this will need an update to handle the convertion of v1 with type: infiniband to v2 where the type: infiniband will be converted to an ethernet type.

review: Needs Fixing

[Darren Birkett (darren-birkett)]

Hi Ryan,

Thanks for your comments. Would you mind expanding on your last comment regarding the conversion between type 1 and type 2? I have added the tests as you suggested, and the sysconfig test is failing because it is expecting 'Type=Ethernet' but has 'Type=InfiniBand' (which we want). I'm happy to add this, just I'm not quite sure what the ask is here and need a bit of guidance.

a706916... by Darren Birkett on 2019-10-08

Add tests for patch to eni to support infiniband

4a94332... by Darren Birkett on 2019-10-08

Ensure sysconfig writes HWADDR when infiniband

[Darren Birkett (darren-birkett)]

I've pushed some additional commits to the branch - does this work, or do I need to do something else for the Type?

Also, I had to fix part of the sysconfig.py to ensure the mac addr is written (in order to pass the tests I added). I know it's not directly related to the bug I was fixing, but it is related to the tests I'm adding. Hope it's OK to include here.

[Ryan Harper (raharper)]

Thanks for adding the test. "type 2" refers to network-config version:2 which is known as netplan.
There's a netplan section in that same test and a netplan renderer. However, I was wondering of the full IB hardware address is required, or would we instead only render the subset of the address that's the same size/format as MAC ?

I know that netplan won't recognize the full hardware address so we currently couldn't support IB in netplan.

review: Needs Information

[Darren Birkett (darren-birkett)]

Hi Ryan. I don't mind working on a patch to support ib in netplan, but it seems like that should probably sit in its own separate patch rather than trying to lump it in with this fix specifically for eni?

I can raise a bug for netplan support and self-assign if you agree? In which case, the unittests for this patchset are passing for eni and sysconfig now so it should be good to go.

[Ryan Harper (raharper)]

Well, we've touched two of the three renderers to fix network config rendering for IB devices; it feels to me the core issue "ib network config isn't rendered correct", is true for all renderers; I think it would be fine to put it all in one place.

I'd like to first confirm if we can use the 8-octet portion of the IB hardware address instead of the full IB hardware address.

Depending on the answer to this query will determine if we can pull netplan support into this branch or not. If we must use the full hardware address, then we'll need to file a bug against netplan to have it add support for parsing the full-length MAC; as now netplan complains if you feed it a MAC address that's not in the standard 8-octet format (where as eni and sysconfig, AFAICT ignore format the value).

[Darren Birkett (darren-birkett)]

Hi Ryan,

I've tested a netplan config with the truncated mac addr (as generated in the current cloud-init code: mac = mac[36:-14] + mac[51:]) from an ib interface, and netplan doesn't seem able to match the device so doesn't configure it properly. I also tried using the final 6 bytes of the long ib mac, and then tried with the first 6 bytes. None of them match the device.

Example not working-

---
network:
    version: 2
    ethernets:
        ib0:
            addresses:
            - 192.168.207.26/24
            mtu: 9000
            match:
                macaddress: a0:00:02:20:fe:80
            set-name: ib0
---

Removing the mac matching completely from the config allows netplan to apply the config and bring the device up, at the risk of not matching the correct device at some point depending on how the device gets enumerated.

Example working-

---
network:
    version: 2
    ethernets:
        ib0:
            addresses:
            - 192.168.207.26/24
            mtu: 9000
---

[Ryan Harper (raharper)]

Thanks for testing that out. We can currently match on name, driver or mac. Let's file a separate bug for getting netplan.io support for IB interfaces, we can sort out whether netplan.io/systemd-networkd need fixes from what cloud-init generates.

[Darren Birkett (darren-birkett)]

Hi Ryan,

Is this patch OK to merge then?

I've separately opened https://bugs.launchpad.net/cloud-init/+bug/1848471 to add netplan support once netplan is able to deal with long-form HWADDR.

[Darren Birkett (darren-birkett)]

I've also added this bug report to netplan to get long-form HWADDR support included.

https://bugs.launchpad.net/netplan/+bug/1848474

[Ryan Harper (raharper)]

Thanks. Ill point ci at this branch and if we're good, then we can land. If it fails, I'll pass along the error.

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:fd252e6dd234dbf980f1a56e0c1465d1f22bd724
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1216/
Executed test runs:
    SUCCESS: Checkout
    FAILED: Unit & Style Tests

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1216//rebuild

review: Needs Fixing (continuous-integration)

[Dan Watkins (oddbloke)]

There are pycodestyle errors:

cloudinit/net/sysconfig.py:333:80: E501 line too long (96 > 79 characters)
tests/unittests/test_net.py:1480:80: E501 line too long (90 > 79 characters)

Let us know when you've fixed those up (bear in mind you can run these tests locally with `tox -e pycodestyle`!) and we'll re-run CI.

[Darren Birkett (darren-birkett)]

Hi Dan,

These errors have now been fixed.

Thanks,
Darren

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:4a94332920192b36e83d5ea71ddacc37f00c6a4e
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1221/
Executed test runs:
    FAILED: Checkout

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1221//rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

FAILED: Continuous integration, rev:4a94332920192b36e83d5ea71ddacc37f00c6a4e
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1222/
Executed test runs:
    FAILED: Checkout

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1222//rebuild

review: Needs Fixing (continuous-integration)

[Server Team CI bot (server-team-bot)]

PASSED: Continuous integration, rev:4a94332920192b36e83d5ea71ddacc37f00c6a4e
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1223/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1223//rebuild

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

Alright, CI is happy!

review: Approve

diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py
index 530922b..a9a80c9 100644
--- a/cloudinit/net/eni.py
+++ b/cloudinit/net/eni.py
@@ -94,7 +94,7 @@ def _iface_add_attrs(iface, index, ipv4_subnet_mtu):
     ]
 
     renames = {'mac_address': 'hwaddress'}
-    if iface['type'] not in ['bond', 'bridge', 'vlan']:
+    if iface['type'] not in ['bond', 'bridge', 'infiniband', 'vlan']:
         ignore_map.append('mac_address')
 
     for key, value in iface.items():
@@ -472,9 +472,10 @@ class Renderer(renderer.Renderer):
         order = {
             'loopback': 0,
             'physical': 1,
-            'bond': 2,
-            'bridge': 3,
-            'vlan': 4,
+            'infiniband': 2,
+            'bond': 3,
+            'bridge': 4,
+            'vlan': 5,
         }
 
         sections = []
diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index 4e65676..e381596 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -330,7 +330,8 @@ class Renderer(renderer.Renderer):
             old_value = iface.get(old_key)
             if old_value is not None:
                 # only set HWADDR on physical interfaces
-                if old_key == 'mac_address' and iface['type'] != 'physical':
+                if (old_key == 'mac_address' and
+                   iface['type'] not in ['physical', 'infiniband']):
                     continue
                 iface_cfg[new_key] = old_value
 
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index f5a9cae..d220199 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -1176,6 +1176,12 @@ iface eth4 inet manual
 # control-manual eth5
 iface eth5 inet dhcp
 
+auto ib0
+iface ib0 inet static
+    address 192.168.200.7/24
+    mtu 9000
+    hwaddress a0:00:02:20:fe:80:00:00:00:00:00:00:ec:0d:9a:03:00:15:e2:c1
+
 auto bond0
 iface bond0 inet6 dhcp
     bond-mode active-backup
@@ -1457,7 +1463,19 @@ pre-down route del -net 10.0.0.0/8 gw 11.0.0.1 metric 3 || true
                 ONBOOT=no
                 STARTMODE=manual
                 TYPE=Ethernet
-                USERCTL=no""")
+                USERCTL=no"""),
+            'ifcfg-ib0': textwrap.dedent("""\
+                BOOTPROTO=none
+                DEVICE=ib0
+                HWADDR=a0:00:02:20:fe:80:00:00:00:00:00:00:ec:0d:9a:03:00:15:e2:c1
+                IPADDR=192.168.200.7
+                MTU=9000
+                NETMASK=255.255.255.0
+                NM_CONTROLLED=no
+                ONBOOT=yes
+                STARTMODE=auto
+                TYPE=InfiniBand
+                USERCTL=no"""),
         },
         'yaml': textwrap.dedent("""
             version: 1
@@ -1532,6 +1550,15 @@ pre-down route del -net 10.0.0.0/8 gw 11.0.0.1 metric 3 || true
                   vlan_id: 200
                   subnets:
                       - type: dhcp4
+                # An infiniband
+                - type: infiniband
+                  name: ib0
+                  mac_address: >-
+                    a0:00:02:20:fe:80:00:00:00:00:00:00:ec:0d:9a:03:00:15:e2:c1
+                  subnets:
+                      - type: static
+                        address: 192.168.200.7/24
+                        mtu: 9000
                 # A bridge.
                 - type: bridge
                   name: br0