cloud-init-dev team mailing list archive

[Chad Smith <chad.smith@xxxxxxxxxxxxx>]

Diff comments:

> diff --git a/tests/cloud_tests/instances/kvm.py b/tests/cloud_tests/instances/kvm.py
> new file mode 100644
> index 0000000..c855e07
> --- /dev/null
> +++ b/tests/cloud_tests/instances/kvm.py
> @@ -0,0 +1,222 @@
> +# 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: image properties
> +        @param config: image config
> +        @param features: 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:
> +            out, err, exit = self.ssh(command)
> +        else:
> +            out, err, exit = self.mount_image_callback(command)
> +
> +        # write data to file descriptors if needed
> +        if stdout:
> +            stdout.write(out.readlines())
> +        if stderr:
> +            stderr.write(err.readlines())
> +
> +        # if the command exited with a code not allowed in rcs, then fail
> +        if exit not in (rcs if rcs else (0,)):
> +            error_desc = ('Failed command to: {}'.format(description)
> +                          if description else None)
> +            raise util.InTargetExecuteError(
> +                out, err, exit, command, self.name, error_desc)
> +
> +        return (out, err, exit)
> +
> +    def mount_image_callback(self, command,):
> +        """Run mount-image-callback."""
> +        img_shell = subprocess.Popen(['sudo', 'mount-image-callback',
> +                                      '--system-mounts', '--system-resolvconf',
> +                                      self.name, 'chroot', '_MOUNTPOINT_',
> +                                      '/bin/sh'],
> +                                     stdin=subprocess.PIPE,
> +                                     stdout=subprocess.PIPE,
> +                                     stderr=subprocess.PIPE)
> +        out, err = img_shell.communicate(command.encode())
> +        exit = img_shell.returncode
> +        img_shell.stdin.close()
> +        img_shell.wait()
> +
> +        return out, err, exit
> +
> +    def generate_seed(self, tmpdir):
> +        """Generate nocloud seed from user-data"""
> +        seed_file = os.path.join(tmpdir, 'seed.img')
> +        user_data_file = os.path.join(tmpdir, 'user_data')
> +
> +        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=''):
> +        """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:
> +            command = ('sudo mount-image-callback --system-mounts '
> +                       '--system-resolvconf %s -- chroot _MOUNTPOINT_ '
> +                       '/bin/sh -ec "cat > %s" < %s'
> +                       % (self.name, remote_path, local_path))
> +            img_shell = subprocess.Popen(command, shell=True,
> +                                         stdin=subprocess.PIPE,
> +                                         stdout=subprocess.PIPE,
> +                                         stderr=subprocess.PIPE)
> +            out, err = img_shell.communicate()
> +            img_shell.stdin.close()
> +            img_shell.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)
> +                break
> +            except paramiko.SSHException:

Looks like we also need to catch paramiko.NoValidConnectionsError here too. (I'm seeing it on my system a few times with a bad traceback

  File "/usr/lib/python3/dist-packages/paramiko/client.py", line 311, in connect
    raise NoValidConnectionsError(errors)
  File "/usr/lib/python3/dist-packages/paramiko/ssh_exception.py", line 166, in __init__
    body = ', '.join([x[0] for x in addrs[:-1]])

Also it looks like paramiko itself may have a bug as it can't generate the NoValidConnectionError exception error message. It falls over with a TypeError which gets raised instead 'dict_keys' object is not subscriptable. Which means that paramiko has a bug in their NoValidConnectionsError definition:
 Bug: addr should be list(errors.keys() instead of just errors.keys() line 165 of /usr/lib/python3/dist-packages/paramiko/ssh_exception.py.

I'm filing a bug and fix against them nowm but we may need to include TypeError here temporarily as well.


Also, since we are retrying, can we log the error message and how many retries remain.

> +                time.sleep(1)
> +                retry_attempts = retry_attempts - 1
> +
> +        return client
> +
> +    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, 'pid')
> +        self.ssh_port = self.get_free_port()
> +
> +        cmd = ('qemu-system-x86_64 -enable-kvm -m 1024 '
> +               '-pidfile %s -vnc none '
> +               '-drive file=%s,format=qcow2,if=virtio '
> +               '-drive file=%s,format=raw,if=virtio '
> +               '-device virtio-net-pci,netdev=net00 '
> +               '-netdev type=user,id=net00,hostfwd=tcp::%s-:22'
> +               % (self.pid_file, self.name, seed, self.ssh_port))
> +
> +        subprocess.Popen(shlex.split(cmd), close_fds=True)
> +        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()
> +
> +        time.sleep(10)
> +
> +        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


-- 
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.