Merge ~chad.smith/cloud-init:bug/azure-dhcp6-metric into cloud-init:master

[Chad Smith]

azure: support matching dhcp route-metrics for dual-stack ipv4 ipv6

Network v2 configuration for Azure will set both dhcp4 and
dhcp6 to False by default.

When IPv6 privateIpAddresses are present for an interface in Azure's
Instance Metadata Service (IMDS), set dhcp6: True and provide a
route-metric value that will match the corresponding dhcp4 route-metric.
The route-metric value will increase by 100 for each additional
interface present to ensure the primary interface has a route to IMDS.

Also fix dhcp route-metric rendering for eni and sysconfig distros.

LP: #1850308

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

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

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

review: Needs Fixing (continuous-integration)

[Ryan Harper (raharper)]

Can we split this into two branches?

1) when we write dhcp4-overrides, write dhcp6-overrides as well

% git diff cloudinit/sources/DataSourceAzure.py
diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index 4984fa8..2c46935 100755
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -1341,6 +1341,10 @@ def parse_network_config(imds_metadata):
                             # primary nic due to lower route-metric value
                             dev_config['dhcp4-overrides'] = {
                                 'route-metric': (idx + 1) * 100}
+ # netplan requires v4 and v6 overrides to match
+ # it's harmless to include v6 override
+ dev_config['dhcp6-overrides'] = {
+ 'route-metric': (idx + 1) * 100}

2) in a separate branch, we can enhance ipv6 config to add secondary
ipv6 static ips if present.

Let me know if I'm missing something that prevents us from doing (1) and (2) separately.

review: Needs Fixing

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

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

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

An updated script I'm trying to adapt to setup a nic with ipv4 and ipv6 addrs. nearly there
http://paste.ubuntu.com/p/DHGj6Z8fzJ/

[Ryan Harper (raharper)]

I think we can simplify the logic if we have a common route-metric value and set the dhcpX-override key when we enable dhcp4 or dhcp6. See inline comment.

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

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

review: Approve (continuous-integration)

[Chad Smith (chad.smith)]

netplan on a clean azure dual stack ipv4/ipv6 dhcp vm https://pastebin.canonical.com/p/GM8B9ZFQST/

[Chad Smith (chad.smith)]

cloud-init generated yaml on an Azure ipv4/ipv6 vm with this fix
 Azure generated netplan yaml for dual stack ipv4/ipv6 dhcp

# This file is generated from information provided by
# the datasource. Changes to it will not persist across an instance.
# To disable cloud-init's network configuration capabilities, write a file
# /etc/cloud/cloud.cfg.d/99-disable-network-config.cfg with the following:
# network: {config: disabled}
network:
    ethernets:
        eth0:
            dhcp4: true
            dhcp4-overrides: &id001
                route-metric: 100
            dhcp6: true
            dhcp6-overrides: *id001
            match:
                macaddress: 00:0d:3a:03:63:b7
            set-name: eth0
    version: 2

[Ryan Harper (raharper)]

review: Needs Fixing

[Ryan Harper (raharper)]

Hrm, the yaml references look odd. The netplan renderer uses safeyaml.dumps whith noalias=True.

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

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

And we need v2 -> (eni, sysconfig) to accept the metric value from dhcpX-overrides

This should do it:

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

review: Needs Fixing

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

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

review: Approve (continuous-integration)

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

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

review: Approve (continuous-integration)

[Ryan Harper (raharper)]

Thanks. This looks good. Let's update the commit message to match what we're doing.

[Ryan Harper (raharper)]

Thanks Chad!

review: Approve

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

Commit message lints:
- Line #7 has 62 too many characters. Line starts with: "route-metric value that"...

review: Needs Fixing

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

review: Approve (continuous-integration)

diff --git a/cloudinit/net/network_state.py b/cloudinit/net/network_state.py
index ba85c69..20b7716 100644
--- a/cloudinit/net/network_state.py
+++ b/cloudinit/net/network_state.py
@@ -22,8 +22,9 @@ NETWORK_STATE_REQUIRED_KEYS = {
     1: ['version', 'config', 'network_state'],
 }
 NETWORK_V2_KEY_FILTER = [
-    'addresses', 'dhcp4', 'dhcp6', 'gateway4', 'gateway6', 'interfaces',
-    'match', 'mtu', 'nameservers', 'renderer', 'set-name', 'wakeonlan'
+    'addresses', 'dhcp4', 'dhcp4-overrides', 'dhcp6', 'dhcp6-overrides',
+    'gateway4', 'gateway6', 'interfaces', 'match', 'mtu', 'nameservers',
+    'renderer', 'set-name', 'wakeonlan'
 ]
 
 NET_CONFIG_TO_V2 = {
@@ -747,12 +748,20 @@ class NetworkStateInterpreter(object):
     def _v2_to_v1_ipcfg(self, cfg):
         """Common ipconfig extraction from v2 to v1 subnets array."""
 
+        def _add_dhcp_overrides(overrides, subnet):
+            if 'route-metric' in overrides:
+                subnet['metric'] = overrides['route-metric']
+
         subnets = []
         if cfg.get('dhcp4'):
-            subnets.append({'type': 'dhcp4'})
+            subnet = {'type': 'dhcp4'}
+            _add_dhcp_overrides(cfg.get('dhcp4-overrides', {}), subnet)
+            subnets.append(subnet)
         if cfg.get('dhcp6'):
+            subnet = {'type': 'dhcp6'}
             self.use_ipv6 = True
-            subnets.append({'type': 'dhcp6'})
+            _add_dhcp_overrides(cfg.get('dhcp6-overrides', {}), subnet)
+            subnets.append(subnet)
 
         gateway4 = None
         gateway6 = None
diff --git a/cloudinit/net/sysconfig.py b/cloudinit/net/sysconfig.py
index 6717d92..fe0c67c 100644
--- a/cloudinit/net/sysconfig.py
+++ b/cloudinit/net/sysconfig.py
@@ -395,6 +395,9 @@ class Renderer(renderer.Renderer):
         ipv6_index = -1
         for i, subnet in enumerate(subnets, start=len(iface_cfg.children)):
             subnet_type = subnet.get('type')
+            # metric may apply to both dhcp and static config
+            if 'metric' in subnet:
+                iface_cfg['METRIC'] = subnet['metric']
             if subnet_type in ['dhcp', 'dhcp4', 'dhcp6']:
                 if has_default_route and iface_cfg['BOOTPROTO'] != 'none':
                     iface_cfg['DHCLIENT_SET_DEFAULT_ROUTE'] = False
@@ -426,9 +429,6 @@ class Renderer(renderer.Renderer):
                     else:
                         iface_cfg['GATEWAY'] = subnet['gateway']
 
-                if 'metric' in subnet:
-                    iface_cfg['METRIC'] = subnet['metric']
-
                 if 'dns_search' in subnet:
                     iface_cfg['DOMAIN'] = ' '.join(subnet['dns_search'])
 
diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index cdf49d3..44cca21 100755
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -1322,7 +1322,8 @@ def parse_network_config(imds_metadata):
             network_metadata = imds_metadata['network']
             for idx, intf in enumerate(network_metadata['interface']):
                 nicname = 'eth{idx}'.format(idx=idx)
-                dev_config = {}
+                dev_config = {'dhcp4': False, 'dhcp6': False}
+                dhcp_override = {'route-metric': (idx + 1) * 100}
                 for addr4 in intf['ipv4']['ipAddress']:
                     privateIpv4 = addr4['privateIpAddress']
                     if privateIpv4:
@@ -1340,12 +1341,15 @@ def parse_network_config(imds_metadata):
                             # non-primary interfaces should have a higher
                             # route-metric (cost) so default routes prefer
                             # primary nic due to lower route-metric value
-                            dev_config['dhcp4-overrides'] = {
-                                'route-metric': (idx + 1) * 100}
+                            dev_config['dhcp4-overrides'] = dhcp_override
                 for addr6 in intf['ipv6']['ipAddress']:
                     privateIpv6 = addr6['privateIpAddress']
                     if privateIpv6:
                         dev_config['dhcp6'] = True
+                        # non-primary interfaces should have a higher
+                        # route-metric (cost) so default routes prefer
+                        # primary nic due to lower route-metric value
+                        dev_config['dhcp6-overrides'] = dhcp_override
                         break
                 if dev_config:
                     mac = ':'.join(re.findall(r'..', intf['macAddress']))
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 80c6f01..d92d7b2 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -153,6 +153,102 @@ SECONDARY_INTERFACE = {
 MOCKPATH = 'cloudinit.sources.DataSourceAzure.'
 
 
+class TestParseNetworkConfig(CiTestCase):
+
+    maxDiff = None
+
+    def test_single_ipv4_nic_configuration(self):
+        """parse_network_config emits dhcp on single nic with ipv4"""
+        expected = {'ethernets': {
+            'eth0': {'dhcp4': True,
+                     'dhcp4-overrides': {'route-metric': 100},
+                     'dhcp6': False,
+                     'match': {'macaddress': '00:0d:3a:04:75:98'},
+                     'set-name': 'eth0'}}, 'version': 2}
+        self.assertEqual(expected, dsaz.parse_network_config(NETWORK_METADATA))
+
+    def test_increases_route_metric_for_non_primary_nics(self):
+        """parse_network_config increases route-metric for each nic"""
+        expected = {'ethernets': {
+            'eth0': {'dhcp4': True,
+                     'dhcp4-overrides': {'route-metric': 100},
+                     'dhcp6': False,
+                     'match': {'macaddress': '00:0d:3a:04:75:98'},
+                     'set-name': 'eth0'},
+            'eth1': {'set-name': 'eth1',
+                     'match': {'macaddress': '22:0d:3a:04:75:98'},
+                     'dhcp6': False,
+                     'dhcp4': True,
+                     'dhcp4-overrides': {'route-metric': 200}},
+            'eth2': {'set-name': 'eth2',
+                     'match': {'macaddress': '33:0d:3a:04:75:98'},
+                     'dhcp6': False,
+                     'dhcp4': True,
+                     'dhcp4-overrides': {'route-metric': 300}}}, 'version': 2}
+        imds_data = copy.deepcopy(NETWORK_METADATA)
+        imds_data['network']['interface'].append(SECONDARY_INTERFACE)
+        third_intf = copy.deepcopy(SECONDARY_INTERFACE)
+        third_intf['macAddress'] = third_intf['macAddress'].replace('22', '33')
+        third_intf['ipv4']['subnet'][0]['address'] = '10.0.2.0'
+        third_intf['ipv4']['ipAddress'][0]['privateIpAddress'] = '10.0.2.6'
+        imds_data['network']['interface'].append(third_intf)
+        self.assertEqual(expected, dsaz.parse_network_config(imds_data))
+
+    def test_ipv4_and_ipv6_route_metrics_match_for_nics(self):
+        """parse_network_config emits matching ipv4 and ipv6 route-metrics."""
+        expected = {'ethernets': {
+            'eth0': {'dhcp4': True,
+                     'dhcp4-overrides': {'route-metric': 100},
+                     'dhcp6': False,
+                     'match': {'macaddress': '00:0d:3a:04:75:98'},
+                     'set-name': 'eth0'},
+            'eth1': {'set-name': 'eth1',
+                     'match': {'macaddress': '22:0d:3a:04:75:98'},
+                     'dhcp4': True,
+                     'dhcp6': False,
+                     'dhcp4-overrides': {'route-metric': 200}},
+            'eth2': {'set-name': 'eth2',
+                     'match': {'macaddress': '33:0d:3a:04:75:98'},
+                     'dhcp4': True,
+                     'dhcp4-overrides': {'route-metric': 300},
+                     'dhcp6': True,
+                     'dhcp6-overrides': {'route-metric': 300}}}, 'version': 2}
+        imds_data = copy.deepcopy(NETWORK_METADATA)
+        imds_data['network']['interface'].append(SECONDARY_INTERFACE)
+        third_intf = copy.deepcopy(SECONDARY_INTERFACE)
+        third_intf['macAddress'] = third_intf['macAddress'].replace('22', '33')
+        third_intf['ipv4']['subnet'][0]['address'] = '10.0.2.0'
+        third_intf['ipv4']['ipAddress'][0]['privateIpAddress'] = '10.0.2.6'
+        third_intf['ipv6'] = {
+            "subnet": [{"prefix": "64", "address": "2001:dead:beef::2"}],
+            "ipAddress": [{"privateIpAddress": "2001:dead:beef::1"}]
+        }
+        imds_data['network']['interface'].append(third_intf)
+        self.assertEqual(expected, dsaz.parse_network_config(imds_data))
+
+    def test_ipv4_secondary_ips_will_be_static_addrs(self):
+        """parse_network_config emits primary ipv4 as dhcp others are static"""
+        expected = {'ethernets': {
+            'eth0': {'addresses': ['10.0.0.5/24'],
+                     'dhcp4': True,
+                     'dhcp4-overrides': {'route-metric': 100},
+                     'dhcp6': True,
+                     'dhcp6-overrides': {'route-metric': 100},
+                     'match': {'macaddress': '00:0d:3a:04:75:98'},
+                     'set-name': 'eth0'}}, 'version': 2}
+        imds_data = copy.deepcopy(NETWORK_METADATA)
+        nic1 = imds_data['network']['interface'][0]
+        nic1['ipv4']['ipAddress'].append({'privateIpAddress': '10.0.0.5'})
+
+        # Secondary ipv6 addresses currently ignored/unconfigured
+        nic1['ipv6'] = {
+            "subnet": [{"prefix": "10", "address": "2001:dead:beef::16"}],
+            "ipAddress": [{"privateIpAddress": "2001:dead:beef::1"},
+                          {"privateIpAddress": "2001:dead:beef::2"}]
+        }
+        self.assertEqual(expected, dsaz.parse_network_config(imds_data))
+
+
 class TestGetMetadataFromIMDS(HttprettyTestCase):
 
     with_logs = True
@@ -641,6 +737,7 @@ scbus-1 on xpt0 bus 0
             'ethernets': {
                 'eth0': {'set-name': 'eth0',
                          'match': {'macaddress': '00:0d:3a:04:75:98'},
+                         'dhcp6': False,
                          'dhcp4': True,
                          'dhcp4-overrides': {'route-metric': 100}}},
             'version': 2}
@@ -658,14 +755,17 @@ scbus-1 on xpt0 bus 0
             'ethernets': {
                 'eth0': {'set-name': 'eth0',
                          'match': {'macaddress': '00:0d:3a:04:75:98'},
+                         'dhcp6': False,
                          'dhcp4': True,
                          'dhcp4-overrides': {'route-metric': 100}},
                 'eth1': {'set-name': 'eth1',
                          'match': {'macaddress': '22:0d:3a:04:75:98'},
+                         'dhcp6': False,
                          'dhcp4': True,
                          'dhcp4-overrides': {'route-metric': 200}},
                 'eth2': {'set-name': 'eth2',
                          'match': {'macaddress': '33:0d:3a:04:75:98'},
+                         'dhcp6': False,
                          'dhcp4': True,
                          'dhcp4-overrides': {'route-metric': 300}}},
             'version': 2}
@@ -999,6 +1099,7 @@ scbus-1 on xpt0 bus 0
             'ethernets': {
                 'eth0': {'dhcp4': True,
                          'dhcp4-overrides': {'route-metric': 100},
+                         'dhcp6': False,
                          'match': {'macaddress': '00:0d:3a:04:75:98'},
                          'set-name': 'eth0'}},
             'version': 2}
diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py
index 6f83ad7..35ce55d 100644
--- a/tests/unittests/test_net.py
+++ b/tests/unittests/test_net.py
@@ -3101,6 +3101,36 @@ USERCTL=no
         self._compare_files_to_expected(
             expected, self._render_and_read(network_config=v2data))
 
+    def test_from_v2_route_metric(self):
+        """verify route-metric gets rendered on nic when source is netplan."""
+        overrides = {'route-metric': 100}
+        v2base = {
+            'version': 2,
+            'ethernets': {
+                'eno1': {'dhcp4': True,
+                         'match': {'macaddress': '07-1c-c6-75-a4-be'}}}}
+        expected = {
+            'ifcfg-eno1': textwrap.dedent("""\
+                BOOTPROTO=dhcp
+                DEVICE=eno1
+                HWADDR=07-1c-c6-75-a4-be
+                METRIC=100
+                NM_CONTROLLED=no
+                ONBOOT=yes
+                STARTMODE=auto
+                TYPE=Ethernet
+                USERCTL=no
+                """),
+            }
+        for dhcp_ver in ('dhcp4', 'dhcp6'):
+            v2data = copy.deepcopy(v2base)
+            if dhcp_ver == 'dhcp6':
+                expected['ifcfg-eno1'] += "IPV6INIT=yes\nDHCPV6C=yes\n"
+            v2data['ethernets']['eno1'].update(
+                {dhcp_ver: True, '{0}-overrides'.format(dhcp_ver): overrides})
+            self._compare_files_to_expected(
+                expected, self._render_and_read(network_config=v2data))
+
 
 class TestOpenSuseSysConfigRendering(CiTestCase):
 
@@ -3466,6 +3496,30 @@ iface eth0 inet dhcp
         self.assertEqual(
             expected, dir2dict(tmp_dir)['/etc/network/interfaces'])
 
+    def test_v2_route_metric_to_eni(self):
+        """Network v2 route-metric overrides are preserved in eni output"""
+        tmp_dir = self.tmp_dir()
+        renderer = eni.Renderer()
+        expected_tmpl = textwrap.dedent("""\
+            auto lo
+            iface lo inet loopback
+
+            auto eth0
+            iface eth0 inet{suffix} dhcp
+                metric 100
+            """)
+        for dhcp_ver in ('dhcp4', 'dhcp6'):
+            suffix = '6' if dhcp_ver == 'dhcp6' else ''
+            dhcp_cfg = {
+                dhcp_ver: True,
+                '{ver}-overrides'.format(ver=dhcp_ver): {'route-metric': 100}}
+            v2_input = {'version': 2, 'ethernets': {'eth0': dhcp_cfg}}
+            ns = network_state.parse_net_config_data(v2_input)
+            renderer.render_network_state(ns, target=tmp_dir)
+            self.assertEqual(
+                expected_tmpl.format(suffix=suffix),
+                dir2dict(tmp_dir)['/etc/network/interfaces'])
+
 
 class TestNetplanNetRendering(CiTestCase):