Merge ~raphael-glon/cloud-init:master into cloud-init:master

[raphael.glon]

net: Fix ipv6 static routes when using eni renderer

When rendering ipv6 static routes in eni format the
post-up/pre down commands were not correct for ipv6.

LP: #1818669

[Dan Watkins (oddbloke)]

Hi Raphael,

Thank you for contributing to cloud-init.

To contribute, you must sign the Canonical Contributor License Agreement
(CLA) [1].

If you have already signed it as an individual, your Launchpad user will
be listed in the contributor-agreement-canonical launchpad group [2].
Unfortunately there is no easy way to check if an organization or company
you are doing work for has signed. If you are unsure or have questions,
email <email address hidden> or ping powersj in #cloud-init channel
via freenode.

For information on how to sign, please see the HACKING document [3].

Thanks again, and please feel free to reach out with any questions.

–
[1] http://www.canonical.com/contributors
[2] https://launchpad.net/~contributor-agreement-canonical/+members
[3] http://cloudinit.readthedocs.io/en/latest/topics/hacking.html

review: Needs Information

1af82aa... by raphael.glon on 2019-03-05

Unittests: add bond rendering test for eni config

[Ryan Harper (raharper)]

I've taken a look and I can confirm that for v6 routes, route needs the network with prefix.
This is information is present in the internal network_state of the route so a little juggling is needed to always use network/prefix when rendering route and ensure that on v6 routes we use -A inet6.

I've added the test-case from the bug and a few others.

This passes tox and does not regress anything.

Please consider applying this instead

http://paste.ubuntu.com/p/s3tYjGv3Jh/

[raphael.glon (raphael-glon)]

Ok for the considered patch
-> but there is still one problem with it:
-net is mandatory for route -4 but unsupported for ipv6
so we need to remove the -net param for ipv6 only

aa64677... by raphael.glon on 2019-03-06

Fix net/eni static routes for ipv6

this fixes:
    https://bugs.launchpad.net/cloud-init/+bug/1818669
    eni: post-up/pre down commands were not correct for ipv6

unittests: additional test to check the
rendered static routes for eni config

[Dan Watkins (oddbloke)]

CLA has been signed; thanks Raphael!

(I'm going to abstain as Ryan has started working with you on this.)

review: Abstain

[Ryan Harper (raharper)]

Yes; that's a good catch on the -net | -A inet6.

[Ryan Harper (raharper)]

I've pointed CI at this branch. If that comes out clean, I'll mark approve and land.

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

PASSED: Continuous integration, rev:1af82aaadd9639ea6e32884a2dfcaa4cb205a5fd
https://jenkins.ubuntu.com/server/job/cloud-init-ci/617/
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/617/rebuild

review: Approve (continuous-integration)

[raphael.glon (raphael-glon)]

Hello Ryan,

Not sure what I should do next about this merge request. Is there something more needed for approval ?

Regards

[Ryan Harper (raharper)]

Sorry for not replying. The branch is good to go. I just need to mark it for landing.

[Ryan Harper (raharper)]

review: Approve

diff --git a/cloudinit/net/eni.py b/cloudinit/net/eni.py
index 6423632..b129bb6 100644
--- a/cloudinit/net/eni.py
+++ b/cloudinit/net/eni.py
@@ -366,8 +366,6 @@ class Renderer(renderer.Renderer):
         down = indent + "pre-down route del"
         or_true = " || true"
         mapping = {
-            'network': '-net',
-            'netmask': 'netmask',
             'gateway': 'gw',
             'metric': 'metric',
         }
@@ -379,13 +377,21 @@ class Renderer(renderer.Renderer):
             default_gw = ' -A inet6 default'
 
         route_line = ''
-        for k in ['network', 'netmask', 'gateway', 'metric']:
-            if default_gw and k in ['network', 'netmask']:
+        for k in ['network', 'gateway', 'metric']:
+            if default_gw and k == 'network':
                 continue
             if k == 'gateway':
                 route_line += '%s %s %s' % (default_gw, mapping[k], route[k])
             elif k in route:
-                route_line += ' %s %s' % (mapping[k], route[k])
+                if k == 'network':
+                    if ':' in route[k]:
+                        route_line += ' -A inet6'
+                    else:
+                        route_line += ' -net'
+                    if 'prefix' in route:
+                        route_line += ' %s/%s' % (route[k], route['prefix'])
+                else:
+                    route_line += ' %s %s' % (mapping[k], route[k])
         content.append(up + route_line + or_true)
         content.append(down + route_line + or_true)
         return content
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index e3b9e02..f725a4b 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -1113,8 +1113,8 @@ iface eth0.101 inet static
 iface eth0.101 inet static
     address 192.168.2.10/24
 
-post-up route add -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
-pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
+post-up route add -net 10.0.0.0/8 gw 11.0.0.1 metric 3 || true
+pre-down route del -net 10.0.0.0/8 gw 11.0.0.1 metric 3 || true
 """),
         'expected_netplan': textwrap.dedent("""
             network:
@@ -1505,17 +1505,18 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
                      - gateway: 192.168.0.3
                        netmask: 255.255.255.0
                        network: 10.1.3.0
-                     - gateway: 2001:67c:1562:1
-                       network: 2001:67c:1
-                       netmask: ffff:ffff:0
-                     - gateway: 3001:67c:1562:1
-                       network: 3001:67c:1
-                       netmask: ffff:ffff:0
-                       metric: 10000
                   - type: static
                     address: 192.168.1.2/24
                   - type: static
                     address: 2001:1::1/92
+                    routes:
+                        - gateway: 2001:67c:1562:1
+                          network: 2001:67c:1
+                          netmask: ffff:ffff:0
+                        - gateway: 3001:67c:1562:1
+                          network: 3001:67c:1
+                          netmask: ffff:ffff:0
+                          metric: 10000
             """),
         'expected_netplan': textwrap.dedent("""
          network:
@@ -1554,6 +1555,51 @@ pre-down route del -net 10.0.0.0 netmask 255.0.0.0 gw 11.0.0.1 metric 3 || true
                          to: 3001:67c:1/32
                          via: 3001:67c:1562:1
         """),
+        'expected_eni': textwrap.dedent("""\
+auto lo
+iface lo inet loopback
+
+auto bond0s0
+iface bond0s0 inet manual
+    bond-master bond0
+    bond-mode active-backup
+    bond-xmit-hash-policy layer3+4
+    bond_miimon 100
+
+auto bond0s1
+iface bond0s1 inet manual
+    bond-master bond0
+    bond-mode active-backup
+    bond-xmit-hash-policy layer3+4
+    bond_miimon 100
+
+auto bond0
+iface bond0 inet static
+    address 192.168.0.2/24
+    gateway 192.168.0.1
+    bond-mode active-backup
+    bond-slaves none
+    bond-xmit-hash-policy layer3+4
+    bond_miimon 100
+    hwaddress aa:bb:cc:dd:e8:ff
+    mtu 9000
+    post-up route add -net 10.1.3.0/24 gw 192.168.0.3 || true
+    pre-down route del -net 10.1.3.0/24 gw 192.168.0.3 || true
+
+# control-alias bond0
+iface bond0 inet static
+    address 192.168.1.2/24
+
+# control-alias bond0
+iface bond0 inet6 static
+    address 2001:1::1/92
+    post-up route add -A inet6 2001:67c:1/32 gw 2001:67c:1562:1 || true
+    pre-down route del -A inet6 2001:67c:1/32 gw 2001:67c:1562:1 || true
+    post-up route add -A inet6 3001:67c:1/32 gw 3001:67c:1562:1 metric 10000 \
+|| true
+    pre-down route del -A inet6 3001:67c:1/32 gw 3001:67c:1562:1 metric 10000 \
+|| true
+        """),
         'yaml-v2': textwrap.dedent("""
             version: 2
             ethernets:
@@ -3570,17 +3616,17 @@ class TestEniRoundTrip(CiTestCase):
             'iface eth0 inet static',
             '    address 172.23.31.42/26',
             '    gateway 172.23.31.2',
-            ('post-up route add -net 10.0.0.0 netmask 255.240.0.0 gw '
+            ('post-up route add -net 10.0.0.0/12 gw '
              '172.23.31.1 metric 0 || true'),
-            ('pre-down route del -net 10.0.0.0 netmask 255.240.0.0 gw '
+            ('pre-down route del -net 10.0.0.0/12 gw '
              '172.23.31.1 metric 0 || true'),
-            ('post-up route add -net 192.168.2.0 netmask 255.255.0.0 gw '
+            ('post-up route add -net 192.168.2.0/16 gw '
              '172.23.31.1 metric 0 || true'),
-            ('pre-down route del -net 192.168.2.0 netmask 255.255.0.0 gw '
+            ('pre-down route del -net 192.168.2.0/16 gw '
              '172.23.31.1 metric 0 || true'),
-            ('post-up route add -net 10.0.200.0 netmask 255.255.0.0 gw '
+            ('post-up route add -net 10.0.200.0/16 gw '
              '172.23.31.1 metric 1 || true'),
-            ('pre-down route del -net 10.0.200.0 netmask 255.255.0.0 gw '
+            ('pre-down route del -net 10.0.200.0/16 gw '
              '172.23.31.1 metric 1 || true'),
         ]
         found = files['/etc/network/interfaces'].splitlines()
@@ -3588,6 +3634,77 @@ class TestEniRoundTrip(CiTestCase):
         self.assertEqual(
             expected, [line for line in found if line])
 
+    def test_ipv6_static_routes(self):
+        # as reported in bug 1818669
+        conf = [
+            {'name': 'eno3', 'type': 'physical',
+             'subnets': [{
+                 'address': 'fd00::12/64',
+                 'dns_nameservers': ['fd00:2::15'],
+                 'gateway': 'fd00::1',
+                 'ipv6': True,
+                 'type': 'static',
+                 'routes': [{'netmask': '32',
+                             'network': 'fd00:12::',
+                             'gateway': 'fd00::2'},
+                            {'network': 'fd00:14::',
+                             'gateway': 'fd00::3'},
+                            {'destination': 'fe00:14::/48',
+                             'gateway': 'fe00::4',
+                             'metric': 500},
+                            {'gateway': '192.168.23.1',
+                             'metric': 999,
+                             'netmask': 24,
+                             'network': '192.168.23.0'},
+                            {'destination': '10.23.23.0/24',
+                             'gateway': '10.23.23.2',
+                             'metric': 300}]}]},
+        ]
+
+        files = self._render_and_read(
+            network_config={'config': conf, 'version': 1})
+        expected = [
+            'auto lo',
+            'iface lo inet loopback',
+            'auto eno3',
+            'iface eno3 inet6 static',
+            '    address fd00::12/64',
+            '    dns-nameservers fd00:2::15',
+            '    gateway fd00::1',
+            ('    post-up route add -A inet6 fd00:12::/32 gw '
+             'fd00::2 || true'),
+            ('    pre-down route del -A inet6 fd00:12::/32 gw '
+             'fd00::2 || true'),
+            ('    post-up route add -A inet6 fd00:14::/64 gw '
+             'fd00::3 || true'),
+            ('    pre-down route del -A inet6 fd00:14::/64 gw '
+             'fd00::3 || true'),
+            ('    post-up route add -A inet6 fe00:14::/48 gw '
+             'fe00::4 metric 500 || true'),
+            ('    pre-down route del -A inet6 fe00:14::/48 gw '
+             'fe00::4 metric 500 || true'),
+            ('    post-up route add -net 192.168.23.0/24 gw '
+             '192.168.23.1 metric 999 || true'),
+            ('    pre-down route del -net 192.168.23.0/24 gw '
+             '192.168.23.1 metric 999 || true'),
+            ('    post-up route add -net 10.23.23.0/24 gw '
+             '10.23.23.2 metric 300 || true'),
+            ('    pre-down route del -net 10.23.23.0/24 gw '
+             '10.23.23.2 metric 300 || true'),
+
+        ]
+        found = files['/etc/network/interfaces'].splitlines()
+
+        self.assertEqual(
+            expected, [line for line in found if line])
+
+    def testsimple_render_bond(self):
+        entry = NETWORK_CONFIGS['bond']
+        files = self._render_and_read(network_config=yaml.load(entry['yaml']))
+        self.assertEqual(
+            entry['expected_eni'].splitlines(),
+            files['/etc/network/interfaces'].splitlines())
+
 
 class TestNetRenderers(CiTestCase):
     @mock.patch("cloudinit.net.renderers.sysconfig.available")