Merge ~rjschwei/cloud-init:netV1ToTranslate into cloud-init:master

[Robert Schweikert]

Handle network configuration translation for the legacy path, do not drop gateway information

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

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

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

review: Needs Fixing (continuous-integration)

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

FAILED: Continuous integration, rev:4d3028d8d083b3781335d5a062038a4ca3cd92b5
https://jenkins.ubuntu.com/server/job/cloud-init-ci/515/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    FAILED: MAAS Compatability Testing

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

review: Needs Fixing (continuous-integration)

[Chad Smith (chad.smith)]

Looking over the failure, that MAAS compatibility testing hit an issue with an include url test unrelated to your branch. Can you git fetch on master and git rebase your branch against master and re-push that branch up. It will contain the fix 6ad23fe9b11f07e4404c8a1f2f1e9cba2640dceb which should resolve integration test issues hit on this branch.

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

PASSED: Continuous integration, rev:462d4c1710f08212d2b84f359e54242b2507f0b0
https://jenkins.ubuntu.com/server/job/cloud-init-ci/548/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

[Robert Schweikert (rjschwei)]

@raharper and @chad.smith

Thanks for review and comments.

Fixed: Test location and IPv4 regular expression.

Comments and questions on the other info you provided.

As far as the test data is concerned, I kind of did a cut and paste from existing tests based on network configuration for the distros. I really have no idea what the various data sources can and cannot produce and what the debian/ubuntu format looks like. So test data that conforms to what the data sources produce would be better. If there is concrete data, that I should be using and have not used, please point me in that direction. Same can be applied to the suggested change of the search for "default gw", I agree that the proposed expression is more flexible, but does it need to be, i.e. do we have a data source where there is a varying space?

Yes there is always the risk that a new data source may format things differently, but the question would then be if we should compensate for such potential future discretions now or not.

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

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

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

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

Robert, I agree on your suggestion to not be too flexible for future formats etc. So +1 on dropping my other re.match suggestion on 'default gw' handling. Yeah no need to waste resources planning for future-formats if we don't need to. Per your handling is duplicate (list type) entries in post-up, did you have specific use-cases in mind where multiple post-up commands are specified in /etc/network/interfaces in a single iface stanza?

review: Needs Information

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

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

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

review: Needs Fixing (continuous-integration)

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

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

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

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:2f69ddd8fa79de3ad8c1b7dc79dbed278ac3b3d8
https://jenkins.ubuntu.com/server/job/cloud-init-ci/7/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

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

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

We think this involves a rework pending the chages discussed at the summit which resulted in Ryan's branch https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/353740. Will mark 'work in progress' on this as the sysconfig rendering changes Ryan addresses will impact this branch.

review: Abstain

[Chad Smith (chad.smith)]

Awaiting the following branch landing which should affect the approach used in this branch https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/353740

[Robert Schweikert (rjschwei)]

> Awaiting the following branch landing which should affect the approach used in
> this branch https://code.launchpad.net/~raharper/cloud-init/+git/cloud-
> init/+merge/353740

Agreed that is what we discussed at the summit. One thing I am not certain about, is the following:

For distros where the old implementation (_write_network) has been superseeded by the new implementation that uses the renderer approach the _write_network implementation has not been removed or rigged to throw and exception, i.e. I would expect a

def _write_network(self, settings):
    raise "You should not be here"

implementation. This is not the case.

So my question is, is there any code path where once the rendered approach is implemented we could still enter _write_network() ?

- If the answer is yes than this change would still be needed.
- If the answer is no, than my follow up question will be, is it OK to then implement the _write_network() with the exception as shown in pseudo code above?

  - If the answer is no, then we'd still want this change.

I do currently carry this patch in the package. I can carry it for a while until https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/353740 and the necessary follow up changes are merged pending the answers to the questions above.

[Scott Moser (smoser)]

Robert,

Please see
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/354962

I think that is what you were after.

[Robert Schweikert (rjschwei)]

Scott,

Yes thanks that answers my question.

Then yes as agreed at the summit lets hang on to this with no changes, when

https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/353740

landed and I have the follow up changes for the config options done, this can just go away :)

Unmerged commits

2f69ddd... by Robert Schweikert on 2017-11-17

- Handle gateway information in the network config translator for
  the apply_network path lp#1732966

diff --git a/cloudinit/distros/net_util.py b/cloudinit/distros/net_util.py
index 1ce1aa7..557a95c 100644
--- a/cloudinit/distros/net_util.py
+++ b/cloudinit/distros/net_util.py
@@ -67,6 +67,11 @@
 #     }
 # }
 
+import re
+
+ipv4 = re.compile(r"\d{1,3}.\d{1,3}.\d{1,3}.\d{1,3}")
+
+
 def translate_network(settings):
     # Get the standard cmd, args from the ubuntu format
     entries = []
@@ -88,7 +93,14 @@ def translate_network(settings):
                 consume = {}
             consume[cmd] = args
         else:
-            consume[cmd] = args
+            if consume.get(cmd):
+                val = consume[cmd]
+                if isinstance(val, list):
+                    consume[cmd].append(args)
+                else:
+                    consume[cmd] = [val, args]
+            else:
+                consume[cmd] = args
     # Check if anything left over to consume
     absorb = False
     for (cmd, args) in consume.items():
@@ -148,6 +160,16 @@ def translate_network(settings):
                     hw_addr = hw_split[1]
                     if hw_addr:
                         iface_info['hwaddress'] = hw_addr
+            if 'post-up' in info:
+                routes = info['post-up']
+                if isinstance(routes, list):
+                    for route_info in routes:
+                        if 'default gw' in route_info:
+                            iface_info['gateway'] = ipv4.search(
+                                route_info).group(0)
+                elif 'default gw' in routes:
+                    iface_info['gateway'] = ipv4.search(routes).group(0)
+
         # If ipv6 is enabled, device will have multiple IPs, so we need to
         # update the dictionary instead of overwriting it...
         if dev_name in real_ifaces:
diff --git a/tests/unittests/test_distros/test_net_util.py b/tests/unittests/test_distros/test_net_util.py
new file mode 100644
index 0000000..93426cf
--- /dev/null
+++ b/tests/unittests/test_distros/test_net_util.py
@@ -0,0 +1,103 @@
+# This file is part of cloud-init. See LICENSE file for license information.
+
+from cloudinit.distros import net_util
+
+from cloudinit.tests.helpers import CiTestCase
+
+
+class TestNetworkConfigTransform(CiTestCase):
+
+    def test_basic_config_ipv4(self):
+        basic_conf = '''
+auto lo
+iface lo inet loopback
+
+auto eth0
+iface eth0 inet static
+    address 192.168.1.5
+    broadcast 192.168.1.0
+    gateway 192.168.1.254
+    netmask 255.255.255.0
+    network 192.168.0.0
+
+auto eth1
+iface eth1 inet dhcp
+'''
+        sysconfig = net_util.translate_network(basic_conf)
+        expected = {
+            'lo': {'auto': True, 'ipv6': {}},
+            'eth0': {
+                'auto': True,
+                'ipv6': {},
+                'broadcast': '192.168.1.0',
+                'netmask': '255.255.255.0',
+                'bootproto': 'static',
+                'address': '192.168.1.5',
+                'gateway': '192.168.1.254'
+            },
+            'eth1': {'auto': True, 'bootproto': 'dhcp', 'ipv6': {}}
+        }
+        self.assertEqual(sysconfig, expected)
+
+    def test_v1_confi_single_route_ipv4(self):
+        v1_conf = '''
+auto lo
+iface lo inet loopback
+
+auto eth0
+iface eth0 inet static
+    hwaddress fa:16:3e:ee:2b:97
+    address 192.168.168.30/24
+    mtu 1500
+    post-up route add default gw 192.168.168.1 || true
+    pre-down route del default gw 192.168.168.1 || true
+'''
+        sysconfig = net_util.translate_network(v1_conf)
+        expected = {
+            'lo': {'auto': True, 'ipv6': {}},
+            'eth0': {
+                'auto': True,
+                'bootproto': 'static',
+                'gateway': '192.168.168.1',
+                'address': '192.168.168.30/24',
+                'ipv6': {}
+            }
+        }
+        self.assertEqual(sysconfig, expected)
+
+    def test_v1_confi_multi_route_multi_nic_ipv4(self):
+        v1_conf = '''
+auto lo
+iface lo inet loopback
+
+auto eth0
+iface eth0 inet static
+    hwaddress fa:16:3e:ee:2b:97
+    address 192.168.168.30/24
+    mtu 1500
+    post-up route add default gw 192.168.168.1 || true
+    pre-down route del default gw 192.168.168.1 || true
+
+auto eth1
+iface eth1 inet dhcp
+    post-up route add 192.168.168.1 || true
+'''
+        sysconfig = net_util.translate_network(v1_conf)
+        expected = {
+            'lo': {'auto': True, 'ipv6': {}},
+            'eth0': {
+                'auto': True,
+                'bootproto': 'static',
+                'gateway': '192.168.168.1',
+                'address': '192.168.168.30/24',
+                'ipv6': {}
+            },
+            'eth1': {
+                'auto': True,
+                'bootproto': 'dhcp',
+                'ipv6': {}
+            }
+        }
+        self.assertEqual(sysconfig, expected)
+
+# vi: ts=4 expandtab