cloud-init-dev team mailing list archive

[Joshua Powers <josh.powers@xxxxxxxxxxxxx>]

Joshua Powers has proposed merging ~powersj/cloud-init:cii-strings into cloud-init:master.

Commit message:
tests: execute: support command as string

If a string is passed to execute, then invoke 'bash', '-c',
'string'. That allows the less verbose execution of simple
commands:
  image.execute("ls /run")
compared to the more explicit but longer winded:
  image.execute(["ls", "/run"])

If 'env' was ever modified in execute or a method that it called,
then the next invocation's default value would be changed. Instead
use None and then set to a new empty dict in the method.

Requested reviews:
  cloud-init commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~powersj/cloud-init/+git/cloud-init/+merge/330535
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~powersj/cloud-init:cii-strings into cloud-init:master.

diff --git a/tests/cloud_tests/bddeb.py b/tests/cloud_tests/bddeb.py
index fe80535..90d7534 100644
--- a/tests/cloud_tests/bddeb.py
+++ b/tests/cloud_tests/bddeb.py
@@ -28,15 +28,14 @@ def build_deb(args, instance):
     # update remote system package list and install build deps
     LOG.debug('installing pre-reqs')
     pkgs = ' '.join(pre_reqs)
-    cmd = 'apt-get update && apt-get install --yes {}'.format(pkgs)
-    instance.execute(['/bin/sh', '-c', cmd])
+    instance.execute('apt-get update && apt-get install --yes {}'.format(pkgs))
 
     # local tmpfile that must be deleted
     local_tarball = tempfile.NamedTemporaryFile().name
 
     # paths to use in remote system
     output_link = '/root/cloud-init_all.deb'
-    remote_tarball = _out(instance.execute(['mktemp']))
+    remote_tarball = _out(instance.execute('mktemp'))
     extract_dir = '/root'
     bddeb_path = os.path.join(extract_dir, 'packages', 'bddeb')
     git_env = {'GIT_DIR': os.path.join(extract_dir, '.git'),
@@ -49,18 +48,17 @@ def build_deb(args, instance):
     instance.push_file(local_tarball, remote_tarball)
 
     LOG.debug('extracting tarball in remote system at: %s', extract_dir)
-    instance.execute(['tar', 'xf', remote_tarball, '-C', extract_dir])
-    instance.execute(['git', 'commit', '-a', '-m', 'tmp', '--allow-empty'],
-                     env=git_env)
+    instance.execute('tar xf %s -C %s' % (remote_tarball, extract_dir))
+    instance.execute('git commit -a -m tmp --allow-empty', env=git_env)
 
     LOG.debug('installing deps')
     deps_path = os.path.join(extract_dir, 'tools', 'read-dependencies')
-    instance.execute([deps_path, '--install', '--test-distro',
-                      '--distro', 'ubuntu', '--python-version', '3'])
+    instance.execute('%s --install --test-distro --distro ubuntu '
+                     '--python-version 3' % deps_path)
 
     LOG.debug('building deb in remote system at: %s', output_link)
-    bddeb_args = args.bddeb_args.split() if args.bddeb_args else []
-    instance.execute([bddeb_path, '-d'] + bddeb_args, env=git_env)
+    bddeb_args = args.bddeb_args if args.bddeb_args else ''
+    instance.execute('%s -d %s' % (bddeb_path, bddeb_args), env=git_env)
 
     # copy the deb back to the host system
     LOG.debug('copying built deb to host at: %s', args.deb)
diff --git a/tests/cloud_tests/instances/base.py b/tests/cloud_tests/instances/base.py
index 959e9cc..42c679a 100644
--- a/tests/cloud_tests/instances/base.py
+++ b/tests/cloud_tests/instances/base.py
@@ -23,7 +23,7 @@ class Instance(object):
         self.config = config
         self.features = features
 
-    def execute(self, command, stdout=None, stderr=None, env={},
+    def execute(self, command, stdout=None, stderr=None, env=None,
                 rcs=None, description=None):
         """Execute command in instance, recording output, error and exit code.
 
@@ -31,6 +31,8 @@ class Instance(object):
         target filesystem being available at /.
 
         @param command: the command to execute as root inside the image
+            if command is a string, then it will be executed as:
+            ['bash', '-c', command]
         @param stdout, stderr: file handles to write output and error to
         @param env: environment variables
         @param rcs: allowed return codes from command
@@ -85,8 +87,8 @@ class Instance(object):
         script_path = self.tmpfile()
         try:
             self.write_data(script_path, script)
-            return self.execute(
-                ['/bin/bash', script_path], rcs=rcs, description=description)
+            return self.execute('/bin/bash %s' % script_path,
+                                rcs=rcs, description=description)
         finally:
             self.execute(['rm', script_path], rcs=rcs)
 
@@ -95,7 +97,7 @@ class Instance(object):
 
         @return_value: path to new file in target
         """
-        return self.execute(['mktemp'])[0].strip()
+        return self.execute('mktemp')[0].strip()
 
     def console_log(self):
         """Instance console.
diff --git a/tests/cloud_tests/instances/lxd.py b/tests/cloud_tests/instances/lxd.py
index b9c2cc6..d1ce33b 100644
--- a/tests/cloud_tests/instances/lxd.py
+++ b/tests/cloud_tests/instances/lxd.py
@@ -31,7 +31,7 @@ class LXDInstance(base.Instance):
         self._pylxd_container.sync()
         return self._pylxd_container
 
-    def execute(self, command, stdout=None, stderr=None, env={},
+    def execute(self, command, stdout=None, stderr=None, env=None,
                 rcs=None, description=None):
         """Execute command in instance, recording output, error and exit code.
 
@@ -46,6 +46,12 @@ class LXDInstance(base.Instance):
         @param description: purpose of command
         @return_value: tuple containing stdout data, stderr data, exit code
         """
+        if env is None:
+            env = {}
+
+        if isinstance(command, str):
+            command = ['bash', '-c', command]
+
         # ensure instance is running and execute the command
         self.start()
         res = self.pylxd_container.execute(command, environment=env)
diff --git a/tests/cloud_tests/setup_image.py b/tests/cloud_tests/setup_image.py
index 8053a09..27356b3 100644
--- a/tests/cloud_tests/setup_image.py
+++ b/tests/cloud_tests/setup_image.py
@@ -19,9 +19,9 @@ def installed_package_version(image, package, ensure_installed=True):
     """
     os_family = util.get_os_family(image.properties['os'])
     if os_family == 'debian':
-        cmd = ['dpkg-query', '-W', "--showformat='${Version}'", package]
+        cmd = 'dpkg-query --show --showformat=\${Version} %s' % package
     elif os_family == 'redhat':
-        cmd = ['rpm', '-q', '--queryformat', "'%{VERSION}'", package]
+        cmd = 'rpm -q --queryformat \'%{VERSION}\' %s' % package
     else:
         raise NotImplementedError
 
@@ -49,16 +49,16 @@ def install_deb(args, image):
     LOG.debug(msg)
     remote_path = os.path.join('/tmp', os.path.basename(args.deb))
     image.push_file(args.deb, remote_path)
-    cmd = 'dpkg -i {} || apt-get install --yes -f'.format(remote_path)
-    image.execute(['/bin/sh', '-c', cmd], description=msg)
+    cmd = 'dpkg -i {}; apt-get install --yes -f'.format(remote_path)
+    image.execute(cmd, description=msg)
 
     # check installed deb version matches package
-    fmt = ['-W', "--showformat='${Version}'"]
-    (out, err, exit) = image.execute(['dpkg-deb'] + fmt + [remote_path])
+    cmd = 'dpkg-deb --show --showformat=\${Version} %s' % remote_path
+    (out, err, exit) = image.execute(cmd)
     expected_version = out.strip()
     found_version = installed_package_version(image, 'cloud-init')
     if expected_version != found_version:
-        raise OSError('install deb version "{}" does not match expected "{}"'
+        raise OSError('install deb version {} does not match expected {}'
                       .format(found_version, expected_version))
 
     LOG.debug('successfully installed: %s, version: %s', args.deb,
@@ -113,7 +113,7 @@ def upgrade(args, image):
 
     msg = 'upgrading cloud-init'
     LOG.debug(msg)
-    image.execute(['/bin/sh', '-c', cmd], description=msg)
+    image.execute(cmd, description=msg)
 
 
 def upgrade_full(args, image):
@@ -134,7 +134,7 @@ def upgrade_full(args, image):
 
     msg = 'full system upgrade'
     LOG.debug(msg)
-    image.execute(['/bin/sh', '-c', cmd], description=msg)
+    image.execute(cmd, description=msg)
 
 
 def run_script(args, image):
@@ -165,7 +165,7 @@ def enable_ppa(args, image):
     msg = 'enable ppa: "{}" in target'.format(ppa)
     LOG.debug(msg)
     cmd = 'add-apt-repository --yes {} && apt-get update'.format(ppa)
-    image.execute(['/bin/sh', '-c', cmd], description=msg)
+    image.execute(cmd, description=msg)
 
 
 def enable_repo(args, image):
@@ -188,7 +188,7 @@ def enable_repo(args, image):
 
     msg = 'enable repo: "{}" in target'.format(args.repo)
     LOG.debug(msg)
-    image.execute(['/bin/sh', '-c', cmd], description=msg)
+    image.execute(cmd, description=msg)
 
 
 def setup_image(args, image):