Merge lp:~smoser/cloud-init/opennebula into lp:~vlastimil-holer/cloud-init/opennebula

[Scott Moser]

[Vlastimil Holer (vlastimil-holer)]

review: Approve

=== modified file 'cloudinit/sources/DataSourceOpenNebula.py'
--- cloudinit/sources/DataSourceOpenNebula.py	2013-09-10 16:35:19 +0000
+++ cloudinit/sources/DataSourceOpenNebula.py	2013-09-10 18:23:38 +0000
@@ -23,9 +23,9 @@
 #    along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 import os
+import pwd
 import re
-import string
-import subprocess
+import string  # pylint: disable=W0402
 
 from cloudinit import log as logging
 from cloudinit import sources
@@ -58,7 +58,7 @@
 
         # decide parseuser for context.sh shell reader
         parseuser = DEFAULT_PARSEUSER
-        if self.ds_cfg.get('parseuser'):
+        if 'parseuser' in self.ds_cfg:
             parseuser = self.ds_cfg.get('parseuser')
 
         candidates = [self.seed_dir]
@@ -260,13 +260,21 @@
     return combined
 
 
-def parse_shell_config(content, keylist=None, bash=None, asuser=None):
+def switch_user_cmd(user):
+    return ['sudo', '-u', user]
+
+
+def parse_shell_config(content, keylist=None, bash=None, asuser=None,
+                       switch_user_cb=None):
 
     if isinstance(bash, str):
         bash = [bash]
     elif bash is None:
         bash = ['bash', '-e']
 
+    if switch_user_cb is None:
+        switch_user_cb = switch_user_cmd
+
     # allvars expands to all existing variables by using '${!x*}' notation
     # where x is lower or upper case letters or '_'
     allvars = ["${!%s*}" % x for x in string.letters + "_"]
@@ -302,23 +310,17 @@
     bcmd = ('unset IFS\n' +
             setup +
             varprinter(allvars) +
-            '{\n%s\n\n} > /dev/null\n' % content +
+            '{\n%s\n\n:\n} > /dev/null\n' % content +
             'unset IFS\n' +
             varprinter(keylist) + "\n")
 
     cmd = []
     if asuser is not None:
-        cmd = ['sudo', '-u', asuser]
+        cmd = switch_user_cb(asuser)
 
     cmd.extend(bash)
 
-    sp = subprocess.Popen(cmd, stdin=subprocess.PIPE,
-                          stdout=subprocess.PIPE,
-                          stderr=subprocess.PIPE)
-    (output, error) = sp.communicate(input=bcmd)
-
-    if sp.returncode != 0:
-        raise Exception("Process returned %d" % sp.returncode)
+    (output, _error) = util.subp(cmd, data=bcmd)
 
     # exclude vars in bash that change on their own or that we used
     excluded = ("RANDOM", "LINENO", "_", "__v")
@@ -329,7 +331,7 @@
 
     # go through output.  First _start_ is for 'preset', second for 'target'.
     # Add to target only things were changed and not in volitile
-    for line in output.split("\0"):
+    for line in output.split("\x00"):
         try:
             (key, val) = line.split("=", 1)
             if target is preset:
@@ -367,21 +369,21 @@
     results = {'userdata': None, 'metadata': {}}
 
     if "context.sh" in found:
+        if asuser is not None:
+            try:
+                pwd.getpwnam(asuser)
+            except KeyError as e:
+                raise BrokenContextDiskDir("configured user '%s' "
+                                           "does not exist", asuser)
         try:
             with open(os.path.join(source_dir, 'context.sh'), 'r') as f:
                 content = f.read().strip()
-                f.close()
 
-                # don't pass empty context script
-                # to shell parser
-                non_empty = re.match(r'.*?^\s*([^# ]+)', content,
-                                     re.MULTILINE | re.DOTALL)
-                if non_empty:
-                    context = parse_shell_config(content, asuser=asuser)
+            context = parse_shell_config(content, asuser=asuser)
+        except util.ProcessExecutionError as e:
+            raise BrokenContextDiskDir("Error processing context.sh: %s" % (e))
         except IOError as e:
             raise NonContextDiskDir("Error reading context.sh: %s" % (e))
-        except Exception as e:
-            raise BrokenContextDiskDir("Error processing context.sh: %s" % (e))
     else:
         raise NonContextDiskDir("Missing context.sh")
 

=== modified file 'cloudinit/sources/__init__.py'
--- cloudinit/sources/__init__.py	2013-09-09 22:46:37 +0000
+++ cloudinit/sources/__init__.py	2013-09-10 18:23:38 +0000
@@ -53,9 +53,16 @@
         self.userdata = None
         self.metadata = None
         self.userdata_raw = None
+
+        # find the datasource config name.
+        # remove 'DataSource' from classname on front, and remove 'Net' on end.
+        # Both Foo and FooNet sources expect config in cfg['sources']['Foo']
         name = type_utils.obj_name(self)
         if name.startswith(DS_PREFIX):
             name = name[len(DS_PREFIX):]
+        if name.endswith('Net'):
+            name = name[0:-3]
+
         self.ds_cfg = util.get_cfg_by_path(self.sys_cfg,
                                           ("datasource", name), {})
         if not ud_proc:

=== modified file 'tests/unittests/test_datasource/test_opennebula.py'
--- tests/unittests/test_datasource/test_opennebula.py	2013-09-10 16:35:19 +0000
+++ tests/unittests/test_datasource/test_opennebula.py	2013-09-10 18:23:38 +0000
@@ -37,6 +37,7 @@
 
 
 class TestOpenNebulaDataSource(MockerTestCase):
+    parsed_user = None
 
     def setUp(self):
         super(TestOpenNebulaDataSource, self).setUp()
@@ -48,6 +49,18 @@
         self.seed_dir = os.path.join(self.paths.seed_dir, "opennebula")
         self.sys_cfg = {'datasource': {'OpenNebula': {'dsmode': 'local'}}}
 
+        # we don't want 'sudo' called in tests. so we patch switch_user_cmd
+        def my_switch_user_cmd(user):
+            self.parsed_user = user
+            return []
+
+        self.switch_user_cmd_real = ds.switch_user_cmd
+        ds.switch_user_cmd = my_switch_user_cmd
+
+    def tearDown(self):
+        ds.switch_user_cmd = self.switch_user_cmd_real
+        super(TestOpenNebulaDataSource, self).tearDown()
+
     def test_get_data_non_contextdisk(self):
         try:
             # dont' try to lookup for CDs
@@ -96,9 +109,9 @@
             util.find_devs_with = orig_find_devs_with
 
     def test_get_data(self):
+        orig_find_devs_with = util.find_devs_with
         try:
             # dont' try to lookup for CDs
-            orig_find_devs_with = util.find_devs_with
             util.find_devs_with = lambda n: []
             populate_context_dir(self.seed_dir, {'KEY1': 'val1'})
             dsrc = self.ds(sys_cfg=self.sys_cfg, distro=None, paths=self.paths)