From 8622491c29f30862a1a1d7ad2cba023981acc8ce Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Mon, 6 Nov 2017 17:39:00 -0500 Subject: tests: integration test cleanup and full pass of nocloud-kvm. Integration test harness changes: * Enable collection of console log in nocloud-kvm and lxd. * Collect the console log to results for all test runs. * change 'tmpfile' to pick name locally instead of using 'mktemp'. * drop the 'instance' attribute from nocloud-kvm Image and demote LXDImage.instance to a private attribute. This is because Images do not actually have instances. (LXDImage internally uses a booted system to modify the image). * Add 'TargetBase' as a superclass of Image and Instance providing implementations of execute, read_data, write_data, pull_file, and push_file. These all depend on an implementation of _execute. * Improve '_execute' implementations to support accepting stdin. * execute supports 'rcs=False' meaning 'do not raise exception'. * Drop support for pylxd < 2.2. older versions cannot determine exit code of 'execute', which makes them unusable. * make NoCloudKVMInstance._execute run as root via sudo. This required some changes so that 'hostname' could be reverse-looked up in order to avoid sudo taking a long time (~20 seconds). * re-use existing ssh connection in nocloud-kvm. Test changes here: * do not use /tmp, but rather /var/tmp (LP: #1707222) * make keys_to_console assertions more strict. * change user test cases to always add default (ubuntu) user so that nocloud-kvm's execute which operates over ssh can work. --- tests/cloud_tests/collect.py | 18 ++- tests/cloud_tests/images/base.py | 19 +-- tests/cloud_tests/images/lxd.py | 32 ++-- tests/cloud_tests/images/nocloudkvm.py | 42 +++--- tests/cloud_tests/instances/base.py | 81 +---------- tests/cloud_tests/instances/lxd.py | 104 +++++++------ tests/cloud_tests/instances/nocloudkvm.py | 143 +++++++----------- .../testcases/examples/run_commands.yaml | 4 +- .../testcases/modules/keys_to_console.py | 8 +- tests/cloud_tests/testcases/modules/runcmd.yaml | 4 +- .../cloud_tests/testcases/modules/set_hostname.py | 4 +- .../testcases/modules/set_hostname.yaml | 3 +- .../testcases/modules/set_hostname_fqdn.py | 10 +- .../testcases/modules/set_hostname_fqdn.yaml | 4 +- .../testcases/modules/set_password_expire.py | 2 +- .../testcases/modules/set_password_expire.yaml | 2 + .../testcases/modules/set_password_list.yaml | 1 + .../modules/set_password_list_string.yaml | 1 + .../modules/ssh_auth_key_fingerprints_disable.py | 8 - .../modules/ssh_auth_key_fingerprints_disable.yaml | 1 - .../testcases/modules/ssh_keys_generate.py | 5 - .../testcases/modules/ssh_keys_generate.yaml | 6 - .../testcases/modules/ssh_keys_provided.py | 11 -- .../testcases/modules/ssh_keys_provided.yaml | 6 - tests/cloud_tests/util.py | 162 ++++++++++++++++++++- 25 files changed, 348 insertions(+), 333 deletions(-) (limited to 'tests') diff --git a/tests/cloud_tests/collect.py b/tests/cloud_tests/collect.py index 4a2422ed..71ee7645 100644 --- a/tests/cloud_tests/collect.py +++ b/tests/cloud_tests/collect.py @@ -22,11 +22,21 @@ def collect_script(instance, base_dir, script, script_name): """ LOG.debug('running collect script: %s', script_name) (out, err, exit) = instance.run_script( - script, rcs=range(0, 256), + script.encode(), rcs=False, description='collect: {}'.format(script_name)) c_util.write_file(os.path.join(base_dir, script_name), out) +def collect_console(instance, base_dir): + LOG.debug('getting console log') + try: + data = instance.console_log() + except NotImplementedError as e: + data = 'Not Implemented: %s' % e + with open(os.path.join(base_dir, 'console.log'), "wb") as fp: + fp.write(data) + + def collect_test_data(args, snapshot, os_name, test_name): """Collect data for test case. @@ -79,8 +89,12 @@ def collect_test_data(args, snapshot, os_name, test_name): test_output_dir, script, script_name)) for script_name, script in test_scripts.items()] + console_log = partial( + run_single, 'collect console', + partial(collect_console, instance, test_output_dir)) + res = run_stage('collect for test: {}'.format(test_name), - [start_call] + collect_calls) + [start_call] + collect_calls + [console_log]) return res diff --git a/tests/cloud_tests/images/base.py b/tests/cloud_tests/images/base.py index 0a1e0563..d503108a 100644 --- a/tests/cloud_tests/images/base.py +++ b/tests/cloud_tests/images/base.py @@ -2,8 +2,10 @@ """Base class for images.""" +from ..util import TargetBase -class Image(object): + +class Image(TargetBase): """Base class for images.""" platform_name = None @@ -43,21 +45,6 @@ class Image(object): # NOTE: more sophisticated options may be requied at some point return self.config.get('setup_overrides', {}) - def execute(self, *args, **kwargs): - """Execute command in image, modifying image.""" - raise NotImplementedError - - def push_file(self, local_path, remote_path): - """Copy file at 'local_path' to instance at 'remote_path'.""" - raise NotImplementedError - - def run_script(self, *args, **kwargs): - """Run script in image, modifying image. - - @return_value: script output - """ - raise NotImplementedError - def snapshot(self): """Create snapshot of image, block until done.""" raise NotImplementedError diff --git a/tests/cloud_tests/images/lxd.py b/tests/cloud_tests/images/lxd.py index fd4e93c2..5caeba41 100644 --- a/tests/cloud_tests/images/lxd.py +++ b/tests/cloud_tests/images/lxd.py @@ -24,7 +24,7 @@ class LXDImage(base.Image): @param config: image configuration """ self.modified = False - self._instance = None + self._img_instance = None self._pylxd_image = None self.pylxd_image = pylxd_image super(LXDImage, self).__init__(platform, config) @@ -38,9 +38,9 @@ class LXDImage(base.Image): @pylxd_image.setter def pylxd_image(self, pylxd_image): - if self._instance: + if self._img_instance: self._instance.destroy() - self._instance = None + self._img_instance = None if (self._pylxd_image and (self._pylxd_image is not pylxd_image) and (not self.config.get('cache_base_image') or self.modified)): @@ -49,15 +49,19 @@ class LXDImage(base.Image): self._pylxd_image = pylxd_image @property - def instance(self): - """Property function.""" - if not self._instance: - self._instance = self.platform.launch_container( + def _instance(self): + """Internal use only, returns a instance + + This starts an lxc instance from the image, so it is "dirty". + Better would be some way to modify this "at rest". + lxc-pstart would be an option.""" + if not self._img_instance: + self._img_instance = self.platform.launch_container( self.properties, self.config, self.features, use_desc='image-modification', image_desc=str(self), image=self.pylxd_image.fingerprint) - self._instance.start() - return self._instance + self._img_instance.start() + return self._img_instance @property def properties(self): @@ -144,20 +148,20 @@ class LXDImage(base.Image): shutil.rmtree(export_dir) shutil.rmtree(extract_dir) - def execute(self, *args, **kwargs): + def _execute(self, *args, **kwargs): """Execute command in image, modifying image.""" - return self.instance.execute(*args, **kwargs) + return self._instance._execute(*args, **kwargs) def push_file(self, local_path, remote_path): """Copy file at 'local_path' to instance at 'remote_path'.""" - return self.instance.push_file(local_path, remote_path) + return self._instance.push_file(local_path, remote_path) def run_script(self, *args, **kwargs): """Run script in image, modifying image. @return_value: script output """ - return self.instance.run_script(*args, **kwargs) + return self._instance.run_script(*args, **kwargs) def snapshot(self): """Create snapshot of image, block until done.""" @@ -169,7 +173,7 @@ class LXDImage(base.Image): # clone current instance instance = self.platform.launch_container( self.properties, self.config, self.features, - container=self.instance.name, image_desc=str(self), + container=self._instance.name, image_desc=str(self), use_desc='snapshot', container_config=conf) # wait for cloud-init before boot_clean_script is run to ensure # /var/lib/cloud is removed cleanly diff --git a/tests/cloud_tests/images/nocloudkvm.py b/tests/cloud_tests/images/nocloudkvm.py index a7af0e59..1e7962cb 100644 --- a/tests/cloud_tests/images/nocloudkvm.py +++ b/tests/cloud_tests/images/nocloudkvm.py @@ -2,6 +2,8 @@ """NoCloud KVM Image Base Class.""" +from cloudinit import util as c_util + from tests.cloud_tests.images import base from tests.cloud_tests.snapshots import nocloudkvm as nocloud_kvm_snapshot @@ -19,23 +21,10 @@ class NoCloudKVMImage(base.Image): @param img_path: path to the image """ self.modified = False - self._instance = None self._img_path = img_path super(NoCloudKVMImage, self).__init__(platform, config) - @property - def instance(self): - """Returns an instance of an image.""" - if not self._instance: - if not self._img_path: - raise RuntimeError() - - self._instance = self.platform.create_image( - self.properties, self.config, self.features, self._img_path, - image_desc=str(self), use_desc='image-modification') - return self._instance - @property def properties(self): """Dictionary containing: 'arch', 'os', 'version', 'release'.""" @@ -46,20 +35,26 @@ class NoCloudKVMImage(base.Image): 'version': self.config['version'], } - def execute(self, *args, **kwargs): + def _execute(self, command, stdin=None, env=None): """Execute command in image, modifying image.""" - return self.instance.execute(*args, **kwargs) + return self.mount_image_callback(command, stdin=stdin, env=env) - def push_file(self, local_path, remote_path): - """Copy file at 'local_path' to instance at 'remote_path'.""" - return self.instance.push_file(local_path, remote_path) + def mount_image_callback(self, command, stdin=None, env=None): + """Run mount-image-callback.""" - def run_script(self, *args, **kwargs): - """Run script in image, modifying image. + env_args = [] + if env: + env_args = ['env'] + ["%s=%s" for k, v in env.items()] - @return_value: script output - """ - return self.instance.run_script(*args, **kwargs) + mic_chroot = ['sudo', 'mount-image-callback', '--system-mounts', + '--system-resolvconf', self._img_path, + '--', 'chroot', '_MOUNTPOINT_'] + try: + out, err = c_util.subp(mic_chroot + env_args + list(command), + data=stdin, decode=False) + return (out, err, 0) + except c_util.ProcessExecutionError as e: + return (e.stdout, e.stderr, e.exit_code) def snapshot(self): """Create snapshot of image, block until done.""" @@ -82,7 +77,6 @@ class NoCloudKVMImage(base.Image): framework decide whether to keep or destroy everything. """ self._img_path = None - self._instance.destroy() super(NoCloudKVMImage, self).destroy() # vi: ts=4 expandtab diff --git a/tests/cloud_tests/instances/base.py b/tests/cloud_tests/instances/base.py index 9bdda608..8c59d62c 100644 --- a/tests/cloud_tests/instances/base.py +++ b/tests/cloud_tests/instances/base.py @@ -2,8 +2,10 @@ """Base instance.""" +from ..util import TargetBase -class Instance(object): + +class Instance(TargetBase): """Base instance object.""" platform_name = None @@ -22,82 +24,7 @@ class Instance(object): self.properties = properties self.config = config self.features = features - - def execute(self, command, stdout=None, stderr=None, env=None, - rcs=None, description=None): - """Execute command in instance, recording output, error and exit code. - - 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 - if command is a string, then it will be executed as: - ['sh', '-c', command] - @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 - """ - raise NotImplementedError - - def read_data(self, remote_path, decode=False): - """Read data from instance filesystem. - - @param remote_path: path in instance - @param decode: return as string - @return_value: data as str or bytes - """ - raise NotImplementedError - - 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 - """ - raise NotImplementedError - - def pull_file(self, remote_path, local_path): - """Copy file at 'remote_path', from instance to 'local_path'. - - @param remote_path: path on remote instance - @param local_path: path on local instance - """ - with open(local_path, 'wb') as fp: - fp.write(self.read_data(remote_path)) - - def push_file(self, local_path, remote_path): - """Copy file at 'local_path' to instance at 'remote_path'. - - @param local_path: path on local instance - @param remote_path: path on remote instance - """ - with open(local_path, 'rb') as fp: - self.write_data(remote_path, fp.read()) - - def run_script(self, script, rcs=None, description=None): - """Run script in target and return stdout. - - @param script: script contents - @param rcs: allowed return codes from script - @param description: purpose of script - @return_value: stdout from script - """ - script_path = self.tmpfile() - try: - self.write_data(script_path, script) - return self.execute( - ['/bin/bash', script_path], rcs=rcs, description=description) - finally: - self.execute(['rm', '-f', script_path], rcs=rcs) - - def tmpfile(self): - """Get a tmp file in the target. - - @return_value: path to new file in target - """ - return self.execute(['mktemp'])[0].strip() + self._tmp_count = 0 def console_log(self): """Instance console. diff --git a/tests/cloud_tests/instances/lxd.py b/tests/cloud_tests/instances/lxd.py index a43918c2..3b035d86 100644 --- a/tests/cloud_tests/instances/lxd.py +++ b/tests/cloud_tests/instances/lxd.py @@ -2,8 +2,11 @@ """Base LXD instance.""" -from tests.cloud_tests.instances import base -from tests.cloud_tests import util +from . import base + +import os +import shutil +from tempfile import mkdtemp class LXDInstance(base.Instance): @@ -24,6 +27,8 @@ class LXDInstance(base.Instance): self._pylxd_container = pylxd_container super(LXDInstance, self).__init__( platform, name, properties, config, features) + self.tmpd = mkdtemp(prefix="%s-%s" % (type(self).__name__, name)) + self._setup_console_log() @property def pylxd_container(self): @@ -31,74 +36,69 @@ class LXDInstance(base.Instance): self._pylxd_container.sync() return self._pylxd_container - def execute(self, command, stdout=None, stderr=None, env=None, - rcs=None, description=None): - """Execute command in instance, recording output, error and exit code. - - 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 - if command is a string, then it will be executed as: - ['sh', '-c', command] - @param stdout: file handler to write output - @param stderr: file handler to write error - @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 - """ + def _setup_console_log(self): + logf = os.path.join(self.tmpd, "console.log") + + # doing this ensures we can read it. Otherwise it ends up root:root. + with open(logf, "w") as fp: + fp.write("# %s\n" % self.name) + + cfg = "lxc.console.logfile=%s" % logf + orig = self._pylxd_container.config.get('raw.lxc', "") + if orig: + orig += "\n" + self._pylxd_container.config['raw.lxc'] = orig + cfg + self._pylxd_container.save() + self._console_log_file = logf + + def _execute(self, command, stdin=None, env=None): if env is None: env = {} - if isinstance(command, str): - command = ['sh', '-c', command] + if stdin is not None: + # pylxd does not support input to execute. + # https://github.com/lxc/pylxd/issues/244 + # + # The solution here is write a tmp file in the container + # and then execute a shell that sets it standard in to + # be from that file, removes it, and calls the comand. + tmpf = self.tmpfile() + self.write_data(tmpf, stdin) + ncmd = 'exec <"{tmpf}"; rm -f "{tmpf}"; exec "$@"' + command = (['sh', '-c', ncmd.format(tmpf=tmpf), 'stdinhack'] + + list(command)) # ensure instance is running and execute the command self.start() + # execute returns a ContainerExecuteResult, named tuple + # (exit_code, stdout, stderr) res = self.pylxd_container.execute(command, environment=env) # get out, exit and err from pylxd return - if hasattr(res, 'exit_code'): - # pylxd 2.2 returns ContainerExecuteResult, named tuple of - # (exit_code, out, err) - (exit, out, err) = res - else: + if not hasattr(res, 'exit_code'): # pylxd 2.1.3 and earlier only return out and err, no exit - # LOG.warning('using pylxd version < 2.2') - (out, err) = res - exit = 0 - - # write data to file descriptors if needed - if stdout: - stdout.write(out) - if stderr: - stderr.write(err) - - # 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) + raise RuntimeError( + "No 'exit_code' in pylxd.container.execute return.\n" + "pylxd > 2.2 is required.") - return (out, err, exit) + return res.stdout, res.stderr, res.exit_code def read_data(self, remote_path, decode=False): """Read data from instance filesystem. @param remote_path: path in instance - @param decode: return as string - @return_value: data as str or bytes + @param decode: decode data before returning. + @return_value: content of remote_path as bytes if 'decode' is False, + and as string if 'decode' is True. """ data = self.pylxd_container.files.get(remote_path) - return data.decode() if decode and isinstance(data, bytes) else data + return data.decode() if decode else data 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 + @param data: data to write in bytes """ self.pylxd_container.files.put(remote_path, data) @@ -107,7 +107,14 @@ class LXDInstance(base.Instance): @return_value: bytes of this instance’s console """ - raise NotImplementedError + if not os.path.exists(self._console_log_file): + raise NotImplementedError( + "Console log '%s' does not exist. If this is a remote " + "lxc, then this is really NotImplementedError. If it is " + "A local lxc, then this is a RuntimeError." + "https://github.com/lxc/lxd/issues/1129") + with open(self._console_log_file, "rb") as fp: + return fp.read() def reboot(self, wait=True): """Reboot instance.""" @@ -144,6 +151,7 @@ class LXDInstance(base.Instance): if self.platform.container_exists(self.name): raise OSError('container {} was not properly removed' .format(self.name)) + shutil.rmtree(self.tmpd) super(LXDInstance, self).destroy() # vi: ts=4 expandtab diff --git a/tests/cloud_tests/instances/nocloudkvm.py b/tests/cloud_tests/instances/nocloudkvm.py index 8a0e5319..fbb04aa2 100644 --- a/tests/cloud_tests/instances/nocloudkvm.py +++ b/tests/cloud_tests/instances/nocloudkvm.py @@ -12,11 +12,19 @@ from cloudinit import util as c_util from tests.cloud_tests.instances import base from tests.cloud_tests import util +# This domain contains reverse lookups for hostnames that are used. +# The primary reason is so sudo will return quickly when it attempts +# to look up the hostname. i9n is just short for 'integration'. +# use i9n.brickies.net until i9n.cloud-init.io is popualted: +# https://portal.admin.canonical.com/107125 +CI_DOMAIN = "i9n.brickies.net" + class NoCloudKVMInstance(base.Instance): """NoCloud KVM backed instance.""" platform_name = "nocloud-kvm" + _ssh_client = None def __init__(self, platform, name, properties, config, features, user_data, meta_data): @@ -35,6 +43,7 @@ class NoCloudKVMInstance(base.Instance): self.ssh_port = None self.pid = None self.pid_file = None + self.console_file = None super(NoCloudKVMInstance, self).__init__( platform, name, properties, config, features) @@ -51,43 +60,18 @@ class NoCloudKVMInstance(base.Instance): os.remove(self.pid_file) self.pid = None - super(NoCloudKVMInstance, self).destroy() - - def execute(self, command, stdout=None, stderr=None, env=None, - 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 - if command is a string, then it will be executed as: - ['sh', '-c', command] - @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 env is None: - env = {} - - if isinstance(command, str): - command = ['sh', '-c', command] + if self._ssh_client: + self._ssh_client.close() + self._ssh_client = None - if self.pid: - return self.ssh(command) - else: - return self.mount_image_callback(command) + (0,) + super(NoCloudKVMInstance, self).destroy() - def mount_image_callback(self, cmd): - """Run mount-image-callback.""" - out, err = c_util.subp(['sudo', 'mount-image-callback', - '--system-mounts', '--system-resolvconf', - self.name, '--', 'chroot', - '_MOUNTPOINT_'] + cmd) + def _execute(self, command, stdin=None, env=None): + env_args = [] + if env: + env_args = ['env'] + ["%s=%s" for k, v in env.items()] - return out, err + return self.ssh(['sudo'] + env_args + list(command), stdin=stdin) def generate_seed(self, tmpdir): """Generate nocloud seed from user-data""" @@ -109,57 +93,31 @@ class NoCloudKVMInstance(base.Instance): s.close() return num - def push_file(self, local_path, remote_path): - """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(NoCloudKVMInstance, self).push_file() - else: - local_file = open(local_path) - p = subprocess.Popen(['sudo', 'mount-image-callback', - '--system-mounts', '--system-resolvconf', - self.name, '--', 'chroot', '_MOUNTPOINT_', - '/bin/sh', '-c', 'cat - > %s' % remote_path], - 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): + def ssh(self, command, stdin=None): """Run a command via SSH.""" client = self._ssh_connect() + cmd = util.shell_pack(command) try: - _, out, err = client.exec_command(util.shell_pack(command)) - except paramiko.SSHException: - raise util.InTargetExecuteError('', '', -1, command, self.name) - - exit = out.channel.recv_exit_status() - out = ''.join(out.readlines()) - err = ''.join(err.readlines()) - client.close() - - return out, err, exit + fp_in, fp_out, fp_err = client.exec_command(cmd) + channel = fp_in.channel + if stdin is not None: + fp_in.write(stdin) + fp_in.close() + + channel.shutdown_write() + rc = channel.recv_exit_status() + return (fp_out.read(), fp_err.read(), rc) + except paramiko.SSHException as e: + raise util.InTargetExecuteError( + b'', b'', -1, command, self.name, reason=e) def _ssh_connect(self, hostname='localhost', username='ubuntu', banner_timeout=120, retry_attempts=30): """Connect via SSH.""" + if self._ssh_client: + return self._ssh_client + private_key = paramiko.RSAKey.from_private_key_file(self.ssh_key_file) client = paramiko.SSHClient() client.set_missing_host_key_policy(paramiko.AutoAddPolicy()) @@ -168,6 +126,7 @@ class NoCloudKVMInstance(base.Instance): client.connect(hostname=hostname, username=username, port=self.ssh_port, pkey=private_key, banner_timeout=banner_timeout) + self._ssh_client = client return client except (paramiko.SSHException, TypeError): time.sleep(1) @@ -183,15 +142,19 @@ class NoCloudKVMInstance(base.Instance): tmpdir = self.platform.config['data_dir'] seed = self.generate_seed(tmpdir) self.pid_file = os.path.join(tmpdir, '%s.pid' % self.name) + self.console_file = os.path.join(tmpdir, '%s-console.log' % self.name) self.ssh_port = self.get_free_port() - subprocess.Popen(['./tools/xkvm', - '--disk', '%s,cache=unsafe' % self.name, - '--disk', '%s,cache=unsafe' % seed, - '--netdev', - 'user,hostfwd=tcp::%s-:22' % self.ssh_port, - '--', '-pidfile', self.pid_file, '-vnc', 'none', - '-m', '2G', '-smp', '2'], + cmd = ['./tools/xkvm', + '--disk', '%s,cache=unsafe' % self.name, + '--disk', '%s,cache=unsafe' % seed, + '--netdev', ','.join(['user', + 'hostfwd=tcp::%s-:22' % self.ssh_port, + 'dnssearch=%s' % CI_DOMAIN]), + '--', '-pidfile', self.pid_file, '-vnc', 'none', + '-m', '2G', '-smp', '2', '-nographic', + '-serial', 'file:' + self.console_file] + subprocess.Popen(cmd, close_fds=True, stdin=subprocess.PIPE, stdout=subprocess.PIPE, @@ -206,12 +169,10 @@ class NoCloudKVMInstance(base.Instance): 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) + def console_log(self): + if not self.console_file: + return b'' + with open(self.console_file, "rb") as fp: + return fp.read() # vi: ts=4 expandtab diff --git a/tests/cloud_tests/testcases/examples/run_commands.yaml b/tests/cloud_tests/testcases/examples/run_commands.yaml index b0e311ba..f80eb8ce 100644 --- a/tests/cloud_tests/testcases/examples/run_commands.yaml +++ b/tests/cloud_tests/testcases/examples/run_commands.yaml @@ -7,10 +7,10 @@ enabled: False cloud_config: | #cloud-config runcmd: - - echo cloud-init run cmd test > /tmp/run_cmd + - echo cloud-init run cmd test > /var/tmp/run_cmd collect_scripts: run_cmd: | #!/bin/bash - cat /tmp/run_cmd + cat /var/tmp/run_cmd # vi: ts=4 expandtab diff --git a/tests/cloud_tests/testcases/modules/keys_to_console.py b/tests/cloud_tests/testcases/modules/keys_to_console.py index 88b6812e..07f38112 100644 --- a/tests/cloud_tests/testcases/modules/keys_to_console.py +++ b/tests/cloud_tests/testcases/modules/keys_to_console.py @@ -10,13 +10,13 @@ class TestKeysToConsole(base.CloudTestCase): def test_excluded_keys(self): """Test excluded keys missing.""" out = self.get_data_file('syslog') - self.assertNotIn('DSA', out) - self.assertNotIn('ECDSA', out) + self.assertNotIn('(DSA)', out) + self.assertNotIn('(ECDSA)', out) def test_expected_keys(self): """Test expected keys exist.""" out = self.get_data_file('syslog') - self.assertIn('ED25519', out) - self.assertIn('RSA', out) + self.assertIn('(ED25519)', out) + self.assertIn('(RSA)', out) # vi: ts=4 expandtab diff --git a/tests/cloud_tests/testcases/modules/runcmd.yaml b/tests/cloud_tests/testcases/modules/runcmd.yaml index 04e5a050..8309a883 100644 --- a/tests/cloud_tests/testcases/modules/runcmd.yaml +++ b/tests/cloud_tests/testcases/modules/runcmd.yaml @@ -4,10 +4,10 @@ cloud_config: | #cloud-config runcmd: - - echo cloud-init run cmd test > /tmp/run_cmd + - echo cloud-init run cmd test > /var/tmp/run_cmd collect_scripts: run_cmd: | #!/bin/bash - cat /tmp/run_cmd + cat /var/tmp/run_cmd # vi: ts=4 expandtab diff --git a/tests/cloud_tests/testcases/modules/set_hostname.py b/tests/cloud_tests/testcases/modules/set_hostname.py index 6e96a75c..1dbe64c2 100644 --- a/tests/cloud_tests/testcases/modules/set_hostname.py +++ b/tests/cloud_tests/testcases/modules/set_hostname.py @@ -7,9 +7,11 @@ from tests.cloud_tests.testcases import base class TestHostname(base.CloudTestCase): """Test hostname module.""" + ex_hostname = "cloudinit2" + def test_hostname(self): """Test hostname command shows correct output.""" out = self.get_data_file('hostname') - self.assertIn('myhostname', out) + self.assertIn(self.ex_hostname, out) # vi: ts=4 expandtab diff --git a/tests/cloud_tests/testcases/modules/set_hostname.yaml b/tests/cloud_tests/testcases/modules/set_hostname.yaml index c96344cf..071fb220 100644 --- a/tests/cloud_tests/testcases/modules/set_hostname.yaml +++ b/tests/cloud_tests/testcases/modules/set_hostname.yaml @@ -5,7 +5,8 @@ required_features: - hostname cloud_config: | #cloud-config - hostname: myhostname + hostname: cloudinit2 + collect_scripts: hosts: | #!/bin/bash diff --git a/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py b/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py index 398f3d40..08ceae01 100644 --- a/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py +++ b/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py @@ -7,20 +7,24 @@ from tests.cloud_tests.testcases import base class TestHostnameFqdn(base.CloudTestCase): """Test Hostname module.""" + ex_hostname = "cloudinit1" + ex_fqdn = "cloudinit2.i9n.brickies.net" + def test_hostname(self): """Test hostname output.""" out = self.get_data_file('hostname') - self.assertIn('myhostname', out) + self.assertIn(self.ex_hostname, out) def test_hostname_fqdn(self): """Test hostname fqdn output.""" out = self.get_data_file('fqdn') - self.assertIn('host.myorg.com', out) + self.assertIn(self.ex_fqdn, out) def test_hosts(self): """Test /etc/hosts file.""" out = self.get_data_file('hosts') - self.assertIn('127.0.1.1 host.myorg.com myhostname', out) + self.assertIn('127.0.1.1 %s %s' % (self.ex_fqdn, self.ex_hostname), + out) self.assertIn('127.0.0.1 localhost', out) # vi: ts=4 expandtab diff --git a/tests/cloud_tests/testcases/modules/set_hostname_fqdn.yaml b/tests/cloud_tests/testcases/modules/set_hostname_fqdn.yaml index daf75931..5320ac57 100644 --- a/tests/cloud_tests/testcases/modules/set_hostname_fqdn.yaml +++ b/tests/cloud_tests/testcases/modules/set_hostname_fqdn.yaml @@ -6,8 +6,8 @@ required_features: cloud_config: | #cloud-config manage_etc_hosts: true - hostname: myhostname - fqdn: host.myorg.com + hostname: cloudinit1 + fqdn: cloudinit2.i9n.brickies.net collect_scripts: hosts: | #!/bin/bash diff --git a/tests/cloud_tests/testcases/modules/set_password_expire.py b/tests/cloud_tests/testcases/modules/set_password_expire.py index a1c3aa08..967aca7b 100644 --- a/tests/cloud_tests/testcases/modules/set_password_expire.py +++ b/tests/cloud_tests/testcases/modules/set_password_expire.py @@ -18,6 +18,6 @@ class TestPasswordExpire(base.CloudTestCase): def test_sshd_config(self): """Test sshd config allows passwords.""" out = self.get_data_file('sshd_config') - self.assertIn('PasswordAuthentication no', out) + self.assertIn('PasswordAuthentication yes', out) # vi: ts=4 expandtab diff --git a/tests/cloud_tests/testcases/modules/set_password_expire.yaml b/tests/cloud_tests/testcases/modules/set_password_expire.yaml index 789604b0..ba6344b9 100644 --- a/tests/cloud_tests/testcases/modules/set_password_expire.yaml +++ b/tests/cloud_tests/testcases/modules/set_password_expire.yaml @@ -6,7 +6,9 @@ required_features: cloud_config: | #cloud-config chpasswd: { expire: True } + ssh_pwauth: yes users: + - default - name: tom password: $1$xyz$sPMsLNmf66Ohl.ol6JvzE. lock_passwd: false diff --git a/tests/cloud_tests/testcases/modules/set_password_list.yaml b/tests/cloud_tests/testcases/modules/set_password_list.yaml index a2a89c9d..fd3e1e44 100644 --- a/tests/cloud_tests/testcases/modules/set_password_list.yaml +++ b/tests/cloud_tests/testcases/modules/set_password_list.yaml @@ -5,6 +5,7 @@ cloud_config: | #cloud-config ssh_pwauth: yes users: + - default - name: tom # md5 gotomgo passwd: "$1$S7$tT1BEDIYrczeryDQJfdPe0" diff --git a/tests/cloud_tests/testcases/modules/set_password_list_string.yaml b/tests/cloud_tests/testcases/modules/set_password_list_string.yaml index c2a0f631..e9fe54b0 100644 --- a/tests/cloud_tests/testcases/modules/set_password_list_string.yaml +++ b/tests/cloud_tests/testcases/modules/set_password_list_string.yaml @@ -5,6 +5,7 @@ cloud_config: | #cloud-config ssh_pwauth: yes users: + - default - name: tom # md5 gotomgo passwd: "$1$S7$tT1BEDIYrczeryDQJfdPe0" diff --git a/tests/cloud_tests/testcases/modules/ssh_auth_key_fingerprints_disable.py b/tests/cloud_tests/testcases/modules/ssh_auth_key_fingerprints_disable.py index 82223217..e7329d48 100644 --- a/tests/cloud_tests/testcases/modules/ssh_auth_key_fingerprints_disable.py +++ b/tests/cloud_tests/testcases/modules/ssh_auth_key_fingerprints_disable.py @@ -13,12 +13,4 @@ class TestSshKeyFingerprintsDisable(base.CloudTestCase): self.assertIn('Skipping module named ssh-authkey-fingerprints, ' 'logging of ssh fingerprints disabled', out) - def test_syslog(self): - """Verify output of syslog.""" - out = self.get_data_file('syslog') - self.assertNotRegex(out, r'256 SHA256:.*(ECDSA)') - self.assertNotRegex(out, r'256 SHA256:.*(ED25519)') - self.assertNotRegex(out, r'1024 SHA256:.*(DSA)') - self.assertNotRegex(out, r'2048 SHA256:.*(RSA)') - # vi: ts=4 expandtab diff --git a/tests/cloud_tests/testcases/modules/ssh_auth_key_fingerprints_disable.yaml b/tests/cloud_tests/testcases/modules/ssh_auth_key_fingerprints_disable.yaml index 746653ec..d93893e2 100644 --- a/tests/cloud_tests/testcases/modules/ssh_auth_key_fingerprints_disable.yaml +++ b/tests/cloud_tests/testcases/modules/ssh_auth_key_fingerprints_disable.yaml @@ -5,7 +5,6 @@ required_features: - syslog cloud_config: | #cloud-config - ssh_genkeytypes: [] no_ssh_fingerprints: true collect_scripts: syslog: | diff --git a/tests/cloud_tests/testcases/modules/ssh_keys_generate.py b/tests/cloud_tests/testcases/modules/ssh_keys_generate.py index fd6d9ba5..b68f5565 100644 --- a/tests/cloud_tests/testcases/modules/ssh_keys_generate.py +++ b/tests/cloud_tests/testcases/modules/ssh_keys_generate.py @@ -9,11 +9,6 @@ class TestSshKeysGenerate(base.CloudTestCase): # TODO: Check cloud-init-output for the correct keys being generated - def test_ubuntu_authorized_keys(self): - """Test passed in key is not in list for ubuntu.""" - out = self.get_data_file('auth_keys_ubuntu') - self.assertEqual('', out) - def test_dsa_public(self): """Test dsa public key not generated.""" out = self.get_data_file('dsa_public') diff --git a/tests/cloud_tests/testcases/modules/ssh_keys_generate.yaml b/tests/cloud_tests/testcases/modules/ssh_keys_generate.yaml index 659fd939..0a7adf62 100644 --- a/tests/cloud_tests/testcases/modules/ssh_keys_generate.yaml +++ b/tests/cloud_tests/testcases/modules/ssh_keys_generate.yaml @@ -10,12 +10,6 @@ cloud_config: | - ed25519 authkey_hash: sha512 collect_scripts: - auth_keys_root: | - #!/bin/bash - cat /root/.ssh/authorized_keys - auth_keys_ubuntu: | - #!/bin/bash - cat /home/ubuntu/ssh/authorized_keys dsa_public: | #!/bin/bash cat /etc/ssh/ssh_host_dsa_key.pub diff --git a/tests/cloud_tests/testcases/modules/ssh_keys_provided.py b/tests/cloud_tests/testcases/modules/ssh_keys_provided.py index 544649da..add3f469 100644 --- a/tests/cloud_tests/testcases/modules/ssh_keys_provided.py +++ b/tests/cloud_tests/testcases/modules/ssh_keys_provided.py @@ -7,17 +7,6 @@ from tests.cloud_tests.testcases import base class TestSshKeysProvided(base.CloudTestCase): """Test ssh keys module.""" - def test_ubuntu_authorized_keys(self): - """Test passed in key is not in list for ubuntu.""" - out = self.get_data_file('auth_keys_ubuntu') - self.assertEqual('', out) - - def test_root_authorized_keys(self): - """Test passed in key is in authorized list for root.""" - out = self.get_data_file('auth_keys_root') - self.assertIn('lzrkPqONphoZx0LDV86w7RUz1ksDzAdcm0tvmNRFMN1a0frDs50' - '6oA3aWK0oDk4Nmvk8sXGTYYw3iQSkOvDUUlIsqdaO+w==', out) - def test_dsa_public(self): """Test dsa public key passed in.""" out = self.get_data_file('dsa_public') diff --git a/tests/cloud_tests/testcases/modules/ssh_keys_provided.yaml b/tests/cloud_tests/testcases/modules/ssh_keys_provided.yaml index 5ceb3623..41f63550 100644 --- a/tests/cloud_tests/testcases/modules/ssh_keys_provided.yaml +++ b/tests/cloud_tests/testcases/modules/ssh_keys_provided.yaml @@ -71,12 +71,6 @@ cloud_config: | -----END EC PRIVATE KEY----- ecdsa_public: ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBFsS5Tvky/IC/dXhE/afxxUG6kdQOvdQJCYGZN42OZqWasYF+L3IG+3/wrV7jOrNrL3AyagHl6+lpPDiSXDMcpQ= root@xenial-lxd collect_scripts: - auth_keys_root: | - #!/bin/bash - cat /root/.ssh/authorized_keys - auth_keys_ubuntu: | - #!/bin/bash - cat /home/ubuntu/ssh/authorized_keys dsa_public: | #!/bin/bash cat /etc/ssh/ssh_host_dsa_key.pub diff --git a/tests/cloud_tests/util.py b/tests/cloud_tests/util.py index 4357fbb0..92c31c3a 100644 --- a/tests/cloud_tests/util.py +++ b/tests/cloud_tests/util.py @@ -7,6 +7,7 @@ import copy import glob import os import random +import shlex import shutil import string import subprocess @@ -285,20 +286,165 @@ def shell_pack(cmd): return 'eval set -- "$(echo %s | base64 --decode)" && exec "$@"' % b64 +def shell_quote(cmd): + if isinstance(cmd, (tuple, list)): + return ' '.join([shlex.quote(x) for x in cmd]) + return shlex.quote(cmd) + + +class TargetBase(object): + _tmp_count = 0 + + def execute(self, command, stdin=None, env=None, + rcs=None, description=None): + """Execute command in instance, recording output, error and exit code. + + 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 + if command is a string, then it will be executed as: + ['sh', '-c', command] + @param stdin: bytes content for standard in + @param env: environment variables + @param rcs: return codes. + None (default): non-zero exit code will raise exception. + False: any is allowed (No execption raised). + list of int: any rc not in the list will raise exception. + @param description: purpose of command + @return_value: tuple containing stdout data, stderr data, exit code + """ + if isinstance(command, str): + command = ['sh', '-c', command] + + if rcs is None: + rcs = (0,) + + if description: + LOG.debug('Executing "%s"', description) + else: + LOG.debug("Executing command: %s", shell_quote(command)) + + out, err, rc = self._execute(command=command, stdin=stdin, env=env) + + # False means accept anything. + if (rcs is False or rc in rcs): + return out, err, rc + + raise InTargetExecuteError(out, err, rc, command, description) + + def _execute(self, command, stdin=None, env=None): + """Execute command in inside, return stdout, stderr and exit code. + + Assumes functional networking and execution as root with the + target filesystem being available at /. + + @param stdin: bytes content for standard in + @param env: environment variables + @return_value: tuple containing stdout data, stderr data, exit code + + This is intended to be implemented by the Image or Instance. + Many callers will use the higher level 'execute'.""" + raise NotImplementedError + + def read_data(self, remote_path, decode=False): + """Read data from instance filesystem. + + @param remote_path: path in instance + @param decode: decode data before returning. + @return_value: content of remote_path as bytes if 'decode' is False, + and as string if 'decode' is True. + """ + # when sh is invoked with '-c', then the first argument is "$0" + # which is commonly understood as the "program name". + # 'read_data' is the program name, and 'remote_path' is '$1' + stdout, stderr, rc = self._execute( + ["sh", "-c", 'exec cat "$1"', 'read_data', remote_path]) + if rc != 0: + raise RuntimeError("Failed to read file '%s'" % remote_path) + + if decode: + return stdout.decode() + return stdout + + def write_data(self, remote_path, data): + """Write data to instance filesystem. + + @param remote_path: path in instance + @param data: data to write in bytes + """ + # when sh is invoked with '-c', then the first argument is "$0" + # which is commonly understood as the "program name". + # 'write_data' is the program name, and 'remote_path' is '$1' + _, _, rc = self._execute( + ["sh", "-c", 'exec cat >"$1"', 'write_data', remote_path], + stdin=data) + + if rc != 0: + raise RuntimeError("Failed to write to '%s'" % remote_path) + return + + def pull_file(self, remote_path, local_path): + """Copy file at 'remote_path', from instance to 'local_path'. + + @param remote_path: path on remote instance + @param local_path: path on local instance + """ + with open(local_path, 'wb') as fp: + fp.write(self.read_data(remote_path)) + + def push_file(self, local_path, remote_path): + """Copy file at 'local_path' to instance at 'remote_path'. + + @param local_path: path on local instance + @param remote_path: path on remote instance""" + with open(local_path, "rb") as fp: + self.write_data(remote_path, data=fp.read()) + + def run_script(self, script, rcs=None, description=None): + """Run script in target and return stdout. + + @param script: script contents + @param rcs: allowed return codes from script + @param description: purpose of script + @return_value: stdout from script + """ + # Just write to a file, add execute, run it, then remove it. + shblob = '; '.join(( + 'set -e', + 's="$1"', + 'shift', + 'cat > "$s"', + 'trap "rm -f $s" EXIT', + 'chmod +x "$s"', + '"$s" "$@"')) + return self.execute( + ['sh', '-c', shblob, 'runscript', self.tmpfile()], + stdin=script, description=description, rcs=rcs) + + def tmpfile(self): + """Get a tmp file in the target. + + @return_value: path to new file in target + """ + path = "/tmp/%s-%04d" % (type(self).__name__, self._tmp_count) + self._tmp_count += 1 + return path + + class InTargetExecuteError(c_util.ProcessExecutionError): """Error type for in target commands that fail.""" - default_desc = 'Unexpected error while running command in target instance' + default_desc = 'Unexpected error while running command.' - def __init__(self, stdout, stderr, exit_code, cmd, instance, - description=None): + def __init__(self, stdout, stderr, exit_code, cmd, description=None, + reason=None): """Init error and parent error class.""" - if isinstance(cmd, (tuple, list)): - cmd = ' '.join(cmd) super(InTargetExecuteError, self).__init__( - stdout=stdout, stderr=stderr, exit_code=exit_code, cmd=cmd, - reason="Instance: {}".format(instance), - description=description if description else self.default_desc) + stdout=stdout, stderr=stderr, exit_code=exit_code, + cmd=shell_quote(cmd), + description=description if description else self.default_desc, + reason=reason) class TempDir(object): -- cgit v1.2.3 From 7bb01ea3a04891bcfcdd8ffbbc6c799a923d8fb3 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Tue, 7 Nov 2017 17:02:22 -0500 Subject: tests: address some minor feedback missed in last merge. 3 things here: a.) link to a bug that we opened to track what made us add dns entries for hostname of our guests. b.) spelling fix. c.) raise an instance of a NotImplementedError not the class. --- tests/cloud_tests/instances/nocloudkvm.py | 3 ++- tests/cloud_tests/util.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) (limited to 'tests') diff --git a/tests/cloud_tests/instances/nocloudkvm.py b/tests/cloud_tests/instances/nocloudkvm.py index fbb04aa2..2427c856 100644 --- a/tests/cloud_tests/instances/nocloudkvm.py +++ b/tests/cloud_tests/instances/nocloudkvm.py @@ -15,8 +15,9 @@ from tests.cloud_tests import util # This domain contains reverse lookups for hostnames that are used. # The primary reason is so sudo will return quickly when it attempts # to look up the hostname. i9n is just short for 'integration'. -# use i9n.brickies.net until i9n.cloud-init.io is popualted: +# use i9n.brickies.net until i9n.cloud-init.io is populated: # https://portal.admin.canonical.com/107125 +# see also bug 1730744 for why we had to do this. CI_DOMAIN = "i9n.brickies.net" diff --git a/tests/cloud_tests/util.py b/tests/cloud_tests/util.py index 92c31c3a..c5cd6974 100644 --- a/tests/cloud_tests/util.py +++ b/tests/cloud_tests/util.py @@ -345,7 +345,7 @@ class TargetBase(object): This is intended to be implemented by the Image or Instance. Many callers will use the higher level 'execute'.""" - raise NotImplementedError + raise NotImplementedError("_execute must be implemented by subclass.") def read_data(self, remote_path, decode=False): """Read data from instance filesystem. -- cgit v1.2.3 From b6deb1d851100a79a89d3c66e74df7bc7efff68c Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Tue, 7 Nov 2017 21:35:23 -0500 Subject: Replace the temporary i9n.brickies.net with i9n.cloud-init.io. We had used some dns records in i9n.brickies.net (my personal domain) as a temporary solution until we got names registered in the cloud-init.io namespace. We now have CNAME records for: ubuntu.i9n.cloud-init.io cloudinit1.cloud-init.io cloudinit2.cloud-init.io --- tests/cloud_tests/instances/nocloudkvm.py | 4 +--- tests/cloud_tests/testcases/modules/set_hostname_fqdn.py | 3 ++- tests/cloud_tests/testcases/modules/set_hostname_fqdn.yaml | 3 ++- 3 files changed, 5 insertions(+), 5 deletions(-) (limited to 'tests') diff --git a/tests/cloud_tests/instances/nocloudkvm.py b/tests/cloud_tests/instances/nocloudkvm.py index 2427c856..cc825800 100644 --- a/tests/cloud_tests/instances/nocloudkvm.py +++ b/tests/cloud_tests/instances/nocloudkvm.py @@ -15,10 +15,8 @@ from tests.cloud_tests import util # This domain contains reverse lookups for hostnames that are used. # The primary reason is so sudo will return quickly when it attempts # to look up the hostname. i9n is just short for 'integration'. -# use i9n.brickies.net until i9n.cloud-init.io is populated: -# https://portal.admin.canonical.com/107125 # see also bug 1730744 for why we had to do this. -CI_DOMAIN = "i9n.brickies.net" +CI_DOMAIN = "i9n.cloud-init.io" class NoCloudKVMInstance(base.Instance): diff --git a/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py b/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py index 08ceae01..eb6f0650 100644 --- a/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py +++ b/tests/cloud_tests/testcases/modules/set_hostname_fqdn.py @@ -1,6 +1,7 @@ # This file is part of cloud-init. See LICENSE file for license information. """cloud-init Integration Test Verify Script.""" +from tests.cloud_tests.instances.nocloudkvm import CI_DOMAIN from tests.cloud_tests.testcases import base @@ -8,7 +9,7 @@ class TestHostnameFqdn(base.CloudTestCase): """Test Hostname module.""" ex_hostname = "cloudinit1" - ex_fqdn = "cloudinit2.i9n.brickies.net" + ex_fqdn = "cloudinit2." + CI_DOMAIN def test_hostname(self): """Test hostname output.""" diff --git a/tests/cloud_tests/testcases/modules/set_hostname_fqdn.yaml b/tests/cloud_tests/testcases/modules/set_hostname_fqdn.yaml index 5320ac57..a85ee79e 100644 --- a/tests/cloud_tests/testcases/modules/set_hostname_fqdn.yaml +++ b/tests/cloud_tests/testcases/modules/set_hostname_fqdn.yaml @@ -7,7 +7,8 @@ cloud_config: | #cloud-config manage_etc_hosts: true hostname: cloudinit1 - fqdn: cloudinit2.i9n.brickies.net + # this needs changing if CI_DOMAIN were updated. + fqdn: cloudinit2.i9n.cloud-init.io collect_scripts: hosts: | #!/bin/bash -- cgit v1.2.3 From 9bc4ce0596544ffa56d9d67245b00e07006a8662 Mon Sep 17 00:00:00 2001 From: Dave Mulford Date: Mon, 9 Oct 2017 15:28:15 -0500 Subject: rh_subscription: Perform null checks for enabled and disabled repos. The rh_subscription module doesn't perform null checks when attempting to iterate on the enabled and disable repos arrays. When only one is specified, cloud-init fails to run. --- cloudinit/config/cc_rh_subscription.py | 46 ++++++++++++++++++++------------- tests/unittests/test_rh_subscription.py | 15 +++++++++++ 2 files changed, 43 insertions(+), 18 deletions(-) (limited to 'tests') diff --git a/cloudinit/config/cc_rh_subscription.py b/cloudinit/config/cc_rh_subscription.py index 7f36cf8f..a9d21e78 100644 --- a/cloudinit/config/cc_rh_subscription.py +++ b/cloudinit/config/cc_rh_subscription.py @@ -38,14 +38,16 @@ Subscription`` example config. server-hostname: """ +from cloudinit import log as logging from cloudinit import util +LOG = logging.getLogger(__name__) + distros = ['fedora', 'rhel'] def handle(name, cfg, _cloud, log, _args): - sm = SubscriptionManager(cfg) - sm.log = log + sm = SubscriptionManager(cfg, log=log) if not sm.is_configured(): log.debug("%s: module not configured.", name) return None @@ -86,10 +88,9 @@ def handle(name, cfg, _cloud, log, _args): if not return_stat: raise SubscriptionError("Unable to attach pools {0}" .format(sm.pools)) - if (sm.enable_repo is not None) or (sm.disable_repo is not None): - return_stat = sm.update_repos(sm.enable_repo, sm.disable_repo) - if not return_stat: - raise SubscriptionError("Unable to add or remove repos") + return_stat = sm.update_repos() + if not return_stat: + raise SubscriptionError("Unable to add or remove repos") sm.log_success("rh_subscription plugin completed successfully") except SubscriptionError as e: sm.log_warn(str(e)) @@ -108,7 +109,10 @@ class SubscriptionManager(object): 'rhsm-baseurl', 'server-hostname', 'auto-attach', 'service-level'] - def __init__(self, cfg): + def __init__(self, cfg, log=None): + if log is None: + log = LOG + self.log = log self.cfg = cfg self.rhel_cfg = self.cfg.get('rh_subscription', {}) self.rhsm_baseurl = self.rhel_cfg.get('rhsm-baseurl') @@ -130,7 +134,7 @@ class SubscriptionManager(object): def log_warn(self, msg): '''Simple wrapper for logging warning messages. Useful for unittests''' - self.log.warn(msg) + self.log.warning(msg) def _verify_keys(self): ''' @@ -245,7 +249,7 @@ class SubscriptionManager(object): return False reg_id = return_out.split("ID: ")[1].rstrip() - self.log.debug("Registered successfully with ID {0}".format(reg_id)) + self.log.debug("Registered successfully with ID %s", reg_id) return True def _set_service_level(self): @@ -347,7 +351,7 @@ class SubscriptionManager(object): try: self._sub_man_cli(cmd) self.log.debug("Attached the following pools to your " - "system: %s" % (", ".join(pool_list)) + "system: %s", (", ".join(pool_list)) .replace('--pool=', '')) return True except util.ProcessExecutionError as e: @@ -355,18 +359,24 @@ class SubscriptionManager(object): "due to {1}".format(pool, e)) return False - def update_repos(self, erepos, drepos): + def update_repos(self): ''' Takes a list of yum repo ids that need to be disabled or enabled; then it verifies if they are already enabled or disabled and finally executes the action to disable or enable ''' - if (erepos is not None) and (not isinstance(erepos, list)): + erepos = self.enable_repo + drepos = self.disable_repo + if erepos is None: + erepos = [] + if drepos is None: + drepos = [] + if not isinstance(erepos, list): self.log_warn("Repo IDs must in the format of a list.") return False - if (drepos is not None) and (not isinstance(drepos, list)): + if not isinstance(drepos, list): self.log_warn("Repo IDs must in the format of a list.") return False @@ -399,14 +409,14 @@ class SubscriptionManager(object): for fail in enable_list_fail: # Check if the repo exists or not if fail in active_repos: - self.log.debug("Repo {0} is already enabled".format(fail)) + self.log.debug("Repo %s is already enabled", fail) else: self.log_warn("Repo {0} does not appear to " "exist".format(fail)) if len(disable_list_fail) > 0: for fail in disable_list_fail: - self.log.debug("Repo {0} not disabled " - "because it is not enabled".format(fail)) + self.log.debug("Repo %s not disabled " + "because it is not enabled", fail) cmd = ['repos'] if len(disable_list) > 0: @@ -422,10 +432,10 @@ class SubscriptionManager(object): return False if len(enable_list) > 0: - self.log.debug("Enabled the following repos: %s" % + self.log.debug("Enabled the following repos: %s", (", ".join(enable_list)).replace('--enable=', '')) if len(disable_list) > 0: - self.log.debug("Disabled the following repos: %s" % + self.log.debug("Disabled the following repos: %s", (", ".join(disable_list)).replace('--disable=', '')) return True diff --git a/tests/unittests/test_rh_subscription.py b/tests/unittests/test_rh_subscription.py index e9d5702a..22718108 100644 --- a/tests/unittests/test_rh_subscription.py +++ b/tests/unittests/test_rh_subscription.py @@ -2,6 +2,7 @@ """Tests for registering RHEL subscription via rh_subscription.""" +import copy import logging from cloudinit.config import cc_rh_subscription @@ -68,6 +69,20 @@ class GoodTests(TestCase): self.assertEqual(self.SM.log_success.call_count, 1) self.assertEqual(self.SM._sub_man_cli.call_count, 2) + @mock.patch.object(cc_rh_subscription.SubscriptionManager, "_getRepos") + @mock.patch.object(cc_rh_subscription.SubscriptionManager, "_sub_man_cli") + def test_update_repos_disable_with_none(self, m_sub_man_cli, m_get_repos): + cfg = copy.deepcopy(self.config) + m_get_repos.return_value = ([], ['repo1']) + m_sub_man_cli.return_value = (b'', b'') + cfg['rh_subscription'].update( + {'enable-repo': ['repo1'], 'disable-repo': None}) + mysm = cc_rh_subscription.SubscriptionManager(cfg) + self.assertEqual(True, mysm.update_repos()) + m_get_repos.assert_called_with() + self.assertEqual(m_sub_man_cli.call_args_list, + [mock.call(['repos', '--enable=repo1'])]) + def test_full_registration(self): ''' Registration with auto-attach, service-level, adding pools, -- cgit v1.2.3 From 22a14a6a6d45ae55d2c2307d7b097eef9863bb0c Mon Sep 17 00:00:00 2001 From: Robert Schweikert Date: Wed, 8 Nov 2017 15:45:53 -0500 Subject: hosts: Fix openSUSE and SLES setup for /etc/hosts and clarify docs. The etc/hosts file is was not properly setup for openSUSE or SLES when manage_etc_hosts is set in the config file. Improve the doc to address the fact that the 'localhost' ip is distribution dependent (not always 127.0.0.1). LP: #1731022 --- cloudinit/config/cc_update_etc_hosts.py | 4 +- templates/hosts.opensuse.tmpl | 26 -------- templates/hosts.suse.tmpl | 10 +++- .../test_handler/test_handler_etc_hosts.py | 69 ++++++++++++++++++++++ 4 files changed, 79 insertions(+), 30 deletions(-) delete mode 100644 templates/hosts.opensuse.tmpl create mode 100644 tests/unittests/test_handler/test_handler_etc_hosts.py (limited to 'tests') diff --git a/cloudinit/config/cc_update_etc_hosts.py b/cloudinit/config/cc_update_etc_hosts.py index b3947849..c96eede1 100644 --- a/cloudinit/config/cc_update_etc_hosts.py +++ b/cloudinit/config/cc_update_etc_hosts.py @@ -23,8 +23,8 @@ using the template located in ``/etc/cloud/templates/hosts.tmpl``. In the If ``manage_etc_hosts`` is set to ``localhost``, then cloud-init will not rewrite ``/etc/hosts`` entirely, but rather will ensure that a entry for the -fqdn with ip ``127.0.1.1`` is present in ``/etc/hosts`` (i.e. -``ping `` will ping ``127.0.1.1``). +fqdn with a distribution dependent ip is present in ``/etc/hosts`` (i.e. +``ping `` will ping ``127.0.0.1`` or ``127.0.1.1`` or other ip). .. note:: if ``manage_etc_hosts`` is set ``true`` or ``template``, the contents diff --git a/templates/hosts.opensuse.tmpl b/templates/hosts.opensuse.tmpl deleted file mode 100644 index 655da3f7..00000000 --- a/templates/hosts.opensuse.tmpl +++ /dev/null @@ -1,26 +0,0 @@ -* - This file /etc/cloud/templates/hosts.opensuse.tmpl is only utilized - if enabled in cloud-config. Specifically, in order to enable it - you need to add the following to config: - manage_etc_hosts: True -*# -# Your system has configured 'manage_etc_hosts' as True. -# As a result, if you wish for changes to this file to persist -# then you will need to either -# a.) make changes to the master file in -# /etc/cloud/templates/hosts.opensuse.tmpl -# b.) change or remove the value of 'manage_etc_hosts' in -# /etc/cloud/cloud.cfg or cloud-config from user-data -# -# The following lines are desirable for IPv4 capable hosts -127.0.0.1 localhost - -# The following lines are desirable for IPv6 capable hosts -::1 localhost ipv6-localhost ipv6-loopback -fe00::0 ipv6-localnet - -ff00::0 ipv6-mcastprefix -ff02::1 ipv6-allnodes -ff02::2 ipv6-allrouters -ff02::3 ipv6-allhosts - diff --git a/templates/hosts.suse.tmpl b/templates/hosts.suse.tmpl index b6082692..8e664dbf 100644 --- a/templates/hosts.suse.tmpl +++ b/templates/hosts.suse.tmpl @@ -13,12 +13,18 @@ you need to add the following to config: # /etc/cloud/cloud.cfg or cloud-config from user-data # # The following lines are desirable for IPv4 capable hosts -127.0.0.1 localhost +127.0.0.1 {{fqdn}} {{hostname}} +127.0.0.1 localhost.localdomain localhost +127.0.0.1 localhost4.localdomain4 localhost4 # The following lines are desirable for IPv6 capable hosts +::1 {{fqdn}} {{hostname}} +::1 localhost.localdomain localhost +::1 localhost6.localdomain6 localhost6 ::1 localhost ipv6-localhost ipv6-loopback -fe00::0 ipv6-localnet + +fe00::0 ipv6-localnet ff00::0 ipv6-mcastprefix ff02::1 ipv6-allnodes ff02::2 ipv6-allrouters diff --git a/tests/unittests/test_handler/test_handler_etc_hosts.py b/tests/unittests/test_handler/test_handler_etc_hosts.py new file mode 100644 index 00000000..ced05a8d --- /dev/null +++ b/tests/unittests/test_handler/test_handler_etc_hosts.py @@ -0,0 +1,69 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +from cloudinit.config import cc_update_etc_hosts + +from cloudinit import cloud +from cloudinit import distros +from cloudinit import helpers +from cloudinit import util + +from cloudinit.tests import helpers as t_help + +import logging +import os +import shutil + +LOG = logging.getLogger(__name__) + + +class TestHostsFile(t_help.FilesystemMockingTestCase): + def setUp(self): + super(TestHostsFile, self).setUp() + self.tmp = self.tmp_dir() + + def _fetch_distro(self, kind): + cls = distros.fetch(kind) + paths = helpers.Paths({}) + return cls(kind, {}, paths) + + def test_write_etc_hosts_suse_localhost(self): + cfg = { + 'manage_etc_hosts': 'localhost', + 'hostname': 'cloud-init.test.us' + } + os.makedirs('%s/etc/' % self.tmp) + hosts_content = '192.168.1.1 blah.blah.us blah\n' + fout = open('%s/etc/hosts' % self.tmp, 'w') + fout.write(hosts_content) + fout.close() + distro = self._fetch_distro('sles') + distro.hosts_fn = '%s/etc/hosts' % self.tmp + paths = helpers.Paths({}) + ds = None + cc = cloud.Cloud(ds, paths, {}, distro, None) + self.patchUtils(self.tmp) + cc_update_etc_hosts.handle('test', cfg, cc, LOG, []) + contents = util.load_file('%s/etc/hosts' % self.tmp) + if '127.0.0.1\tcloud-init.test.us\tcloud-init' not in contents: + self.assertIsNone('No entry for 127.0.0.1 in etc/hosts') + if '192.168.1.1\tblah.blah.us\tblah' not in contents: + self.assertIsNone('Default etc/hosts content modified') + + def test_write_etc_hosts_suse_template(self): + cfg = { + 'manage_etc_hosts': 'template', + 'hostname': 'cloud-init.test.us' + } + shutil.copytree('templates', '%s/etc/cloud/templates' % self.tmp) + distro = self._fetch_distro('sles') + paths = helpers.Paths({}) + paths.template_tpl = '%s' % self.tmp + '/etc/cloud/templates/%s.tmpl' + ds = None + cc = cloud.Cloud(ds, paths, {}, distro, None) + self.patchUtils(self.tmp) + cc_update_etc_hosts.handle('test', cfg, cc, LOG, []) + contents = util.load_file('%s/etc/hosts' % self.tmp) + if '127.0.0.1 cloud-init.test.us cloud-init' not in contents: + self.assertIsNone('No entry for 127.0.0.1 in etc/hosts') + if '::1 cloud-init.test.us cloud-init' not in contents: + self.assertIsNone('No entry for 127.0.0.1 in etc/hosts') -- cgit v1.2.3 From e10ad2d7854b87024b5d051db50166125fce2279 Mon Sep 17 00:00:00 2001 From: Andrew Jorgensen Date: Thu, 6 Mar 2014 13:26:05 -0800 Subject: Catch UrlError when #include'ing URLs Without this the entire stage can fail, which will leave an instance unaccessible. Reviewed-by: Tom Kirchner Reviewed-by: Matt Nierzwicki Reviewed-by: Ben Cressey --- cloudinit/user_data.py | 28 ++++++++++++++++--------- tests/unittests/test_data.py | 50 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 10 deletions(-) (limited to 'tests') diff --git a/cloudinit/user_data.py b/cloudinit/user_data.py index 88cb7f84..e163c722 100644 --- a/cloudinit/user_data.py +++ b/cloudinit/user_data.py @@ -19,6 +19,7 @@ import six from cloudinit import handlers from cloudinit import log as logging +from cloudinit.url_helper import UrlError from cloudinit import util LOG = logging.getLogger(__name__) @@ -222,16 +223,23 @@ class UserDataProcessor(object): if include_once_on and os.path.isfile(include_once_fn): content = util.load_file(include_once_fn) else: - resp = util.read_file_or_url(include_url, - ssl_details=self.ssl_details) - if include_once_on and resp.ok(): - util.write_file(include_once_fn, resp.contents, mode=0o600) - if resp.ok(): - content = resp.contents - else: - LOG.warning(("Fetching from %s resulted in" - " a invalid http code of %s"), - include_url, resp.code) + try: + resp = util.read_file_or_url(include_url, + ssl_details=self.ssl_details) + if include_once_on and resp.ok(): + util.write_file(include_once_fn, resp.contents, + mode=0o600) + if resp.ok(): + content = resp.contents + else: + LOG.warning(("Fetching from %s resulted in" + " a invalid http code of %s"), + include_url, resp.code) + except UrlError as urle: + LOG.warning(urle) + except IOError as ioe: + LOG.warning("Fetching from %s resulted in %s", + include_url, ioe) if content is not None: new_msg = convert_string(content) diff --git a/tests/unittests/test_data.py b/tests/unittests/test_data.py index 6d621d26..275b16d2 100644 --- a/tests/unittests/test_data.py +++ b/tests/unittests/test_data.py @@ -18,6 +18,8 @@ from email.mime.application import MIMEApplication from email.mime.base import MIMEBase from email.mime.multipart import MIMEMultipart +import httpretty + from cloudinit import handlers from cloudinit import helpers as c_helpers from cloudinit import log @@ -522,6 +524,54 @@ c: 4 self.assertEqual(cfg.get('password'), 'gocubs') self.assertEqual(cfg.get('locale'), 'chicago') + @httpretty.activate + @mock.patch('cloudinit.url_helper.time.sleep') + def test_include(self, mock_sleep): + """Test #include.""" + included_url = 'http://hostname/path' + included_data = '#cloud-config\nincluded: true\n' + httpretty.register_uri(httpretty.GET, included_url, included_data) + + blob = '#include\n%s\n' % included_url + + self.reRoot() + ci = stages.Init() + ci.datasource = FakeDataSource(blob) + ci.fetch() + ci.consume_data() + cc_contents = util.load_file(ci.paths.get_ipath("cloud_config")) + cc = util.load_yaml(cc_contents) + self.assertTrue(cc.get('included')) + + @httpretty.activate + @mock.patch('cloudinit.url_helper.time.sleep') + def test_include_bad_url(self, mock_sleep): + """Test #include with a bad URL.""" + bad_url = 'http://bad/forbidden' + bad_data = '#cloud-config\nbad: true\n' + httpretty.register_uri(httpretty.GET, bad_url, bad_data, status=403) + + included_url = 'http://hostname/path' + included_data = '#cloud-config\nincluded: true\n' + httpretty.register_uri(httpretty.GET, included_url, included_data) + + blob = '#include\n%s\n%s' % (bad_url, included_url) + + self.reRoot() + ci = stages.Init() + ci.datasource = FakeDataSource(blob) + log_file = self.capture_log(logging.WARNING) + ci.fetch() + ci.consume_data() + + self.assertIn("403 Client Error: Forbidden for url: %s" % bad_url, + log_file.getvalue()) + + cc_contents = util.load_file(ci.paths.get_ipath("cloud_config")) + cc = util.load_yaml(cc_contents) + self.assertIsNone(cc.get('bad')) + self.assertTrue(cc.get('included')) + class TestUDProcess(helpers.ResourceUsingTestCase): -- cgit v1.2.3 From d90318b21d0379e24337bcb92a0a90ebfa359c35 Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Thu, 16 Nov 2017 20:58:45 -0700 Subject: ntp: fix configuration template rendering for openSUSE and SLES Add opensuse distro support to cc_ntp module. LP: #1726572 --- cloudinit/config/cc_ntp.py | 9 ++- templates/ntp.conf.opensuse.tmpl | 88 ++++++++++++++++++++++++ templates/ntp.conf.sles.tmpl | 12 ---- tests/unittests/test_handler/test_handler_ntp.py | 26 +++++++ 4 files changed, 121 insertions(+), 14 deletions(-) create mode 100644 templates/ntp.conf.opensuse.tmpl (limited to 'tests') diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py index d43d060c..f50bcb35 100644 --- a/cloudinit/config/cc_ntp.py +++ b/cloudinit/config/cc_ntp.py @@ -23,7 +23,7 @@ frequency = PER_INSTANCE NTP_CONF = '/etc/ntp.conf' TIMESYNCD_CONF = '/etc/systemd/timesyncd.conf.d/cloud-init.conf' NR_POOL_SERVERS = 4 -distros = ['centos', 'debian', 'fedora', 'opensuse', 'ubuntu'] +distros = ['centos', 'debian', 'fedora', 'opensuse', 'sles', 'ubuntu'] # The schema definition for each cloud-config module is a strict contract for @@ -174,8 +174,13 @@ def rename_ntp_conf(config=None): def generate_server_names(distro): names = [] + pool_distro = distro + # For legal reasons x.pool.sles.ntp.org does not exist, + # use the opensuse pool + if distro == 'sles': + pool_distro = 'opensuse' for x in range(0, NR_POOL_SERVERS): - name = "%d.%s.pool.ntp.org" % (x, distro) + name = "%d.%s.pool.ntp.org" % (x, pool_distro) names.append(name) return names diff --git a/templates/ntp.conf.opensuse.tmpl b/templates/ntp.conf.opensuse.tmpl new file mode 100644 index 00000000..f3ab565f --- /dev/null +++ b/templates/ntp.conf.opensuse.tmpl @@ -0,0 +1,88 @@ +## template:jinja + +## +## Radio and modem clocks by convention have addresses in the +## form 127.127.t.u, where t is the clock type and u is a unit +## number in the range 0-3. +## +## Most of these clocks require support in the form of a +## serial port or special bus peripheral. The particular +## device is normally specified by adding a soft link +## /dev/device-u to the particular hardware device involved, +## where u correspond to the unit number above. +## +## Generic DCF77 clock on serial port (Conrad DCF77) +## Address: 127.127.8.u +## Serial Port: /dev/refclock-u +## +## (create soft link /dev/refclock-0 to the particular ttyS?) +## +# server 127.127.8.0 mode 5 prefer + +## +## Undisciplined Local Clock. This is a fake driver intended for backup +## and when no outside source of synchronized time is available. +## +# server 127.127.1.0 # local clock (LCL) +# fudge 127.127.1.0 stratum 10 # LCL is unsynchronized + +## +## Add external Servers using +## # rcntpd addserver +## The servers will only be added to the currently running instance, not +## to /etc/ntp.conf. +## +{% if pools %}# pools +{% endif %} +{% for pool in pools -%} +pool {{pool}} iburst +{% endfor %} +{%- if servers %}# servers +{% endif %} +{% for server in servers -%} +server {{server}} iburst +{% endfor %} + +# Access control configuration; see /usr/share/doc/packages/ntp/html/accopt.html for +# details. The web page +# might also be helpful. +# +# Note that "restrict" applies to both servers and clients, so a configuration +# that might be intended to block requests from certain clients could also end +# up blocking replies from your own upstream servers. + +# By default, exchange time with everybody, but don't allow configuration. +restrict -4 default notrap nomodify nopeer noquery +restrict -6 default notrap nomodify nopeer noquery + +# Local users may interrogate the ntp server more closely. +restrict 127.0.0.1 +restrict ::1 + +# Clients from this (example!) subnet have unlimited access, but only if +# cryptographically authenticated. +#restrict 192.168.123.0 mask 255.255.255.0 notrust + +## +## Miscellaneous stuff +## + +driftfile /var/lib/ntp/drift/ntp.drift # path for drift file + +logfile /var/log/ntp # alternate log file +# logconfig =syncstatus + sysevents +# logconfig =all + +# statsdir /tmp/ # directory for statistics files +# filegen peerstats file peerstats type day enable +# filegen loopstats file loopstats type day enable +# filegen clockstats file clockstats type day enable + +# +# Authentication stuff +# +keys /etc/ntp.keys # path for keys file +trustedkey 1 # define trusted keys +requestkey 1 # key (7) for accessing server variables +controlkey 1 # key (6) for accessing server variables + diff --git a/templates/ntp.conf.sles.tmpl b/templates/ntp.conf.sles.tmpl index 5c5fc4db..f3ab565f 100644 --- a/templates/ntp.conf.sles.tmpl +++ b/templates/ntp.conf.sles.tmpl @@ -1,17 +1,5 @@ ## template:jinja -################################################################################ -## /etc/ntp.conf -## -## Sample NTP configuration file. -## See package 'ntp-doc' for documentation, Mini-HOWTO and FAQ. -## Copyright (c) 1998 S.u.S.E. GmbH Fuerth, Germany. -## -## Author: Michael Andres, -## Michael Skibbe, -## -################################################################################ - ## ## Radio and modem clocks by convention have addresses in the ## form 127.127.t.u, where t is the clock type and u is a unit diff --git a/tests/unittests/test_handler/test_handler_ntp.py b/tests/unittests/test_handler/test_handler_ntp.py index 3abe5786..28a8455d 100644 --- a/tests/unittests/test_handler/test_handler_ntp.py +++ b/tests/unittests/test_handler/test_handler_ntp.py @@ -430,5 +430,31 @@ class TestNtp(FilesystemMockingTestCase): "[Time]\nNTP=192.168.2.1 192.168.2.2 0.mypool.org \n", content.decode()) + def test_write_ntp_config_template_defaults_pools_empty_lists_sles(self): + """write_ntp_config_template defaults pools servers upon empty config. + + When both pools and servers are empty, default NR_POOL_SERVERS get + configured. + """ + distro = 'sles' + mycloud = self._get_cloud(distro) + ntp_conf = self.tmp_path('ntp.conf', self.new_root) # Doesn't exist + # Create ntp.conf.tmpl + with open('{0}.tmpl'.format(ntp_conf), 'wb') as stream: + stream.write(NTP_TEMPLATE) + with mock.patch('cloudinit.config.cc_ntp.NTP_CONF', ntp_conf): + cc_ntp.write_ntp_config_template({}, mycloud, ntp_conf) + content = util.read_file_or_url('file://' + ntp_conf).contents + default_pools = [ + "{0}.opensuse.pool.ntp.org".format(x) + for x in range(0, cc_ntp.NR_POOL_SERVERS)] + self.assertEqual( + "servers []\npools {0}\n".format(default_pools), + content.decode()) + self.assertIn( + "Adding distro default ntp pool servers: {0}".format( + ",".join(default_pools)), + self.logs.getvalue()) + # vi: ts=4 expandtab -- cgit v1.2.3 From 281a82181716183d526e76f4e0415e0a6f680cbe Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Mon, 20 Nov 2017 15:56:40 -0500 Subject: EC2: Fix bug using fallback_nic and metadata when restoring from cache. If user upgraded to new cloud-init and attempted to run 'cloud-init init' without rebooting, cloud-init restores the datasource object from pickle. The older version pickled datasource object had no value for _network_config or fallback_nic. This caused the Ec2 datasource to attempt to reconfigure networking with a None fallback_nic. The pickled object also cached an older version of ec2 metadata which didn't contain network information. This branch does two things: - Add a fallback_interface property to DatasourceEC2 to support reading the old .fallback_nic attribute if it was set. New versions will call net.find_fallback_nic() if there has not been one found. - Re-crawl metadata if we are on Ec2 and don't have a 'network' key in metadata LP: #1732917 --- cloudinit/net/dhcp.py | 3 +- cloudinit/sources/DataSourceEc2.py | 44 +++++++++++++++++++++-------- tests/unittests/test_datasource/test_ec2.py | 33 ++++++++++++++++++++++ 3 files changed, 67 insertions(+), 13 deletions(-) (limited to 'tests') diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index f3a412a9..d8624d82 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -42,8 +42,7 @@ def maybe_perform_dhcp_discovery(nic=None): if nic is None: nic = find_fallback_nic() if nic is None: - LOG.debug( - 'Skip dhcp_discovery: Unable to find fallback nic.') + LOG.debug('Skip dhcp_discovery: Unable to find fallback nic.') return {} elif nic not in get_devicelist(): LOG.debug( diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index 0ef22174..7bbbfb63 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -65,7 +65,7 @@ class DataSourceEc2(sources.DataSource): get_network_metadata = False # Track the discovered fallback nic for use in configuration generation. - fallback_nic = None + _fallback_interface = None def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) @@ -92,18 +92,17 @@ class DataSourceEc2(sources.DataSource): elif self.cloud_platform == Platforms.NO_EC2_METADATA: return False - self.fallback_nic = net.find_fallback_nic() if self.get_network_metadata: # Setup networking in init-local stage. if util.is_FreeBSD(): LOG.debug("FreeBSD doesn't support running dhclient with -sf") return False - dhcp_leases = dhcp.maybe_perform_dhcp_discovery(self.fallback_nic) + dhcp_leases = dhcp.maybe_perform_dhcp_discovery( + self.fallback_interface) if not dhcp_leases: # DataSourceEc2Local failed in init-local stage. DataSourceEc2 # will still run in init-network stage. return False dhcp_opts = dhcp_leases[-1] - self.fallback_nic = dhcp_opts.get('interface') net_params = {'interface': dhcp_opts.get('interface'), 'ip': dhcp_opts.get('fixed-address'), 'prefix_or_mask': dhcp_opts.get('subnet-mask'), @@ -301,21 +300,44 @@ class DataSourceEc2(sources.DataSource): return None result = None - net_md = self.metadata.get('network') + no_network_metadata_on_aws = bool( + 'network' not in self.metadata and + self.cloud_platform == Platforms.AWS) + if no_network_metadata_on_aws: + LOG.debug("Metadata 'network' not present:" + " Refreshing stale metadata from prior to upgrade.") + util.log_time( + logfunc=LOG.debug, msg='Re-crawl of metadata service', + func=self._crawl_metadata) + # Limit network configuration to only the primary/fallback nic - macs_to_nics = { - net.get_interface_mac(self.fallback_nic): self.fallback_nic} + iface = self.fallback_interface + macs_to_nics = {net.get_interface_mac(iface): iface} + net_md = self.metadata.get('network') if isinstance(net_md, dict): result = convert_ec2_metadata_network_config( - net_md, macs_to_nics=macs_to_nics, - fallback_nic=self.fallback_nic) + net_md, macs_to_nics=macs_to_nics, fallback_nic=iface) else: - LOG.warning("unexpected metadata 'network' key not valid: %s", - net_md) + LOG.warning("Metadata 'network' key not valid: %s.", net_md) self._network_config = result return self._network_config + @property + def fallback_interface(self): + if self._fallback_interface is None: + # fallback_nic was used at one point, so restored objects may + # have an attribute there. respect that if found. + _legacy_fbnic = getattr(self, 'fallback_nic', None) + if _legacy_fbnic: + self._fallback_interface = _legacy_fbnic + self.fallback_nic = None + else: + self._fallback_interface = net.find_fallback_nic() + if self._fallback_interface is None: + LOG.warning("Did not find a fallback interface on EC2.") + return self._fallback_interface + def _crawl_metadata(self): """Crawl metadata service when available. diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index 6af699a6..ba328ee9 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -305,6 +305,39 @@ class TestEc2(test_helpers.HttprettyTestCase): ds._network_config = {'cached': 'data'} self.assertEqual({'cached': 'data'}, ds.network_config) + @httpretty.activate + @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') + def test_network_config_cached_property_refreshed_on_upgrade(self, m_dhcp): + """Refresh the network_config Ec2 cache if network key is absent. + + This catches an upgrade issue where obj.pkl contained stale metadata + which lacked newly required network key. + """ + old_metadata = copy.deepcopy(DEFAULT_METADATA) + old_metadata.pop('network') + ds = self._setup_ds( + platform_data=self.valid_platform_data, + sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, + md=old_metadata) + self.assertTrue(ds.get_data()) + # Provide new revision of metadata that contains network data + register_mock_metaserver( + 'http://169.254.169.254/2009-04-04/meta-data/', DEFAULT_METADATA) + mac1 = '06:17:04:d7:26:09' # Defined in DEFAULT_METADATA + get_interface_mac_path = ( + 'cloudinit.sources.DataSourceEc2.net.get_interface_mac') + ds.fallback_nic = 'eth9' + with mock.patch(get_interface_mac_path) as m_get_interface_mac: + m_get_interface_mac.return_value = mac1 + ds.network_config # Will re-crawl network metadata + self.assertIn('Re-crawl of metadata service', self.logs.getvalue()) + expected = {'version': 1, 'config': [ + {'mac_address': '06:17:04:d7:26:09', + 'name': 'eth9', + 'subnets': [{'type': 'dhcp4'}, {'type': 'dhcp6'}], + 'type': 'physical'}]} + self.assertEqual(expected, ds.network_config) + @httpretty.activate @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery') def test_valid_platform_with_strict_true(self, m_dhcp): -- cgit v1.2.3 From 7624348712b4502f0085d30c05b34dce3f2ceeae Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Mon, 20 Nov 2017 16:51:09 -0500 Subject: integration test: replace curtin test ppa with cloud-init test ppa. Cloud-init integration tests should not depend on a curtin test ppa. We already had a cloud-init test ppa for explicitly this purpose. Just use it instead. --- tests/cloud_tests/testcases/modules/apt_configure_sources_ppa.py | 6 +++--- tests/cloud_tests/testcases/modules/apt_configure_sources_ppa.yaml | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'tests') diff --git a/tests/cloud_tests/testcases/modules/apt_configure_sources_ppa.py b/tests/cloud_tests/testcases/modules/apt_configure_sources_ppa.py index d299e9ad..dfbdeadf 100644 --- a/tests/cloud_tests/testcases/modules/apt_configure_sources_ppa.py +++ b/tests/cloud_tests/testcases/modules/apt_configure_sources_ppa.py @@ -11,13 +11,13 @@ class TestAptconfigureSourcesPPA(base.CloudTestCase): """Test specific ppa added.""" out = self.get_data_file('sources.list') self.assertIn( - 'http://ppa.launchpad.net/curtin-dev/test-archive/ubuntu', out) + 'http://ppa.launchpad.net/cloud-init-dev/test-archive/ubuntu', out) def test_ppa_key(self): """Test ppa key added.""" out = self.get_data_file('apt-key') self.assertIn( - '1BC3 0F71 5A3B 8612 47A8 1A5E 55FE 7C8C 0165 013E', out) - self.assertIn('Launchpad PPA for curtin developers', out) + '1FF0 D853 5EF7 E719 E5C8 1B9C 083D 06FB E4D3 04DF', out) + self.assertIn('Launchpad PPA for cloud init development team', out) # vi: ts=4 expandtab diff --git a/tests/cloud_tests/testcases/modules/apt_configure_sources_ppa.yaml b/tests/cloud_tests/testcases/modules/apt_configure_sources_ppa.yaml index 9efdae52..b997bcfb 100644 --- a/tests/cloud_tests/testcases/modules/apt_configure_sources_ppa.yaml +++ b/tests/cloud_tests/testcases/modules/apt_configure_sources_ppa.yaml @@ -2,7 +2,7 @@ # Add a PPA to source.list # # NOTE: on older ubuntu releases the sources file added is named -# 'curtin-dev-test-archive-trusty', without 'ubuntu' in the middle +# 'cloud-init-dev-test-archive-trusty', without 'ubuntu' in the middle required_features: - apt - ppa @@ -14,11 +14,11 @@ cloud_config: | source1: keyid: 0165013E keyserver: keyserver.ubuntu.com - source: "ppa:curtin-dev/test-archive" + source: "ppa:cloud-init-dev/test-archive" collect_scripts: sources.list: | #!/bin/bash - cat /etc/apt/sources.list.d/curtin-dev-ubuntu-test-archive-*.list + cat /etc/apt/sources.list.d/cloud-init-dev-ubuntu-test-archive-*.list apt-key: | #!/bin/bash apt-key finger -- cgit v1.2.3