cloud-init-dev team mailing list archive

[Ryan Harper <ryan.harper@xxxxxxxxxxxxx>]

There are quite a few with open() as fh: fh.read() chunks; if you're using cloudinit.util  then you can just use util.load_file()

same for writing (util.write_file())

Diff comments:

> diff --git a/tests/cloud_tests/instances/base.py b/tests/cloud_tests/instances/base.py
> index 959e9cc..af6c896 100644
> --- a/tests/cloud_tests/instances/base.py
> +++ b/tests/cloud_tests/instances/base.py
> @@ -85,17 +85,17 @@ 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)
> +            self.execute('rm %s' % script_path, rcs=rcs)

This isn't really an issue for this merge but I wonder why the rcs (return codes) for the script are used here when we're doing cleanup; I would expect a -f, or rcs=0 (ignoring what may be acceptable for the script execution.

>  
>      def tmpfile(self):
>          """Get a tmp file in the target.
>  
>          @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/kvm.py b/tests/cloud_tests/instances/kvm.py
> new file mode 100644
> index 0000000..0c504e5
> --- /dev/null
> +++ b/tests/cloud_tests/instances/kvm.py
> @@ -0,0 +1,201 @@
> +# This file is part of cloud-init. See LICENSE file for license information.
> +
> +"""Base KVM instance."""
> +
> +import os
> +import paramiko
> +import shlex
> +import socket
> +import subprocess
> +import time
> +
> +from cloudinit import util as c_util
> +from tests.cloud_tests.instances import base
> +from tests.cloud_tests import util
> +
> +
> +class KVMInstance(base.Instance):
> +    """KVM backed instance."""
> +
> +    platform_name = "kvm"
> +
> +    def __init__(self, platform, name, properties, config, features,
> +                 user_data, meta_data):
> +        """Set up instance.
> +
> +        @param platform: platform object
> +        @param name: image path
> +        @param properties: dictionary of properties
> +        @param config: dictionary of configuration values
> +        @param features: dictionary of supported feature flags
> +        """
> +        self.user_data = user_data
> +        self.meta_data = meta_data
> +        self.ssh_key_file = os.path.join(platform.config['data_dir'],
> +                                         platform.config['private_key'])
> +        self.ssh_port = None
> +        self.pid = None
> +        self.pid_file = None
> +
> +        super(KVMInstance, self).__init__(
> +            platform, name, properties, config, features)
> +
> +    def destroy(self):
> +        """Clean up instance."""
> +        if self.pid:
> +            c_util.subp(['kill', '-9', self.pid])
> +            os.remove(self.pid_file)
> +        super(KVMInstance, self).destroy()
> +
> +    def execute(self, command, stdout=None, stderr=None, env={},
> +                rcs=None, description=None):
> +        """Execute command in instance.
> +
> +        Assumes functional networking and execution as root with the
> +        target filesystem being available at /.
> +
> +        @param command: the command to execute as root inside the image
> +        @param stdout, stderr: file handles to write output and error to
> +        @param env: environment variables
> +        @param rcs: allowed return codes from command
> +        @param description: purpose of command
> +        @return_value: tuple containing stdout data, stderr data, exit code
> +        """
> +        if self.pid:
> +            return self.ssh(command)
> +        else:
> +            return self.mount_image_callback(command)
> +
> +    def mount_image_callback(self, cmd):
> +        """Run mount-image-callback."""
> +        mic = ('sudo mount-image-callback --system-mounts --system-resolvconf '
> +               '%s -- chroot _MOUNTPOINT_ /bin/sh' % self.name)
> +
> +        out, err = c_util.subp(shlex.split(mic), data=cmd)
> +
> +        return out, err, 0
> +
> +    def generate_seed(self, tmpdir):
> +        """Generate nocloud seed from user-data"""
> +        seed_file = os.path.join(tmpdir, '%s_seed.img' % self.name)
> +        user_data_file = os.path.join(tmpdir, '%s_user_data' % self.name)
> +
> +        if os.path.exists(seed_file):
> +            os.remove(seed_file)
> +        if os.path.exists(user_data_file):
> +            os.remove(user_data_file)
> +
> +        with open(user_data_file, "w") as ud_file:
> +            ud_file.write(self.user_data)
> +
> +        c_util.subp(['cloud-localds', seed_file, user_data_file])
> +
> +        return seed_file
> +
> +    def get_free_port(self):
> +        """Get a free port assigned by the kernel."""
> +        s = socket.socket()
> +        s.bind(('', 0))
> +        num = s.getsockname()[1]
> +        s.close()
> +        return num
> +
> +    def push_file(self, local_path, remote_path, description=''):

looks like description is unused here

> +        """Copy file at 'local_path' to instance at 'remote_path'.
> +
> +        If we have a pid then SSH is up, otherwise, use
> +        mount-image-callback.
> +
> +        @param local_path: path on local instance
> +        @param remote_path: path on remote instance
> +        """
> +        if self.pid:
> +            super(KVMInstance, self).push_file()
> +        else:
> +            cmd = ("sudo mount-image-callback --system-mounts "
> +                   "--system-resolvconf %s -- chroot _MOUNTPOINT_ "
> +                   "/bin/sh -c 'cat - > %s'" % (self.name, remote_path))
> +            local_file = open(local_path)
> +            p = subprocess.Popen(shlex.split(cmd),
> +                                 stdin=local_file,
> +                                 stdout=subprocess.PIPE,
> +                                 stderr=subprocess.PIPE)
> +            p.wait()
> +
> +    def sftp_put(self, path, data):
> +        """SFTP put a file."""
> +        client = self._ssh_connect()
> +        sftp = client.open_sftp()
> +
> +        with sftp.open(path, 'w') as f:
> +            f.write(data)
> +
> +        client.close()
> +
> +    def ssh(self, command):
> +        """Run a command via SSH."""
> +        client = self._ssh_connect()
> +        _, out, err = client.exec_command(command)
> +        exit = out.channel.recv_exit_status()
> +        out = ''.join(out.readlines())
> +        err = ''.join(err.readlines())
> +        client.close()
> +
> +        return out, err, exit
> +
> +    def _ssh_connect(self, hostname='localhost', username='ubuntu',
> +                     banner_timeout=120, retry_attempts=30):
> +        """Connect via SSH."""
> +        private_key = paramiko.RSAKey.from_private_key_file(self.ssh_key_file)
> +        client = paramiko.SSHClient()
> +        client.set_missing_host_key_policy(paramiko.AutoAddPolicy())
> +        while retry_attempts:
> +            try:
> +                client.connect(hostname=hostname, username=username,
> +                               port=self.ssh_port, pkey=private_key,
> +                               banner_timeout=banner_timeout)
> +                return client
> +            except (paramiko.SSHException, TypeError):
> +                time.sleep(1)
> +                retry_attempts = retry_attempts - 1
> +
> +        error_desc = 'Failed command to: %s@%s:%s' % (username, hostname,
> +                                                      self.ssh_port)
> +        raise util.InTargetExecuteError('', '', -1, 'ssh connect',
> +                                        self.name, error_desc)
> +
> +    def start(self, wait=True, wait_for_cloud_init=False):
> +        """Start instance."""
> +        tmpdir = self.platform.config['data_dir']
> +        seed = self.generate_seed(tmpdir)
> +        self.pid_file = os.path.join(tmpdir, '%s.pid' % self.name)
> +        self.ssh_port = self.get_free_port()
> +
> +        cmd = ('./tools/xkvm --disk %s,cache=unsafe --disk %s,cache=unsafe '
> +               '--netdev user,hostfwd=tcp::%s-:22 '
> +               '-- -pidfile %s -vnc none -m 2G -smp 2'
> +               % (self.name, seed, self.ssh_port, self.pid_file))
> +
> +        subprocess.Popen(shlex.split(cmd), close_fds=True,
> +                         stdin=subprocess.PIPE,
> +                         stdout=subprocess.PIPE,
> +                         stderr=subprocess.PIPE)
> +
> +        while not os.path.exists(self.pid_file):
> +            time.sleep(1)
> +
> +        with open(self.pid_file, 'r') as pid_f:
> +            self.pid = pid_f.readlines()[0].strip()
> +
> +        if wait:
> +            self._wait_for_system(wait_for_cloud_init)
> +
> +    def write_data(self, remote_path, data):
> +        """Write data to instance filesystem.
> +
> +        @param remote_path: path in instance
> +        @param data: data to write, either str or bytes
> +        """
> +        self.sftp_put(remote_path, data)
> +
> +# vi: ts=4 expandtab
> diff --git a/tests/cloud_tests/instances/lxd.py b/tests/cloud_tests/instances/lxd.py
> index b9c2cc6..c3a7c13 100644
> --- a/tests/cloud_tests/instances/lxd.py
> +++ b/tests/cloud_tests/instances/lxd.py
> @@ -48,6 +48,7 @@ class LXDInstance(base.Instance):
>          """
>          # ensure instance is running and execute the command
>          self.start()
> +        command = ['/bin/bash', '-c'] + [command]

this seems unrelated?

>          res = self.pylxd_container.execute(command, environment=env)
>  
>          # get out, exit and err from pylxd return
> diff --git a/tests/cloud_tests/platforms/kvm.py b/tests/cloud_tests/platforms/kvm.py
> new file mode 100644
> index 0000000..70808f2
> --- /dev/null
> +++ b/tests/cloud_tests/platforms/kvm.py
> @@ -0,0 +1,98 @@
> +# This file is part of cloud-init. See LICENSE file for license information.
> +
> +"""Base KVM platform."""
> +import glob
> +import os
> +import shutil
> +
> +from simplestreams import filters
> +from simplestreams import mirrors
> +from simplestreams import objectstores
> +from simplestreams import util as s_util
> +
> +from cloudinit import util as c_util
> +from tests.cloud_tests.images import kvm as kvm_image
> +from tests.cloud_tests.instances import kvm as kvm_instance
> +from tests.cloud_tests.platforms import base
> +from tests.cloud_tests import util
> +
> +
> +class KVMPlatform(base.Platform):
> +    """KVM test platform."""
> +
> +    platform_name = 'kvm'
> +
> +    def __init__(self, config):
> +        """Set up platform."""
> +        super(KVMPlatform, self).__init__(config)
> +
> +    def get_image(self, img_conf):
> +        """Get image using specified image configuration.
> +
> +        @param img_conf: configuration for image
> +        @return_value: cloud_tests.images instance
> +        """
> +        (url, path) = s_util.path_from_mirror_url(img_conf['mirror_url'], None)
> +
> +        filter = filters.get_filters(['arch=%s' % c_util.get_architecture(),
> +                                      'release=%s' % img_conf['release'],
> +                                      'ftype=disk1.img'])
> +        mirror_config = {'filters': filter,
> +                         'keep_items': False,
> +                         'max_items': 1,
> +                         'checksumming_reader': True,
> +                         'item_download': True
> +                         }
> +
> +        def policy(content, path):
> +            return s_util.read_signed(content, keyring=img_conf['keyring'])
> +
> +        smirror = mirrors.UrlMirrorReader(url,
> +                                          policy=policy)
> +        tstore = objectstores.FileStore(img_conf['mirror_dir'])
> +        tmirror = mirrors.ObjectFilterMirror(config=mirror_config,
> +                                             objectstore=tstore)
> +        tmirror.sync(smirror, path)
> +
> +        search_d = os.path.join(img_conf['mirror_dir'], '**',
> +                                img_conf['release'], '**', '*.img')
> +
> +        images = []
> +        for fname in glob.iglob(search_d, recursive=True):
> +            images.append(fname)
> +
> +        if len(images) != 1:
> +            raise Exception('No unique images found')
> +
> +        image = kvm_image.KVMImage(self, img_conf, images[0])
> +        if img_conf.get('override_templates', False):
> +            image.update_templates(self.config.get('template_overrides', {}),
> +                                   self.config.get('template_files', {}))
> +        return image
> +
> +    def create_image(self, properties, config, features,
> +                     src_img_path, image_desc=None, use_desc=None,
> +                     user_data=None, meta_data=None):
> +        """Create an image
> +
> +        @param src_image_path: image path to launch from
> +        @param properties: image properties
> +        @param config: image configuration
> +        @param features: image features
> +        @param image_desc: description of image being launched
> +        @param use_desc: description of container's use
> +        @return_value: cloud_tests.instances instance
> +        """
> +        name = util.gen_instance_name(image_desc=image_desc, use_desc=use_desc)
> +        img_path = os.path.join(self.config['data_dir'], name + '.qcow2')
> +        shutil.copy2(src_img_path, img_path)

we could instead use qemu-img create -f qcow2 -b /path/to/src/image /path/to/instance/img

this would avoid copy io and provide each test a clean copy of the source image.

> +
> +        # create copy of the latest image, return that as an instance
> +        return kvm_instance.KVMInstance(self, img_path, properties, config,
> +                                        features, user_data, meta_data)
> +
> +    def destroy(self):
> +        """Clean up platform data."""
> +        super(KVMPlatform, self).destroy()
> +
> +# vi: ts=4 expandtab


-- 
https://code.launchpad.net/~powersj/cloud-init/+git/cloud-init/+merge/327646
Your team cloud-init commiters is requested to review the proposed merge of ~powersj/cloud-init:cii-kvm into cloud-init:master.