PEP8 and Pylint fixes. Move context.sh "parser" into separate

function. Fix fetching user specified dsmode (from context).
Rename context_sh->context. Reuse unittests.helpers.populate_dir. 

2 files changed, 119 insertions, 106 deletions

diff --git a/cloudinit/sources/DataSourceOpenNebula.py b/cloudinit/sources/DataSourceOpenNebula.py
index fef7beac..87ec51bd 100644
--- a/cloudinit/sources/DataSourceOpenNebula.py
+++ b/cloudinit/sources/DataSourceOpenNebula.py
@@ -3,7 +3,7 @@
 #    Copyright (C) 2012 Canonical Ltd.
 #    Copyright (C) 2012 Yahoo! Inc.
 #    Copyright (C) 2012-2013 CERIT Scientific Cloud
-#    Copyright (C) 2012 OpenNebula.org
+#    Copyright (C) 2012-2013 OpenNebula.org
 #
 #    Author: Scott Moser <scott.moser@canonical.com>
 #    Author: Joshua Harlow <harlowja@yahoo-inc.com>
@@ -45,45 +45,47 @@ class DataSourceOpenNebula(sources.DataSource):
         self.seed_dir = os.path.join(paths.seed_dir, 'opennebula')
 
     def __str__(self):
-        return "%s [seed=%s][dsmode=%s]" % \
-            (util.obj_name(self), self.seed, self.dsmode)
+        root = sources.DataSource.__str__(self)
+        return "%s [seed=%s][dsmode=%s]" % (root, self.seed, self.dsmode)
 
     def get_data(self):
         defaults = {
             "instance-id": DEFAULT_IID,
-            "dsmode": self.dsmode}
+            "dsmode": self.dsmode
+        }
 
-        found = None
-        md = {}
+        seed = None
         results = {}
 
+        # first try to read local seed_dir
         if os.path.isdir(self.seed_dir):
             try:
                 results = read_context_disk_dir(self.seed_dir)
-                found = self.seed_dir
+                seed = self.seed_dir
             except NonContextDiskDir:
-                util.logexc(LOG, "Failed reading context disk from %s",
+                util.logexc(LOG, "Failed reading context from %s",
                             self.seed_dir)
 
-        # find candidate devices, try to mount them and
-        # read context script if present
-        if not found:
+        if not seed:
+            # then try to detect and mount candidate devices and
+            # read contextualization if present
             for dev in find_candidate_devs():
                 try:
                     results = util.mount_cb(dev, read_context_disk_dir)
-                    found = dev
+                    seed = dev
                     break
                 except (NonContextDiskDir, util.MountFailedError):
                     pass
 
-        if not found:
+        if not seed:
             return False
 
+        # merge fetched metadata with datasource defaults
         md = results['metadata']
-        md = util.mergedict(md, defaults)
+        md = util.mergemanydict([md, defaults])
 
         # check for valid user specified dsmode
-        user_dsmode = results.get('dsmode', None)
+        user_dsmode = results['metadata'].get('dsmode', None)
         if user_dsmode not in VALID_DSMODES + (None,):
             LOG.warn("user specified invalid mode: %s", user_dsmode)
             user_dsmode = None
@@ -109,10 +111,9 @@ class DataSourceOpenNebula(sources.DataSource):
             LOG.debug("%s: not claiming datasource, dsmode=%s", self, dsmode)
             return False
 
-        self.seed = found
+        self.seed = seed
         self.metadata = md
         self.userdata_raw = results.get('userdata')
-
         return True
 
     def get_hostname(self, fqdn=False, resolve_ip=None):
@@ -139,9 +140,9 @@ class OpenNebulaNetwork(object):
                     r'^\d+: (eth\d+):.*?link\/ether (..:..:..:..:..:..) ?',
                     re.MULTILINE | re.DOTALL)
 
-    def __init__(self, ip, context_sh):
+    def __init__(self, ip, context):
         self.ip = ip
-        self.context_sh = context_sh
+        self.context = context
         self.ifaces = self.get_ifaces()
 
     def get_ifaces(self):
@@ -153,50 +154,50 @@ class OpenNebulaNetwork(object):
 
     def get_ip(self, dev, components):
         var_name = dev + '_ip'
-        if var_name in self.context_sh:
-            return self.context_sh[var_name]
+        if var_name in self.context:
+            return self.context[var_name]
         else:
             return '.'.join(components)
 
     def get_mask(self, dev):
         var_name = dev + '_mask'
-        if var_name in self.context_sh:
-            return self.context_sh[var_name]
+        if var_name in self.context:
+            return self.context[var_name]
         else:
             return '255.255.255.0'
 
     def get_network(self, dev, components):
         var_name = dev + '_network'
-        if var_name in self.context_sh:
-            return self.context_sh[var_name]
+        if var_name in self.context:
+            return self.context[var_name]
         else:
             return '.'.join(components[:-1]) + '.0'
 
     def get_gateway(self, dev):
         var_name = dev + '_gateway'
-        if var_name in self.context_sh:
-            return self.context_sh[var_name]
+        if var_name in self.context:
+            return self.context[var_name]
         else:
             return None
 
     def get_dns(self, dev):
         var_name = dev + '_dns'
-        if var_name in self.context_sh:
-            return self.context_sh[var_name]
+        if var_name in self.context:
+            return self.context[var_name]
         else:
             return None
 
     def get_domain(self, dev):
         var_name = dev + '_domain'
-        if var_name in self.context_sh:
-            return self.context_sh[var_name]
+        if var_name in self.context:
+            return self.context[var_name]
         else:
             return None
 
     def gen_conf(self):
         global_dns = []
-        if 'dns' in self.context_sh:
-            global_dns.append(self.context_sh['dns'])
+        if 'dns' in self.context:
+            global_dns.append(self.context['dns'])
 
         conf = []
         conf.append('auto lo')
@@ -241,101 +242,113 @@ def find_candidate_devs():
     """
     combined = []
     for f in ('LABEL=CONTEXT', 'LABEL=CDROM', 'TYPE=iso9660'):
-        for d in util.find_devs_with(f).sort():
+        devs = util.find_devs_with(f)
+        devs.sort()
+        for d in devs:
             if d not in combined:
                 combined.append(d)
 
     return combined
 
 
+def parse_context_data(data):
+    """
+    parse_context_data(data)
+    parse context.sh variables provided as a single string. Uses
+    very simple matching RE. Returns None if nothing is matched.
+    """
+    # RE groups:
+    # 1: key
+    # 2: single quoted value, respect '\''
+    # 3: old double quoted value, but doesn't end with \"
+    context_reg = re.compile(
+        r"^([\w_]+)=(?:'((?:[^']|'\\'')*?)'|\"(.*?[^\\])\")$",
+        re.MULTILINE | re.DOTALL)
+
+    found = context_reg.findall(data)
+    if not found:
+        return None
+
+    variables = {}
+    for k, v1, v2 in found:
+        k = k.lower()
+        if v1:
+            # take single quoted variable 'xyz'
+            # (ON>=4) and unquote '\'' -> '
+            variables[k] = v1.replace(r"'\''", r"'")
+        elif v2:
+            # take double quoted variable "xyz"
+            # (old ON<4) and unquote \" -> "
+            variables[k] = v2.replace(r'\"', r'"')
+
+    return variables
+
+
 def read_context_disk_dir(source_dir):
     """
     read_context_disk_dir(source_dir):
     read source_dir and return a tuple with metadata dict and user-data
     string populated.  If not a valid dir, raise a NonContextDiskDir
     """
-
     found = {}
     for af in CONTEXT_DISK_FILES:
         fn = os.path.join(source_dir, af)
         if os.path.isfile(fn):
             found[af] = fn
 
-    if len(found) == 0:
+    if not found:
         raise NonContextDiskDir("%s: %s" % (source_dir, "no files found"))
 
     results = {'userdata': None, 'metadata': {}}
-    context_sh = {}
+    context = {}
 
     if "context.sh" in found:
         try:
-            f = open('%s/context.sh' % (source_dir), 'r')
-            text = f.read()
-            f.close()
-
-            # lame matching:
-            # 1. group = key
-            # 2. group = single quoted value, respect '\''
-            # 3. group = old double quoted value, but doesn't end with \"
-            context_reg = re.compile(
-                r"^([\w_]+)=(?:'((?:[^']|'\\'')*?)'|\"(.*?[^\\])\")$",
-                re.MULTILINE | re.DOTALL)
-
-            variables = context_reg.findall(text)
-            if not variables:
+            with open(os.path.join(source_dir, 'context.sh'), 'r') as f:
+                context = parse_context_data(f.read())
+                f.close()
+            if not context:
                 raise NonContextDiskDir("No variables in context")
 
-            for k, v1, v2 in variables:
-                k = k.lower()
-                if v1:
-                    # take single quoted variable 'xyz'
-                    # (ON>=4) and unquote '\'' -> '
-                    context_sh[k] = v1.replace(r"'\''", r"'")
-                elif v2:
-                    # take double quoted variable "xyz"
-                    # (old ON<4) and unquote \" -> "
-                    context_sh[k] = v2.replace(r'\"', r'"')
-
         except (IOError, NonContextDiskDir) as e:
             raise NonContextDiskDir("Error reading context.sh: %s" % (e))
 
-        results['metadata'] = context_sh
+        results['metadata'] = context
     else:
         raise NonContextDiskDir("Missing context.sh")
 
     # process single or multiple SSH keys
     ssh_key_var = None
-
-    if "ssh_key" in context_sh:
+    if "ssh_key" in context:
         ssh_key_var = "ssh_key"
-    elif "ssh_public_key" in context_sh:
+    elif "ssh_public_key" in context:
         ssh_key_var = "ssh_public_key"
 
     if ssh_key_var:
-        lines = context_sh.get(ssh_key_var).splitlines()
+        lines = context.get(ssh_key_var).splitlines()
         results['metadata']['public-keys'] = [l for l in lines
             if len(l) and not l.startswith("#")]
 
     # custom hostname -- try hostname or leave cloud-init
     # itself create hostname from IP address later
     for k in ('hostname', 'public_ip', 'ip_public', 'eth0_ip'):
-        if k in context_sh:
-            results['metadata']['local-hostname'] = context_sh[k]
+        if k in context:
+            results['metadata']['local-hostname'] = context[k]
             break
 
     # raw user data
-    if "user_data" in context_sh:
-        results['userdata'] = context_sh["user_data"]
-    elif "userdata" in context_sh:
-        results['userdata'] = context_sh["userdata"]
+    if "user_data" in context:
+        results['userdata'] = context["user_data"]
+    elif "userdata" in context:
+        results['userdata'] = context["userdata"]
 
     # generate static /etc/network/interfaces
     # only if there are any required context variables
     # http://opennebula.org/documentation:rel3.8:cong#network_configuration
-    for k in context_sh.keys():
+    for k in context.keys():
         if re.match(r'^eth\d+_ip$', k):
             (out, _) = util.subp(['/sbin/ip', 'link'])
-            net = OpenNebulaNetwork(out, context_sh)
+            net = OpenNebulaNetwork(out, context)
             results['network-interfaces'] = net.gen_conf()
             break
 
diff --git a/tests/unittests/test_datasource/test_opennebula.py b/tests/unittests/test_datasource/test_opennebula.py
index 66a38e31..4b82a49c 100644
--- a/tests/unittests/test_datasource/test_opennebula.py
+++ b/tests/unittests/test_datasource/test_opennebula.py
@@ -1,8 +1,9 @@
-import os
-
 from cloudinit.sources import DataSourceOpenNebula as ds
 from cloudinit import util
 from mocker import MockerTestCase
+from tests.unittests.helpers import populate_dir
+
+import os
 
 TEST_VARS = {
     'var1': 'single',
@@ -13,7 +14,11 @@ TEST_VARS = {
     'var6': "'multi\nline\n'",
     'var7': 'single\\t',
     'var8': 'double\\tword',
-    'var9': 'multi\\t\nline\n'}
+    'var9': 'multi\\t\nline\n',
+    'var10': '\\',  # expect \
+    'var11': '\'',  # expect '
+    'var12': '$',   # expect $
+}
 
 USER_DATA = '#cloud-config\napt_upgrade: true'
 SSH_KEY = 'ssh-rsa AAAAB3NzaC1....sIkJhq8wdX+4I3A4cYbYP ubuntu@server-460-%i'
@@ -38,17 +43,15 @@ class TestOpenNebulaDataSource(MockerTestCase):
         my_d = os.path.join(self.tmp, 'non-contextdisk')
         self.assertRaises(ds.NonContextDiskDir, ds.read_context_disk_dir, my_d)
 
-    def test_seed_dir_bad_context_sh(self):
-        my_d = os.path.join(self.tmp, 'bad-context-sh')
+    def test_seed_dir_bad_context(self):
+        my_d = os.path.join(self.tmp, 'bad-context')
         os.mkdir(my_d)
-        with open(os.path.join(my_d, "context.sh"), "w") as fp:
-            fp.write('/bin/false\n')
-            fp.close()
+        open(os.path.join(my_d, "context.sh"), "w").close()
         self.assertRaises(ds.NonContextDiskDir, ds.read_context_disk_dir, my_d)
 
-    def test_context_sh_parser(self):
-        my_d = os.path.join(self.tmp, 'context-sh-parser')
-        populate_dir(my_d, TEST_VARS)
+    def test_context_parser(self):
+        my_d = os.path.join(self.tmp, 'context-parser')
+        populate_context_dir(my_d, TEST_VARS)
         results = ds.read_context_disk_dir(my_d)
 
         self.assertTrue('metadata' in results)
@@ -59,7 +62,7 @@ class TestOpenNebulaDataSource(MockerTestCase):
         for c in range(4):
             for k in ('SSH_KEY', 'SSH_PUBLIC_KEY'):
                 my_d = os.path.join(self.tmp, "%s-%i" % (k, c))
-                populate_dir(my_d, {k: '\n'.join(public_keys)})
+                populate_context_dir(my_d, {k: '\n'.join(public_keys)})
                 results = ds.read_context_disk_dir(my_d)
 
                 self.assertTrue('metadata' in results)
@@ -72,7 +75,7 @@ class TestOpenNebulaDataSource(MockerTestCase):
     def test_user_data(self):
         for k in ('USER_DATA', 'USERDATA'):
             my_d = os.path.join(self.tmp, k)
-            populate_dir(my_d, {k: USER_DATA})
+            populate_context_dir(my_d, {k: USER_DATA})
             results = ds.read_context_disk_dir(my_d)
 
             self.assertTrue('userdata' in results)
@@ -81,7 +84,7 @@ class TestOpenNebulaDataSource(MockerTestCase):
     def test_hostname(self):
         for k in ('HOSTNAME', 'PUBLIC_IP', 'IP_PUBLIC', 'ETH0_IP'):
             my_d = os.path.join(self.tmp, k)
-            populate_dir(my_d, {k: PUBLIC_IP})
+            populate_context_dir(my_d, {k: PUBLIC_IP})
             results = ds.read_context_disk_dir(my_d)
 
             self.assertTrue('metadata' in results)
@@ -89,14 +92,12 @@ class TestOpenNebulaDataSource(MockerTestCase):
             self.assertEqual(PUBLIC_IP, results['metadata']['local-hostname'])
 
     def test_find_candidates(self):
-        devs_with_answers = {
-            "TYPE=iso9660": ["/dev/vdb"],
-            "LABEL=CDROM": ["/dev/sr0"],
-            "LABEL=CONTEXT": ["/dev/sdb"],
-        }
-
         def my_devs_with(criteria):
-            return devs_with_answers[criteria]
+            return {
+                "LABEL=CONTEXT": ["/dev/sdb"],
+                "LABEL=CDROM": ["/dev/sr0"],
+                "TYPE=iso9660": ["/dev/vdb"],
+            }.get(criteria, [])
 
         try:
             orig_find_devs_with = util.find_devs_with
@@ -133,16 +134,17 @@ iface eth0 inet static
 ''')
 
     def test_eth0_override(self):
-        context_sh = {
+        context = {
             'dns': '1.2.3.8',
             'eth0_ip': '1.2.3.4',
             'eth0_network': '1.2.3.0',
             'eth0_mask': '255.255.0.0',
             'eth0_gateway': '1.2.3.5',
             'eth0_domain': 'example.com',
-            'eth0_dns': '1.2.3.6 1.2.3.7'}
+            'eth0_dns': '1.2.3.6 1.2.3.7'
+        }
 
-        net = ds.OpenNebulaNetwork(CMD_IP_OUT, context_sh)
+        net = ds.OpenNebulaNetwork(CMD_IP_OUT, context)
         self.assertEqual(net.gen_conf(), u'''\
 auto lo
 iface lo inet loopback
@@ -158,12 +160,10 @@ iface eth0 inet static
 ''')
 
 
-def populate_dir(seed_dir, files):
-    os.mkdir(seed_dir)
-    with open(os.path.join(seed_dir, "context.sh"), "w") as fp:
-        fp.write("# Context variables generated by OpenNebula\n")
-        for (name, content) in files.iteritems():
-            fp.write("%s='%s'\n" % (name.upper(), content.replace(r"'", r"'\''")))
-        fp.close()
+def populate_context_dir(path, variables):
+    data = "# Context variables generated by OpenNebula\n"
+    for (k, v) in variables.iteritems():
+        data += ("%s='%s'\n" % (k.upper(), v.replace(r"'", r"'\''")))
+    populate_dir(path, {'context.sh': data})
 
 # vi: ts=4 expandtab