Merge ~utlemming/cloud-init/+git/cloud-init:digitalocean_networking into cloud-init:master

[Ben Howard]

On DigitalOcean, the meta-data provides network configuration on a link-local address (ip4LL). This change makes uses Cloud-init's ability to setup netwokrking from data provided in the meta-data.

Rather than using avahi-autoipd (and thus adding an additional depedency on Cloud-init, a random link-local address is chosen. DigitalOcean's networking guarantees that the entire 169.254.0.0/16 net block is private to the fabric and the droplet. Therefore, any arbitrary address in 169.254.0.0/16 can be used (with the notable exception of 169.254.269.254). That said, the datasource will select a random address between 169.254.[1-168[.[0-254], query the metadata service and then flush the interface.

Included in this merge proposal is an updated test case to use actual meta-data from a droplet.

[Scott Moser (smoser)]

Over all, this is really nice. Its mostly exactly what i would like to have network datasources doing.

General flow in the 'get' for your datasource then looks like:

a.) quickly check (ie, via dmi info) if you are on the cloud. if not rreturn
b.) you are in all likely hood on your cloud, so you this datasource is in charge
c.) configure local networking transiently as you did
d.) read network metadata service
e.) un-do your network changes

there are some nits in line below, and please make sure that in the local datasource 'get', you're un-configuring your ipv4 link local addresses.

Also, need some unit tests. the unit tests can focus on parse_network_configuration as that is the brunt of the work.

review: Needs Fixing

[Scott Moser (smoser)]

fyi, you can push to
 git+ssh://<email address hidden>/~utlemming/cloud-init

then its referencable via shorter url of
 https://code.launchpad.net/~utlemming/cloud-init/
rather than
 https://code.launchpad.net/~utlemming/cloud-init/

see Repository Urls at
  https://help.launchpad.net/Code/Git
for more information.

[Scott Moser (smoser)]

fixing previous comment

fyi, you can push to
 git+ssh://<email address hidden>/~utlemming/cloud-init

then its referencable via shorter url of
 https://code.launchpad.net/~utlemming/cloud-init/
compared to
 https://code.launchpad.net/~utlemming/cloud-init/+git/cloud-init

The first is the "default" repository for cloud-init for you. You can have multiples under the +git/.

see Repository Urls at
  https://help.launchpad.net/Code/Git
for more information.

using the shorter form allows me to do something like:

for user in lpuser1 lpuser2 utlemming; do
   git remote add $user git://git.launchpad.net/~$user/cloud-init
   git fetch $user
done

739d113... by Ben Howard on 2016-08-30

Minors changes per the MP.
- fixed function names
- changed scope of some messages

[Ben Howard (utlemming)]

Thanks Scott for the review. I just got back from vacation. I've fixed the nit picks and will look at getting the Unit tests done shortly.

In the meantime, I've updated my branch.

4d78b9f... by Ben Howard on 2016-08-31

DigitalOcean: add test cases for networking config

[Ben Howard (utlemming)]

Added test cases for the networking configuration.

[Ben Howard (utlemming)]

Also, I've push the branch to:
git+ssh://<email address hidden>/~utlemming/cloud-init

I figured I would leave this MP open rather than creating a new one so that the conversation wasn't lost.

[Scott Moser (smoser)]

Some changes i made at http://paste.ubuntu.com/23146037/ that i'd like you to take.
I've pasted the comments http://paste.ubuntu.com/23146043/ in case this doesn't format well.

Over all, this is good, and thank you.

Comments below, where i put 'FIX' i'd like you to make sure that things are as you'd expect
and then a 'TEST' comment, please test that.

 - DataSourceDigitalOcean.get()
 - explicitly populate the self.metadata from keys in the full
     digital ocean metadata rather than assuming that values are there, and
     not having a explicit list of what we expect. FIX: there may be other
     values that were implied by copying the metadata to .metadata, please
     check that.

   this also allowed me to drop get_public_ssh_keys and availability
     zone methods.

   - changed
        dns = self.metadata.get('dns')
        nameservers = dns.get('nameservers')
        self._network_config = do_helper.parse_network_configuration(
     to
        nameservers = self.metadata_full['dns']['nameservers']

     that would fail badly if metadata might not have a 'dns' entry,
     but really no worse as before, since if there was None in 'dns' you'd
     get a TypeError on None.get('nameservers)'

   - put the reading of the metadata into a function in do_helpers.
   the value of doing this is that mocking the DataSource.get() is
     much easier as you can just mock read_metadata() and have it return
     what you expect.

   FIX: change your tests to just use mock and mock that rather than
     using httpretty.

 - fixed a logexc usage, improved another one with more useful information.
 - dropped use of 'util.which("ip")' in one case, rather just let
   execution find that.
 - renamed do_helper.parse_network_configuration to
   convert_network_configuration

 - TEST:
 You can easily add tests of the network configuration conversion
   that do not need the datasource infrastructure in place. Its good to
   have the test of the datasource but you can test more network config
   parsing directly by just calling it with digital ocean values and
   making expectations on the response.

  See my example TestNetworkConvert and how its easier than dealing with
  the datasource.

review: Needs Fixing

de8b146... by Ben Howard on 2016-09-13

Applied proposed patches from Scott Moser in MP

528541a... by Ben Howard on 2016-09-13

updated test cases to not use httpretty

a9d8221... by Ben Howard on 2016-09-14

explicitly test actual JSON for expected types and values

f274cfc... by Ben Howard on 2016-09-14

updated patch from MP for pylint/pep8 compliance

[Ben Howard (utlemming)]

okay, I think that the issues are addressed. I added a test case to explicitly test the JSON that is returned from the metadata service; the logic being that using actual JSON gives a better, real world test. I can change it be explicit, but would rather be testing against real data, not mocked up data.

This code is all tested as well.

[Scott Moser (smoser)]

I'm going to merge this now. But for your reference, the changes I've made are below.

They generally include:
 - move '_get_sysinfo' implementation into do_helper.read_sysinfo() just
   for easier patching
 - some modifications to tests
 - remove httppretty completely
 - drop your test 'TestDataSourceDigitalOcean'. I think you misunderstood
   what I was asking. I just wanted to actually use a response from
   the metadata service.
 - use of .copy() on DO_META so it didn't inadvertantly get updated.
 - drop no longer used 'apply_filter' on get_data()
 - drop 'launch_index' as that is inherited from DataSource.
 - flake8 related cleanups.
 - assertEqual rather than assertEquals
 - add negative test (test_returns_false_not_on_docean)
 - log critical *and* raise exception if DigitalOcean is found as
   system manufacture and droplet_id is not found.
 - some LOG usage cleanups (use LOG.debug("msg %s", key) rather than
   LOG.debug("msg {}".format(key)) as the ladder requires rendering/
   formatting to string of 'key'. and is also harder to make work
   with long messages.

diff --git a/cloudinit/sources/DataSourceDigitalOcean.py b/cloudinit/sources/DataSourceDigitalOcean.py
index 95a939f..c5770d5 100644
--- a/cloudinit/sources/DataSourceDigitalOcean.py
+++ b/cloudinit/sources/DataSourceDigitalOcean.py
@@ -54,38 +54,17 @@ class DataSourceDigitalOcean(sources.DataSource):
         self._network_config = None

     def _get_sysinfo(self):
- # DigitalOcean embeds vendor ID and instance/droplet_id in the
- # SMBIOS information
+ return do_helper.read_sysinfo()

- LOG.debug("checking if instance is a DigitalOcean droplet")
-
- # Detect if we are on DigitalOcean and return the Droplet's ID
- vendor_name = util.read_dmi_data("system-manufacturer")
- if vendor_name != "DigitalOcean":
- return (False, None)
-
- LOG.info("running on DigitalOcean")
-
- droplet_id = util.read_dmi_data("system-serial-number")
- if droplet_id:
- LOG.debug(("system identified via SMBIOS as DigitalOcean Droplet"
- "{}").format(droplet_id))
- else:
- LOG.critical(("system identified via SMBIOS as a DigitalOcean "
- "Droplet, but did not provide an ID. Please file a "
- "support ticket at: "
- "https://cloud.digitalocean.com/support/tickets/"
- "new"))
-
- return (True, droplet_id)
-
- def get_data(self, apply_filter=False):
+ def get_data(self):
         (is_do, droplet_id) = self._get_sysinfo()

         # only proceed if we know we are on DigitalOcean
         if not is_do:
             return False

+ LOG.info("Running on digital ocean. droplet_id=%s" % droplet_id)
+
         ipv4LL_nic = None
         if self.use_ip4LL:
             ipv4LL_nic = do_helper.assign_ipv4_link_local()
@@ -108,10 +87,6 @@ class DataSourceDigitalOcean(sources.DataSource):

         return True

- @property
- def launch_index(self):
- return None
-
     def check_instance_id(self, sys_cfg):
         return sources.instance_id_ma...

review: Approve

[Scott Moser (smoser)]

Wow.
More work... further reading of this.

I have a branch i'm happy with, and proposed merging into this one...
 https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/307093

the one question i have is in NIC_MAP.
the network data we get from digital ocean is like:
 {
  'public': [{'mac': '04:01...', ...}],
  'private': ['mac': '05:01...', ...]],
 }

the top level is a dictionary, but the second is a list.
Currently you assign the name by a map.
What if there are 2 entries in 'public'?

At this point they'd both get 'eth0' as their name.

review: Needs Information

[Ben Howard (utlemming)]

> the one question i have is in NIC_MAP.
> the network data we get from digital ocean is like:
> {
> 'public': [{'mac': '04:01...', ...}],
> 'private': ['mac': '05:01...', ...]],
> }
>
> the top level is a dictionary, but the second is a list.
> Currently you assign the name by a map.
> What if there are 2 entries in 'public'?

I can certify that there won't be, and if there are, then this is a _huge_ bug. We have notated that changes to the meta-data should be checked against Cloud-init first.

That said, I've double checked and I'm seeing (from actual droplets, i.e. 'curl http://169.254.169.254/metadata/v1.json | jq')

  "interfaces": {
    "public": [
      {
        "ipv4": {
          "ip_address": "REDACTED",
          "netmask": "255.255.240.0",
          "gateway": "159.203.96.1"
        },
        "anchor_ipv4": {
          "ip_address": "10.17.0.5",
          "netmask": "255.255.0.0",
          "gateway": "10.17.0.1"
        },
        "mac": "1e:9a:51:a2:e4:0a",
        "type": "public"
      }
    ]
  },

And
  "interfaces": {
    "private": [
      {
        "ipv4": {
          "ip_address": "10.132.84.146",
          "netmask": "255.255.0.0",
          "gateway": "10.132.0.1"
        },
        "mac": "96:b5:74:db:dd:85",
        "type": "private"
      }
    ],
    "public": [
      {
        "ipv4": {
          "ip_address": "REDACED",
          "netmask": "255.255.240.0",
          "gateway": "159.203.96.1"
        },
        "ipv6": {
          "ip_address": "REDACTED",
          "cidr": 64,
          "gateway": "2604:A880:0800:00A1:0000:0000:0000:0001"
        },
        "anchor_ipv4": {
          "ip_address": "10.17.0.6",
          "netmask": "255.255.0.0",
          "gateway": "10.17.0.1"
        },
        "mac": "5a:b1:61:a0:b8:98",
        "type": "public"
      }
    ]
  },

[Scott Moser (smoser)]

Yeah. Just odd that it is a list if there can not be more than one entry.

diff --git a/cloudinit/sources/DataSourceDigitalOcean.py b/cloudinit/sources/DataSourceDigitalOcean.py
index fc596e1..95a939f 100644
--- a/cloudinit/sources/DataSourceDigitalOcean.py
+++ b/cloudinit/sources/DataSourceDigitalOcean.py
@@ -18,13 +18,12 @@
 # DigitalOcean Droplet API:
 # https://developers.digitalocean.com/documentation/metadata/
 
-import json
-
 from cloudinit import log as logging
 from cloudinit import sources
-from cloudinit import url_helper
 from cloudinit import util
 
+import cloudinit.sources.helpers.digitalocean as do_helper
+
 LOG = logging.getLogger(__name__)
 
 BUILTIN_DS_CONFIG = {
@@ -36,11 +35,13 @@ BUILTIN_DS_CONFIG = {
 MD_RETRIES = 30
 MD_TIMEOUT = 2
 MD_WAIT_RETRY = 2
+MD_USE_IPV4LL = True
 
 
 class DataSourceDigitalOcean(sources.DataSource):
     def __init__(self, sys_cfg, distro, paths):
         sources.DataSource.__init__(self, sys_cfg, distro, paths)
+        self.distro = distro
         self.metadata = dict()
         self.ds_cfg = util.mergemanydict([
             util.get_cfg_by_path(sys_cfg, ["datasource", "DigitalOcean"], {}),
@@ -48,7 +49,9 @@ class DataSourceDigitalOcean(sources.DataSource):
         self.metadata_address = self.ds_cfg['metadata_url']
         self.retries = self.ds_cfg.get('retries', MD_RETRIES)
         self.timeout = self.ds_cfg.get('timeout', MD_TIMEOUT)
+        self.use_ip4LL = self.ds_cfg.get('use_ip4LL', MD_USE_IPV4LL)
         self.wait_retry = self.ds_cfg.get('wait_retry', MD_WAIT_RETRY)
+        self._network_config = None
 
     def _get_sysinfo(self):
         # DigitalOcean embeds vendor ID and instance/droplet_id in the
@@ -83,32 +86,27 @@ class DataSourceDigitalOcean(sources.DataSource):
         if not is_do:
             return False
 
-        LOG.debug("reading metadata from {}".format(self.metadata_address))
-        response = url_helper.readurl(self.metadata_address,
-                                      timeout=self.timeout,
-                                      sec_between=self.wait_retry,
-                                      retries=self.retries)
+        ipv4LL_nic = None
+        if self.use_ip4LL:
+            ipv4LL_nic = do_helper.assign_ipv4_link_local()
 
-        contents = util.decode_binary(response.contents)
-        decoded = json.loads(contents)
+        md = do_helper.read_metadata(
+            self.metadata_address, timeout=self.timeout,
+            sec_between=self.wait_retry, retries=self.retries)
 
-        self.metadata = decoded
-        self.metadata['instance-id'] = decoded.get('droplet_id', droplet_id)
-        self.metadata['local-hostname'] = decoded.get('hostname', droplet_id)
-        self.vendordata_raw = decoded.get("vendor_data", None)
-        self.userdata_raw = decoded.get("user_data", None)
-        return True
+        self.metadata_full = md
+        self.metadata['instance-id'] = md.get('droplet_id', droplet_id)
+        self.metadata['local-hostname'] = md.get('hostname', droplet_id)
+        self.metadata['interfaces'] = md.get('interfaces')
+        self.metadata['public-keys'] = md.get('public_keys')
+        self.metadata['availability_zone'] = md.get('region', 'default')
+        self.vendordata_raw = md.get("vendor_data", None)
+        self.userdata_raw = md.get("user_data", None)
 
-    def get_public_ssh_keys(self):
-        public_keys = self.metadata.get('public_keys', [])
-        if isinstance(public_keys, list):
-            return public_keys
-        else:
-            return [public_keys]
+        if ipv4LL_nic:
+            do_helper.del_ipv4_link_local(ipv4LL_nic)
 
-    @property
-    def availability_zone(self):
-        return self.metadata.get('region', 'default')
+        return True
 
     @property
     def launch_index(self):
@@ -118,10 +116,30 @@ class DataSourceDigitalOcean(sources.DataSource):
         return sources.instance_id_matches_system_uuid(
             self.get_instance_id(), 'system-serial-number')
 
+    @property
+    def network_config(self):
+        """Configure the networking. This needs to be done each boot, since
+           the IP information may have changed due to snapshot and/or
+           migration.
+        """
+
+        if self._network_config:
+            return self._network_config
+
+        interfaces = self.metadata.get('interfaces')
+        LOG.debug(interfaces)
+        if not interfaces:
+            raise Exception("Unable to get meta-data from server....")
+
+        nameservers = self.metadata_full['dns']['nameservers']
+        self._network_config = do_helper.convert_network_configuration(
+                                    interfaces, nameservers)
+        return self._network_config
+
 
 # Used to match classes to dependencies
 datasources = [
-    (DataSourceDigitalOcean, (sources.DEP_FILESYSTEM, sources.DEP_NETWORK)),
+    (DataSourceDigitalOcean, (sources.DEP_FILESYSTEM, )),
 ]
 
 
diff --git a/cloudinit/sources/helpers/digitalocean.py b/cloudinit/sources/helpers/digitalocean.py
new file mode 100644
index 0000000..2e112a2
--- /dev/null
+++ b/cloudinit/sources/helpers/digitalocean.py
@@ -0,0 +1,187 @@
+# vi: ts=4 expandtab
+#
+#    Author: Ben Howard  <[e-mail]>
+
+#    This program is free software: you can redistribute it and/or modify
+#    it under the terms of the GNU General Public License version 3, as
+#    published by the Free Software Foundation.
+#
+#    This program is distributed in the hope that it will be useful,
+#    but WITHOUT ANY WARRANTY; without even the implied warranty of
+#    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+#    GNU General Public License for more details.
+#
+#    You should have received a copy of the GNU General Public License
+#    along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+import logging
+import random
+import json
+
+from cloudinit import net as cloudnet
+from cloudinit import util
+from cloudinit import url_helper
+
+NIC_MAP = {'public': 'eth0', 'private': 'eth1'}
+
+LOG = logging.getLogger(__name__)
+
+
+def assign_ipv4_link_local(nic=None):
+    """Bring up NIC using an address using link-local (ip4LL) IPs. On
+       DigitalOcean, the link-local domain is per-droplet routed, so there
+       is no risk of collisions. However, to be more safe, the ip4LL
+       address is random.
+    """
+
+    if not nic:
+        for cdev in sorted(cloudnet.get_devicelist()):
+            if cloudnet.is_physical(cdev):
+                nic = cdev
+                LOG.debug("assigned nic '%s' for link-local discovery", nic)
+                break
+
+    if not nic:
+        raise RuntimeError("unable to find interfaces to access the"
+                           "meta-data server. This droplet is broken.")
+
+    addr = "169.254.{0}.{1}/16".format(random.randint(1, 168),
+                                       random.randint(0, 255))
+
+    ip_addr_cmd = ['ip', 'addr', 'add', addr, 'dev', nic]
+    ip_link_cmd = ['ip', 'link', 'set', 'dev', nic, 'up']
+
+    if not util.which('ip'):
+        raise RuntimeError("No 'ip' command available to configure ip4LL "
+                           "address")
+
+    try:
+        (result, _err) = util.subp(ip_addr_cmd)
+        LOG.debug("assigned ip4LL address '%s' to '%s'", addr, nic)
+
+        (result, _err) = util.subp(ip_link_cmd)
+        LOG.debug("brought device '%s' up", nic)
+    except Exception:
+        util.logexc(LOG, "ip4LL address assignment of '%s' to '%s' failed."
+                         " Droplet networking will be broken", addr, nic)
+        raise
+
+    return nic
+
+
+def del_ipv4_link_local(nic=None):
+    """Remove the ip4LL address. While this is not necessary, the ip4LL
+       address is extraneous and confusing to users.
+    """
+    if not nic:
+        LOG.debug("no link_local address interface defined, skipping link "
+                  "local address cleanup")
+        return
+
+    LOG.debug("cleaning up ipv4LL address")
+
+    ip_addr_cmd = ['ip', 'addr', 'flush', 'dev', nic]
+
+    try:
+        (result, _err) = util.subp(ip_addr_cmd)
+        LOG.debug("removed ip4LL addresses from %s", nic)
+
+    except Exception as e:
+        util.logexc(LOG, "failed to remove ip4LL address from '%s'.", nic, e)
+
+
+def convert_network_configuration(config, dns_servers):
+    """Convert the DigitalOcean Network description into Cloud-init's netconfig
+       format.
+
+       Example JSON:
+        {'public': [
+              {'mac': '04:01:58:27:7f:01',
+               'ipv4': {'gateway': '45.55.32.1',
+                        'netmask': '255.255.224.0',
+                        'ip_address': '45.55.50.93'},
+               'anchor_ipv4': {
+                        'gateway': '10.17.0.1',
+                        'netmask': '255.255.0.0',
+                        'ip_address': '10.17.0.9'},
+               'type': 'public',
+               'ipv6': {'gateway': '....',
+                        'ip_address': '....',
+                        'cidr': 64}}
+           ],
+          'private': [
+              {'mac': '04:01:58:27:7f:02',
+               'ipv4': {'gateway': '10.132.0.1',
+                        'netmask': '255.255.0.0',
+                        'ip_address': '10.132.75.35'},
+               'type': 'private'}
+           ]
+        }
+    """
+
+    def _get_subnet_part(pcfg, nameservers=None):
+        subpart = {'type': 'static',
+                   'control': 'auto',
+                   'address': pcfg.get('ip_address'),
+                   'gateway': pcfg.get('gateway')}
+
+        if nameservers:
+            subpart['dns_nameservers'] = " ".join(nameservers)
+
+        if ":" in pcfg.get('ip_address'):
+            subpart['address'] = "{0}/{1}".format(pcfg.get('ip_address'),
+                                                  pcfg.get('cidr'))
+        else:
+            subpart['netmask'] = pcfg.get('netmask')
+
+        return subpart
+
+    all_nics = config.get('public')
+    all_nics.extend(config.get('private'))
+    macs_to_nics = cloudnet.get_interfaces_by_mac()
+    nic_configs = []
+
+    for nic in all_nics:
+
+        mac_address = nic.get('mac')
+        sysfs_name = macs_to_nics.get(mac_address)
+        nic_type = nic.get('type', 'unknown')
+        if_name = NIC_MAP.get(nic_type, sysfs_name)
+
+        LOG.debug("mapped {0} interface to {1}, assigning name of {2}".format(
+                  mac_address, sysfs_name, if_name))
+
+        ncfg = {'type': 'physical',
+                'mac_address': mac_address,
+                'name': if_name}
+
+        subnets = []
+        for netdef in ('ipv4', 'ipv6', 'anchor_ipv4', 'anchor_ipv6'):
+            raw_subnet = nic.get(netdef, None)
+            if not raw_subnet:
+                continue
+
+            sub_part = _get_subnet_part(raw_subnet)
+            if nic_type == 'public' and 'anchor' not in netdef:
+                # add DNS resolvers to the public interfaces only
+                sub_part = _get_subnet_part(raw_subnet, dns_servers)
+            else:
+                # remove the gateway any non-public interfaces
+                if 'gateway' in sub_part:
+                    del sub_part['gateway']
+
+            subnets.append(sub_part)
+
+        ncfg['subnets'] = subnets
+        nic_configs.append(ncfg)
+        LOG.debug("nic {0} configuration: {1}".format(if_name, ncfg))
+
+    return {'version': 1, 'config': nic_configs}
+
+
+def read_metadata(url, timeout=2, sec_between=2, retries=30):
+    response = url_helper.readurl(url, timeout=timeout,
+                                  sec_between=sec_between, retries=retries)
+    if not response.ok():
+        raise RuntimeError("unable to read metadata at %s" % url)
+    return json.loads(response.contents.decode())
diff --git a/tests/unittests/test_datasource/test_digitalocean.py b/tests/unittests/test_datasource/test_digitalocean.py
index f5d2ef3..2e27f89 100644
--- a/tests/unittests/test_datasource/test_digitalocean.py
+++ b/tests/unittests/test_datasource/test_digitalocean.py
@@ -20,6 +20,7 @@ import json
 from cloudinit import helpers
 from cloudinit import settings
 from cloudinit.sources import DataSourceDigitalOcean
+from cloudinit.sources.helpers import digitalocean
 
 from .. import helpers as test_helpers
 from ..helpers import HttprettyTestCase
@@ -30,14 +31,66 @@ DO_MULTIPLE_KEYS = ["ssh-rsa AAAAB3NzaC1yc2EAAAA... [e-mail]",
                     "ssh-rsa AAAAB3NzaC1yc2EAAAA... [e-mail]"]
 DO_SINGLE_KEY = "ssh-rsa AAAAB3NzaC1yc2EAAAA... [e-mail]"
 
-DO_META = {
-    'user_data': 'user_data_here',
-    'vendor_data': 'vendor_data_here',
-    'public_keys': DO_SINGLE_KEY,
-    'region': 'nyc3',
-    'id': '2000000',
-    'hostname': 'cloudinit-test',
+# the following JSON was taken from droplet (that's why its a string)
+DO_META = json.loads("""
+{
+  "droplet_id": "22532410",
+  "hostname": "utl-96268",
+  "vendor_data": "vendordata goes here",
+  "user_data": "userdata goes here",
+  "public_keys": "",
+  "auth_key": "authorization_key",
+  "region": "nyc3",
+  "interfaces": {
+    "private": [
+      {
+        "ipv4": {
+          "ip_address": "10.132.6.205",
+          "netmask": "255.255.0.0",
+          "gateway": "10.132.0.1"
+        },
+        "mac": "04:01:57:d1:9e:02",
+        "type": "private"
+      }
+    ],
+    "public": [
+      {
+        "ipv4": {
+          "ip_address": "192.0.0.20",
+          "netmask": "255.255.255.0",
+          "gateway": "104.236.0.1"
+        },
+        "ipv6": {
+          "ip_address": "2604:A880:0800:0000:1000:0000:0000:0000",
+          "cidr": 64,
+          "gateway": "2604:A880:0800:0000:0000:0000:0000:0001"
+        },
+        "anchor_ipv4": {
+          "ip_address": "10.0.0.5",
+          "netmask": "255.255.0.0",
+          "gateway": "10.0.0.1"
+        },
+        "mac": "04:01:57:d1:9e:01",
+        "type": "public"
+      }
+    ]
+  },
+  "floating_ip": {
+    "ipv4": {
+      "active": false
+    }
+  },
+  "dns": {
+    "nameservers": [
+      "2001:4860:4860::8844",
+      "2001:4860:4860::8888",
+      "8.8.8.8"
+    ]
+  }
 }
+""")
+
+DO_META['public_keys'] = DO_SINGLE_KEY
 
 MD_URL = 'http://169.254.169.254/metadata/v1.json'
 
@@ -50,6 +103,86 @@ def _request_callback(method, uri, headers):
     return (200, headers, json.dumps(DO_META))
 
 
+class TestJSONValid(test_helpers.TestCase):
+    """
+    Test that the JSON is valid. While the JSON above is actual JSON is
+    ACTUAL JSON returned from the meta-data server on DigitalOcean, we
+    should check for explicit values. This is likely an exercise in pendantry,
+    (it would be easier to just mock out the data structure, but this gives
+    us the advantage of testing an actual meta-data response
+    """
+
+    def testExpectedKeys(self):
+        required = ['droplet_id', 'hostname', 'user_data', 'vendor_data',
+                    'public_keys', 'region', 'interfaces', 'dns',
+                    'floating_ip']
+
+        for req in required:
+            self.assertIn(req, DO_META)
+
+    def testExpectedStrings(self):
+        should_be_strings = ["hostname", "vendor_data", "user_data",
+                             "auth_key", "region", "droplet_id"]
+
+        for sbs in should_be_strings:
+            print("checking type %s" % sbs)
+            self.assertIs(str, type(DO_META[sbs]))
+
+    def testInterfaceTypes(self):
+        '''test for expected types in interface definitions'''
+
+        expected_ip_strings = {
+             'anchor_ipv4': ["ip_address", "netmask", "gateway"],
+             'ipv4': ["ip_address", "netmask", "gateway"],
+             'ipv6': ["ip_address", "cidr", "gateway"]
+        }
+
+        expected_int_strings = ["mac", "type"]
+
+        expected_nets = {
+            'public': ["ipv4", "ipv6", "anchor_ipv4"],
+            'private': ["private"]
+        }
+
+        interfaces = DO_META['interfaces']
+        self.assertIs(dict, type(interfaces))
+
+        # make sure that expected interfaces types are there
+        for if_type in expected_nets:
+            int_def = interfaces[if_type]
+            self.assertIs(list, type(int_def))
+            self.assertIn(if_type, expected_nets)
+
+            for enet in expected_nets[if_type]:
+                print(enet)
+                for sub in int_def:
+                    for sub_key in sub:
+                        sk = sub[sub_key]
+                        if sk is str:
+                            self.assertIn(sub_key, expected_int_strings)
+                        elif sk is dict:
+                            self.assertIn(sk, expected_ip_strings)
+                            for ssk in expected_ip_strings[sk]:
+                                self.assertIs(str, type(sk[ssk]))
+
+    def testfloatingIP(self):
+        '''test to make sure the floating ip is defined'''
+        floating_ip = DO_META['floating_ip']
+
+        self.assertIs(dict, type(floating_ip))
+        self.assertIs(dict, type(floating_ip['ipv4']))
+        self.assertIs(bool, type(floating_ip['ipv4']['active']))
+
+    def testDNS(self):
+        '''test to make sure that DNS is defined'''
+        dns = DO_META['dns']
+        self.assertIs(dict, type(dns))
+        self.assertIs(list, type(dns['nameservers']))
+
+        for servers in dns['nameservers']:
+            self.assertIs(str, type(servers))
+
+
 class TestDataSourceDigitalOcean(HttprettyTestCase):
     """
     Test reading the meta-data
@@ -59,6 +192,7 @@ class TestDataSourceDigitalOcean(HttprettyTestCase):
         self.ds = DataSourceDigitalOcean.DataSourceDigitalOcean(
             settings.CFG_BUILTIN, None,
             helpers.Paths({}))
+        self.ds.use_ip4LL = False
         self.ds._get_sysinfo = _mock_dmi
         super(TestDataSourceDigitalOcean, self).setUp()
 
@@ -87,7 +221,7 @@ class TestDataSourceDigitalOcean(HttprettyTestCase):
         self.assertEqual(DO_META.get('region'),
                          self.ds.availability_zone)
 
-        self.assertEqual(DO_META.get('id'),
+        self.assertEqual(DO_META.get('droplet_id'),
                          self.ds.get_instance_id())
 
         self.assertEqual(DO_META.get('hostname'),
@@ -112,3 +246,115 @@ class TestDataSourceDigitalOcean(HttprettyTestCase):
                          self.ds.get_public_ssh_keys())
 
         self.assertIsInstance(self.ds.get_public_ssh_keys(), list)
+
+
+class TestNetworkConvert(test_helpers.TestCase):
+
+    def _get_networking(self):
+        netcfg = digitalocean.convert_network_configuration(
+            DO_META['interfaces'], DO_META['dns']['nameservers'])
+        self.assertIn('config', netcfg)
+        return netcfg
+
+    def test_networking_defined(self):
+        netcfg = self._get_networking()
+        self.assertIsNotNone(netcfg)
+
+        for nic_def in netcfg.get('config'):
+            print(json.dumps(nic_def, indent=3))
+            n_type = nic_def.get('type')
+            n_subnets = nic_def.get('type')
+            n_name = nic_def.get('name')
+            n_mac = nic_def.get('mac_address')
+
+            self.assertIsNotNone(n_type)
+            self.assertIsNotNone(n_subnets)
+            self.assertIsNotNone(n_name)
+            self.assertIsNotNone(n_mac)
+
+    def _get_nic_definition(self, int_type, expected_name):
+        """helper function to return if_type (i.e. public) and the expected
+           name used by cloud-init (i.e eth0)"""
+        netcfg = self._get_networking()
+        meta_def = (DO_META.get('interfaces')).get(int_type)[0]
+
+        self.assertEquals(int_type, meta_def.get('type'))
+
+        for nic_def in netcfg.get('config'):
+            print(nic_def)
+            if nic_def.get('name') == expected_name:
+                return nic_def, meta_def
+
+    def _get_match_subn(self, subnets, ip_addr):
+        """get the matching subnet definition based on ip address"""
+        for subn in subnets:
+            address = subn.get('address')
+            self.assertIsNotNone(address)
+
+            # equals won't work because of ipv6 addressing being in
+            # cidr notation, i.e fe00::1/64
+            if ip_addr in address:
+                print(json.dumps(subn, indent=3))
+                return subn
+
+    def test_public_interface_defined(self):
+        """test that the public interface is defined as eth0"""
+        (nic_def, meta_def) = self._get_nic_definition('public', 'eth0')
+        self.assertEquals('eth0', nic_def.get('name'))
+        self.assertEquals(meta_def.get('mac'), nic_def.get('mac_address'))
+        self.assertEquals('physical', nic_def.get('type'))
+
+    def test_private_interface_defined(self):
+        """test that the private interface is defined as eth1"""
+        (nic_def, meta_def) = self._get_nic_definition('private', 'eth1')
+        self.assertEquals('eth1', nic_def.get('name'))
+        self.assertEquals(meta_def.get('mac'), nic_def.get('mac_address'))
+        self.assertEquals('physical', nic_def.get('type'))
+
+    def _check_dns_nameservers(self, subn_def):
+        self.assertIn('dns_nameservers', subn_def)
+        expected_nameservers = DO_META['dns']['nameservers']
+        nic_nameservers = subn_def.get('dns_nameservers').split()
+        for dns_server in expected_nameservers:
+            self.assertIn(dns_server, nic_nameservers)
+
+    def test_public_interface_ipv6(self):
+        """test public ipv6 addressing"""
+        (nic_def, meta_def) = self._get_nic_definition('public', 'eth0')
+        ipv6_def = meta_def.get('ipv6')
+        self.assertIsNotNone(ipv6_def)
+
+        subn_def = self._get_match_subn(nic_def.get('subnets'),
+                                        ipv6_def.get('ip_address'))
+
+        cidr_notated_address = "{0}/{1}".format(ipv6_def.get('ip_address'),
+                                                ipv6_def.get('cidr'))
+
+        self.assertEquals(cidr_notated_address, subn_def.get('address'))
+        self.assertEquals(ipv6_def.get('gateway'), subn_def.get('gateway'))
+        self._check_dns_nameservers(subn_def)
+
+    def test_public_interface_ipv4(self):
+        """test public ipv4 addressing"""
+        (nic_def, meta_def) = self._get_nic_definition('public', 'eth0')
+        ipv4_def = meta_def.get('ipv4')
+        self.assertIsNotNone(ipv4_def)
+
+        subn_def = self._get_match_subn(nic_def.get('subnets'),
+                                        ipv4_def.get('ip_address'))
+
+        self.assertEquals(ipv4_def.get('netmask'), subn_def.get('netmask'))
+        self.assertEquals(ipv4_def.get('gateway'), subn_def.get('gateway'))
+        self._check_dns_nameservers(subn_def)
+
+    def test_public_interface_anchor_ipv4(self):
+        """test public ipv4 addressing"""
+        (nic_def, meta_def) = self._get_nic_definition('public', 'eth0')
+        ipv4_def = meta_def.get('anchor_ipv4')
+        self.assertIsNotNone(ipv4_def)
+
+        subn_def = self._get_match_subn(nic_def.get('subnets'),
+                                        ipv4_def.get('ip_address'))
+
+        self.assertEquals(ipv4_def.get('netmask'), subn_def.get('netmask'))
+        self.assertNotIn('gateway', subn_def)