Fix extra ipv6 issues, code reduction and simplification (#1243)

Eliminated the duplicate code and now run the entire configuration
routine against both public and private interfaces.
Also addressed an inconsistency from our metadata api for ipv6
address configuration.

2 files changed, 84 insertions, 96 deletions

diff --git a/cloudinit/sources/helpers/vultr.py b/cloudinit/sources/helpers/vultr.py
index 190a5640..88a21034 100644
--- a/cloudinit/sources/helpers/vultr.py
+++ b/cloudinit/sources/helpers/vultr.py
@@ -7,7 +7,7 @@ from functools import lru_cache
 
 from cloudinit import dmi
 from cloudinit import log as log
-from cloudinit import net, netinfo, subp, url_helper, util
+from cloudinit import net, subp, url_helper, util
 from cloudinit.net.dhcp import EphemeralDHCPv4, NoDHCPLeaseError
 
 # Get LOG
@@ -30,9 +30,6 @@ def get_metadata(url, timeout, retries, sec_between, agent):
             with EphemeralDHCPv4(
                 iface=iface[0], connectivity_url_data={"url": url}
             ):
-                # Set metadata route
-                set_route(iface[0])
-
                 # Fetch the metadata
                 v1 = read_metadata(url, timeout, retries, sec_between, agent)
 
@@ -43,49 +40,6 @@ def get_metadata(url, timeout, retries, sec_between, agent):
     raise exception
 
 
-# Set route for metadata
-def set_route(iface):
-    # Get routes, confirm entry does not exist
-    routes = netinfo.route_info()
-
-    # If no tools exist and empty dict is returned
-    if "ipv4" not in routes:
-        return
-
-    # We only care about IPv4
-    routes = routes["ipv4"]
-
-    # Searchable list
-    dests = []
-
-    # Parse each route into a more searchable format
-    for route in routes:
-        dests.append(route["destination"])
-
-    gw_present = "100.64.0.0" in dests or "100.64.0.0/10" in dests
-    dest_present = "169.254.169.254" in dests
-
-    # If not IPv6 only (No link local)
-    # or the route is already present
-    if not gw_present or dest_present:
-        return
-
-    # Set metadata route
-    if subp.which("ip"):
-        subp.subp(
-            [
-                "ip",
-                "route",
-                "add",
-                "169.254.169.254/32",
-                "dev",
-                iface,
-            ]
-        )
-    elif subp.which("route"):
-        subp.subp(["route", "add", "-net", "169.254.169.254/32", "100.64.0.1"])
-
-
 # Read the system information from SMBIOS
 def get_sysinfo():
     return {
@@ -165,19 +119,18 @@ def generate_network_config(interfaces):
 
     # Prepare interface 0, public
     if len(interfaces) > 0:
-        public = generate_public_network_interface(interfaces[0])
+        public = generate_interface(interfaces[0], primary=True)
         network["config"].append(public)
 
     # Prepare additional interfaces, private
     for i in range(1, len(interfaces)):
-        private = generate_private_network_interface(interfaces[i])
+        private = generate_interface(interfaces[i])
         network["config"].append(private)
 
     return network
 
 
-# Input Metadata and generate public network config part
-def generate_public_network_interface(interface):
+def generate_interface(interface, primary=False):
     interface_name = get_interface_name(interface["mac"])
     if not interface_name:
         raise RuntimeError(
@@ -188,13 +141,33 @@ def generate_public_network_interface(interface):
         "name": interface_name,
         "type": "physical",
         "mac_address": interface["mac"],
-        "accept-ra": 1,
-        "subnets": [
+    }
+
+    if primary:
+        netcfg["accept-ra"] = 1
+        netcfg["subnets"] = [
             {"type": "dhcp", "control": "auto"},
             {"type": "ipv6_slaac", "control": "auto"},
-        ],
-    }
+        ]
+
+    if not primary:
+        netcfg["subnets"] = [
+            {
+                "type": "static",
+                "control": "auto",
+                "address": interface["ipv4"]["address"],
+                "netmask": interface["ipv4"]["netmask"],
+            }
+        ]
 
+    generate_interface_routes(interface, netcfg)
+    generate_interface_additional_addresses(interface, netcfg)
+
+    # Add config to template
+    return netcfg
+
+
+def generate_interface_routes(interface, netcfg):
     # Options that may or may not be used
     if "mtu" in interface:
         netcfg["mtu"] = interface["mtu"]
@@ -205,6 +178,8 @@ def generate_public_network_interface(interface):
     if "routes" in interface:
         netcfg["subnets"][0]["routes"] = interface["routes"]
 
+
+def generate_interface_additional_addresses(interface, netcfg):
     # Check for additional IP's
     additional_count = len(interface["ipv4"]["additional"])
     if "ipv4" in interface and additional_count > 0:
@@ -228,8 +203,8 @@ def generate_public_network_interface(interface):
             add = {
                 "type": "static6",
                 "control": "auto",
-                "address": additional["address"],
-                "netmask": additional["netmask"],
+                "address": "%s/%s"
+                % (additional["network"], additional["prefix"]),
             }
 
             if "routes" in additional:
@@ -237,44 +212,6 @@ def generate_public_network_interface(interface):
 
             netcfg["subnets"].append(add)
 
-    # Add config to template
-    return netcfg
-
-
-# Input Metadata and generate private network config part
-def generate_private_network_interface(interface):
-    interface_name = get_interface_name(interface["mac"])
-    if not interface_name:
-        raise RuntimeError(
-            "Interface: %s could not be found on the system" % interface["mac"]
-        )
-
-    netcfg = {
-        "name": interface_name,
-        "type": "physical",
-        "mac_address": interface["mac"],
-        "subnets": [
-            {
-                "type": "static",
-                "control": "auto",
-                "address": interface["ipv4"]["address"],
-                "netmask": interface["ipv4"]["netmask"],
-            }
-        ],
-    }
-
-    # Options that may or may not be used
-    if "mtu" in interface:
-        netcfg["mtu"] = interface["mtu"]
-
-    if "accept-ra" in interface:
-        netcfg["accept-ra"] = interface["accept-ra"]
-
-    if "routes" in interface:
-        netcfg["subnets"][0]["routes"] = interface["routes"]
-
-    return netcfg
-
 
 # Make required adjustments to the network configs provided
 def add_interface_names(interfaces):
diff --git a/tests/unittests/sources/test_vultr.py b/tests/unittests/sources/test_vultr.py
index 21d5bc17..18b2c084 100644
--- a/tests/unittests/sources/test_vultr.py
+++ b/tests/unittests/sources/test_vultr.py
@@ -8,6 +8,7 @@
 import json
 
 from cloudinit import helpers, settings
+from cloudinit.net.dhcp import NoDHCPLeaseError
 from cloudinit.sources import DataSourceVultr
 from cloudinit.sources.helpers import vultr
 from tests.unittests.helpers import CiTestCase, mock
@@ -95,7 +96,9 @@ VULTR_V1_2 = {
                 "netmask": "255.255.254.0",
             },
             "ipv6": {
-                "additional": [],
+                "additional": [
+                    {"network": "2002:19f0:5:28a7::", "prefix": "64"}
+                ],
                 "address": "2001:19f0:5:28a7:5400:03ff:fe1b:4eca",
                 "network": "2001:19f0:5:28a7::",
                 "prefix": "64",
@@ -138,6 +141,14 @@ VULTR_V1_2 = {
 
 SSH_KEYS_1 = ["ssh-rsa AAAAB3NzaC1y...IQQhv5PAOKaIl+mM3c= test3@key"]
 
+INTERFACES = [
+    ["lo", "56:00:03:15:c4:00", "drv", "devid0"],
+    ["dummy0", "56:00:03:15:c4:01", "drv", "devid1"],
+    ["eth1", "56:00:03:15:c4:02", "drv", "devid2"],
+    ["eth0", "56:00:03:15:c4:04", "drv", "devid4"],
+    ["eth2", "56:00:03:15:c4:03", "drv", "devid3"],
+]
+
 # Expected generated objects
 
 # Expected config
@@ -182,6 +193,11 @@ EXPECTED_VULTR_NETWORK_2 = {
             "subnets": [
                 {"type": "dhcp", "control": "auto"},
                 {"type": "ipv6_slaac", "control": "auto"},
+                {
+                    "type": "static6",
+                    "control": "auto",
+                    "address": "2002:19f0:5:28a7::/64",
+                },
             ],
         },
         {
@@ -208,6 +224,9 @@ INTERFACE_MAP = {
 }
 
 
+EPHERMERAL_USED = ""
+
+
 class TestDataSourceVultr(CiTestCase):
     def setUp(self):
         super(TestDataSourceVultr, self).setUp()
@@ -284,5 +303,37 @@ class TestDataSourceVultr(CiTestCase):
             EXPECTED_VULTR_NETWORK_2, vultr.generate_network_config(interf)
         )
 
+    def ephemeral_init(self, iface="", connectivity_url_data=None):
+        global EPHERMERAL_USED
+        EPHERMERAL_USED = iface
+        if iface == "eth0":
+            return
+        raise NoDHCPLeaseError("Generic for testing")
+
+    # Test interface seeking to ensure we are able to find the correct one
+    @mock.patch("cloudinit.net.dhcp.EphemeralDHCPv4.__init__", ephemeral_init)
+    @mock.patch("cloudinit.sources.helpers.vultr.is_vultr")
+    @mock.patch("cloudinit.sources.helpers.vultr.read_metadata")
+    @mock.patch("cloudinit.net.get_interfaces")
+    def test_interface_seek(
+        self, mock_get_interfaces, mock_read_metadata, mock_isvultr
+    ):
+        mock_read_metadata.side_effect = NoDHCPLeaseError(
+            "Generic for testing"
+        )
+        mock_isvultr.return_value = True
+        mock_get_interfaces.return_value = INTERFACES
+
+        source = DataSourceVultr.DataSourceVultr(
+            settings.CFG_BUILTIN, None, helpers.Paths({"run_dir": self.tmp})
+        )
+
+        try:
+            source._get_data()
+        except Exception:
+            pass
+
+        self.assertEqual(EPHERMERAL_USED, INTERFACES[3][0])
+
 
 # vi: ts=4 expandtab