Merge ~xnox/cloud-init:dash_underscore into cloud-init:master

[Dimitri John Ledkov]

nplan: For bonds, allow dashed or underscore names of keys.

As some of the bond paramemters are passed in as dashed, or
underscored, depending on the input source.

Also correct transmit-hash-policy netplan target key.

LP: #1690480

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

PASSED: Continuous integration, rev:24bfe483a1007f7980544f03220c12ab7cf69ecf
https://jenkins.ubuntu.com/server/job/cloud-init-ci/332/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/332
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/332
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/332
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/332

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

review: Approve (continuous-integration)

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

PASSED: Continuous integration, rev:2c0ffcabfc1424c90a8861401ec79406294fe90d
https://jenkins.ubuntu.com/server/job/cloud-init-ci/333/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/333
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/333
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/333
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/333

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

AFAICT, bond-* parameters are almost always directly related to the kernel sysfs parameters as defined in /sys/class/net/<bond>/bonding/* which are kernel properties of the bonding driver.

Ifupdown appears to have a habit of using bond-<sysfs-key-name>, netplan has a habit of the same, but in some cases rewords the value of the sysfs-key-name (e.g. num_grat_arp vs. gratiutous-arp).

So it's not clear that just a few adds of underbar here are going to resolve the issue.
In the renderers class, we have a currently known mapping of what's expected, but it's certainly somewhat error prone as new kernel features may surface where by both cloud-init and renderer will need updating w.r.t to validation.

It's also been expressed that netplan should not expose sysfs entries directly in the configuration but higher level names.

In any case for this; I suggest that instead of fixing a few keys, we accept either '-' or '_' in the input:

bondoption = 'bond_mode'

netplan_param = NET_CONFIG_TO_V2.get(bondoption.replace("_", "-"))

review: Needs Fixing

[Dimitri John Ledkov (xnox)]

Hm, i think i should start a new clean branch for these things.

[Dimitri John Ledkov (xnox)]

Did a force push, please re-review.

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

FAILED: Continuous integration, rev:b996c59ee11412e199449804eab2262d0cf0fd7b
https://jenkins.ubuntu.com/server/job/cloud-init-ci/355/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/355/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/355/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/355/console
    FAILURE: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/355/console

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

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:1736fea2085e2df5fc06d59640da01612531e2ff
https://jenkins.ubuntu.com/server/job/cloud-init-ci/356/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/356
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/356
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/356
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/356

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

Looks good; let's add some element to the unittest to catch and verify this change:

diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 7c5dc4e..956570d 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -517,11 +517,15 @@ auto eth1
 iface eth1 inet manual
     bond-master bond0
     bond-mode active-backup
+ bond-xmit-hash-policy layer3+4
+ bond_miimon 100

 auto eth2
 iface eth2 inet manual
     bond-master bond0
     bond-mode active-backup
+ bond-xmit-hash-policy layer3+4
+ bond_miimon 100

 iface eth3 inet manual

@@ -534,6 +538,8 @@ auto bond0
 iface bond0 inet6 dhcp
     bond-mode active-backup
     bond-slaves none
+ bond-xmit-hash-policy layer3+4
+ bond_miimon 100
     hwaddress aa:bb:cc:dd:ee:ff

 auto br0
@@ -658,7 +664,9 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
                         - eth1
                         - eth2
                         parameters:
+ mii-monitor-interval: 100
                             mode: active-backup
+ transmit-hash-policy: layer3+4
                 bridges:
                     br0:
                         addresses:
@@ -747,6 +755,8 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
                     - eth2
                   params:
                     bond-mode: active-backup
+ bond_miimon: 100
+ bond-xmit-hash-policy: "layer3+4"
                   subnets:
                     - type: dhcp6
                 # A Bond VLAN.

47a3f8c... by Dimitri John Ledkov on 2017-05-22

Add tests as per rharper suggestion.

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

PASSED: Continuous integration, rev:47a3f8ceae5ac27e1497a42e5576ae7bdfa610c5
https://jenkins.ubuntu.com/server/job/cloud-init-ci/365/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-amd64/365
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-arm64/365
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-ppc64el/365
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=metal-s390x/365
    SUCCESS: https://jenkins.ubuntu.com/server/job/cloud-init-ci/nodes=vm-i386/365

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

review: Approve (continuous-integration)

diff --git a/cloudinit/net/netplan.py b/cloudinit/net/netplan.py
index 9b71de9..d7ddf0c 100644
--- a/cloudinit/net/netplan.py
+++ b/cloudinit/net/netplan.py
@@ -41,7 +41,7 @@ NET_CONFIG_TO_V2 = {
              'bond-num-grat-arp': 'gratuitious-arp',
              'bond-primary-reselect': 'primary-reselect-policy',
              'bond-updelay': 'up-delay',
-             'bond-xmit_hash_policy': 'transmit_hash_policy'},
+             'bond-xmit-hash-policy': 'transmit-hash-policy'},
     'bridge': {'bridge_ageing': 'ageing-time',
                'bridge_bridgeprio': 'priority',
                'bridge_fd': 'forward-delay',
@@ -294,7 +294,7 @@ class Renderer(renderer.Renderer):
                 for match in ['bond_', 'bond-']:
                     bond_params = _get_params_dict_by_match(ifcfg, match)
                     for (param, value) in bond_params.items():
-                        newname = v2_bond_map.get(param)
+                        newname = v2_bond_map.get(param.replace('_', '-'))
                         if newname is None:
                             continue
                         bond_config.update({newname: value})
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 68a0157..ea4c789 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -518,11 +518,15 @@ auto eth1
 iface eth1 inet manual
     bond-master bond0
     bond-mode active-backup
+    bond-xmit-hash-policy layer3+4
+    bond_miimon 100
 
 auto eth2
 iface eth2 inet manual
     bond-master bond0
     bond-mode active-backup
+    bond-xmit-hash-policy layer3+4
+    bond_miimon 100
 
 iface eth3 inet manual
 
@@ -535,6 +539,8 @@ auto bond0
 iface bond0 inet6 dhcp
     bond-mode active-backup
     bond-slaves none
+    bond-xmit-hash-policy layer3+4
+    bond_miimon 100
     hwaddress aa:bb:cc:dd:ee:ff
 
 auto br0
@@ -660,7 +666,9 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
                         - eth1
                         - eth2
                         parameters:
+                            mii-monitor-interval: 100
                             mode: active-backup
+                            transmit-hash-policy: layer3+4
                 bridges:
                     br0:
                         addresses:
@@ -751,6 +759,8 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
                     - eth2
                   params:
                     bond-mode: active-backup
+                    bond_miimon: 100
+                    bond-xmit-hash-policy: "layer3+4"
                   subnets:
                     - type: dhcp6
                 # A Bond VLAN.