Complete initial cleanup for refactoring/rework.

Some of the cleanups were the following

1. Using standard (logged) utility functions for sub process work, writing, reading files, and other file system/operating system options
2. Having distrobutions impelement there own subclasses to handle system specifics (if applicable)
3. Having a cloud wrapper that provides just the functionality we want to expose (cloud.py)
4. Using a path class instead of globals for all cloud init paths (it is configured via config)
5. Removal of as much shared global state as possible (there should be none, minus a set of constants)
6. Other various cleanups that remove transforms/handlers/modules from reading/writing/chmoding there own files. 
 a. They should be using util functions to take advantage of the logging that is now enabled in those util functions (very useful for debugging)
7. Urls being read and checked from a single module that serves this and only this purpose (+1 for code organization)
8. Updates to log whenever a transform decides not to run
9. Ensure whenever a exception is thrown (and possibly captured) that the util.logexc function is called 
 a. For debugging, tracing this is important to not just drop them on the floor.
10. Code shuffling into utils.py where it makes sense (and where it could serve a benefit for other code now or in the future)

4 files changed, 234 insertions, 374 deletions

diff --git a/cloudinit/sources/DataSourceMAAS.py b/cloudinit/sources/DataSourceMAAS.py
index 61a0038f..27196265 100644
--- a/cloudinit/sources/DataSourceMAAS.py
+++ b/cloudinit/sources/DataSourceMAAS.py
@@ -1,8 +1,10 @@
 # vi: ts=4 expandtab
 #
 #    Copyright (C) 2012 Canonical Ltd.
+#    Copyright (C) 2012 Yahoo! Inc.
 #
 #    Author: Scott Moser <scott.moser@canonical.com>
+#    Author: Joshua Harlow <harlowja@yahoo-inc.com>
 #
 #    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
@@ -16,22 +18,22 @@
 #    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 cloudinit.DataSource as DataSource
-
-from cloudinit import seeddir as base_seeddir
-from cloudinit import log
-import cloudinit.util as util
+import os
 import errno
 import oauth.oauth as oauth
-import os.path
-import urllib2
 import time
+import urllib2
 
+from cloudinit import log as logging
+from cloudinit import sources
+from cloudinit import url_helper as uhelp
+from cloudinit import util
 
+LOG = logging.getLogger(__name__)
 MD_VERSION = "2012-03-01"
 
 
-class DataSourceMAAS(DataSource.DataSource):
+class DataSourceMAAS(sources.DataSource):
     """
     DataSourceMAAS reads instance information from MAAS.
     Given a config metadata_url, and oauth tokens, it expects to find
@@ -40,61 +42,64 @@ class DataSourceMAAS(DataSource.DataSource):
       user-data
       hostname
     """
-    seeddir = base_seeddir + '/maas'
-    baseurl = None
+    def __init__(self, sys_cfg, distro, paths):
+        sources.DataSource.__init__(self, sys_cfg, distro, paths)
+        self.base_url = None
+        self.seed_dir = os.path.join(paths.seed_dir, 'maas')
 
     def __str__(self):
-        return("DataSourceMAAS[%s]" % self.baseurl)
+        return "%s[%s]" % (util.obj_name(self), self.base_url)
 
     def get_data(self):
         mcfg = self.ds_cfg
 
         try:
-            (userdata, metadata) = read_maas_seed_dir(self.seeddir)
+            (userdata, metadata) = read_maas_seed_dir(self.seed_dir)
             self.userdata_raw = userdata
             self.metadata = metadata
-            self.baseurl = self.seeddir
+            self.base_url = self.seed_dir
             return True
         except MAASSeedDirNone:
             pass
         except MAASSeedDirMalformed as exc:
-            log.warn("%s was malformed: %s\n" % (self.seeddir, exc))
+            LOG.warn("%s was malformed: %s" % (self.seed_dir, exc))
             raise
 
-        try:
-            # if there is no metadata_url, then we're not configured
-            url = mcfg.get('metadata_url', None)
-            if url == None:
-                return False
+        # If there is no metadata_url, then we're not configured
+        url = mcfg.get('metadata_url', None)
+        if not url:
+            return False
 
+        try:
             if not self.wait_for_metadata_service(url):
                 return False
 
-            self.baseurl = url
+            self.base_url = url
 
-            (userdata, metadata) = read_maas_seed_url(self.baseurl,
-                self.md_headers)
+            (userdata, metadata) = read_maas_seed_url(self.base_url,
+                                                      self.md_headers)
             self.userdata_raw = userdata
             self.metadata = metadata
             return True
         except Exception:
-            util.logexc(log)
+            util.logexc(LOG, "Failed fetching metadata from url %s", url)
             return False
 
     def md_headers(self, url):
         mcfg = self.ds_cfg
 
-        # if we are missing token_key, token_secret or consumer_key
+        # If we are missing token_key, token_secret or consumer_key
         # then just do non-authed requests
         for required in ('token_key', 'token_secret', 'consumer_key'):
             if required not in mcfg:
-                return({})
+                return {}
 
         consumer_secret = mcfg.get('consumer_secret', "")
-
-        return(oauth_headers(url=url, consumer_key=mcfg['consumer_key'],
-            token_key=mcfg['token_key'], token_secret=mcfg['token_secret'],
-            consumer_secret=consumer_secret))
+        return oauth_headers(url=url,
+                             consumer_key=mcfg['consumer_key'],
+                             token_key=mcfg['token_key'],
+                             token_secret=mcfg['token_secret'],
+                             consumer_secret=consumer_secret)
 
     def wait_for_metadata_service(self, url):
         mcfg = self.ds_cfg
@@ -103,32 +108,31 @@ class DataSourceMAAS(DataSource.DataSource):
         try:
             max_wait = int(mcfg.get("max_wait", max_wait))
         except Exception:
-            util.logexc(log)
-            log.warn("Failed to get max wait. using %s" % max_wait)
+            util.logexc(LOG, "Failed to get max wait. using %s", max_wait)
 
         if max_wait == 0:
             return False
 
         timeout = 50
         try:
-            timeout = int(mcfg.get("timeout", timeout))
+            if timeout in mcfg:
+                timeout = int(mcfg.get("timeout", timeout))
         except Exception:
-            util.logexc(log)
-            log.warn("Failed to get timeout, using %s" % timeout)
+            LOG.warn("Failed to get timeout, using %s" % timeout)
 
         starttime = time.time()
         check_url = "%s/%s/meta-data/instance-id" % (url, MD_VERSION)
         url = util.wait_for_url(urls=[check_url], max_wait=max_wait,
-            timeout=timeout, status_cb=log.warn,
-            headers_cb=self.md_headers)
+                                timeout=timeout, status_cb=LOG.warn,
+                                headers_cb=self.md_headers)
 
         if url:
-            log.debug("Using metadata source: '%s'" % url)
+            LOG.info("Using metadata source: '%s'", url)
         else:
-            log.critical("giving up on md after %i seconds\n" %
-                         int(time.time() - starttime))
+            LOG.critical("Giving up on md from %s after %i seconds",
+                            urls, int(time.time() - starttime))
 
-        return (bool(url))
+        return bool(url)
 
 
 def read_maas_seed_dir(seed_d):
@@ -139,22 +143,19 @@ def read_maas_seed_dir(seed_d):
       * local-hostname
       * user-data
     """
-    files = ('local-hostname', 'instance-id', 'user-data', 'public-keys')
-    md = {}
-
     if not os.path.isdir(seed_d):
         raise MAASSeedDirNone("%s: not a directory")
 
+    files = ('local-hostname', 'instance-id', 'user-data', 'public-keys')
+    md = {}
     for fname in files:
         try:
-            with open(os.path.join(seed_d, fname)) as fp:
-                md[fname] = fp.read()
-                fp.close()
+            md[fname] = util.load_file(os.path.join(seed_d, fname))
         except IOError as e:
             if e.errno != errno.ENOENT:
                 raise
 
-    return(check_seed_contents(md, seed_d))
+    return check_seed_contents(md, seed_d)
 
 
 def read_maas_seed_url(seed_url, header_cb=None, timeout=None,
@@ -169,29 +170,26 @@ def read_maas_seed_url(seed_url, header_cb=None, timeout=None,
       * <seed_url>/<version>/meta-data/local-hostname
       * <seed_url>/<version>/user-data
     """
-    files = ('meta-data/local-hostname',
-             'meta-data/instance-id',
-             'meta-data/public-keys',
-             'user-data')
-
     base_url = "%s/%s" % (seed_url, version)
+    files = {
+        'local-hostname': "%s/%s" % (base_url, 'meta-data/local-hostname'),
+        'instance-id': "%s/%s" % (base_url, 'meta-data/instance-id'),
+        'public-keys': "%s/%s" % (base_url, 'meta-data/public-keys'),
+        'user-data': "%s/%s" % (base_url, 'user-data'),
+    }
     md = {}
-    for fname in files:
-        url = "%s/%s" % (base_url, fname)
+    for (name, url) in files:
         if header_cb:
             headers = header_cb(url)
         else:
             headers = {}
-
         try:
-            req = urllib2.Request(url, data=None, headers=headers)
-            resp = urllib2.urlopen(req, timeout=timeout)
-            md[os.path.basename(fname)] = resp.read()
+            (resp, sc) = uhelp.readurl(url, headers=headers, timeout=timeout)
+            md[name] = resp
         except urllib2.HTTPError as e:
             if e.code != 404:
                 raise
-
-    return(check_seed_contents(md, seed_url))
+    return check_seed_contents(md, seed_url)
 
 
 def check_seed_contents(content, seed):
@@ -201,11 +199,10 @@ def check_seed_contents(content, seed):
        Raise MAASSeedDirMalformed or MAASSeedDirNone
     """
     md_required = ('instance-id', 'local-hostname')
-    found = content.keys()
-
     if len(content) == 0:
         raise MAASSeedDirNone("%s: no data files found" % seed)
 
+    found = content.keys()
     missing = [k for k in md_required if k not in found]
     if len(missing):
         raise MAASSeedDirMalformed("%s: missing files %s" % (seed, missing))
@@ -217,7 +214,7 @@ def check_seed_contents(content, seed):
             continue
         md[key] = val
 
-    return(userdata, md)
+    return (userdata, md)
 
 
 def oauth_headers(url, consumer_key, token_key, token_secret, consumer_secret):
@@ -232,8 +229,8 @@ def oauth_headers(url, consumer_key, token_key, token_secret, consumer_secret):
     }
     req = oauth.OAuthRequest(http_url=url, parameters=params)
     req.sign_request(oauth.OAuthSignatureMethod_PLAINTEXT(),
-        consumer, token)
-    return(req.to_header())
+                     consumer, token)
+    return req.to_header()
 
 
 class MAASSeedDirNone(Exception):
@@ -244,102 +241,11 @@ class MAASSeedDirMalformed(Exception):
     pass
 
 
+# Used to match classes to dependencies
 datasources = [
-  (DataSourceMAAS, (DataSource.DEP_FILESYSTEM, DataSource.DEP_NETWORK)),
+  (DataSourceMAAS, (sources.DEP_FILESYSTEM, sources.DEP_NETWORK)),
 ]
 
-
-# return a list of data sources that match this set of dependencies
+# Return a list of data sources that match this set of dependencies
 def get_datasource_list(depends):
-    return(DataSource.list_from_depends(depends, datasources))
-
-
-if __name__ == "__main__":
-    def main():
-        """
-        Call with single argument of directory or http or https url.
-        If url is given additional arguments are allowed, which will be
-        interpreted as consumer_key, token_key, token_secret, consumer_secret
-        """
-        import argparse
-        import pprint
-
-        parser = argparse.ArgumentParser(description='Interact with MAAS DS')
-        parser.add_argument("--config", metavar="file",
-            help="specify DS config file", default=None)
-        parser.add_argument("--ckey", metavar="key",
-            help="the consumer key to auth with", default=None)
-        parser.add_argument("--tkey", metavar="key",
-            help="the token key to auth with", default=None)
-        parser.add_argument("--csec", metavar="secret",
-            help="the consumer secret (likely '')", default="")
-        parser.add_argument("--tsec", metavar="secret",
-            help="the token secret to auth with", default=None)
-        parser.add_argument("--apiver", metavar="version",
-            help="the apiver to use ("" can be used)", default=MD_VERSION)
-
-        subcmds = parser.add_subparsers(title="subcommands", dest="subcmd")
-        subcmds.add_parser('crawl', help="crawl the datasource")
-        subcmds.add_parser('get', help="do a single GET of provided url")
-        subcmds.add_parser('check-seed', help="read andn verify seed at url")
-
-        parser.add_argument("url", help="the data source to query")
-
-        args = parser.parse_args()
-
-        creds = {'consumer_key': args.ckey, 'token_key': args.tkey,
-            'token_secret': args.tsec, 'consumer_secret': args.csec}
-
-        if args.config:
-            import yaml
-            with open(args.config) as fp:
-                cfg = yaml.load(fp)
-            if 'datasource' in cfg:
-                cfg = cfg['datasource']['MAAS']
-            for key in creds.keys():
-                if key in cfg and creds[key] == None:
-                    creds[key] = cfg[key]
-
-        def geturl(url, headers_cb):
-            req = urllib2.Request(url, data=None, headers=headers_cb(url))
-            return(urllib2.urlopen(req).read())
-
-        def printurl(url, headers_cb):
-            print "== %s ==\n%s\n" % (url, geturl(url, headers_cb))
-
-        def crawl(url, headers_cb=None):
-            if url.endswith("/"):
-                for line in geturl(url, headers_cb).splitlines():
-                    if line.endswith("/"):
-                        crawl("%s%s" % (url, line), headers_cb)
-                    else:
-                        printurl("%s%s" % (url, line), headers_cb)
-            else:
-                printurl(url, headers_cb)
-
-        def my_headers(url):
-            headers = {}
-            if creds.get('consumer_key', None) != None:
-                headers = oauth_headers(url, **creds)
-            return headers
-
-        if args.subcmd == "check-seed":
-            if args.url.startswith("http"):
-                (userdata, metadata) = read_maas_seed_url(args.url,
-                    header_cb=my_headers, version=args.apiver)
-            else:
-                (userdata, metadata) = read_maas_seed_url(args.url)
-            print "=== userdata ==="
-            print userdata
-            print "=== metadata ==="
-            pprint.pprint(metadata)
-
-        elif args.subcmd == "get":
-            printurl(args.url, my_headers)
-
-        elif args.subcmd == "crawl":
-            if not args.url.endswith("/"):
-                args.url = "%s/" % args.url
-            crawl(args.url, my_headers)
-
-    main()
+    return sources.list_from_depends(depends, datasources)
diff --git a/cloudinit/sources/DataSourceNoCloud.py b/cloudinit/sources/DataSourceNoCloud.py
index e8c56b8f..84d0f99d 100644
--- a/cloudinit/sources/DataSourceNoCloud.py
+++ b/cloudinit/sources/DataSourceNoCloud.py
@@ -2,9 +2,11 @@
 #
 #    Copyright (C) 2009-2010 Canonical Ltd.
 #    Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
+#    Copyright (C) 2012 Yahoo! Inc.
 #
 #    Author: Scott Moser <scott.moser@canonical.com>
 #    Author: Juerg Hafliger <juerg.haefliger@hp.com>
+#    Author: Joshua Harlow <harlowja@yahoo-inc.com>
 #
 #    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
@@ -18,33 +20,34 @@
 #    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 cloudinit.DataSource as DataSource
-
-from cloudinit import seeddir as base_seeddir
-from cloudinit import log
-import cloudinit.util as util
 import errno
-import subprocess
+import os
+
+from cloudinit import log as logging
+from cloudinit import sources
+from cloudinit import util
+
+LOG = logging.getLogger(__name__)
 
 
-class DataSourceNoCloud(DataSource.DataSource):
-    metadata = None
-    userdata = None
-    userdata_raw = None
-    supported_seed_starts = ("/", "file://")
-    dsmode = "local"
-    seed = None
-    cmdline_id = "ds=nocloud"
-    seeddir = base_seeddir + '/nocloud'
+class DataSourceNoCloud(sources.DataSource):
+    def __init__(self, sys_cfg, distro, paths):
+        sources.DataSource.__init__(self, sys_cfg, distro, paths)
+        self.dsmode = 'local'
+        self.seed = None
+        self.cmdline_id = "ds=nocloud"
+        self.seed_dir = os.path.join(paths.seed_dir, 'nocloud')
+        self.supported_seed_starts = ("/", "file://")
 
     def __str__(self):
-        mstr = "DataSourceNoCloud"
-        mstr = mstr + " [seed=%s]" % self.seed
-        return(mstr)
+        mstr = "%s [seed=%s][dsmode=%s]" % (util.obj_name(self),
+                                            self.seed, self.dsmode)
+        return mstr
 
     def get_data(self):
         defaults = {
-            "instance-id": "nocloud", "dsmode": self.dsmode
+            "instance-id": "nocloud",
+            "dsmode": self.dsmode,
         }
 
         found = []
@@ -52,24 +55,24 @@ class DataSourceNoCloud(DataSource.DataSource):
         ud = ""
 
         try:
-            # parse the kernel command line, getting data passed in
+            # Parse the kernel command line, getting data passed in
             if parse_cmdline_data(self.cmdline_id, md):
                 found.append("cmdline")
         except:
-            util.logexc(log)
+            util.logexc(LOG, "Unable to parse command line data")
             return False
 
-        # check to see if the seeddir has data.
+        # Check to see if the seed dir has data.
         seedret = {}
-        if util.read_optional_seed(seedret, base=self.seeddir + "/"):
+        if util.read_optional_seed(seedret, base=self.seed_dir + "/"):
             md = util.mergedict(md, seedret['meta-data'])
             ud = seedret['user-data']
-            found.append(self.seeddir)
-            log.debug("using seeded cache data in %s" % self.seeddir)
+            found.append(self.seed_dir)
+            LOG.debug("Using seeded cache data from %s", self.seed_dir)
 
-        # if the datasource config had a 'seedfrom' entry, then that takes
+        # If the datasource config had a 'seedfrom' entry, then that takes
         # precedence over a 'seedfrom' that was found in a filesystem
-        # but not over external medi
+        # but not over external media
         if 'seedfrom' in self.ds_cfg and self.ds_cfg['seedfrom']:
             found.append("ds_config")
             md["seedfrom"] = self.ds_cfg['seedfrom']
@@ -83,35 +86,36 @@ class DataSourceNoCloud(DataSource.DataSource):
 
         for dev in devlist:
             try:
-                (newmd, newud) = util.mount_callback_umount(dev,
-                    util.read_seeded)
+                LOG.debug("Attempting to use data from %s", dev)
+
+                (newmd, newud) = util.mount_cb(dev, util.read_seeded)
                 md = util.mergedict(newmd, md)
                 ud = newud
 
-                # for seed from a device, the default mode is 'net'.
+                # For seed from a device, the default mode is 'net'.
                 # that is more likely to be what is desired.
                 # If they want dsmode of local, then they must
                 # specify that.
                 if 'dsmode' not in md:
                     md['dsmode'] = "net"
 
-                log.debug("using data from %s" % dev)
+                LOG.debug("Using data from %s", dev)
                 found.append(dev)
                 break
-            except OSError, e:
+            except OSError as e:
                 if e.errno != errno.ENOENT:
                     raise
-            except util.mountFailedError:
-                log.warn("Failed to mount %s when looking for seed" % dev)
+            except util.MountFailedError:
+                util.logexc(LOG, "Failed to mount %s when looking for seed", dev)
 
-        # there was no indication on kernel cmdline or data
+        # There was no indication on kernel cmdline or data
         # in the seeddir suggesting this handler should be used.
         if len(found) == 0:
             return False
 
         seeded_interfaces = None
 
-        # the special argument "seedfrom" indicates we should
+        # The special argument "seedfrom" indicates we should
         # attempt to seed the userdata / metadata from its value
         # its primarily value is in allowing the user to type less
         # on the command line, ie: ds=nocloud;s=http://bit.ly/abcdefg
@@ -123,57 +127,46 @@ class DataSourceNoCloud(DataSource.DataSource):
                     seedfound = proto
                     break
             if not seedfound:
-                log.debug("seed from %s not supported by %s" %
-                    (seedfrom, self.__class__))
+                LOG.debug("Seed from %s not supported by %s", seedfrom, self)
                 return False
 
             if 'network-interfaces' in md:
                 seeded_interfaces = self.dsmode
 
-            # this could throw errors, but the user told us to do it
+            # This could throw errors, but the user told us to do it
             # so if errors are raised, let them raise
             (md_seed, ud) = util.read_seeded(seedfrom, timeout=None)
-            log.debug("using seeded cache data from %s" % seedfrom)
+            LOG.debug("Using seeded cache data from %s", seedfrom)
 
-            # values in the command line override those from the seed
+            # Values in the command line override those from the seed
             md = util.mergedict(md, md_seed)
             found.append(seedfrom)
 
+        # Now that we have exhausted any other places merge in the defaults
         md = util.mergedict(md, defaults)
 
-        # update the network-interfaces if metadata had 'network-interfaces'
+        # Update the network-interfaces if metadata had 'network-interfaces'
         # entry and this is the local datasource, or 'seedfrom' was used
         # and the source of the seed was self.dsmode
         # ('local' for NoCloud, 'net' for NoCloudNet')
         if ('network-interfaces' in md and
             (self.dsmode in ("local", seeded_interfaces))):
-            log.info("updating network interfaces from nocloud")
-
-            util.write_file("/etc/network/interfaces",
-                md['network-interfaces'])
-            try:
-                (out, err) = util.subp(['ifup', '--all'])
-                if len(out) or len(err):
-                    log.warn("ifup --all had stderr: %s" % err)
-
-            except subprocess.CalledProcessError as exc:
-                log.warn("ifup --all failed: %s" % (exc.output[1]))
-
-        self.seed = ",".join(found)
-        self.metadata = md
-        self.userdata_raw = ud
-
+            LOG.info("Updating network interfaces from %s", self)
+            self.distro.apply_network(md['network-interfaces'])
+            
         if md['dsmode'] == self.dsmode:
+            self.seed = ",".join(found)
+            self.metadata = md
+            self.userdata_raw = ud
             return True
 
-        log.debug("%s: not claiming datasource, dsmode=%s" %
-            (self, md['dsmode']))
+        LOG.debug("%s: not claiming datasource, dsmode=%s", self, md['dsmode'])
         return False
 
 
-# returns true or false indicating if cmdline indicated
+# Returns true or false indicating if cmdline indicated
 # that this module should be used
-# example cmdline:
+# Example cmdline:
 #  root=LABEL=uec-rootfs ro ds=nocloud
 def parse_cmdline_data(ds_id, fill, cmdline=None):
     if cmdline is None:
@@ -210,23 +203,25 @@ def parse_cmdline_data(ds_id, fill, cmdline=None):
             k = s2l[k]
         fill[k] = v
 
-    return(True)
+    return True
 
 
 class DataSourceNoCloudNet(DataSourceNoCloud):
-    cmdline_id = "ds=nocloud-net"
-    supported_seed_starts = ("http://", "https://", "ftp://")
-    seeddir = base_seeddir + '/nocloud-net'
-    dsmode = "net"
+    def __init__(self, sys_cfg, distro, paths):
+        DataSourceNoCloud.__init__(self, sys_cfg, distro, paths)
+        self.cmdline_id = "ds=nocloud-net"
+        self.supported_seed_starts = ("http://", "https://", "ftp://")
+        self.seed_dir = os.path.join(paths.seed_dir, 'nocloud-net')
+        self.dsmode = "net"
 
 
-datasources = (
-  (DataSourceNoCloud, (DataSource.DEP_FILESYSTEM, )),
-  (DataSourceNoCloudNet,
-    (DataSource.DEP_FILESYSTEM, DataSource.DEP_NETWORK)),
-)
+# Used to match classes to dependencies
+datasources = [
+  (DataSourceNoCloud, (sources.DEP_FILESYSTEM, )),
+  (DataSourceNoCloudNet, (sources.DEP_FILESYSTEM, sources.DEP_NETWORK)),
+]
 
 
-# return a list of data sources that match this set of dependencies
+# Return a list of data sources that match this set of dependencies
 def get_datasource_list(depends):
-    return(DataSource.list_from_depends(depends, datasources))
+    return sources.list_from_depends(depends, datasources)
diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py
index a0b1b518..bb0f46c2 100644
--- a/cloudinit/sources/DataSourceOVF.py
+++ b/cloudinit/sources/DataSourceOVF.py
@@ -2,9 +2,11 @@
 #
 #    Copyright (C) 2011 Canonical Ltd.
 #    Copyright (C) 2012 Hewlett-Packard Development Company, L.P.
+#    Copyright (C) 2012 Yahoo! Inc.
 #
 #    Author: Scott Moser <scott.moser@canonical.com>
 #    Author: Juerg Hafliger <juerg.haefliger@hp.com>
+#    Author: Joshua Harlow <harlowja@yahoo-inc.com>
 #
 #    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
@@ -18,33 +20,30 @@
 #    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 cloudinit.DataSource as DataSource
-
-from cloudinit import seeddir as base_seeddir
-from cloudinit import log
-import cloudinit.util as util
-import os.path
-import os
 from xml.dom import minidom
 import base64
+import os
 import re
 import tempfile
-import subprocess
 
+from cloudinit import log as logging
+from cloudinit import sources
+from cloudinit import util
 
-class DataSourceOVF(DataSource.DataSource):
-    seed = None
-    seeddir = base_seeddir + '/ovf'
-    environment = None
-    cfg = {}
-    userdata_raw = None
-    metadata = None
-    supported_seed_starts = ("/", "file://")
+LOG = logging.getLogger(__name__)
+
+
+class DataSourceOVF(sources.DataSource):
+    def __init__(self, sys_cfg, distro, paths):
+        sources.DataSource.__init__(self, sys_cfg, distro, paths)
+        self.seed = None
+        self.seed_dir = os.path.join(paths.seed_dir, 'ovf')
+        self.environment = None
+        self.cfg = {}
+        self.supported_seed_starts = ("/", "file://")
 
     def __str__(self):
-        mstr = "DataSourceOVF"
-        mstr = mstr + " [seed=%s]" % self.seed
-        return(mstr)
+        return "%s [seed=%s]" % (util.obj_name(self), self.seed)
 
     def get_data(self):
         found = []
@@ -55,13 +54,12 @@ class DataSourceOVF(DataSource.DataSource):
             "instance-id": "iid-dsovf"
         }
 
-        (seedfile, contents) = get_ovf_env(base_seeddir)
+        (seedfile, contents) = get_ovf_env(self.paths.seed_dir)
         if seedfile:
-            # found a seed dir
-            seed = "%s/%s" % (base_seeddir, seedfile)
+            # Found a seed dir
+            seed = os.path.join(self.paths.seed_dir, seedfile)
             (md, ud, cfg) = read_ovf_environment(contents)
             self.environment = contents
-
             found.append(seed)
         else:
             np = {'iso': transport_iso9660,
@@ -71,7 +69,6 @@ class DataSourceOVF(DataSource.DataSource):
                 (contents, _dev, _fname) = transfunc()
                 if contents:
                     break
-
             if contents:
                 (md, ud, cfg) = read_ovf_environment(contents)
                 self.environment = contents
@@ -89,17 +86,19 @@ class DataSourceOVF(DataSource.DataSource):
                     seedfound = proto
                     break
             if not seedfound:
-                log.debug("seed from %s not supported by %s" %
-                    (seedfrom, self.__class__))
+                LOG.debug("Seed from %s not supported by %s",
+                          seedfrom, self)
                 return False
 
             (md_seed, ud) = util.read_seeded(seedfrom, timeout=None)
-            log.debug("using seeded cache data from %s" % seedfrom)
+            LOG.debug("Using seeded cache data from %s", seedfrom)
 
             md = util.mergedict(md, md_seed)
             found.append(seedfrom)
 
+        # Now that we have exhausted any other places merge in the defaults
         md = util.mergedict(md, defaults)
+
         self.seed = ",".join(found)
         self.metadata = md
         self.userdata_raw = ud
@@ -108,31 +107,37 @@ class DataSourceOVF(DataSource.DataSource):
 
     def get_public_ssh_keys(self):
         if not 'public-keys' in self.metadata:
-            return([])
-        return([self.metadata['public-keys'], ])
+            return []
+        pks = self.metadata['public-keys']
+        if isinstance(pks, (list)):
+            return pks
+        else:
+            return [pks]
 
-    # the data sources' config_obj is a cloud-config formated
+    # The data sources' config_obj is a cloud-config formatted
     # object that came to it from ways other than cloud-config
     # because cloud-config content would be handled elsewhere
     def get_config_obj(self):
-        return(self.cfg)
+        return self.cfg
 
 
 class DataSourceOVFNet(DataSourceOVF):
-    seeddir = base_seeddir + '/ovf-net'
-    supported_seed_starts = ("http://", "https://", "ftp://")
+    def __init__(self, sys_cfg, distro, paths):
+        DataSourceOVF.__init__(self, sys_cfg, distro, paths)
+        self.seed_dir = os.path.join(paths.seed_dir, 'ovf-net')
+        self.supported_seed_starts = ("http://", "https://", "ftp://")
 
 
-# this will return a dict with some content
-#  meta-data, user-data
+# This will return a dict with some content
+#  meta-data, user-data, some config
 def read_ovf_environment(contents):
-    props = getProperties(contents)
+    props = get_properties(contents)
     md = {}
     cfg = {}
     ud = ""
-    cfg_props = ['password', ]
+    cfg_props = ['password']
     md_props = ['seedfrom', 'local-hostname', 'public-keys', 'instance-id']
-    for prop, val in props.iteritems():
+    for (prop, val) in props.iteritems():
         if prop == 'hostname':
             prop = "local-hostname"
         if prop in md_props:
@@ -144,23 +149,25 @@ def read_ovf_environment(contents):
                 ud = base64.decodestring(val)
             except:
                 ud = val
-    return(md, ud, cfg)
+    return (md, ud, cfg)
 
 
-# returns tuple of filename (in 'dirname', and the contents of the file)
+# Returns tuple of filename (in 'dirname', and the contents of the file)
 # on "not found", returns 'None' for filename and False for contents
 def get_ovf_env(dirname):
     env_names = ("ovf-env.xml", "ovf_env.xml", "OVF_ENV.XML", "OVF-ENV.XML")
     for fname in env_names:
-        if os.path.isfile("%s/%s" % (dirname, fname)):
-            fp = open("%s/%s" % (dirname, fname))
-            contents = fp.read()
-            fp.close()
-            return(fname, contents)
-    return(None, False)
+        full_fn = os.path.join(dirname, fname)
+        if os.path.isfile(full_fn):
+            try:
+                contents = util.load_file(full_fn)
+                return (fname, contents)
+            except:
+                util.logexc(LOG, "Failed loading ovf file %s", full_fn)
+    return (None, False)
 
 
-# transport functions take no input and return
+# Transport functions take no input and return
 # a 3 tuple of content, path, filename
 def transport_iso9660(require_iso=True):
 
@@ -173,79 +180,45 @@ def transport_iso9660(require_iso=True):
     devname_regex = os.environ.get(envname, default_regex)
     cdmatch = re.compile(devname_regex)
 
-    # go through mounts to see if it was already mounted
-    fp = open("/proc/mounts")
-    mounts = fp.readlines()
-    fp.close()
-
-    mounted = {}
-    for mpline in mounts:
-        (dev, mp, fstype, _opts, _freq, _passno) = mpline.split()
-        mounted[dev] = (dev, fstype, mp, False)
-        mp = mp.replace("\\040", " ")
+    # Go through mounts to see if it was already mounted
+    mounts = util.mounts()
+    for (dev, info) in mounts.iteritems():
+        fstype = info['fstype']
         if fstype != "iso9660" and require_iso:
             continue
-
         if cdmatch.match(dev[5:]) == None:  # take off '/dev/'
             continue
-
+        mp = info['mountpoint']
         (fname, contents) = get_ovf_env(mp)
         if contents is not False:
-            return(contents, dev, fname)
-
-    tmpd = None
-    dvnull = None
+            return (contents, dev, fname)
 
     devs = os.listdir("/dev/")
     devs.sort()
-
     for dev in devs:
-        fullp = "/dev/%s" % dev
+        fullp = os.path.join("/dev/", dev)
 
-        if fullp in mounted or not cdmatch.match(dev) or os.path.isdir(fullp):
+        if (fullp in mounted or
+            not cdmatch.match(dev) or os.path.isdir(fullp)):
             continue
 
-        fp = None
         try:
-            fp = open(fullp, "rb")
-            fp.read(512)
-            fp.close()
+            # See if we can read anything at all...??
+            with open(fullp, 'rb') as fp:
+                fp.read(512)
         except:
-            if fp:
-                fp.close()
             continue
 
-        if tmpd is None:
-            tmpd = tempfile.mkdtemp()
-        if dvnull is None:
-            try:
-                dvnull = open("/dev/null")
-            except:
-                pass
-
-        cmd = ["mount", "-o", "ro", fullp, tmpd]
-        if require_iso:
-            cmd.extend(('-t', 'iso9660'))
-
-        rc = subprocess.call(cmd, stderr=dvnull, stdout=dvnull, stdin=dvnull)
-        if rc:
+        try:
+            (fname, contents) = utils.mount_cb(fullp, get_ovf_env, mtype="iso9660")
+        except util.MountFailedError:
+            util.logexc(LOG, "Failed mounting %s", fullp)
             continue
 
-        (fname, contents) = get_ovf_env(tmpd)
-
-        subprocess.call(["umount", tmpd])
-
         if contents is not False:
-            os.rmdir(tmpd)
-            return(contents, fullp, fname)
-
-    if tmpd:
-        os.rmdir(tmpd)
-
-    if dvnull:
-        dvnull.close()
+            return (contents, fullp, fname)
 
-    return(False, None, None)
+    return (False, None, None)
 
 
 def transport_vmware_guestd():
@@ -259,74 +232,60 @@ def transport_vmware_guestd():
     #     # would need to error check here and see why this failed
     #     # to know if log/error should be raised
     #     return(False, None, None)
-    return(False, None, None)
+    return (False, None, None)
 
 
-def findChild(node, filter_func):
+def find_child(node, filter_func):
     ret = []
     if not node.hasChildNodes():
         return ret
     for child in node.childNodes:
         if filter_func(child):
             ret.append(child)
-    return(ret)
+    return ret
 
 
-def getProperties(environString):
-    dom = minidom.parseString(environString)
+def get_properties(contents):
+
+    dom = minidom.parseString(contents)
     if dom.documentElement.localName != "Environment":
-        raise Exception("No Environment Node")
+        raise XmlError("No Environment Node")
 
     if not dom.documentElement.hasChildNodes():
-        raise Exception("No Child Nodes")
+        raise XmlError("No Child Nodes")
 
     envNsURI = "http://schemas.dmtf.org/ovf/environment/1"
 
     # could also check here that elem.namespaceURI ==
     #   "http://schemas.dmtf.org/ovf/environment/1"
-    propSections = findChild(dom.documentElement,
+    propSections = find_child(dom.documentElement,
         lambda n: n.localName == "PropertySection")
 
     if len(propSections) == 0:
-        raise Exception("No 'PropertySection's")
+        raise XmlError("No 'PropertySection's")
 
     props = {}
-    propElems = findChild(propSections[0], lambda n: n.localName == "Property")
+    propElems = find_child(propSections[0], lambda n: n.localName == "Property")
 
     for elem in propElems:
         key = elem.attributes.getNamedItemNS(envNsURI, "key").value
         val = elem.attributes.getNamedItemNS(envNsURI, "value").value
         props[key] = val
 
-    return(props)
+    return props
+
+
+class XmlError(Exception):
+    pass
 
 
+# Used to match classes to dependencies
 datasources = (
-  (DataSourceOVF, (DataSource.DEP_FILESYSTEM, )),
-  (DataSourceOVFNet,
-    (DataSource.DEP_FILESYSTEM, DataSource.DEP_NETWORK)),
+  (DataSourceOVF, (sources.DEP_FILESYSTEM, )),
+  (DataSourceOVFNet, (sources.DEP_FILESYSTEM, sources.DEP_NETWORK)),
 )
 
 
-# return a list of data sources that match this set of dependencies
+# Return a list of data sources that match this set of dependencies
 def get_datasource_list(depends):
-    return(DataSource.list_from_depends(depends, datasources))
-
-
-if __name__ == "__main__":
-    def main():
-        import sys
-        envStr = open(sys.argv[1]).read()
-        props = getProperties(envStr)
-        import pprint
-        pprint.pprint(props)
-
-        md, ud, cfg = read_ovf_environment(envStr)
-        print "=== md ==="
-        pprint.pprint(md)
-        print "=== ud ==="
-        pprint.pprint(ud)
-        print "=== cfg ==="
-        pprint.pprint(cfg)
-
-    main()
+    return sources.list_from_depends(depends, datasources)
diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py
index dfd1fff3..08669f5d 100644
--- a/cloudinit/sources/__init__.py
+++ b/cloudinit/sources/__init__.py
@@ -39,10 +39,6 @@ class DataSourceNotFoundException(Exception):
 
 class DataSource(object):
     def __init__(self, sys_cfg, distro, paths):
-        name = util.obj_name(self)
-        if name.startswith(DS_PREFIX):
-            name = name[DS_PREFIX:]
-        self.cfgname = name
         self.sys_cfg = sys_cfg
         self.distro = distro
         self.paths = paths
@@ -50,8 +46,11 @@ class DataSource(object):
         self.userdata = None
         self.metadata = None
         self.userdata_raw = None
+        name = util.obj_name(self)
+        if name.startswith(DS_PREFIX):
+            name = name[DS_PREFIX:]
         self.ds_cfg = util.get_cfg_by_path(self.sys_cfg,
-                        ("datasource", self.cfgname), {})
+                                          ("datasource", name), {})
 
     def get_userdata(self):
         if self.userdata is None:
@@ -112,6 +111,7 @@ class DataSource(object):
 
     def get_instance_id(self):
         if not self.metadata or 'instance-id' not in self.metadata:
+            # Return a magic not really instance id string
             return "iid-datasource"
         return str(self.metadata['instance-id'])
 
@@ -166,7 +166,7 @@ def find_source(sys_cfg, distro, paths, ds_deps, cfg_list, pkg_list):
             if s.get_data():
                 return (s, ds)
         except Exception as e:
-            LOG.exception("Getting data from %s failed due to %s", ds, e)
+            util.logexc(LOG, "Getting data from %s failed", ds)
 
     msg = "Did not find any data source, searched classes: %s" % (ds_names)
     raise DataSourceNotFoundException(msg)