Looks pretty solid. Just a few in-line comments/fixes
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:
> + 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 '
I think you already collected host arch, we could use that to expand the
qemu-system-${ARCH};
Also, we may be interested in re-using curtin's tools/xkvm (it handles the arch
automatically as well as abstracting some of the device naming so this works
more generically where ever *kvm* can run.
> + '-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
> diff --git a/tests/cloud_tests/platforms/kvm.py
> b/tests/cloud_tests/platforms/kvm.py
> new file mode 100644
> index 0000000..83ca4d6
> --- /dev/null
> +++ b/tests/cloud_tests/platforms/kvm.py
> @@ -0,0 +1,96 @@
> +# 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):
> + """LXD test platform."""
s/LXD/KVM
> +
> + 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
> + """
> + 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(img_conf['mirror_url'],
> + policy=policy)
> + tstore = objectstores.FileStore(img_conf['mirror_dir'])
> + tmirror = mirrors.ObjectFilterMirror(config=mirror_config,
> + objectstore=tstore)
> + tmirror.sync(smirror, img_conf['mirror_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)
> +
> + # 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.
_______________________________________________
Mailing list: https://launchpad.net/~cloud-init-dev
Post to : cloud-init-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~cloud-init-dev
More help : https://help.launchpad.net/ListHelp
