From d5f855dd96ccbea77f61b0515b574ad2c43d116d Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Wed, 9 Aug 2017 21:55:52 -0600 Subject: ec2: Allow Ec2 to run in init-local using dhclient in a sandbox. This branch is a prerequisite for IPv6 support in AWS by allowing Ec2 datasource to query the metadata source version 2016-09-02 about whether or not it needs to configure IPv6 on interfaces. If version 2016-09-02 is not present, fallback to the min_metadata_version of 2009-04-04. The DataSourceEc2Local not run on FreeBSD because dhclient in doesn't support the -sf flag allowing us to run dhclient without filesystem side-effects. To query AWS' metadata address @ 169.254.169.254, the instance must have a dhcp-allocated address configured. Configuring IPv4 link-local addresses result in timeouts from the metadata service. We introduced a DataSourceEc2Local subclass which will perform a sandboxed dhclient discovery which obtains an authorized IP address on eth0 and crawl metadata about full instance network configuration. Since ec2 IPv6 metadata is not sufficient in itself to tell us all the ipv6 knownledge we need, it only be used as a boolean to tell us which nics need IPv6. Cloud-init will then configure desired interfaces to DHCPv6 versus DHCPv4. Performance side note: Shifting the dhcp work into init-local for Ec2 actually gets us 1 second faster deployments by skipping init-network phase of alternate datasource checks because Ec2Local is configured in an ealier boot stage. In 3 test runs prior to this change: cloud-init runs were 5.5 seconds, with the change we now average 4.6 seconds. This efficiency could be even further improved if we avoiding dhcp discovery in order to talk to the metadata service from an AWS authorized dhcp address if there were some way to advertize the dhcp configuration via DMI/SMBIOS or system environment variables. Inspecting time costs of the dhclient setup/teardown in 3 live runs the time cost for the dhcp setup round trip on AWS is: test 1: 76 milliseconds dhcp discovery + metadata: 0.347 seconds metadata alone: 0.271 seconds test 2: 88 milliseconds dhcp discovery + metadata: 0.388 seconds metadata alone: 0.300 seconds test 3: 75 milliseconds dhcp discovery + metadata: 0.366 seconds metadata alone: 0.291 seconds LP: #1709772 --- cloudinit/sources/DataSourceAliYun.py | 9 ++- cloudinit/sources/DataSourceEc2.py | 121 +++++++++++++++++++++++++++------- 2 files changed, 105 insertions(+), 25 deletions(-) (limited to 'cloudinit/sources') diff --git a/cloudinit/sources/DataSourceAliYun.py b/cloudinit/sources/DataSourceAliYun.py index 380e27cb..43a7e42c 100644 --- a/cloudinit/sources/DataSourceAliYun.py +++ b/cloudinit/sources/DataSourceAliYun.py @@ -6,17 +6,20 @@ from cloudinit import sources from cloudinit.sources import DataSourceEc2 as EC2 from cloudinit import util -DEF_MD_VERSION = "2016-01-01" ALIYUN_PRODUCT = "Alibaba Cloud ECS" class DataSourceAliYun(EC2.DataSourceEc2): - metadata_urls = ["http://100.100.100.200"] + + metadata_urls = ['http://100.100.100.200'] + + # The minimum supported metadata_version from the ec2 metadata apis + min_metadata_version = '2016-01-01' + extended_metadata_versions = [] def __init__(self, sys_cfg, distro, paths): super(DataSourceAliYun, self).__init__(sys_cfg, distro, paths) self.seed_dir = os.path.join(paths.seed_dir, "AliYun") - self.api_ver = DEF_MD_VERSION def get_hostname(self, fqdn=False, _resolve_ip=False): return self.metadata.get('hostname', 'localhost.localdomain') diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index 4ec9592f..8e5f8ee4 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -13,6 +13,8 @@ import time from cloudinit import ec2_utils as ec2 from cloudinit import log as logging +from cloudinit import net +from cloudinit.net import dhcp from cloudinit import sources from cloudinit import url_helper as uhelp from cloudinit import util @@ -20,8 +22,7 @@ from cloudinit import warnings LOG = logging.getLogger(__name__) -# Which version we are requesting of the ec2 metadata apis -DEF_MD_VERSION = '2009-04-04' +SKIP_METADATA_URL_CODES = frozenset([uhelp.NOT_FOUND]) STRICT_ID_PATH = ("datasource", "Ec2", "strict_id") STRICT_ID_DEFAULT = "warn" @@ -41,17 +42,28 @@ class Platforms(object): class DataSourceEc2(sources.DataSource): + # Default metadata urls that will be used if none are provided # They will be checked for 'resolveability' and some of the # following may be discarded if they do not resolve metadata_urls = ["http://169.254.169.254", "http://instance-data.:8773"] + + # The minimum supported metadata_version from the ec2 metadata apis + min_metadata_version = '2009-04-04' + + # Priority ordered list of additional metadata versions which will be tried + # for extended metadata content. IPv6 support comes in 2016-09-02 + extended_metadata_versions = ['2016-09-02'] + _cloud_platform = None + # Whether we want to get network configuration from the metadata service. + get_network_metadata = False + def __init__(self, sys_cfg, distro, paths): sources.DataSource.__init__(self, sys_cfg, distro, paths) self.metadata_address = None self.seed_dir = os.path.join(paths.seed_dir, "ec2") - self.api_ver = DEF_MD_VERSION def get_data(self): seed_ret = {} @@ -73,21 +85,27 @@ class DataSourceEc2(sources.DataSource): elif self.cloud_platform == Platforms.NO_EC2_METADATA: return False - try: - if not self.wait_for_metadata_service(): + 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 - start_time = time.time() - self.userdata_raw = \ - ec2.get_instance_userdata(self.api_ver, self.metadata_address) - self.metadata = ec2.get_instance_metadata(self.api_ver, - self.metadata_address) - LOG.debug("Crawl of metadata service took %.3f seconds", - time.time() - start_time) - return True - except Exception: - util.logexc(LOG, "Failed reading from metadata address %s", - self.metadata_address) - return False + dhcp_leases = dhcp.maybe_perform_dhcp_discovery() + 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] + net_params = {'interface': dhcp_opts.get('interface'), + 'ip': dhcp_opts.get('fixed-address'), + 'prefix_or_mask': dhcp_opts.get('subnet-mask'), + 'broadcast': dhcp_opts.get('broadcast-address'), + 'router': dhcp_opts.get('routers')} + with net.EphemeralIPv4Network(**net_params): + return util.log_time( + logfunc=LOG.debug, msg='Crawl of metadata service', + func=self._crawl_metadata) + else: + return self._crawl_metadata() @property def launch_index(self): @@ -95,6 +113,32 @@ class DataSourceEc2(sources.DataSource): return None return self.metadata.get('ami-launch-index') + def get_metadata_api_version(self): + """Get the best supported api version from the metadata service. + + Loop through all extended support metadata versions in order and + return the most-fully featured metadata api version discovered. + + If extended_metadata_versions aren't present, return the datasource's + min_metadata_version. + """ + # Assumes metadata service is already up + for api_ver in self.extended_metadata_versions: + url = '{0}/{1}/meta-data/instance-id'.format( + self.metadata_address, api_ver) + try: + resp = uhelp.readurl(url=url) + except uhelp.UrlError as e: + LOG.debug('url %s raised exception %s', url, e) + else: + if resp.code == 200: + LOG.debug('Found preferred metadata version %s', api_ver) + return api_ver + elif resp.code == 404: + msg = 'Metadata api version %s not present. Headers: %s' + LOG.debug(msg, api_ver, resp.headers) + return self.min_metadata_version + def get_instance_id(self): return self.metadata['instance-id'] @@ -138,21 +182,22 @@ class DataSourceEc2(sources.DataSource): urls = [] url2base = {} for url in mdurls: - cur = "%s/%s/meta-data/instance-id" % (url, self.api_ver) + cur = '{0}/{1}/meta-data/instance-id'.format( + url, self.min_metadata_version) urls.append(cur) url2base[cur] = url start_time = time.time() - url = uhelp.wait_for_url(urls=urls, max_wait=max_wait, - timeout=timeout, status_cb=LOG.warn) + url = uhelp.wait_for_url( + urls=urls, max_wait=max_wait, timeout=timeout, status_cb=LOG.warn) if url: - LOG.debug("Using metadata source: '%s'", url2base[url]) + self.metadata_address = url2base[url] + LOG.debug("Using metadata source: '%s'", self.metadata_address) else: LOG.critical("Giving up on md from %s after %s seconds", urls, int(time.time() - start_time)) - self.metadata_address = url2base.get(url) return bool(url) def device_name_to_device(self, name): @@ -234,6 +279,37 @@ class DataSourceEc2(sources.DataSource): util.get_cfg_by_path(cfg, STRICT_ID_PATH, STRICT_ID_DEFAULT), cfg) + def _crawl_metadata(self): + """Crawl metadata service when available. + + @returns: True on success, False otherwise. + """ + if not self.wait_for_metadata_service(): + return False + api_version = self.get_metadata_api_version() + try: + self.userdata_raw = ec2.get_instance_userdata( + api_version, self.metadata_address) + self.metadata = ec2.get_instance_metadata( + api_version, self.metadata_address) + except Exception: + util.logexc( + LOG, "Failed reading from metadata address %s", + self.metadata_address) + return False + return True + + +class DataSourceEc2Local(DataSourceEc2): + """Datasource run at init-local which sets up network to query metadata. + + In init-local, no network is available. This subclass sets up minimal + networking with dhclient on a viable nic so that it can talk to the + metadata service. If the metadata service provides network configuration + then render the network configuration for that instance based on metadata. + """ + get_network_metadata = True # Get metadata network config if present + def read_strict_mode(cfgval, default): try: @@ -349,6 +425,7 @@ def _collect_platform_data(): # Used to match classes to dependencies datasources = [ + (DataSourceEc2Local, (sources.DEP_FILESYSTEM,)), # Run at init-local (DataSourceEc2, (sources.DEP_FILESYSTEM, sources.DEP_NETWORK)), ] -- cgit v1.2.3 From 1f8183ff4750cc7f8798749987ef10912719544d Mon Sep 17 00:00:00 2001 From: Maitreyee Saikia Date: Tue, 15 Aug 2017 09:33:50 -0600 Subject: vcloud directory: Guest Customization support for passwords This feature enables the following VMware VCloud Director functionality: 1. Setting admin password 2. Expire password. 3. Set admin password and expire. Password configuration is triggered only as part of a full recustomization, that happens either on first power on or when "poweron and full recustomization" is selected. Full customization flow is determined by marker files. Unique marker ids are generated when full recustomization is requested. And marker file based on these marker ids help to determine if we need to execute the above configuration. --- cloudinit/sources/DataSourceOVF.py | 63 +++++++++++++++++++- cloudinit/sources/helpers/vmware/imc/config.py | 24 +++++++- .../sources/helpers/vmware/imc/config_passwd.py | 67 ++++++++++++++++++++++ tests/unittests/test_vmware_config_file.py | 32 ++++++++++- 4 files changed, 180 insertions(+), 6 deletions(-) create mode 100644 cloudinit/sources/helpers/vmware/imc/config_passwd.py (limited to 'cloudinit/sources') diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py index f20c9a65..73d38771 100644 --- a/cloudinit/sources/DataSourceOVF.py +++ b/cloudinit/sources/DataSourceOVF.py @@ -25,6 +25,8 @@ from cloudinit.sources.helpers.vmware.imc.config_file \ import ConfigFile from cloudinit.sources.helpers.vmware.imc.config_nic \ import NicConfigurator +from cloudinit.sources.helpers.vmware.imc.config_passwd \ + import PasswordConfigurator from cloudinit.sources.helpers.vmware.imc.guestcust_error \ import GuestCustErrorEnum from cloudinit.sources.helpers.vmware.imc.guestcust_event \ @@ -117,6 +119,8 @@ class DataSourceOVF(sources.DataSource): (md, ud, cfg) = read_vmware_imc(conf) dirpath = os.path.dirname(vmwareImcConfigFilePath) nics = get_nics_to_enable(dirpath) + markerid = conf.marker_id + markerexists = check_marker_exists(markerid) except Exception as e: LOG.debug("Error parsing the customization Config File") LOG.exception(e) @@ -127,7 +131,6 @@ class DataSourceOVF(sources.DataSource): return False finally: util.del_dir(os.path.dirname(vmwareImcConfigFilePath)) - try: LOG.debug("Applying the Network customization") nicConfigurator = NicConfigurator(conf.nics) @@ -140,6 +143,35 @@ class DataSourceOVF(sources.DataSource): GuestCustEventEnum.GUESTCUST_EVENT_NETWORK_SETUP_FAILED) enable_nics(nics) return False + if markerid and not markerexists: + LOG.debug("Applying password customization") + pwdConfigurator = PasswordConfigurator() + adminpwd = conf.admin_password + try: + resetpwd = conf.reset_password + if adminpwd or resetpwd: + pwdConfigurator.configure(adminpwd, resetpwd, + self.distro) + else: + LOG.debug("Changing password is not needed") + except Exception as e: + LOG.debug("Error applying Password Configuration: %s", e) + set_customization_status( + GuestCustStateEnum.GUESTCUST_STATE_RUNNING, + GuestCustEventEnum.GUESTCUST_EVENT_CUSTOMIZE_FAILED) + enable_nics(nics) + return False + if markerid: + LOG.debug("Handle marker creation") + try: + setup_marker_files(markerid) + except Exception as e: + LOG.debug("Error creating marker files: %s", e) + set_customization_status( + GuestCustStateEnum.GUESTCUST_STATE_RUNNING, + GuestCustEventEnum.GUESTCUST_EVENT_CUSTOMIZE_FAILED) + enable_nics(nics) + return False vmwarePlatformFound = True set_customization_status( @@ -445,4 +477,33 @@ datasources = ( def get_datasource_list(depends): return sources.list_from_depends(depends, datasources) + +# To check if marker file exists +def check_marker_exists(markerid): + """ + Check the existence of a marker file. + Presence of marker file determines whether a certain code path is to be + executed. It is needed for partial guest customization in VMware. + """ + if not markerid: + return False + markerfile = "/.markerfile-" + markerid + if os.path.exists(markerfile): + return True + return False + + +# Create a marker file +def setup_marker_files(markerid): + """ + Create a new marker file. + Marker files are unique to a full customization workflow in VMware + environment. + """ + if not markerid: + return + markerfile = "/.markerfile-" + markerid + util.del_file("/.markerfile-*.txt") + open(markerfile, 'w').close() + # vi: ts=4 expandtab diff --git a/cloudinit/sources/helpers/vmware/imc/config.py b/cloudinit/sources/helpers/vmware/imc/config.py index 9a5e3a8a..49d441db 100644 --- a/cloudinit/sources/helpers/vmware/imc/config.py +++ b/cloudinit/sources/helpers/vmware/imc/config.py @@ -5,6 +5,7 @@ # # This file is part of cloud-init. See LICENSE file for license information. + from .nic import Nic @@ -14,13 +15,16 @@ class Config(object): Specification file. """ + CUSTOM_SCRIPT = 'CUSTOM-SCRIPT|SCRIPT-NAME' DNS = 'DNS|NAMESERVER|' - SUFFIX = 'DNS|SUFFIX|' + DOMAINNAME = 'NETWORK|DOMAINNAME' + HOSTNAME = 'NETWORK|HOSTNAME' + MARKERID = 'MISC|MARKER-ID' PASS = 'PASSWORD|-PASS' + RESETPASS = 'PASSWORD|RESET' + SUFFIX = 'DNS|SUFFIX|' TIMEZONE = 'DATETIME|TIMEZONE' UTC = 'DATETIME|UTC' - HOSTNAME = 'NETWORK|HOSTNAME' - DOMAINNAME = 'NETWORK|DOMAINNAME' def __init__(self, configFile): self._configFile = configFile @@ -82,4 +86,18 @@ class Config(object): return res + @property + def reset_password(self): + """Retreives if the root password needs to be reset.""" + resetPass = self._configFile.get(Config.RESETPASS, 'no') + resetPass = resetPass.lower() + if resetPass not in ('yes', 'no'): + raise ValueError('ResetPassword value should be yes/no') + return resetPass == 'yes' + + @property + def marker_id(self): + """Returns marker id.""" + return self._configFile.get(Config.MARKERID, None) + # vi: ts=4 expandtab diff --git a/cloudinit/sources/helpers/vmware/imc/config_passwd.py b/cloudinit/sources/helpers/vmware/imc/config_passwd.py new file mode 100644 index 00000000..75cfbaaf --- /dev/null +++ b/cloudinit/sources/helpers/vmware/imc/config_passwd.py @@ -0,0 +1,67 @@ +# Copyright (C) 2016 Canonical Ltd. +# Copyright (C) 2016 VMware INC. +# +# Author: Maitreyee Saikia +# +# This file is part of cloud-init. See LICENSE file for license information. + + +import logging +import os + +from cloudinit import util + +LOG = logging.getLogger(__name__) + + +class PasswordConfigurator(object): + """ + Class for changing configurations related to passwords in a VM. Includes + setting and expiring passwords. + """ + def configure(self, passwd, resetPasswd, distro): + """ + Main method to perform all functionalities based on configuration file + inputs. + @param passwd: encoded admin password. + @param resetPasswd: boolean to determine if password needs to be reset. + @return cfg: dict to be used by cloud-init set_passwd code. + """ + LOG.info('Starting password configuration') + if passwd: + passwd = util.b64d(passwd) + allRootUsers = [] + for line in open('/etc/passwd', 'r'): + if line.split(':')[2] == '0': + allRootUsers.append(line.split(':')[0]) + # read shadow file and check for each user, if its uid0 or root. + uidUsersList = [] + for line in open('/etc/shadow', 'r'): + user = line.split(':')[0] + if user in allRootUsers: + uidUsersList.append(user) + if passwd: + LOG.info('Setting admin password') + distro.set_passwd('root', passwd) + if resetPasswd: + self.reset_password(uidUsersList) + LOG.info('Configure Password completed!') + + def reset_password(self, uidUserList): + """ + Method to reset password. Use passwd --expire command. Use chage if + not succeeded using passwd command. Log failure message otherwise. + @param: list of users for which to expire password. + """ + LOG.info('Expiring password.') + for user in uidUserList: + try: + out, err = util.subp(['passwd', '--expire', user]) + except util.ProcessExecutionError as e: + if os.path.exists('/usr/bin/chage'): + out, e = util.subp(['chage', '-d', '0', user]) + else: + LOG.warning('Failed to expire password for %s with error: ' + '%s', user, e) + +# vi: ts=4 expandtab diff --git a/tests/unittests/test_vmware_config_file.py b/tests/unittests/test_vmware_config_file.py index 18475f10..03b36d31 100644 --- a/tests/unittests/test_vmware_config_file.py +++ b/tests/unittests/test_vmware_config_file.py @@ -7,8 +7,8 @@ import logging import sys -import unittest +from .helpers import CiTestCase from cloudinit.sources.helpers.vmware.imc.boot_proto import BootProtoEnum from cloudinit.sources.helpers.vmware.imc.config import Config from cloudinit.sources.helpers.vmware.imc.config_file import ConfigFile @@ -17,7 +17,7 @@ logging.basicConfig(level=logging.DEBUG, stream=sys.stdout) logger = logging.getLogger(__name__) -class TestVmwareConfigFile(unittest.TestCase): +class TestVmwareConfigFile(CiTestCase): def test_utility_methods(self): cf = ConfigFile("tests/data/vmware/cust-dhcp-2nic.cfg") @@ -90,4 +90,32 @@ class TestVmwareConfigFile(unittest.TestCase): self.assertEqual('00:50:56:a6:8c:08', nics[0].mac, "mac0") self.assertEqual(BootProtoEnum.DHCP, nics[0].bootProto, "bootproto0") + def test_config_password(self): + cf = ConfigFile("tests/data/vmware/cust-dhcp-2nic.cfg") + + cf._insertKey("PASSWORD|-PASS", "test-password") + cf._insertKey("PASSWORD|RESET", "no") + + conf = Config(cf) + self.assertEqual('test-password', conf.admin_password, "password") + self.assertFalse(conf.reset_password, "do not reset password") + + def test_config_reset_passwd(self): + cf = ConfigFile("tests/data/vmware/cust-dhcp-2nic.cfg") + + cf._insertKey("PASSWORD|-PASS", "test-password") + cf._insertKey("PASSWORD|RESET", "random") + + conf = Config(cf) + with self.assertRaises(ValueError): + conf.reset_password() + + cf.clear() + cf._insertKey("PASSWORD|RESET", "yes") + self.assertEqual(1, len(cf), "insert size") + + conf = Config(cf) + self.assertTrue(conf.reset_password, "reset password") + + # vi: ts=4 expandtab -- cgit v1.2.3 From 3c45330af2a301f2bf219da556844d01cef6778e Mon Sep 17 00:00:00 2001 From: Chad Smith Date: Tue, 29 Aug 2017 10:34:19 -0600 Subject: ec2: Add IPv6 dhcp support to Ec2DataSource. DataSourceEc2 now parses the metadata for each nic to determine if configured for ipv6 and/or ipv4 addresses. In AWS for metadata version 2016-09-02, nics configured for ipv4 or ipv6 addresses will have non-zero values stored in metadata at network/interfaces/macs//public-ipv4 or ipv6s respectively. Those metadata files are only non-zero when an ipv4 or ipv6 ip is associated to the specific nic. A new DataSourceEc2.network_config property is added which parses the metadata and renders a network version 1 dictionary representing both dhcp4 and dhcp6 configuration for associated nics. The network configuration returned from the datasource will also 'pin' the nic name to the name presented on the instance for each nic. LP: #1639030 --- cloudinit/sources/DataSourceEc2.py | 38 +++++++++++ tests/unittests/test_datasource/test_ec2.py | 97 +++++++++++++++++++++++++++++ 2 files changed, 135 insertions(+) (limited to 'cloudinit/sources') diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index 8e5f8ee4..07c12bb4 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -57,6 +57,8 @@ class DataSourceEc2(sources.DataSource): _cloud_platform = None + _network_config = None # Used for caching calculated network config v1 + # Whether we want to get network configuration from the metadata service. get_network_metadata = False @@ -279,6 +281,15 @@ class DataSourceEc2(sources.DataSource): util.get_cfg_by_path(cfg, STRICT_ID_PATH, STRICT_ID_DEFAULT), cfg) + @property + def network_config(self): + """Return a network config dict for rendering ENI or netplan files.""" + if self._network_config is None: + if self.metadata is not None: + self._network_config = convert_ec2_metadata_network_config( + self.metadata) + return self._network_config + def _crawl_metadata(self): """Crawl metadata service when available. @@ -423,6 +434,33 @@ def _collect_platform_data(): return data +def convert_ec2_metadata_network_config(metadata=None, macs_to_nics=None): + """Convert ec2 metadata to network config version 1 data dict. + + @param: metadata: Dictionary of metadata crawled from EC2 metadata url. + @param: macs_to_name: Optional dict mac addresses and the nic name. If + not provided, get_interfaces_by_mac is called to get it from the OS. + + @return A dict of network config version 1 based on the metadata and macs. + """ + netcfg = {'version': 1, 'config': []} + if not macs_to_nics: + macs_to_nics = net.get_interfaces_by_mac() + macs_metadata = metadata['network']['interfaces']['macs'] + for mac, nic_name in macs_to_nics.items(): + nic_metadata = macs_metadata.get(mac) + if not nic_metadata: + continue # Not a physical nic represented in metadata + nic_cfg = {'type': 'physical', 'name': nic_name, 'subnets': []} + nic_cfg['mac_address'] = mac + if nic_metadata.get('public-ipv4s'): + nic_cfg['subnets'].append({'type': 'dhcp4'}) + if nic_metadata.get('ipv6s'): + nic_cfg['subnets'].append({'type': 'dhcp6'}) + netcfg['config'].append(nic_cfg) + return netcfg + + # Used to match classes to dependencies datasources = [ (DataSourceEc2Local, (sources.DEP_FILESYSTEM,)), # Run at init-local diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index 33d02619..e1ce6446 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -1,5 +1,6 @@ # This file is part of cloud-init. See LICENSE file for license information. +import copy import httpretty import mock @@ -194,6 +195,34 @@ class TestEc2(test_helpers.HttprettyTestCase): return ds + @httpretty.activate + def test_network_config_property_returns_version_1_network_data(self): + """network_config property returns network version 1 for metadata.""" + ds = self._setup_ds( + platform_data=self.valid_platform_data, + sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, + md=DEFAULT_METADATA) + ds.get_data() + mac1 = '06:17:04:d7:26:09' # Defined in DEFAULT_METADATA + expected = {'version': 1, 'config': [ + {'mac_address': '06:17:04:d7:26:09', 'name': 'eth9', + 'subnets': [{'type': 'dhcp4'}, {'type': 'dhcp6'}], + 'type': 'physical'}]} + patch_path = ( + 'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac') + with mock.patch(patch_path) as m_get_interfaces_by_mac: + m_get_interfaces_by_mac.return_value = {mac1: 'eth9'} + self.assertEqual(expected, ds.network_config) + + def test_network_config_property_is_cached_in_datasource(self): + """network_config property is cached in DataSourceEc2.""" + ds = self._setup_ds( + platform_data=self.valid_platform_data, + sys_cfg={'datasource': {'Ec2': {'strict_id': True}}}, + md=DEFAULT_METADATA) + 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_valid_platform_with_strict_true(self, m_dhcp): @@ -287,4 +316,72 @@ class TestEc2(test_helpers.HttprettyTestCase): self.assertIn('Crawl of metadata service took', self.logs.getvalue()) +class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): + + def setUp(self): + super(TestConvertEc2MetadataNetworkConfig, self).setUp() + self.mac1 = '06:17:04:d7:26:09' + self.network_metadata = { + 'network': {'interfaces': {'macs': { + self.mac1: {'public-ipv4s': '172.31.2.16'}}}}} + + def test_convert_ec2_metadata_network_config_skips_absent_macs(self): + """Any mac absent from metadata is skipped by network config.""" + macs_to_nics = {self.mac1: 'eth9', 'DE:AD:BE:EF:FF:FF': 'vitualnic2'} + + # DE:AD:BE:EF:FF:FF represented by OS but not in metadata + expected = {'version': 1, 'config': [ + {'mac_address': self.mac1, 'type': 'physical', + 'name': 'eth9', 'subnets': [{'type': 'dhcp4'}]}]} + self.assertEqual( + expected, + ec2.convert_ec2_metadata_network_config( + self.network_metadata, macs_to_nics)) + + def test_convert_ec2_metadata_network_config_handles_only_dhcp6(self): + """Config dhcp6 when ipv6s is in metadata for a mac.""" + macs_to_nics = {self.mac1: 'eth9'} + network_metadata_ipv6 = copy.deepcopy(self.network_metadata) + nic1_metadata = ( + network_metadata_ipv6['network']['interfaces']['macs'][self.mac1]) + nic1_metadata['ipv6s'] = '2620:0:1009:fd00:e442:c88d:c04d:dc85/64' + nic1_metadata.pop('public-ipv4s') + expected = {'version': 1, 'config': [ + {'mac_address': self.mac1, 'type': 'physical', + 'name': 'eth9', 'subnets': [{'type': 'dhcp6'}]}]} + self.assertEqual( + expected, + ec2.convert_ec2_metadata_network_config( + network_metadata_ipv6, macs_to_nics)) + + def test_convert_ec2_metadata_network_config_handles_dhcp4_and_dhcp6(self): + """Config both dhcp4 and dhcp6 when both vpc-ipv6 and ipv4 exists.""" + macs_to_nics = {self.mac1: 'eth9'} + network_metadata_both = copy.deepcopy(self.network_metadata) + nic1_metadata = ( + network_metadata_both['network']['interfaces']['macs'][self.mac1]) + nic1_metadata['ipv6s'] = '2620:0:1009:fd00:e442:c88d:c04d:dc85/64' + expected = {'version': 1, 'config': [ + {'mac_address': self.mac1, 'type': 'physical', + 'name': 'eth9', + 'subnets': [{'type': 'dhcp4'}, {'type': 'dhcp6'}]}]} + self.assertEqual( + expected, + ec2.convert_ec2_metadata_network_config( + network_metadata_both, macs_to_nics)) + + def test_convert_ec2_metadata_gets_macs_from_get_interfaces_by_mac(self): + """Convert Ec2 Metadata calls get_interfaces_by_mac by default.""" + expected = {'version': 1, 'config': [ + {'mac_address': self.mac1, 'type': 'physical', + 'name': 'eth9', + 'subnets': [{'type': 'dhcp4'}]}]} + patch_path = ( + 'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac') + with mock.patch(patch_path) as m_get_interfaces_by_mac: + m_get_interfaces_by_mac.return_value = {self.mac1: 'eth9'} + self.assertEqual( + expected, + ec2.convert_ec2_metadata_network_config(self.network_metadata)) + # vi: ts=4 expandtab -- cgit v1.2.3 From 44529c1de0098ccd684b46b0bc18d48312c4097c Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Tue, 22 Aug 2017 15:31:48 -0400 Subject: GCE: Add a main to the GCE Datasource. This just adds a main to the GCE datasource so that it is easily callable: python3 -m cloudinit.sources.DataSourceGCE It also adds a log of the time it took to crawl. --- cloudinit/sources/DataSourceGCE.py | 182 +++++++++++++++++++++++-------------- 1 file changed, 112 insertions(+), 70 deletions(-) (limited to 'cloudinit/sources') diff --git a/cloudinit/sources/DataSourceGCE.py b/cloudinit/sources/DataSourceGCE.py index 684eac86..94484d60 100644 --- a/cloudinit/sources/DataSourceGCE.py +++ b/cloudinit/sources/DataSourceGCE.py @@ -11,9 +11,8 @@ from cloudinit import util LOG = logging.getLogger(__name__) -BUILTIN_DS_CONFIG = { - 'metadata_url': 'http://metadata.google.internal/computeMetadata/v1/' -} +MD_V1_URL = 'http://metadata.google.internal/computeMetadata/v1/' +BUILTIN_DS_CONFIG = {'metadata_url': MD_V1_URL} REQUIRED_FIELDS = ('instance-id', 'availability-zone', 'local-hostname') @@ -51,75 +50,20 @@ class DataSourceGCE(sources.DataSource): BUILTIN_DS_CONFIG]) self.metadata_address = self.ds_cfg['metadata_url'] - # GCE takes sshKeys attribute in the format of ':' - # so we have to trim each key to remove the username part - def _trim_key(self, public_key): - try: - index = public_key.index(':') - if index > 0: - return public_key[(index + 1):] - except Exception: - return public_key - def get_data(self): - if not platform_reports_gce(): - return False + ret = util.log_time( + LOG.debug, 'Crawl of GCE metadata service', + read_md, kwargs={'address': self.metadata_address}) - # url_map: (our-key, path, required, is_text) - url_map = [ - ('instance-id', ('instance/id',), True, True), - ('availability-zone', ('instance/zone',), True, True), - ('local-hostname', ('instance/hostname',), True, True), - ('public-keys', ('project/attributes/sshKeys', - 'instance/attributes/ssh-keys'), False, True), - ('user-data', ('instance/attributes/user-data',), False, False), - ('user-data-encoding', ('instance/attributes/user-data-encoding',), - False, True), - ] - - # if we cannot resolve the metadata server, then no point in trying - if not util.is_resolvable_url(self.metadata_address): - LOG.debug("%s is not resolvable", self.metadata_address) - return False - - metadata_fetcher = GoogleMetadataFetcher(self.metadata_address) - # iterate over url_map keys to get metadata items - running_on_gce = False - for (mkey, paths, required, is_text) in url_map: - value = None - for path in paths: - new_value = metadata_fetcher.get_value(path, is_text) - if new_value is not None: - value = new_value - if value: - running_on_gce = True - if required and value is None: - msg = "required key %s returned nothing. not GCE" - if not running_on_gce: - LOG.debug(msg, mkey) - else: - LOG.warning(msg, mkey) - return False - self.metadata[mkey] = value - - if self.metadata['public-keys']: - lines = self.metadata['public-keys'].splitlines() - self.metadata['public-keys'] = [self._trim_key(k) for k in lines] - - if self.metadata['availability-zone']: - self.metadata['availability-zone'] = self.metadata[ - 'availability-zone'].split('/')[-1] - - encoding = self.metadata.get('user-data-encoding') - if encoding: - if encoding == 'base64': - self.metadata['user-data'] = b64decode( - self.metadata['user-data']) + if not ret['success']: + if ret['platform_reports_gce']: + LOG.warning(ret['reason']) else: - LOG.warning('unknown user-data-encoding: %s, ignoring', - encoding) - - return running_on_gce + LOG.debug(ret['reason']) + return False + self.metadata = ret['meta-data'] + self.userdata = ret['user-data'] + return True @property def launch_index(self): @@ -137,7 +81,7 @@ class DataSourceGCE(sources.DataSource): return self.metadata['local-hostname'].split('.')[0] def get_userdata_raw(self): - return self.metadata['user-data'] + return self.userdata @property def availability_zone(self): @@ -148,6 +92,87 @@ class DataSourceGCE(sources.DataSource): return self.availability_zone.rsplit('-', 1)[0] +def _trim_key(public_key): + # GCE takes sshKeys attribute in the format of ':' + # so we have to trim each key to remove the username part + try: + index = public_key.index(':') + if index > 0: + return public_key[(index + 1):] + except Exception: + return public_key + + +def read_md(address=None, platform_check=True): + + if address is None: + address = MD_V1_URL + + ret = {'meta-data': None, 'user-data': None, + 'success': False, 'reason': None} + ret['platform_reports_gce'] = platform_reports_gce() + + if platform_check and not ret['platform_reports_gce']: + ret['reason'] = "Not running on GCE." + return ret + + # if we cannot resolve the metadata server, then no point in trying + if not util.is_resolvable_url(address): + LOG.debug("%s is not resolvable", address) + ret['reason'] = 'address "%s" is not resolvable' % address + return ret + + # url_map: (our-key, path, required, is_text) + url_map = [ + ('instance-id', ('instance/id',), True, True), + ('availability-zone', ('instance/zone',), True, True), + ('local-hostname', ('instance/hostname',), True, True), + ('public-keys', ('project/attributes/sshKeys', + 'instance/attributes/ssh-keys'), False, True), + ('user-data', ('instance/attributes/user-data',), False, False), + ('user-data-encoding', ('instance/attributes/user-data-encoding',), + False, True), + ] + + metadata_fetcher = GoogleMetadataFetcher(address) + md = {} + # iterate over url_map keys to get metadata items + for (mkey, paths, required, is_text) in url_map: + value = None + for path in paths: + new_value = metadata_fetcher.get_value(path, is_text) + if new_value is not None: + value = new_value + if required and value is None: + msg = "required key %s returned nothing. not GCE" + ret['reason'] = msg % mkey + return ret + md[mkey] = value + + if md['public-keys']: + lines = md['public-keys'].splitlines() + md['public-keys'] = [_trim_key(k) for k in lines] + + if md['availability-zone']: + md['availability-zone'] = md['availability-zone'].split('/')[-1] + + encoding = md.get('user-data-encoding') + if encoding: + if encoding == 'base64': + md['user-data'] = b64decode(md['user-data']) + else: + LOG.warning('unknown user-data-encoding: %s, ignoring', encoding) + + if 'user-data' in md: + ret['user-data'] = md['user-data'] + del md['user-data'] + + ret['meta-data'] = md + ret['success'] = True + + return ret + + def platform_reports_gce(): pname = util.read_dmi_data('system-product-name') or "N/A" if pname == "Google Compute Engine": @@ -173,4 +198,21 @@ datasources = [ def get_datasource_list(depends): return sources.list_from_depends(depends, datasources) + +if __name__ == "__main__": + import argparse + import json + + parser = argparse.ArgumentParser(description='Query GCE Metadata Service') + parser.add_argument("--endpoint", metavar="URL", + help="The url of the metadata service.", + default=MD_V1_URL) + parser.add_argument("--no-platform-check", dest="platform_check", + help="Ignore smbios platform check", + action='store_false', default=True) + args = parser.parse_args() + print(json.dumps( + read_md(address=args.endpoint, platform_check=args.platform_check), + indent=1, sort_keys=True, separators=(',', ': '))) + # vi: ts=4 expandtab -- cgit v1.2.3 From 7e76c57b590c7c2c13f7b1a2a8b5b7d4f2d18396 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Wed, 16 Aug 2017 16:50:07 -0500 Subject: distro: allow distro to specify a default locale Currently the cloud-init default locale (en_US.UTF-8) is set by the base datasource class. This patch allows a distro to overide the fallback value with one that's available in the distro but continues to respect an image which has preconfigured a locale. - Distro object now has a get_locale method which will return a preconfigure locale setting by checking the distros locale system configuration file. If not set or not present, return the default locale of en_US.UTF-8 which retains behavior of all previous cloud-init releases. - Apply locale now handles regenerating locales or system configuration files as needed. - Adjust apply_locale logic to skip locale-regen if the specified LANG value is C.UTF-8,C, or POSIX; they do not require regeneration. - Further add unittests to exercise the default paths for Ubuntu and non-ubuntu paths to validate they get the LANG expected. --- cloudinit/distros/__init__.py | 3 + cloudinit/distros/debian.py | 94 ++++++++++++++++------ cloudinit/sources/__init__.py | 9 ++- tests/unittests/test_distros/test_debian.py | 66 +++++++++------ tests/unittests/test_distros/test_generic.py | 16 ++++ tests/unittests/test_handler/test_handler_debug.py | 11 ++- .../unittests/test_handler/test_handler_locale.py | 48 +++++++++++ 7 files changed, 195 insertions(+), 52 deletions(-) (limited to 'cloudinit/sources') diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index 807b3ea2..b714b9ab 100755 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -188,6 +188,9 @@ class Distro(object): def _get_localhost_ip(self): return "127.0.0.1" + def get_locale(self): + raise NotImplementedError() + @abc.abstractmethod def _read_hostname(self, filename, default=None): raise NotImplementedError() diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py index abfb81f4..33cc0bf1 100644 --- a/cloudinit/distros/debian.py +++ b/cloudinit/distros/debian.py @@ -61,11 +61,49 @@ class Distro(distros.Distro): # should only happen say once per instance...) self._runner = helpers.Runners(paths) self.osfamily = 'debian' + self.default_locale = 'en_US.UTF-8' + self.system_locale = None - def apply_locale(self, locale, out_fn=None): + def get_locale(self): + """Return the default locale if set, else use default locale""" + + # read system locale value + if not self.system_locale: + self.system_locale = read_system_locale() + + # Return system_locale setting if valid, else use default locale + return (self.system_locale if self.system_locale else + self.default_locale) + + def apply_locale(self, locale, out_fn=None, keyname='LANG'): + """Apply specified locale to system, regenerate if specified locale + differs from system default.""" if not out_fn: out_fn = LOCALE_CONF_FN - apply_locale(locale, out_fn) + + if not locale: + raise ValueError('Failed to provide locale value.') + + # Only call locale regeneration if needed + # Update system locale config with specified locale if needed + distro_locale = self.get_locale() + conf_fn_exists = os.path.exists(out_fn) + sys_locale_unset = False if self.system_locale else True + need_regen = (locale.lower() != distro_locale.lower() or + not conf_fn_exists or sys_locale_unset) + need_conf = not conf_fn_exists or need_regen or sys_locale_unset + + if need_regen: + regenerate_locale(locale, out_fn, keyname=keyname) + else: + LOG.debug( + "System has '%s=%s' requested '%s', skipping regeneration.", + keyname, self.system_locale, locale) + + if need_conf: + update_locale_conf(locale, out_fn, keyname=keyname) + # once we've updated the system config, invalidate cache + self.system_locale = None def install_packages(self, pkglist): self.update_package_sources() @@ -218,37 +256,47 @@ def _maybe_remove_legacy_eth0(path="/etc/network/interfaces.d/eth0.cfg"): LOG.warning(msg) -def apply_locale(locale, sys_path=LOCALE_CONF_FN, keyname='LANG'): - """Apply the locale. - - Run locale-gen for the provided locale and set the default - system variable `keyname` appropriately in the provided `sys_path`. - - If sys_path indicates that `keyname` is already set to `locale` - then no changes will be made and locale-gen not called. - This allows images built with a locale already generated to not re-run - locale-gen which can be very heavy. - """ - if not locale: - raise ValueError('Failed to provide locale value.') - +def read_system_locale(sys_path=LOCALE_CONF_FN, keyname='LANG'): + """Read system default locale setting, if present""" + sys_val = "" if not sys_path: raise ValueError('Invalid path: %s' % sys_path) if os.path.exists(sys_path): locale_content = util.load_file(sys_path) - # if LANG isn't present, regen sys_defaults = util.load_shell_content(locale_content) sys_val = sys_defaults.get(keyname, "") - if sys_val.lower() == locale.lower(): - LOG.debug( - "System has '%s=%s' requested '%s', skipping regeneration.", - keyname, sys_val, locale) - return - util.subp(['locale-gen', locale], capture=False) + return sys_val + + +def update_locale_conf(locale, sys_path, keyname='LANG'): + """Update system locale config""" + LOG.debug('Updating %s with locale setting %s=%s', + sys_path, keyname, locale) util.subp( ['update-locale', '--locale-file=' + sys_path, '%s=%s' % (keyname, locale)], capture=False) + +def regenerate_locale(locale, sys_path, keyname='LANG'): + """ + Run locale-gen for the provided locale and set the default + system variable `keyname` appropriately in the provided `sys_path`. + + """ + # special case for locales which do not require regen + # % locale -a + # C + # C.UTF-8 + # POSIX + if locale.lower() in ['c', 'c.utf-8', 'posix']: + LOG.debug('%s=%s does not require rengeneration', keyname, locale) + return + + # finally, trigger regeneration + LOG.debug('Generating locales for %s', locale) + util.subp(['locale-gen', locale], capture=False) + + # vi: ts=4 expandtab diff --git a/cloudinit/sources/__init__.py b/cloudinit/sources/__init__.py index 952caf35..9a43fbee 100644 --- a/cloudinit/sources/__init__.py +++ b/cloudinit/sources/__init__.py @@ -44,6 +44,7 @@ class DataSourceNotFoundException(Exception): class DataSource(object): dsmode = DSMODE_NETWORK + default_locale = 'en_US.UTF-8' def __init__(self, sys_cfg, distro, paths, ud_proc=None): self.sys_cfg = sys_cfg @@ -150,7 +151,13 @@ class DataSource(object): return None def get_locale(self): - return 'en_US.UTF-8' + """Default locale is en_US.UTF-8, but allow distros to override""" + locale = self.default_locale + try: + locale = self.distro.get_locale() + except NotImplementedError: + pass + return locale @property def availability_zone(self): diff --git a/tests/unittests/test_distros/test_debian.py b/tests/unittests/test_distros/test_debian.py index 2330ad52..72d3aad6 100644 --- a/tests/unittests/test_distros/test_debian.py +++ b/tests/unittests/test_distros/test_debian.py @@ -1,67 +1,85 @@ # This file is part of cloud-init. See LICENSE file for license information. -from ..helpers import (CiTestCase, mock) - -from cloudinit.distros.debian import apply_locale +from cloudinit import distros from cloudinit import util +from ..helpers import (FilesystemMockingTestCase, mock) @mock.patch("cloudinit.distros.debian.util.subp") -class TestDebianApplyLocale(CiTestCase): +class TestDebianApplyLocale(FilesystemMockingTestCase): + + def setUp(self): + super(TestDebianApplyLocale, self).setUp() + self.new_root = self.tmp_dir() + self.patchOS(self.new_root) + self.patchUtils(self.new_root) + self.spath = self.tmp_path('etc/default/locale', self.new_root) + cls = distros.fetch("debian") + self.distro = cls("debian", {}, None) + def test_no_rerun(self, m_subp): """If system has defined locale, no re-run is expected.""" - spath = self.tmp_path("default-locale") m_subp.return_value = (None, None) locale = 'en_US.UTF-8' - util.write_file(spath, 'LANG=%s\n' % locale, omode="w") - apply_locale(locale, sys_path=spath) + util.write_file(self.spath, 'LANG=%s\n' % locale, omode="w") + self.distro.apply_locale(locale, out_fn=self.spath) m_subp.assert_not_called() + def test_no_regen_on_c_utf8(self, m_subp): + """If locale is set to C.UTF8, do not attempt to call locale-gen""" + m_subp.return_value = (None, None) + locale = 'C.UTF-8' + util.write_file(self.spath, 'LANG=%s\n' % 'en_US.UTF-8', omode="w") + self.distro.apply_locale(locale, out_fn=self.spath) + self.assertEqual( + [['update-locale', '--locale-file=' + self.spath, + 'LANG=%s' % locale]], + [p[0][0] for p in m_subp.call_args_list]) + def test_rerun_if_different(self, m_subp): """If system has different locale, locale-gen should be called.""" - spath = self.tmp_path("default-locale") m_subp.return_value = (None, None) locale = 'en_US.UTF-8' - util.write_file(spath, 'LANG=fr_FR.UTF-8', omode="w") - apply_locale(locale, sys_path=spath) + util.write_file(self.spath, 'LANG=fr_FR.UTF-8', omode="w") + self.distro.apply_locale(locale, out_fn=self.spath) self.assertEqual( [['locale-gen', locale], - ['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale]], + ['update-locale', '--locale-file=' + self.spath, + 'LANG=%s' % locale]], [p[0][0] for p in m_subp.call_args_list]) def test_rerun_if_no_file(self, m_subp): """If system has no locale file, locale-gen should be called.""" - spath = self.tmp_path("default-locale") m_subp.return_value = (None, None) locale = 'en_US.UTF-8' - apply_locale(locale, sys_path=spath) + self.distro.apply_locale(locale, out_fn=self.spath) self.assertEqual( [['locale-gen', locale], - ['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale]], + ['update-locale', '--locale-file=' + self.spath, + 'LANG=%s' % locale]], [p[0][0] for p in m_subp.call_args_list]) def test_rerun_on_unset_system_locale(self, m_subp): """If system has unset locale, locale-gen should be called.""" m_subp.return_value = (None, None) - spath = self.tmp_path("default-locale") locale = 'en_US.UTF-8' - util.write_file(spath, 'LANG=', omode="w") - apply_locale(locale, sys_path=spath) + util.write_file(self.spath, 'LANG=', omode="w") + self.distro.apply_locale(locale, out_fn=self.spath) self.assertEqual( [['locale-gen', locale], - ['update-locale', '--locale-file=' + spath, 'LANG=%s' % locale]], + ['update-locale', '--locale-file=' + self.spath, + 'LANG=%s' % locale]], [p[0][0] for p in m_subp.call_args_list]) def test_rerun_on_mismatched_keys(self, m_subp): """If key is LC_ALL and system has only LANG, rerun is expected.""" m_subp.return_value = (None, None) - spath = self.tmp_path("default-locale") locale = 'en_US.UTF-8' - util.write_file(spath, 'LANG=', omode="w") - apply_locale(locale, sys_path=spath, keyname='LC_ALL') + util.write_file(self.spath, 'LANG=', omode="w") + self.distro.apply_locale(locale, out_fn=self.spath, keyname='LC_ALL') self.assertEqual( [['locale-gen', locale], - ['update-locale', '--locale-file=' + spath, + ['update-locale', '--locale-file=' + self.spath, 'LC_ALL=%s' % locale]], [p[0][0] for p in m_subp.call_args_list]) @@ -69,14 +87,14 @@ class TestDebianApplyLocale(CiTestCase): """locale as None or "" is invalid and should raise ValueError.""" with self.assertRaises(ValueError) as ctext_m: - apply_locale(None) + self.distro.apply_locale(None) m_subp.assert_not_called() self.assertEqual( 'Failed to provide locale value.', str(ctext_m.exception)) with self.assertRaises(ValueError) as ctext_m: - apply_locale("") + self.distro.apply_locale("") m_subp.assert_not_called() self.assertEqual( 'Failed to provide locale value.', str(ctext_m.exception)) diff --git a/tests/unittests/test_distros/test_generic.py b/tests/unittests/test_distros/test_generic.py index c9be277e..b355a19e 100644 --- a/tests/unittests/test_distros/test_generic.py +++ b/tests/unittests/test_distros/test_generic.py @@ -228,5 +228,21 @@ class TestGenericDistro(helpers.FilesystemMockingTestCase): os.symlink('/', '/run/systemd/system') self.assertFalse(d.uses_systemd()) + @mock.patch('cloudinit.distros.debian.read_system_locale') + def test_get_locale_ubuntu(self, m_locale): + """Test ubuntu distro returns locale set to C.UTF-8""" + m_locale.return_value = 'C.UTF-8' + cls = distros.fetch("ubuntu") + d = cls("ubuntu", {}, None) + locale = d.get_locale() + self.assertEqual('C.UTF-8', locale) + + def test_get_locale_rhel(self): + """Test rhel distro returns NotImplementedError exception""" + cls = distros.fetch("rhel") + d = cls("rhel", {}, None) + with self.assertRaises(NotImplementedError): + d.get_locale() + # vi: ts=4 expandtab diff --git a/tests/unittests/test_handler/test_handler_debug.py b/tests/unittests/test_handler/test_handler_debug.py index 929f786e..1873c3e1 100644 --- a/tests/unittests/test_handler/test_handler_debug.py +++ b/tests/unittests/test_handler/test_handler_debug.py @@ -11,7 +11,7 @@ from cloudinit import util from cloudinit.sources import DataSourceNone -from .. import helpers as t_help +from ..helpers import (FilesystemMockingTestCase, mock) import logging import shutil @@ -20,7 +20,8 @@ import tempfile LOG = logging.getLogger(__name__) -class TestDebug(t_help.FilesystemMockingTestCase): +@mock.patch('cloudinit.distros.debian.read_system_locale') +class TestDebug(FilesystemMockingTestCase): def setUp(self): super(TestDebug, self).setUp() self.new_root = tempfile.mkdtemp() @@ -36,7 +37,8 @@ class TestDebug(t_help.FilesystemMockingTestCase): ds.metadata.update(metadata) return cloud.Cloud(ds, paths, {}, d, None) - def test_debug_write(self): + def test_debug_write(self, m_locale): + m_locale.return_value = 'en_US.UTF-8' cfg = { 'abc': '123', 'c': u'\u20a0', @@ -54,7 +56,8 @@ class TestDebug(t_help.FilesystemMockingTestCase): for k in cfg.keys(): self.assertIn(k, contents) - def test_debug_no_write(self): + def test_debug_no_write(self, m_locale): + m_locale.return_value = 'en_US.UTF-8' cfg = { 'abc': '123', 'debug': { diff --git a/tests/unittests/test_handler/test_handler_locale.py b/tests/unittests/test_handler/test_handler_locale.py index cba5cae8..a789db32 100644 --- a/tests/unittests/test_handler/test_handler_locale.py +++ b/tests/unittests/test_handler/test_handler_locale.py @@ -20,6 +20,8 @@ from configobj import ConfigObj from six import BytesIO import logging +import mock +import os import shutil import tempfile @@ -27,6 +29,9 @@ LOG = logging.getLogger(__name__) class TestLocale(t_help.FilesystemMockingTestCase): + + with_logs = True + def setUp(self): super(TestLocale, self).setUp() self.new_root = tempfile.mkdtemp() @@ -60,4 +65,47 @@ class TestLocale(t_help.FilesystemMockingTestCase): else: self.assertEqual({'RC_LANG': cfg['locale']}, dict(n_cfg)) + def test_set_locale_sles_default(self): + cfg = {} + cc = self._get_cloud('sles') + cc_locale.handle('cc_locale', cfg, cc, LOG, []) + + if cc.distro.uses_systemd(): + locale_conf = cc.distro.systemd_locale_conf_fn + keyname = 'LANG' + else: + locale_conf = cc.distro.locale_conf_fn + keyname = 'RC_LANG' + + contents = util.load_file(locale_conf, decode=False) + n_cfg = ConfigObj(BytesIO(contents)) + self.assertEqual({keyname: 'en_US.UTF-8'}, dict(n_cfg)) + + def test_locale_update_config_if_different_than_default(self): + """Test cc_locale writes updates conf if different than default""" + locale_conf = os.path.join(self.new_root, "etc/default/locale") + util.write_file(locale_conf, 'LANG="en_US.UTF-8"\n') + cfg = {'locale': 'C.UTF-8'} + cc = self._get_cloud('ubuntu') + with mock.patch('cloudinit.distros.debian.util.subp') as m_subp: + with mock.patch('cloudinit.distros.debian.LOCALE_CONF_FN', + locale_conf): + cc_locale.handle('cc_locale', cfg, cc, LOG, []) + m_subp.assert_called_with(['update-locale', + '--locale-file=%s' % locale_conf, + 'LANG=C.UTF-8'], capture=False) + + def test_locale_rhel_defaults_en_us_utf8(self): + """Test cc_locale gets en_US.UTF-8 from distro get_locale fallback""" + cfg = {} + cc = self._get_cloud('rhel') + update_sysconfig = 'cloudinit.distros.rhel_util.update_sysconfig_file' + with mock.patch.object(cc.distro, 'uses_systemd') as m_use_sd: + m_use_sd.return_value = True + with mock.patch(update_sysconfig) as m_update_syscfg: + cc_locale.handle('cc_locale', cfg, cc, LOG, []) + m_update_syscfg.assert_called_with('/etc/locale.conf', + {'LANG': 'en_US.UTF-8'}) + + # vi: ts=4 expandtab -- cgit v1.2.3 From 409918f9ba83e45e9bc5cc0b6c589e2fc8ae9b60 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Tue, 29 Aug 2017 09:59:20 -0400 Subject: Use /run/cloud-init for tempfile operations. During boot, the usage of /tmp is not safe. In systemd systems, systemd-tmpfiles-clean may run at any point and clear out a temp file while cloud-init is using it. The solution here is to use /run/cloud-init/tmp. LP: #1707222 --- cloudinit/config/cc_bootcmd.py | 3 +- cloudinit/config/cc_chef.py | 3 +- cloudinit/config/cc_snappy.py | 4 +- cloudinit/net/dhcp.py | 3 +- cloudinit/sources/helpers/azure.py | 4 +- cloudinit/temp_utils.py | 93 ++++++++++++++++++++++ cloudinit/util.py | 36 +-------- packages/bddeb | 5 +- .../unittests/test_datasource/test_azure_helper.py | 4 +- tests/unittests/test_net.py | 3 +- 10 files changed, 112 insertions(+), 46 deletions(-) create mode 100644 cloudinit/temp_utils.py (limited to 'cloudinit/sources') diff --git a/cloudinit/config/cc_bootcmd.py b/cloudinit/config/cc_bootcmd.py index 604f93b0..9c0476af 100644 --- a/cloudinit/config/cc_bootcmd.py +++ b/cloudinit/config/cc_bootcmd.py @@ -37,6 +37,7 @@ specified either as lists or strings. For invocation details, see ``runcmd``. import os from cloudinit.settings import PER_ALWAYS +from cloudinit import temp_utils from cloudinit import util frequency = PER_ALWAYS @@ -49,7 +50,7 @@ def handle(name, cfg, cloud, log, _args): " no 'bootcmd' key in configuration"), name) return - with util.ExtendedTemporaryFile(suffix=".sh") as tmpf: + with temp_utils.ExtendedTemporaryFile(suffix=".sh") as tmpf: try: content = util.shellify(cfg["bootcmd"]) tmpf.write(util.encode_text(content)) diff --git a/cloudinit/config/cc_chef.py b/cloudinit/config/cc_chef.py index 02c70b10..c192dd32 100644 --- a/cloudinit/config/cc_chef.py +++ b/cloudinit/config/cc_chef.py @@ -71,6 +71,7 @@ import itertools import json import os +from cloudinit import temp_utils from cloudinit import templater from cloudinit import url_helper from cloudinit import util @@ -303,7 +304,7 @@ def install_chef(cloud, chef_cfg, log): "omnibus_url_retries", default=OMNIBUS_URL_RETRIES)) content = url_helper.readurl(url=url, retries=retries).contents - with util.tempdir() as tmpd: + with temp_utils.tempdir() as tmpd: # Use tmpdir over tmpfile to avoid 'text file busy' on execute tmpf = "%s/chef-omnibus-install" % tmpd util.write_file(tmpf, content, mode=0o700) diff --git a/cloudinit/config/cc_snappy.py b/cloudinit/config/cc_snappy.py index a9682f19..eecb8178 100644 --- a/cloudinit/config/cc_snappy.py +++ b/cloudinit/config/cc_snappy.py @@ -63,11 +63,11 @@ is ``auto``. Options are: from cloudinit import log as logging from cloudinit.settings import PER_INSTANCE +from cloudinit import temp_utils from cloudinit import util import glob import os -import tempfile LOG = logging.getLogger(__name__) @@ -183,7 +183,7 @@ def render_snap_op(op, name, path=None, cfgfile=None, config=None): # config # Note, however, we do not touch config files on disk. nested_cfg = {'config': {shortname: config}} - (fd, cfg_tmpf) = tempfile.mkstemp() + (fd, cfg_tmpf) = temp_utils.mkstemp() os.write(fd, util.yaml_dumps(nested_cfg).encode()) os.close(fd) cfgfile = cfg_tmpf diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index c7febc57..c842c839 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -9,6 +9,7 @@ import os import re from cloudinit.net import find_fallback_nic, get_devicelist +from cloudinit import temp_utils from cloudinit import util LOG = logging.getLogger(__name__) @@ -47,7 +48,7 @@ def maybe_perform_dhcp_discovery(nic=None): if not dhclient_path: LOG.debug('Skip dhclient configuration: No dhclient command found.') return {} - with util.tempdir(prefix='cloud-init-dhcp-') as tmpdir: + with temp_utils.tempdir(prefix='cloud-init-dhcp-') as tmpdir: return dhcp_discovery(dhclient_path, nic, tmpdir) diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index e22409d1..28ed0ae2 100644 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -6,10 +6,10 @@ import os import re import socket import struct -import tempfile import time from cloudinit import stages +from cloudinit import temp_utils from contextlib import contextmanager from xml.etree import ElementTree @@ -111,7 +111,7 @@ class OpenSSLManager(object): } def __init__(self): - self.tmpdir = tempfile.mkdtemp() + self.tmpdir = temp_utils.mkdtemp() self.certificate = None self.generate_certificate() diff --git a/cloudinit/temp_utils.py b/cloudinit/temp_utils.py new file mode 100644 index 00000000..0355f19d --- /dev/null +++ b/cloudinit/temp_utils.py @@ -0,0 +1,93 @@ +# This file is part of cloud-init. See LICENSE file for license information. + +import contextlib +import errno +import os +import shutil +import tempfile + +_TMPDIR = None +_ROOT_TMPDIR = "/run/cloud-init/tmp" + + +def _tempfile_dir_arg(odir=None): + """Return the proper 'dir' argument for tempfile functions. + + When root, cloud-init will use /run/cloud-init/tmp to avoid + any cleaning that a distro boot might do on /tmp (such as + systemd-tmpfiles-clean). + + If the caller of this function (mkdtemp or mkstemp) was provided + with a 'dir' argument, then that is respected. + + @param odir: original 'dir' arg to 'mkdtemp' or other.""" + + if odir is not None: + return odir + + global _TMPDIR + if _TMPDIR: + return _TMPDIR + + if os.getuid() == 0: + tdir = _ROOT_TMPDIR + else: + tdir = os.environ.get('TMPDIR', '/tmp') + if not os.path.isdir(tdir): + os.makedirs(tdir) + os.chmod(tdir, 0o1777) + + _TMPDIR = tdir + return tdir + + +def ExtendedTemporaryFile(**kwargs): + kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None)) + fh = tempfile.NamedTemporaryFile(**kwargs) + # Replace its unlink with a quiet version + # that does not raise errors when the + # file to unlink has been unlinked elsewhere.. + + def _unlink_if_exists(path): + try: + os.unlink(path) + except OSError as e: + if e.errno != errno.ENOENT: + raise e + + fh.unlink = _unlink_if_exists + + # Add a new method that will unlink + # right 'now' but still lets the exit + # method attempt to remove it (which will + # not throw due to our del file being quiet + # about files that are not there) + def unlink_now(): + fh.unlink(fh.name) + + setattr(fh, 'unlink_now', unlink_now) + return fh + + +@contextlib.contextmanager +def tempdir(**kwargs): + # This seems like it was only added in python 3.2 + # Make it since its useful... + # See: http://bugs.python.org/file12970/tempdir.patch + tdir = mkdtemp(**kwargs) + try: + yield tdir + finally: + shutil.rmtree(tdir) + + +def mkdtemp(**kwargs): + kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None)) + return tempfile.mkdtemp(**kwargs) + + +def mkstemp(**kwargs): + kwargs['dir'] = _tempfile_dir_arg(kwargs.pop('dir', None)) + return tempfile.mkstemp(**kwargs) + +# vi: ts=4 expandtab diff --git a/cloudinit/util.py b/cloudinit/util.py index 609e94c8..ae5cda8d 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -30,7 +30,6 @@ import stat import string import subprocess import sys -import tempfile import time from errno import ENOENT, ENOEXEC @@ -45,6 +44,7 @@ from cloudinit import importer from cloudinit import log as logging from cloudinit import mergers from cloudinit import safeyaml +from cloudinit import temp_utils from cloudinit import type_utils from cloudinit import url_helper from cloudinit import version @@ -349,26 +349,6 @@ class DecompressionError(Exception): pass -def ExtendedTemporaryFile(**kwargs): - fh = tempfile.NamedTemporaryFile(**kwargs) - # Replace its unlink with a quiet version - # that does not raise errors when the - # file to unlink has been unlinked elsewhere.. - LOG.debug("Created temporary file %s", fh.name) - fh.unlink = del_file - - # Add a new method that will unlink - # right 'now' but still lets the exit - # method attempt to remove it (which will - # not throw due to our del file being quiet - # about files that are not there) - def unlink_now(): - fh.unlink(fh.name) - - setattr(fh, 'unlink_now', unlink_now) - return fh - - def fork_cb(child_cb, *args, **kwargs): fid = os.fork() if fid == 0: @@ -790,18 +770,6 @@ def umask(n_msk): os.umask(old) -@contextlib.contextmanager -def tempdir(**kwargs): - # This seems like it was only added in python 3.2 - # Make it since its useful... - # See: http://bugs.python.org/file12970/tempdir.patch - tdir = tempfile.mkdtemp(**kwargs) - try: - yield tdir - finally: - del_dir(tdir) - - def center(text, fill, max_len): return '{0:{fill}{align}{size}}'.format(text, fill=fill, align="^", size=max_len) @@ -1587,7 +1555,7 @@ def mount_cb(device, callback, data=None, rw=False, mtype=None, sync=True): mtypes = [''] mounted = mounts() - with tempdir() as tmpd: + with temp_utils.tempdir() as tmpd: umount = False if os.path.realpath(device) in mounted: mountpoint = mounted[os.path.realpath(device)]['mountpoint'] diff --git a/packages/bddeb b/packages/bddeb index 7c123548..4f2e2ddf 100755 --- a/packages/bddeb +++ b/packages/bddeb @@ -21,8 +21,9 @@ def find_root(): if "avoid-pep8-E402-import-not-top-of-file": # Use the util functions from cloudinit sys.path.insert(0, find_root()) - from cloudinit import templater from cloudinit import util + from cloudinit import temp_utils + from cloudinit import templater DEBUILD_ARGS = ["-S", "-d"] @@ -148,7 +149,7 @@ def main(): capture = False templ_data = {'debian_release': args.release} - with util.tempdir() as tdir: + with temp_utils.tempdir() as tdir: # output like 0.7.6-1022-g36e92d3 ver_data = read_version() diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py index 80ce003d..44b99eca 100644 --- a/tests/unittests/test_datasource/test_azure_helper.py +++ b/tests/unittests/test_datasource/test_azure_helper.py @@ -275,7 +275,7 @@ class TestOpenSSLManager(TestCase): mock.patch('builtins.open')) @mock.patch.object(azure_helper, 'cd', mock.MagicMock()) - @mock.patch.object(azure_helper.tempfile, 'mkdtemp') + @mock.patch.object(azure_helper.temp_utils, 'mkdtemp') def test_openssl_manager_creates_a_tmpdir(self, mkdtemp): manager = azure_helper.OpenSSLManager() self.assertEqual(mkdtemp.return_value, manager.tmpdir) @@ -292,7 +292,7 @@ class TestOpenSSLManager(TestCase): manager.clean_up() @mock.patch.object(azure_helper, 'cd', mock.MagicMock()) - @mock.patch.object(azure_helper.tempfile, 'mkdtemp', mock.MagicMock()) + @mock.patch.object(azure_helper.temp_utils, 'mkdtemp', mock.MagicMock()) @mock.patch.object(azure_helper.util, 'del_dir') def test_clean_up(self, del_dir): manager = azure_helper.OpenSSLManager() diff --git a/tests/unittests/test_net.py b/tests/unittests/test_net.py index c10ef905..f2496151 100644 --- a/tests/unittests/test_net.py +++ b/tests/unittests/test_net.py @@ -9,6 +9,7 @@ from cloudinit.net import network_state from cloudinit.net import renderers from cloudinit.net import sysconfig from cloudinit.sources.helpers import openstack +from cloudinit import temp_utils from cloudinit import util from cloudinit.tests.helpers import CiTestCase @@ -2150,7 +2151,7 @@ class TestCmdlineConfigParsing(CiTestCase): static['mac_address'] = macs['eth1'] expected = {'version': 1, 'config': [dhcp, static]} - with util.tempdir() as tmpd: + with temp_utils.tempdir() as tmpd: for fname, content in pairs: fp = os.path.join(tmpd, fname) files.append(fp) -- cgit v1.2.3 From 922c3c5c1a86f2d58e95a328e72b49a3bb234ca8 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Thu, 7 Sep 2017 09:59:47 -0400 Subject: Ec2: only attempt to operate at local mode on known platforms. This change makes the DataSourceEc2Local do nothing unless it is on actual AWS platform. The motivation is twofold: a.) It is generally safer to only make this function available to Ec2 clones that explicitly identify themselves to the guest. (It also gives them a reason to supply identification code to cloud-init.) b.) On non-intel OpenStack platforms ds-identify would enable both the Ec2 and OpenStack sources. That is because there is not good data (such as dmi) to positively identify the platform. Previously that would be fine as OpenStack would run first and be successful. The change to add Ec2Local meant that an Ec2 now runs first. The best case for 'b' would be a slow down as attempts at the Ec2 metadata service time out. The discovered case was worse. Additionally we add a simple check for datatype of 'network' in the metadata before attempting to read it. LP: #1715128 --- cloudinit/sources/DataSourceEc2.py | 43 +++++++++++++++++++++++------ tests/unittests/test_datasource/test_ec2.py | 29 ++++++++++++++++--- 2 files changed, 60 insertions(+), 12 deletions(-) (limited to 'cloudinit/sources') diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index 07c12bb4..41367a8b 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -27,6 +27,8 @@ SKIP_METADATA_URL_CODES = frozenset([uhelp.NOT_FOUND]) STRICT_ID_PATH = ("datasource", "Ec2", "strict_id") STRICT_ID_DEFAULT = "warn" +_unset = "_unset" + class Platforms(object): ALIYUN = "AliYun" @@ -57,7 +59,7 @@ class DataSourceEc2(sources.DataSource): _cloud_platform = None - _network_config = None # Used for caching calculated network config v1 + _network_config = _unset # Used for caching calculated network config v1 # Whether we want to get network configuration from the metadata service. get_network_metadata = False @@ -284,10 +286,24 @@ class DataSourceEc2(sources.DataSource): @property def network_config(self): """Return a network config dict for rendering ENI or netplan files.""" - if self._network_config is None: - if self.metadata is not None: - self._network_config = convert_ec2_metadata_network_config( - self.metadata) + if self._network_config != _unset: + return self._network_config + + if self.metadata is None: + # this would happen if get_data hadn't been called. leave as _unset + LOG.warning( + "Unexpected call to network_config when metadata is None.") + return None + + result = None + net_md = self.metadata.get('network') + if isinstance(net_md, dict): + result = convert_ec2_metadata_network_config(net_md) + else: + LOG.warning("unexpected metadata 'network' key not valid: %s", + net_md) + self._network_config = result + return self._network_config def _crawl_metadata(self): @@ -321,6 +337,14 @@ class DataSourceEc2Local(DataSourceEc2): """ get_network_metadata = True # Get metadata network config if present + def get_data(self): + supported_platforms = (Platforms.AWS,) + if self.cloud_platform not in supported_platforms: + LOG.debug("Local Ec2 mode only supported on %s, not %s", + supported_platforms, self.cloud_platform) + return False + return super(DataSourceEc2Local, self).get_data() + def read_strict_mode(cfgval, default): try: @@ -434,10 +458,13 @@ def _collect_platform_data(): return data -def convert_ec2_metadata_network_config(metadata=None, macs_to_nics=None): +def convert_ec2_metadata_network_config(network_md, macs_to_nics=None): """Convert ec2 metadata to network config version 1 data dict. - @param: metadata: Dictionary of metadata crawled from EC2 metadata url. + @param: network_md: 'network' portion of EC2 metadata. + generally formed as {"interfaces": {"macs": {}} where + 'macs' is a dictionary with mac address as key and contents like: + {"device-number": "0", "interface-id": "...", "local-ipv4s": ...} @param: macs_to_name: Optional dict mac addresses and the nic name. If not provided, get_interfaces_by_mac is called to get it from the OS. @@ -446,7 +473,7 @@ def convert_ec2_metadata_network_config(metadata=None, macs_to_nics=None): netcfg = {'version': 1, 'config': []} if not macs_to_nics: macs_to_nics = net.get_interfaces_by_mac() - macs_metadata = metadata['network']['interfaces']['macs'] + macs_metadata = network_md['interfaces']['macs'] for mac, nic_name in macs_to_nics.items(): nic_metadata = macs_metadata.get(mac) if not nic_metadata: diff --git a/tests/unittests/test_datasource/test_ec2.py b/tests/unittests/test_datasource/test_ec2.py index 9fb90483..a7301dbf 100644 --- a/tests/unittests/test_datasource/test_ec2.py +++ b/tests/unittests/test_datasource/test_ec2.py @@ -279,6 +279,27 @@ class TestEc2(test_helpers.HttprettyTestCase): ret = ds.get_data() self.assertTrue(ret) + def test_ec2_local_returns_false_on_non_aws(self): + """DataSourceEc2Local returns False when platform is not AWS.""" + self.datasource = ec2.DataSourceEc2Local + ds = self._setup_ds( + platform_data=self.valid_platform_data, + sys_cfg={'datasource': {'Ec2': {'strict_id': False}}}, + md=DEFAULT_METADATA) + platform_attrs = [ + attr for attr in ec2.Platforms.__dict__.keys() + if not attr.startswith('__')] + for attr_name in platform_attrs: + platform_name = getattr(ec2.Platforms, attr_name) + if platform_name != 'AWS': + ds._cloud_platform = platform_name + ret = ds.get_data() + self.assertFalse(ret) + message = ( + "Local Ec2 mode only supported on ('AWS',)," + ' not {0}'.format(platform_name)) + self.assertIn(message, self.logs.getvalue()) + @httpretty.activate @mock.patch('cloudinit.sources.DataSourceEc2.util.is_FreeBSD') def test_ec2_local_returns_false_on_bsd(self, m_is_freebsd): @@ -336,8 +357,8 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): super(TestConvertEc2MetadataNetworkConfig, self).setUp() self.mac1 = '06:17:04:d7:26:09' self.network_metadata = { - 'network': {'interfaces': {'macs': { - self.mac1: {'public-ipv4s': '172.31.2.16'}}}}} + 'interfaces': {'macs': { + self.mac1: {'public-ipv4s': '172.31.2.16'}}}} def test_convert_ec2_metadata_network_config_skips_absent_macs(self): """Any mac absent from metadata is skipped by network config.""" @@ -357,7 +378,7 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): macs_to_nics = {self.mac1: 'eth9'} network_metadata_ipv6 = copy.deepcopy(self.network_metadata) nic1_metadata = ( - network_metadata_ipv6['network']['interfaces']['macs'][self.mac1]) + network_metadata_ipv6['interfaces']['macs'][self.mac1]) nic1_metadata['ipv6s'] = '2620:0:1009:fd00:e442:c88d:c04d:dc85/64' nic1_metadata.pop('public-ipv4s') expected = {'version': 1, 'config': [ @@ -373,7 +394,7 @@ class TestConvertEc2MetadataNetworkConfig(test_helpers.CiTestCase): macs_to_nics = {self.mac1: 'eth9'} network_metadata_both = copy.deepcopy(self.network_metadata) nic1_metadata = ( - network_metadata_both['network']['interfaces']['macs'][self.mac1]) + network_metadata_both['interfaces']['macs'][self.mac1]) nic1_metadata['ipv6s'] = '2620:0:1009:fd00:e442:c88d:c04d:dc85/64' expected = {'version': 1, 'config': [ {'mac_address': self.mac1, 'type': 'physical', -- cgit v1.2.3 From a1dfdda2a2ae20fe026881980ddf7d16110f06e2 Mon Sep 17 00:00:00 2001 From: Sankar Tanguturi Date: Thu, 7 Sep 2017 22:16:16 -0600 Subject: vmware customization: return network config format For customizing the machines hosted on 'VMWare' hypervisor, the datasource should return the 'network config' data in 'curtin' format. This branch also fixes /etc/network/interfaces replacing the line "source /etc/network/interfaces.d/*.cfg" which is incorrectly removed when VMWare's Perl Customization Engine writes /etc/network/interfaces. Modify the code to read the customization configuration and return the converted data. Added few tests. LP: #1675063 --- cloudinit/sources/DataSourceOVF.py | 91 ++++++--- cloudinit/sources/helpers/vmware/imc/config_nic.py | 201 ++++++++++++------- .../sources/helpers/vmware/imc/guestcust_util.py | 12 +- tests/unittests/test_vmware_config_file.py | 217 +++++++++++++++++++++ 4 files changed, 418 insertions(+), 103 deletions(-) (limited to 'cloudinit/sources') diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py index 73d38771..aa5f798d 100644 --- a/cloudinit/sources/DataSourceOVF.py +++ b/cloudinit/sources/DataSourceOVF.py @@ -51,6 +51,10 @@ class DataSourceOVF(sources.DataSource): self.cfg = {} self.supported_seed_starts = ("/", "file://") self.vmware_customization_supported = True + self._network_config = None + self._vmware_nics_to_enable = None + self._vmware_cust_conf = None + self._vmware_cust_found = False def __str__(self): root = sources.DataSource.__str__(self) @@ -60,8 +64,8 @@ class DataSourceOVF(sources.DataSource): found = [] md = {} ud = "" - vmwarePlatformFound = False - vmwareImcConfigFilePath = '' + vmwareImcConfigFilePath = None + nicspath = None defaults = { "instance-id": "iid-dsovf", @@ -101,25 +105,26 @@ class DataSourceOVF(sources.DataSource): logfunc=LOG.debug, msg="waiting for configuration file", func=wait_for_imc_cfg_file, - args=("/var/run/vmware-imc", "cust.cfg", max_wait)) + args=("cust.cfg", max_wait)) if vmwareImcConfigFilePath: LOG.debug("Found VMware Customization Config File at %s", vmwareImcConfigFilePath) + nicspath = wait_for_imc_cfg_file( + filename="nics.txt", maxwait=10, naplen=5) else: LOG.debug("Did not find VMware Customization Config File") else: LOG.debug("Customization for VMware platform is disabled.") if vmwareImcConfigFilePath: - nics = "" + self._vmware_nics_to_enable = "" try: cf = ConfigFile(vmwareImcConfigFilePath) - conf = Config(cf) - (md, ud, cfg) = read_vmware_imc(conf) - dirpath = os.path.dirname(vmwareImcConfigFilePath) - nics = get_nics_to_enable(dirpath) - markerid = conf.marker_id + self._vmware_cust_conf = Config(cf) + (md, ud, cfg) = read_vmware_imc(self._vmware_cust_conf) + self._vmware_nics_to_enable = get_nics_to_enable(nicspath) + markerid = self._vmware_cust_conf.marker_id markerexists = check_marker_exists(markerid) except Exception as e: LOG.debug("Error parsing the customization Config File") @@ -127,28 +132,29 @@ class DataSourceOVF(sources.DataSource): set_customization_status( GuestCustStateEnum.GUESTCUST_STATE_RUNNING, GuestCustEventEnum.GUESTCUST_EVENT_CUSTOMIZE_FAILED) - enable_nics(nics) - return False + raise e finally: util.del_dir(os.path.dirname(vmwareImcConfigFilePath)) try: - LOG.debug("Applying the Network customization") - nicConfigurator = NicConfigurator(conf.nics) - nicConfigurator.configure() + LOG.debug("Preparing the Network configuration") + self._network_config = get_network_config_from_conf( + self._vmware_cust_conf, + True, + True, + self.distro.osfamily) except Exception as e: - LOG.debug("Error applying the Network Configuration") LOG.exception(e) set_customization_status( GuestCustStateEnum.GUESTCUST_STATE_RUNNING, GuestCustEventEnum.GUESTCUST_EVENT_NETWORK_SETUP_FAILED) - enable_nics(nics) - return False + raise e + if markerid and not markerexists: LOG.debug("Applying password customization") pwdConfigurator = PasswordConfigurator() - adminpwd = conf.admin_password + adminpwd = self._vmware_cust_conf.admin_password try: - resetpwd = conf.reset_password + resetpwd = self._vmware_cust_conf.reset_password if adminpwd or resetpwd: pwdConfigurator.configure(adminpwd, resetpwd, self.distro) @@ -159,7 +165,6 @@ class DataSourceOVF(sources.DataSource): set_customization_status( GuestCustStateEnum.GUESTCUST_STATE_RUNNING, GuestCustEventEnum.GUESTCUST_EVENT_CUSTOMIZE_FAILED) - enable_nics(nics) return False if markerid: LOG.debug("Handle marker creation") @@ -170,14 +175,18 @@ class DataSourceOVF(sources.DataSource): set_customization_status( GuestCustStateEnum.GUESTCUST_STATE_RUNNING, GuestCustEventEnum.GUESTCUST_EVENT_CUSTOMIZE_FAILED) - enable_nics(nics) return False - vmwarePlatformFound = True + self._vmware_cust_found = True + found.append('vmware-tools') + + # TODO: Need to set the status to DONE only when the + # customization is done successfully. set_customization_status( GuestCustStateEnum.GUESTCUST_STATE_DONE, GuestCustErrorEnum.GUESTCUST_ERROR_SUCCESS) - enable_nics(nics) + enable_nics(self._vmware_nics_to_enable) + else: np = {'iso': transport_iso9660, 'vmware-guestd': transport_vmware_guestd, } @@ -192,7 +201,7 @@ class DataSourceOVF(sources.DataSource): found.append(name) # There was no OVF transports found - if len(found) == 0 and not vmwarePlatformFound: + if len(found) == 0: return False if 'seedfrom' in md and md['seedfrom']: @@ -237,6 +246,10 @@ class DataSourceOVF(sources.DataSource): def get_config_obj(self): return self.cfg + @property + def network_config(self): + return self._network_config + class DataSourceOVFNet(DataSourceOVF): def __init__(self, sys_cfg, distro, paths): @@ -268,12 +281,13 @@ def get_max_wait_from_cfg(cfg): return max_wait -def wait_for_imc_cfg_file(dirpath, filename, maxwait=180, naplen=5): +def wait_for_imc_cfg_file(filename, maxwait=180, naplen=5, + dirpath="/var/run/vmware-imc"): waited = 0 while waited < maxwait: - fileFullPath = search_file(dirpath, filename) - if fileFullPath: + fileFullPath = os.path.join(dirpath, filename) + if os.path.isfile(fileFullPath): return fileFullPath LOG.debug("Waiting for VMware Customization Config File") time.sleep(naplen) @@ -281,6 +295,26 @@ def wait_for_imc_cfg_file(dirpath, filename, maxwait=180, naplen=5): return None +def get_network_config_from_conf(config, use_system_devices=True, + configure=False, osfamily=None): + nicConfigurator = NicConfigurator(config.nics, use_system_devices) + nics_cfg_list = nicConfigurator.generate(configure, osfamily) + + return get_network_config(nics_cfg_list, + config.name_servers, + config.dns_suffixes) + + +def get_network_config(nics=None, nameservers=None, search=None): + config_list = nics + + if nameservers or search: + config_list.append({'type': 'nameserver', 'address': nameservers, + 'search': search}) + + return {'version': 1, 'config': config_list} + + # This will return a dict with some content # meta-data, user-data, some config def read_vmware_imc(config): @@ -296,6 +330,9 @@ def read_vmware_imc(config): if config.timezone: cfg['timezone'] = config.timezone + # Generate a unique instance-id so that re-customization will + # happen in cloud-init + md['instance-id'] = "iid-vmware-" + util.rand_str(strlen=8) return (md, ud, cfg) diff --git a/cloudinit/sources/helpers/vmware/imc/config_nic.py b/cloudinit/sources/helpers/vmware/imc/config_nic.py index 67ac21db..2fb07c59 100644 --- a/cloudinit/sources/helpers/vmware/imc/config_nic.py +++ b/cloudinit/sources/helpers/vmware/imc/config_nic.py @@ -9,22 +9,48 @@ import logging import os import re +from cloudinit.net.network_state import mask_to_net_prefix from cloudinit import util logger = logging.getLogger(__name__) +def gen_subnet(ip, netmask): + """ + Return the subnet for a given ip address and a netmask + @return (str): the subnet + @param ip: ip address + @param netmask: netmask + """ + ip_array = ip.split(".") + mask_array = netmask.split(".") + result = [] + for index in list(range(4)): + result.append(int(ip_array[index]) & int(mask_array[index])) + + return ".".join([str(x) for x in result]) + + class NicConfigurator(object): - def __init__(self, nics): + def __init__(self, nics, use_system_devices=True): """ Initialize the Nic Configurator @param nics (list) an array of nics to configure + @param use_system_devices (Bool) Get the MAC names from the system + if this is True. If False, then mac names will be retrieved from + the specified nics. """ self.nics = nics self.mac2Name = {} self.ipv4PrimaryGateway = None self.ipv6PrimaryGateway = None - self.find_devices() + + if use_system_devices: + self.find_devices() + else: + for nic in self.nics: + self.mac2Name[nic.mac.lower()] = nic.name + self._primaryNic = self.get_primary_nic() def get_primary_nic(self): @@ -61,138 +87,163 @@ class NicConfigurator(object): def gen_one_nic(self, nic): """ - Return the lines needed to configure a nic - @return (str list): the string list to configure the nic + Return the config list needed to configure a nic + @return (list): the subnets and routes list to configure the nic @param nic (NicBase): the nic to configure """ - lines = [] - name = self.mac2Name.get(nic.mac.lower()) + mac = nic.mac.lower() + name = self.mac2Name.get(mac) if not name: raise ValueError('No known device has MACADDR: %s' % nic.mac) - if nic.onboot: - lines.append('auto %s' % name) + nics_cfg_list = [] + + cfg = {'type': 'physical', 'name': name, 'mac_address': mac} + + subnet_list = [] + route_list = [] # Customize IPv4 - lines.extend(self.gen_ipv4(name, nic)) + (subnets, routes) = self.gen_ipv4(name, nic) + subnet_list.extend(subnets) + route_list.extend(routes) # Customize IPv6 - lines.extend(self.gen_ipv6(name, nic)) + (subnets, routes) = self.gen_ipv6(name, nic) + subnet_list.extend(subnets) + route_list.extend(routes) + + cfg.update({'subnets': subnet_list}) - lines.append('') + nics_cfg_list.append(cfg) + if route_list: + nics_cfg_list.extend(route_list) - return lines + return nics_cfg_list def gen_ipv4(self, name, nic): """ - Return the lines needed to configure the IPv4 setting of a nic - @return (str list): the string list to configure the gateways - @param name (str): name of the nic + Return the set of subnets and routes needed to configure the + IPv4 settings of a nic + @return (set): the set of subnet and routes to configure the gateways + @param name (str): subnet and route list for the nic @param nic (NicBase): the nic to configure """ - lines = [] + + subnet = {} + route_list = [] + + if nic.onboot: + subnet.update({'control': 'auto'}) bootproto = nic.bootProto.lower() if nic.ipv4_mode.lower() == 'disabled': bootproto = 'manual' - lines.append('iface %s inet %s' % (name, bootproto)) if bootproto != 'static': - return lines + subnet.update({'type': 'dhcp'}) + return ([subnet], route_list) + else: + subnet.update({'type': 'static'}) # Static Ipv4 addrs = nic.staticIpv4 if not addrs: - return lines + return ([subnet], route_list) v4 = addrs[0] if v4.ip: - lines.append(' address %s' % v4.ip) + subnet.update({'address': v4.ip}) if v4.netmask: - lines.append(' netmask %s' % v4.netmask) + subnet.update({'netmask': v4.netmask}) # Add the primary gateway if nic.primary and v4.gateways: self.ipv4PrimaryGateway = v4.gateways[0] - lines.append(' gateway %s metric 0' % self.ipv4PrimaryGateway) - return lines + subnet.update({'gateway': self.ipv4PrimaryGateway}) + return [subnet] # Add routes if there is no primary nic if not self._primaryNic: - lines.extend(self.gen_ipv4_route(nic, v4.gateways)) + route_list.extend(self.gen_ipv4_route(nic, + v4.gateways, + v4.netmask)) - return lines + return ([subnet], route_list) - def gen_ipv4_route(self, nic, gateways): + def gen_ipv4_route(self, nic, gateways, netmask): """ - Return the lines needed to configure additional Ipv4 route - @return (str list): the string list to configure the gateways + Return the routes list needed to configure additional Ipv4 route + @return (list): the route list to configure the gateways @param nic (NicBase): the nic to configure @param gateways (str list): the list of gateways """ - lines = [] + route_list = [] + + cidr = mask_to_net_prefix(netmask) for gateway in gateways: - lines.append(' up route add default gw %s metric 10000' % - gateway) + destination = "%s/%d" % (gen_subnet(gateway, netmask), cidr) + route_list.append({'destination': destination, + 'type': 'route', + 'gateway': gateway, + 'metric': 10000}) - return lines + return route_list def gen_ipv6(self, name, nic): """ - Return the lines needed to configure the gateways for a nic - @return (str list): the string list to configure the gateways + Return the set of subnets and routes needed to configure the + gateways for a nic + @return (set): the set of subnets and routes to configure the gateways @param name (str): name of the nic @param nic (NicBase): the nic to configure """ - lines = [] if not nic.staticIpv6: - return lines + return ([], []) + subnet_list = [] # Static Ipv6 addrs = nic.staticIpv6 - lines.append('iface %s inet6 static' % name) - lines.append(' address %s' % addrs[0].ip) - lines.append(' netmask %s' % addrs[0].netmask) - for addr in addrs[1:]: - lines.append(' up ifconfig %s inet6 add %s/%s' % (name, addr.ip, - addr.netmask)) - # Add the primary gateway - if nic.primary: - for addr in addrs: - if addr.gateway: - self.ipv6PrimaryGateway = addr.gateway - lines.append(' gateway %s' % self.ipv6PrimaryGateway) - return lines + for addr in addrs: + subnet = {'type': 'static6', + 'address': addr.ip, + 'netmask': addr.netmask} + subnet_list.append(subnet) - # Add routes if there is no primary nic - if not self._primaryNic: - lines.extend(self._genIpv6Route(name, nic, addrs)) + # TODO: Add the primary gateway + + route_list = [] + # TODO: Add routes if there is no primary nic + # if not self._primaryNic: + # route_list.extend(self._genIpv6Route(name, nic, addrs)) - return lines + return (subnet_list, route_list) def _genIpv6Route(self, name, nic, addrs): - lines = [] + route_list = [] for addr in addrs: - lines.append(' up route -A inet6 add default gw ' - '%s metric 10000' % addr.gateway) + route_list.append({'type': 'route', + 'gateway': addr.gateway, + 'metric': 10000}) + + return route_list - return lines + def generate(self, configure=False, osfamily=None): + """Return the config elements that are needed to configure the nics""" + if configure: + logger.info("Configuring the interfaces file") + self.configure(osfamily) - def generate(self): - """Return the lines that is needed to configure the nics""" - lines = [] - lines.append('iface lo inet loopback') - lines.append('auto lo') - lines.append('') + nics_cfg_list = [] for nic in self.nics: - lines.extend(self.gen_one_nic(nic)) + nics_cfg_list.extend(self.gen_one_nic(nic)) - return lines + return nics_cfg_list def clear_dhcp(self): logger.info('Clearing DHCP leases') @@ -201,11 +252,16 @@ class NicConfigurator(object): util.subp(["pkill", "dhclient"], rcs=[0, 1]) util.subp(["rm", "-f", "/var/lib/dhcp/*"]) - def configure(self): + def configure(self, osfamily=None): """ - Configure the /etc/network/intefaces + Configure the /etc/network/interfaces Make a back up of the original """ + + if not osfamily or osfamily != "debian": + logger.info("Debian OS not detected. Skipping the configure step") + return + containingDir = '/etc/network' interfaceFile = os.path.join(containingDir, 'interfaces') @@ -215,10 +271,13 @@ class NicConfigurator(object): if not os.path.exists(originalFile) and os.path.exists(interfaceFile): os.rename(interfaceFile, originalFile) - lines = self.generate() - with open(interfaceFile, 'w') as fp: - for line in lines: - fp.write('%s\n' % line) + lines = [ + "# DO NOT EDIT THIS FILE BY HAND --" + " AUTOMATICALLY GENERATED BY cloud-init", + "source /etc/network/interfaces.d/*.cfg", + ] + + util.write_file(interfaceFile, content='\n'.join(lines)) self.clear_dhcp() diff --git a/cloudinit/sources/helpers/vmware/imc/guestcust_util.py b/cloudinit/sources/helpers/vmware/imc/guestcust_util.py index 1ab6bd41..44075255 100644 --- a/cloudinit/sources/helpers/vmware/imc/guestcust_util.py +++ b/cloudinit/sources/helpers/vmware/imc/guestcust_util.py @@ -59,14 +59,16 @@ def set_customization_status(custstate, custerror, errormessage=None): return (out, err) -# This will read the file nics.txt in the specified directory -# and return the content -def get_nics_to_enable(dirpath): - if not dirpath: +def get_nics_to_enable(nicsfilepath): + """Reads the NICS from the specified file path and returns the content + + @param nicsfilepath: Absolute file path to the NICS.txt file. + """ + + if not nicsfilepath: return None NICS_SIZE = 1024 - nicsfilepath = os.path.join(dirpath, "nics.txt") if not os.path.exists(nicsfilepath): return None diff --git a/tests/unittests/test_vmware_config_file.py b/tests/unittests/test_vmware_config_file.py index d8651077..808d303a 100644 --- a/tests/unittests/test_vmware_config_file.py +++ b/tests/unittests/test_vmware_config_file.py @@ -8,9 +8,13 @@ import logging import sys +from cloudinit.sources.DataSourceOVF import get_network_config_from_conf +from cloudinit.sources.DataSourceOVF import read_vmware_imc from cloudinit.sources.helpers.vmware.imc.boot_proto import BootProtoEnum from cloudinit.sources.helpers.vmware.imc.config import Config from cloudinit.sources.helpers.vmware.imc.config_file import ConfigFile +from cloudinit.sources.helpers.vmware.imc.config_nic import gen_subnet +from cloudinit.sources.helpers.vmware.imc.config_nic import NicConfigurator from cloudinit.tests.helpers import CiTestCase logging.basicConfig(level=logging.DEBUG, stream=sys.stdout) @@ -20,6 +24,7 @@ logger = logging.getLogger(__name__) class TestVmwareConfigFile(CiTestCase): def test_utility_methods(self): + """Tests basic utility methods of ConfigFile class""" cf = ConfigFile("tests/data/vmware/cust-dhcp-2nic.cfg") cf.clear() @@ -43,7 +48,26 @@ class TestVmwareConfigFile(CiTestCase): self.assertFalse(cf.should_keep_current_value("BAR"), "keepBar") self.assertTrue(cf.should_remove_current_value("BAR"), "removeBar") + def test_datasource_instance_id(self): + """Tests instance id for the DatasourceOVF""" + cf = ConfigFile("tests/data/vmware/cust-dhcp-2nic.cfg") + + instance_id_prefix = 'iid-vmware-' + + conf = Config(cf) + + (md1, _, _) = read_vmware_imc(conf) + self.assertIn(instance_id_prefix, md1["instance-id"]) + self.assertEqual(len(md1["instance-id"]), len(instance_id_prefix) + 8) + + (md2, _, _) = read_vmware_imc(conf) + self.assertIn(instance_id_prefix, md2["instance-id"]) + self.assertEqual(len(md2["instance-id"]), len(instance_id_prefix) + 8) + + self.assertNotEqual(md1["instance-id"], md2["instance-id"]) + def test_configfile_static_2nics(self): + """Tests Config class for a configuration with two static NICs.""" cf = ConfigFile("tests/data/vmware/cust-static-2nic.cfg") conf = Config(cf) @@ -81,6 +105,7 @@ class TestVmwareConfigFile(CiTestCase): self.assertTrue(not nics[1].staticIpv6, "ipv61 dhcp") def test_config_file_dhcp_2nics(self): + """Tests Config class for a configuration with two DHCP NICs.""" cf = ConfigFile("tests/data/vmware/cust-dhcp-2nic.cfg") conf = Config(cf) @@ -117,5 +142,197 @@ class TestVmwareConfigFile(CiTestCase): conf = Config(cf) self.assertTrue(conf.reset_password, "reset password") + def test_get_config_nameservers(self): + """Tests DNS and nameserver settings in a configuration.""" + cf = ConfigFile("tests/data/vmware/cust-static-2nic.cfg") + + config = Config(cf) + + network_config = get_network_config_from_conf(config, False) + + self.assertEqual(1, network_config.get('version')) + + config_types = network_config.get('config') + name_servers = None + dns_suffixes = None + + for type in config_types: + if type.get('type') == 'nameserver': + name_servers = type.get('address') + dns_suffixes = type.get('search') + break + + self.assertEqual(['10.20.145.1', '10.20.145.2'], + name_servers, + "dns") + self.assertEqual(['eng.vmware.com', 'proxy.vmware.com'], + dns_suffixes, + "suffixes") + + def test_gen_subnet(self): + """Tests if gen_subnet properly calculates network subnet from + IPv4 address and netmask""" + ip_subnet_list = [['10.20.87.253', '255.255.252.0', '10.20.84.0'], + ['10.20.92.105', '255.255.252.0', '10.20.92.0'], + ['192.168.0.10', '255.255.0.0', '192.168.0.0']] + for entry in ip_subnet_list: + self.assertEqual(entry[2], gen_subnet(entry[0], entry[1]), + "Subnet for a specified ip and netmask") + + def test_get_config_dns_suffixes(self): + """Tests if get_network_config_from_conf properly + generates nameservers and dns settings from a + specified configuration""" + cf = ConfigFile("tests/data/vmware/cust-dhcp-2nic.cfg") + + config = Config(cf) + + network_config = get_network_config_from_conf(config, False) + + self.assertEqual(1, network_config.get('version')) + + config_types = network_config.get('config') + name_servers = None + dns_suffixes = None + + for type in config_types: + if type.get('type') == 'nameserver': + name_servers = type.get('address') + dns_suffixes = type.get('search') + break + + self.assertEqual([], + name_servers, + "dns") + self.assertEqual(['eng.vmware.com'], + dns_suffixes, + "suffixes") + + def test_get_nics_list_dhcp(self): + """Tests if NicConfigurator properly calculates network subnets + for a configuration with a list of DHCP NICs""" + cf = ConfigFile("tests/data/vmware/cust-dhcp-2nic.cfg") + + config = Config(cf) + + nicConfigurator = NicConfigurator(config.nics, False) + nics_cfg_list = nicConfigurator.generate() + + self.assertEqual(2, len(nics_cfg_list), "number of config elements") + + nic1 = {'name': 'NIC1'} + nic2 = {'name': 'NIC2'} + for cfg in nics_cfg_list: + if cfg.get('name') == nic1.get('name'): + nic1.update(cfg) + elif cfg.get('name') == nic2.get('name'): + nic2.update(cfg) + + self.assertEqual('physical', nic1.get('type'), 'type of NIC1') + self.assertEqual('NIC1', nic1.get('name'), 'name of NIC1') + self.assertEqual('00:50:56:a6:8c:08', nic1.get('mac_address'), + 'mac address of NIC1') + subnets = nic1.get('subnets') + self.assertEqual(1, len(subnets), 'number of subnets for NIC1') + subnet = subnets[0] + self.assertEqual('dhcp', subnet.get('type'), 'DHCP type for NIC1') + self.assertEqual('auto', subnet.get('control'), 'NIC1 Control type') + + self.assertEqual('physical', nic2.get('type'), 'type of NIC2') + self.assertEqual('NIC2', nic2.get('name'), 'name of NIC2') + self.assertEqual('00:50:56:a6:5a:de', nic2.get('mac_address'), + 'mac address of NIC2') + subnets = nic2.get('subnets') + self.assertEqual(1, len(subnets), 'number of subnets for NIC2') + subnet = subnets[0] + self.assertEqual('dhcp', subnet.get('type'), 'DHCP type for NIC2') + self.assertEqual('auto', subnet.get('control'), 'NIC2 Control type') + + def test_get_nics_list_static(self): + """Tests if NicConfigurator properly calculates network subnets + for a configuration with 2 static NICs""" + cf = ConfigFile("tests/data/vmware/cust-static-2nic.cfg") + + config = Config(cf) + + nicConfigurator = NicConfigurator(config.nics, False) + nics_cfg_list = nicConfigurator.generate() + + self.assertEqual(5, len(nics_cfg_list), "number of elements") + + nic1 = {'name': 'NIC1'} + nic2 = {'name': 'NIC2'} + route_list = [] + for cfg in nics_cfg_list: + cfg_type = cfg.get('type') + if cfg_type == 'physical': + if cfg.get('name') == nic1.get('name'): + nic1.update(cfg) + elif cfg.get('name') == nic2.get('name'): + nic2.update(cfg) + elif cfg_type == 'route': + route_list.append(cfg) + + self.assertEqual('physical', nic1.get('type'), 'type of NIC1') + self.assertEqual('NIC1', nic1.get('name'), 'name of NIC1') + self.assertEqual('00:50:56:a6:8c:08', nic1.get('mac_address'), + 'mac address of NIC1') + + subnets = nic1.get('subnets') + self.assertEqual(2, len(subnets), 'Number of subnets') + + static_subnet = [] + static6_subnet = [] + + for subnet in subnets: + subnet_type = subnet.get('type') + if subnet_type == 'static': + static_subnet.append(subnet) + elif subnet_type == 'static6': + static6_subnet.append(subnet) + else: + self.assertEqual(True, False, 'Unknown type') + + self.assertEqual(1, len(static_subnet), 'Number of static subnet') + self.assertEqual(1, len(static6_subnet), 'Number of static6 subnet') + + subnet = static_subnet[0] + self.assertEqual('10.20.87.154', subnet.get('address'), + 'IPv4 address of static subnet') + self.assertEqual('255.255.252.0', subnet.get('netmask'), + 'NetMask of static subnet') + self.assertEqual('auto', subnet.get('control'), + 'control for static subnet') + + subnet = static6_subnet[0] + self.assertEqual('fc00:10:20:87::154', subnet.get('address'), + 'IPv6 address of static subnet') + self.assertEqual('64', subnet.get('netmask'), + 'NetMask of static6 subnet') + + route_set = set(['10.20.87.253', '10.20.87.105', '192.168.0.10']) + for route in route_list: + self.assertEqual(10000, route.get('metric'), 'metric of route') + gateway = route.get('gateway') + if gateway in route_set: + route_set.discard(gateway) + else: + self.assertEqual(True, False, 'invalid gateway %s' % (gateway)) + + self.assertEqual('physical', nic2.get('type'), 'type of NIC2') + self.assertEqual('NIC2', nic2.get('name'), 'name of NIC2') + self.assertEqual('00:50:56:a6:ef:7d', nic2.get('mac_address'), + 'mac address of NIC2') + + subnets = nic2.get('subnets') + self.assertEqual(1, len(subnets), 'Number of subnets for NIC2') + + subnet = subnets[0] + self.assertEqual('static', subnet.get('type'), 'Subnet type') + self.assertEqual('192.168.6.102', subnet.get('address'), + 'Subnet address') + self.assertEqual('255.255.0.0', subnet.get('netmask'), + 'Subnet netmask') + # vi: ts=4 expandtab -- cgit v1.2.3 From 7629702ca0956fb26d27ee19ed99306f73421c66 Mon Sep 17 00:00:00 2001 From: Sankar Tanguturi Date: Mon, 11 Sep 2017 07:50:01 -0700 Subject: vmware: Enable nics before sending the SUCCESS event. The network devices should be enabled before sending the 'SUCCESS' event to the underlying hypervisor. --- cloudinit/sources/DataSourceOVF.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cloudinit/sources') diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py index aa5f798d..24b45d55 100644 --- a/cloudinit/sources/DataSourceOVF.py +++ b/cloudinit/sources/DataSourceOVF.py @@ -182,10 +182,10 @@ class DataSourceOVF(sources.DataSource): # TODO: Need to set the status to DONE only when the # customization is done successfully. + enable_nics(self._vmware_nics_to_enable) set_customization_status( GuestCustStateEnum.GUESTCUST_STATE_DONE, GuestCustErrorEnum.GUESTCUST_ERROR_SUCCESS) - enable_nics(self._vmware_nics_to_enable) else: np = {'iso': transport_iso9660, -- cgit v1.2.3 From da1db792b2721d94ef85df8c136e78012c49c6e5 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Fri, 15 Sep 2017 13:37:55 -0600 Subject: CloudStack: consider dhclient lease files named with a hyphen. A regression in 'get_latest_lease' made it ignore files starting with 'dhclient-' rather than just 'dhclient.'. The fix here is to allow those files to be considered. There is a lot more we could do here to better ensure that we pick the most recent lease, but this change fixes the regression. LP: #1717147 --- cloudinit/sources/DataSourceCloudStack.py | 34 +++++++--- tests/unittests/test_datasource/test_cloudstack.py | 79 +++++++++++++++++++++- 2 files changed, 100 insertions(+), 13 deletions(-) (limited to 'cloudinit/sources') diff --git a/cloudinit/sources/DataSourceCloudStack.py b/cloudinit/sources/DataSourceCloudStack.py index 0188d894..7e0f9bb8 100644 --- a/cloudinit/sources/DataSourceCloudStack.py +++ b/cloudinit/sources/DataSourceCloudStack.py @@ -187,22 +187,36 @@ def get_dhclient_d(): return None -def get_latest_lease(): +def get_latest_lease(lease_d=None): # find latest lease file - lease_d = get_dhclient_d() + if lease_d is None: + lease_d = get_dhclient_d() if not lease_d: return None lease_files = os.listdir(lease_d) latest_mtime = -1 latest_file = None - for file_name in lease_files: - if file_name.startswith("dhclient.") and \ - (file_name.endswith(".lease") or file_name.endswith(".leases")): - abs_path = os.path.join(lease_d, file_name) - mtime = os.path.getmtime(abs_path) - if mtime > latest_mtime: - latest_mtime = mtime - latest_file = abs_path + + # lease files are named inconsistently across distros. + # We assume that 'dhclient6' indicates ipv6 and ignore it. + # ubuntu: + # dhclient..leases, dhclient.leases, dhclient6.leases + # centos6: + # dhclient-.leases, dhclient6.leases + # centos7: ('--' is not a typo) + # dhclient--.lease, dhclient6.leases + for fname in lease_files: + if fname.startswith("dhclient6"): + # avoid files that start with dhclient6 assuming dhcpv6. + continue + if not (fname.endswith(".lease") or fname.endswith(".leases")): + continue + + abs_path = os.path.join(lease_d, fname) + mtime = os.path.getmtime(abs_path) + if mtime > latest_mtime: + latest_mtime = mtime + latest_file = abs_path return latest_file diff --git a/tests/unittests/test_datasource/test_cloudstack.py b/tests/unittests/test_datasource/test_cloudstack.py index 2dc90305..8e98e1bb 100644 --- a/tests/unittests/test_datasource/test_cloudstack.py +++ b/tests/unittests/test_datasource/test_cloudstack.py @@ -1,12 +1,17 @@ # This file is part of cloud-init. See LICENSE file for license information. from cloudinit import helpers -from cloudinit.sources.DataSourceCloudStack import DataSourceCloudStack +from cloudinit import util +from cloudinit.sources.DataSourceCloudStack import ( + DataSourceCloudStack, get_latest_lease) -from cloudinit.tests.helpers import TestCase, mock, ExitStack +from cloudinit.tests.helpers import CiTestCase, ExitStack, mock +import os +import time -class TestCloudStackPasswordFetching(TestCase): + +class TestCloudStackPasswordFetching(CiTestCase): def setUp(self): super(TestCloudStackPasswordFetching, self).setUp() @@ -89,4 +94,72 @@ class TestCloudStackPasswordFetching(TestCase): def test_password_not_saved_if_bad_request(self): self._check_password_not_saved_for('bad_request') + +class TestGetLatestLease(CiTestCase): + + def _populate_dir_list(self, bdir, files): + """populate_dir_list([(name, data), (name, data)]) + + writes files to bdir, and updates timestamps to ensure + that their mtime increases with each file.""" + + start = int(time.time()) + for num, fname in enumerate(reversed(files)): + fpath = os.path.sep.join((bdir, fname)) + util.write_file(fpath, fname.encode()) + os.utime(fpath, (start - num, start - num)) + + def _pop_and_test(self, files, expected): + lease_d = self.tmp_dir() + self._populate_dir_list(lease_d, files) + self.assertEqual(self.tmp_path(expected, lease_d), + get_latest_lease(lease_d)) + + def test_skips_dhcpv6_files(self): + """files started with dhclient6 should be skipped.""" + expected = "dhclient.lease" + self._pop_and_test([expected, "dhclient6.lease"], expected) + + def test_selects_dhclient_dot_files(self): + """files named dhclient.lease or dhclient.leases should be used. + + Ubuntu names files dhclient.eth0.leases dhclient6.leases and + sometimes dhclient.leases.""" + self._pop_and_test(["dhclient.lease"], "dhclient.lease") + self._pop_and_test(["dhclient.leases"], "dhclient.leases") + + def test_selects_dhclient_dash_files(self): + """files named dhclient-lease or dhclient-leases should be used. + + Redhat/Centos names files with dhclient--eth0.lease (centos 7) or + dhclient-eth0.leases (centos 6). + """ + self._pop_and_test(["dhclient-eth0.lease"], "dhclient-eth0.lease") + self._pop_and_test(["dhclient--eth0.lease"], "dhclient--eth0.lease") + + def test_ignores_by_extension(self): + """only .lease or .leases file should be considered.""" + + self._pop_and_test(["dhclient.lease", "dhclient.lease.bk", + "dhclient.lease-old", "dhclient.leaselease"], + "dhclient.lease") + + def test_selects_newest_matching(self): + """If multiple files match, the newest written should be used.""" + lease_d = self.tmp_dir() + valid_1 = "dhclient.leases" + valid_2 = "dhclient.lease" + valid_1_path = self.tmp_path(valid_1, lease_d) + valid_2_path = self.tmp_path(valid_2, lease_d) + + self._populate_dir_list(lease_d, [valid_1, valid_2]) + self.assertEqual(valid_2_path, get_latest_lease(lease_d)) + + # now update mtime on valid_2 to be older than valid_1 and re-check. + mtime = int(os.path.getmtime(valid_1_path)) - 1 + os.utime(valid_2_path, (mtime, mtime)) + + self.assertEqual(valid_1_path, get_latest_lease(lease_d)) + + # vi: ts=4 expandtab -- cgit v1.2.3 From 10f067d87a2d48092c593862e686c517c57b987c Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Fri, 15 Sep 2017 22:50:21 -0400 Subject: GCE: Fix usage of user-data. This regressed in the rework of GCE datasource to have a main. The fix really just stores the user-data that was read in self.userdata_raw, rather than self.userdata. That is consistent with other datasources and ulitimately how it was before the refactor. The main is updated to address the fact that user-data is binary data and may not be able to be printed. LP: #1717598 --- cloudinit/sources/DataSourceGCE.py | 26 +++++++++++++++++++------- tests/unittests/test_datasource/test_gce.py | 3 ++- 2 files changed, 21 insertions(+), 8 deletions(-) (limited to 'cloudinit/sources') diff --git a/cloudinit/sources/DataSourceGCE.py b/cloudinit/sources/DataSourceGCE.py index 94484d60..ccae4200 100644 --- a/cloudinit/sources/DataSourceGCE.py +++ b/cloudinit/sources/DataSourceGCE.py @@ -62,7 +62,7 @@ class DataSourceGCE(sources.DataSource): LOG.debug(ret['reason']) return False self.metadata = ret['meta-data'] - self.userdata = ret['user-data'] + self.userdata_raw = ret['user-data'] return True @property @@ -80,9 +80,6 @@ class DataSourceGCE(sources.DataSource): # GCE has long FDQN's and has asked for short hostnames return self.metadata['local-hostname'].split('.')[0] - def get_userdata_raw(self): - return self.userdata - @property def availability_zone(self): return self.metadata['availability-zone'] @@ -202,6 +199,9 @@ def get_datasource_list(depends): if __name__ == "__main__": import argparse import json + import sys + + from base64 import b64encode parser = argparse.ArgumentParser(description='Query GCE Metadata Service') parser.add_argument("--endpoint", metavar="URL", @@ -211,8 +211,20 @@ if __name__ == "__main__": help="Ignore smbios platform check", action='store_false', default=True) args = parser.parse_args() - print(json.dumps( - read_md(address=args.endpoint, platform_check=args.platform_check), - indent=1, sort_keys=True, separators=(',', ': '))) + data = read_md(address=args.endpoint, platform_check=args.platform_check) + if 'user-data' in data: + # user-data is bytes not string like other things. Handle it specially. + # if it can be represented as utf-8 then do so. Otherwise print base64 + # encoded value in the key user-data-b64. + try: + data['user-data'] = data['user-data'].decode() + except UnicodeDecodeError: + sys.stderr.write("User-data cannot be decoded. " + "Writing as base64\n") + del data['user-data'] + # b64encode returns a bytes value. decode to get the string. + data['user-data-b64'] = b64encode(data['user-data']).decode() + + print(json.dumps(data, indent=1, sort_keys=True, separators=(',', ': '))) # vi: ts=4 expandtab diff --git a/tests/unittests/test_datasource/test_gce.py b/tests/unittests/test_datasource/test_gce.py index 50e49a10..d399ae7a 100644 --- a/tests/unittests/test_datasource/test_gce.py +++ b/tests/unittests/test_datasource/test_gce.py @@ -23,7 +23,8 @@ GCE_META = { 'instance/zone': 'foo/bar', 'project/attributes/sshKeys': 'user:ssh-rsa AA2..+aRD0fyVw== root@server', 'instance/hostname': 'server.project-foo.local', - 'instance/attributes/user-data': b'/bin/echo foo\n', + # UnicodeDecodeError below if set to ds.userdata instead of userdata_raw + 'instance/attributes/user-data': b'/bin/echo \xff\n', } GCE_META_PARTIAL = { -- cgit v1.2.3 From eaadf52b1010cf189bde2a6abb3265b890f6d36d Mon Sep 17 00:00:00 2001 From: Paul Meyer Date: Mon, 18 Sep 2017 16:07:46 -0600 Subject: Azure: wait longer for SSH pub keys to arrive. Currently the Azure data source waits up to 60 seconds. This has proven not to be sufficient to provide resiliency to unrelated transient failures in other parts of the infrastructure. Azure already has logic outside of the VM to abort hung provisioning. This changes lengthens the time out to 15 minutes. LP: #1717611 --- cloudinit/sources/DataSourceAzure.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'cloudinit/sources') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index b5a95a1f..80c2bd12 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -317,9 +317,13 @@ class DataSourceAzure(sources.DataSource): LOG.debug("ssh authentication: " "using fingerprint from fabirc") - missing = util.log_time(logfunc=LOG.debug, msg="waiting for files", + # wait very long for public SSH keys to arrive + # https://bugs.launchpad.net/cloud-init/+bug/1717611 + missing = util.log_time(logfunc=LOG.debug, + msg="waiting for SSH public key files", func=wait_for_files, - args=(fp_files,)) + args=(fp_files, 900)) + if len(missing): LOG.warning("Did not find files, but going on: %s", missing) @@ -656,7 +660,7 @@ def pubkeys_from_crt_files(flist): return pubkeys -def wait_for_files(flist, maxwait=60, naplen=.5, log_pre=""): +def wait_for_files(flist, maxwait, naplen=.5, log_pre=""): need = set(flist) waited = 0 while True: -- cgit v1.2.3 From da6562e21d0b17a0957adc0c5a2c9da076e0d219 Mon Sep 17 00:00:00 2001 From: Ryan Harper Date: Tue, 19 Sep 2017 11:10:09 -0500 Subject: DataSourceOVF: use util.find_devs_with(TYPE=iso9660) DataSourceOVF attempts to find iso files via walking os.listdir('/dev/') which is far too wide. This approach is too invasive and can sometimes race with systemd attempting to fsck and mount devices. Instead, utilize cloudinit.util.find_devs_with to filter devices by criteria (which uses blkid under the covers). This results in fewer attempts to mount block devices which do not contain iso filesystems. Unittest changes include: - cloudinit.tests.helpers; introduce add_patch() helper - Add unittest coverage for DataSourceOVF use of transport_iso9660 LP: #1718287 --- cloudinit/sources/DataSourceOVF.py | 74 ++++++++----- cloudinit/tests/helpers.py | 10 ++ tests/unittests/test_datasource/test_ovf.py | 164 ++++++++++++++++++++++++++++ 3 files changed, 221 insertions(+), 27 deletions(-) (limited to 'cloudinit/sources') diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py index 24b45d55..ccebf11a 100644 --- a/cloudinit/sources/DataSourceOVF.py +++ b/cloudinit/sources/DataSourceOVF.py @@ -375,26 +375,56 @@ def get_ovf_env(dirname): return (None, False) -# Transport functions take no input and return -# a 3 tuple of content, path, filename -def transport_iso9660(require_iso=True): +def maybe_cdrom_device(devname): + """Test if devname matches known list of devices which may contain iso9660 + filesystems. - # default_regex matches values in - # /lib/udev/rules.d/60-cdrom_id.rules - # KERNEL!="sr[0-9]*|hd[a-z]|xvd*", GOTO="cdrom_end" - envname = "CLOUD_INIT_CDROM_DEV_REGEX" - default_regex = "^(sr[0-9]+|hd[a-z]|xvd.*)" + Be helpful in accepting either knames (with no leading /dev/) or full path + names, but do not allow paths outside of /dev/, like /dev/foo/bar/xxx. + """ + if not devname: + return False + elif not isinstance(devname, util.string_types): + raise ValueError("Unexpected input for devname: %s" % devname) + + # resolve '..' and multi '/' elements + devname = os.path.normpath(devname) - devname_regex = os.environ.get(envname, default_regex) + # drop leading '/dev/' + if devname.startswith("/dev/"): + # partition returns tuple (before, partition, after) + devname = devname.partition("/dev/")[-1] + + # ignore leading slash (/sr0), else fail on / in name (foo/bar/xvdc) + if devname.startswith("/"): + devname = devname.split("/")[-1] + elif devname.count("/") > 0: + return False + + # if empty string + if not devname: + return False + + # default_regex matches values in /lib/udev/rules.d/60-cdrom_id.rules + # KERNEL!="sr[0-9]*|hd[a-z]|xvd*", GOTO="cdrom_end" + default_regex = r"^(sr[0-9]+|hd[a-z]|xvd.*)" + devname_regex = os.environ.get("CLOUD_INIT_CDROM_DEV_REGEX", default_regex) cdmatch = re.compile(devname_regex) + return cdmatch.match(devname) is not None + + +# Transport functions take no input and return +# a 3 tuple of content, path, filename +def transport_iso9660(require_iso=True): + # Go through mounts to see if it was already mounted mounts = util.mounts() for (dev, info) in mounts.items(): fstype = info['fstype'] if fstype != "iso9660" and require_iso: continue - if cdmatch.match(dev[5:]) is None: # take off '/dev/' + if not maybe_cdrom_device(dev): continue mp = info['mountpoint'] (fname, contents) = get_ovf_env(mp) @@ -406,29 +436,19 @@ def transport_iso9660(require_iso=True): else: mtype = None - devs = os.listdir("/dev/") - devs.sort() + # generate a list of devices with mtype filesystem, filter by regex + devs = [dev for dev in + util.find_devs_with("TYPE=%s" % mtype if mtype else None) + if maybe_cdrom_device(dev)] for dev in devs: - fullp = os.path.join("/dev/", dev) - - if (fullp in mounts or - not cdmatch.match(dev) or os.path.isdir(fullp)): - continue - - try: - # See if we can read anything at all...?? - util.peek_file(fullp, 512) - except IOError: - continue - try: - (fname, contents) = util.mount_cb(fullp, get_ovf_env, mtype=mtype) + (fname, contents) = util.mount_cb(dev, get_ovf_env, mtype=mtype) except util.MountFailedError: - LOG.debug("%s not mountable as iso9660", fullp) + LOG.debug("%s not mountable as iso9660", dev) continue if contents is not False: - return (contents, fullp, fname) + return (contents, dev, fname) return (False, None, None) diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py index 28e26622..6f88a5b7 100644 --- a/cloudinit/tests/helpers.py +++ b/cloudinit/tests/helpers.py @@ -104,6 +104,16 @@ class TestCase(unittest2.TestCase): super(TestCase, self).setUp() self.reset_global_state() + def add_patch(self, target, attr, **kwargs): + """Patches specified target object and sets it as attr on test + instance also schedules cleanup""" + if 'autospec' not in kwargs: + kwargs['autospec'] = True + m = mock.patch(target, **kwargs) + p = m.start() + self.addCleanup(m.stop) + setattr(self, attr, p) + class CiTestCase(TestCase): """This is the preferred test case base class unless user diff --git a/tests/unittests/test_datasource/test_ovf.py b/tests/unittests/test_datasource/test_ovf.py index 9dbf4dd9..700da86c 100644 --- a/tests/unittests/test_datasource/test_ovf.py +++ b/tests/unittests/test_datasource/test_ovf.py @@ -5,6 +5,7 @@ # This file is part of cloud-init. See LICENSE file for license information. import base64 +from collections import OrderedDict from cloudinit.tests import helpers as test_helpers @@ -70,4 +71,167 @@ class TestReadOvfEnv(test_helpers.TestCase): self.assertEqual({'password': "passw0rd"}, cfg) self.assertIsNone(ud) + +class TestTransportIso9660(test_helpers.CiTestCase): + + def setUp(self): + super(TestTransportIso9660, self).setUp() + self.add_patch('cloudinit.util.find_devs_with', + 'm_find_devs_with') + self.add_patch('cloudinit.util.mounts', 'm_mounts') + self.add_patch('cloudinit.util.mount_cb', 'm_mount_cb') + self.add_patch('cloudinit.sources.DataSourceOVF.get_ovf_env', + 'm_get_ovf_env') + self.m_get_ovf_env.return_value = ('myfile', 'mycontent') + + def test_find_already_mounted(self): + """Check we call get_ovf_env from on matching mounted devices""" + mounts = { + '/dev/sr9': { + 'fstype': 'iso9660', + 'mountpoint': 'wark/media/sr9', + 'opts': 'ro', + } + } + self.m_mounts.return_value = mounts + + (contents, fullp, fname) = dsovf.transport_iso9660() + self.assertEqual("mycontent", contents) + self.assertEqual("/dev/sr9", fullp) + self.assertEqual("myfile", fname) + + def test_find_already_mounted_skips_non_iso9660(self): + """Check we call get_ovf_env ignoring non iso9660""" + mounts = { + '/dev/xvdb': { + 'fstype': 'vfat', + 'mountpoint': 'wark/foobar', + 'opts': 'defaults,noatime', + }, + '/dev/xvdc': { + 'fstype': 'iso9660', + 'mountpoint': 'wark/media/sr9', + 'opts': 'ro', + } + } + # We use an OrderedDict here to ensure we check xvdb before xvdc + # as we're not mocking the regex matching, however, if we place + # an entry in the results then we can be reasonably sure that + # we're skipping an entry which fails to match. + self.m_mounts.return_value = ( + OrderedDict(sorted(mounts.items(), key=lambda t: t[0]))) + + (contents, fullp, fname) = dsovf.transport_iso9660() + self.assertEqual("mycontent", contents) + self.assertEqual("/dev/xvdc", fullp) + self.assertEqual("myfile", fname) + + def test_find_already_mounted_matches_kname(self): + """Check we dont regex match on basename of the device""" + mounts = { + '/dev/foo/bar/xvdc': { + 'fstype': 'iso9660', + 'mountpoint': 'wark/media/sr9', + 'opts': 'ro', + } + } + # we're skipping an entry which fails to match. + self.m_mounts.return_value = mounts + + (contents, fullp, fname) = dsovf.transport_iso9660() + self.assertEqual(False, contents) + self.assertIsNone(fullp) + self.assertIsNone(fname) + + def test_mount_cb_called_on_blkdevs_with_iso9660(self): + """Check we call mount_cb on blockdevs with iso9660 only""" + self.m_mounts.return_value = {} + self.m_find_devs_with.return_value = ['/dev/sr0'] + self.m_mount_cb.return_value = ("myfile", "mycontent") + + (contents, fullp, fname) = dsovf.transport_iso9660() + + self.m_mount_cb.assert_called_with( + "/dev/sr0", dsovf.get_ovf_env, mtype="iso9660") + self.assertEqual("mycontent", contents) + self.assertEqual("/dev/sr0", fullp) + self.assertEqual("myfile", fname) + + def test_mount_cb_called_on_blkdevs_with_iso9660_check_regex(self): + """Check we call mount_cb on blockdevs with iso9660 and match regex""" + self.m_mounts.return_value = {} + self.m_find_devs_with.return_value = [ + '/dev/abc', '/dev/my-cdrom', '/dev/sr0'] + self.m_mount_cb.return_value = ("myfile", "mycontent") + + (contents, fullp, fname) = dsovf.transport_iso9660() + + self.m_mount_cb.assert_called_with( + "/dev/sr0", dsovf.get_ovf_env, mtype="iso9660") + self.assertEqual("mycontent", contents) + self.assertEqual("/dev/sr0", fullp) + self.assertEqual("myfile", fname) + + def test_mount_cb_not_called_no_matches(self): + """Check we don't call mount_cb if nothing matches""" + self.m_mounts.return_value = {} + self.m_find_devs_with.return_value = ['/dev/vg/myovf'] + + (contents, fullp, fname) = dsovf.transport_iso9660() + + self.assertEqual(0, self.m_mount_cb.call_count) + self.assertEqual(False, contents) + self.assertIsNone(fullp) + self.assertIsNone(fname) + + def test_mount_cb_called_require_iso_false(self): + """Check we call mount_cb on blockdevs with require_iso=False""" + self.m_mounts.return_value = {} + self.m_find_devs_with.return_value = ['/dev/xvdz'] + self.m_mount_cb.return_value = ("myfile", "mycontent") + + (contents, fullp, fname) = dsovf.transport_iso9660(require_iso=False) + + self.m_mount_cb.assert_called_with( + "/dev/xvdz", dsovf.get_ovf_env, mtype=None) + self.assertEqual("mycontent", contents) + self.assertEqual("/dev/xvdz", fullp) + self.assertEqual("myfile", fname) + + def test_maybe_cdrom_device_none(self): + """Test maybe_cdrom_device returns False for none/empty input""" + self.assertFalse(dsovf.maybe_cdrom_device(None)) + self.assertFalse(dsovf.maybe_cdrom_device('')) + + def test_maybe_cdrom_device_non_string_exception(self): + """Test maybe_cdrom_device raises ValueError on non-string types""" + with self.assertRaises(ValueError): + dsovf.maybe_cdrom_device({'a': 'eleven'}) + + def test_maybe_cdrom_device_false_on_multi_dir_paths(self): + """Test maybe_cdrom_device is false on /dev[/.*]/* paths""" + self.assertFalse(dsovf.maybe_cdrom_device('/dev/foo/sr0')) + self.assertFalse(dsovf.maybe_cdrom_device('foo/sr0')) + self.assertFalse(dsovf.maybe_cdrom_device('../foo/sr0')) + self.assertFalse(dsovf.maybe_cdrom_device('../foo/sr0')) + + def test_maybe_cdrom_device_true_on_hd_partitions(self): + """Test maybe_cdrom_device is false on /dev/hd[a-z][0-9]+ paths""" + self.assertTrue(dsovf.maybe_cdrom_device('/dev/hda1')) + self.assertTrue(dsovf.maybe_cdrom_device('hdz9')) + + def test_maybe_cdrom_device_true_on_valid_relative_paths(self): + """Test maybe_cdrom_device normalizes paths""" + self.assertTrue(dsovf.maybe_cdrom_device('/dev/wark/../sr9')) + self.assertTrue(dsovf.maybe_cdrom_device('///sr0')) + self.assertTrue(dsovf.maybe_cdrom_device('/sr0')) + self.assertTrue(dsovf.maybe_cdrom_device('//dev//hda')) + + def test_maybe_cdrom_device_true_on_xvd_partitions(self): + """Test maybe_cdrom_device returns true on xvd*""" + self.assertTrue(dsovf.maybe_cdrom_device('/dev/xvda')) + self.assertTrue(dsovf.maybe_cdrom_device('/dev/xvda1')) + self.assertTrue(dsovf.maybe_cdrom_device('xvdza1')) + +# # vi: ts=4 expandtab -- cgit v1.2.3 From ad099a53d120e88719a5ad50f29d22e9f7a52bc7 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Mon, 25 Sep 2017 14:29:13 -0400 Subject: AltCloud: Trust PATH for udevadm and modprobe. Previously we had hard coded paths in /sbin for the udevadm and modprobe programs invoked by AltCloud. Its more flexible to expect the PATH to be set correctly. Debian: #852564 --- cloudinit/sources/DataSourceAltCloud.py | 4 ++-- tests/unittests/test_datasource/test_altcloud.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'cloudinit/sources') diff --git a/cloudinit/sources/DataSourceAltCloud.py b/cloudinit/sources/DataSourceAltCloud.py index ed1d691a..c78ad9eb 100644 --- a/cloudinit/sources/DataSourceAltCloud.py +++ b/cloudinit/sources/DataSourceAltCloud.py @@ -28,8 +28,8 @@ LOG = logging.getLogger(__name__) CLOUD_INFO_FILE = '/etc/sysconfig/cloud-info' # Shell command lists -CMD_PROBE_FLOPPY = ['/sbin/modprobe', 'floppy'] -CMD_UDEVADM_SETTLE = ['/sbin/udevadm', 'settle', '--timeout=5'] +CMD_PROBE_FLOPPY = ['modprobe', 'floppy'] +CMD_UDEVADM_SETTLE = ['udevadm', 'settle', '--timeout=5'] META_DATA_NOT_SUPPORTED = { 'block-device-mapping': {}, diff --git a/tests/unittests/test_datasource/test_altcloud.py b/tests/unittests/test_datasource/test_altcloud.py index 3b274d90..a4dfb540 100644 --- a/tests/unittests/test_datasource/test_altcloud.py +++ b/tests/unittests/test_datasource/test_altcloud.py @@ -280,8 +280,8 @@ class TestUserDataRhevm(TestCase): pass dsac.CLOUD_INFO_FILE = '/etc/sysconfig/cloud-info' - dsac.CMD_PROBE_FLOPPY = ['/sbin/modprobe', 'floppy'] - dsac.CMD_UDEVADM_SETTLE = ['/sbin/udevadm', 'settle', + dsac.CMD_PROBE_FLOPPY = ['modprobe', 'floppy'] + dsac.CMD_UDEVADM_SETTLE = ['udevadm', 'settle', '--quiet', '--timeout=5'] def test_mount_cb_fails(self): -- cgit v1.2.3 From 9d2a87dc386b7aed1a8243d599676e78ed358749 Mon Sep 17 00:00:00 2001 From: Dimitri John Ledkov Date: Sat, 30 Sep 2017 00:00:18 -0400 Subject: Azure, CloudStack: Support reading dhcp options from systemd-networkd. Systems that used systemd-networkd's dhcp client would not be able to get information on the Azure endpoint (placed in Option 245) or the CloudStack server (in 'server_address'). The change here supports reading these files in /run/systemd/netif/leases. The files declare that "This is private data. Do not parse.", but at this point we do not have another option. LP: #1718029 --- cloudinit/net/dhcp.py | 42 ++++++ cloudinit/net/tests/test_dhcp.py | 113 +++++++++++++++- cloudinit/sources/DataSourceCloudStack.py | 17 ++- cloudinit/sources/helpers/azure.py | 20 ++- .../unittests/test_datasource/test_azure_helper.py | 143 ++++++++++++++------- tests/unittests/test_datasource/test_cloudstack.py | 11 +- 6 files changed, 282 insertions(+), 64 deletions(-) (limited to 'cloudinit/sources') diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py index 05350639..0cba7032 100644 --- a/cloudinit/net/dhcp.py +++ b/cloudinit/net/dhcp.py @@ -4,6 +4,7 @@ # # This file is part of cloud-init. See LICENSE file for license information. +import configobj import logging import os import re @@ -11,9 +12,12 @@ import re from cloudinit.net import find_fallback_nic, get_devicelist from cloudinit import temp_utils from cloudinit import util +from six import StringIO LOG = logging.getLogger(__name__) +NETWORKD_LEASES_DIR = '/run/systemd/netif/leases' + class InvalidDHCPLeaseFileError(Exception): """Raised when parsing an empty or invalid dhcp.leases file. @@ -118,4 +122,42 @@ def dhcp_discovery(dhclient_cmd_path, interface, cleandir): return parse_dhcp_lease_file(lease_file) +def networkd_parse_lease(content): + """Parse a systemd lease file content as in /run/systemd/netif/leases/ + + Parse this (almost) ini style file even though it says: + # This is private data. Do not parse. + + Simply return a dictionary of key/values.""" + + return dict(configobj.ConfigObj(StringIO(content), list_values=False)) + + +def networkd_load_leases(leases_d=None): + """Return a dictionary of dictionaries representing each lease + found in lease_d.i + + The top level key will be the filename, which is typically the ifindex.""" + + if leases_d is None: + leases_d = NETWORKD_LEASES_DIR + + ret = {} + if not os.path.isdir(leases_d): + return ret + for lfile in os.listdir(leases_d): + ret[lfile] = networkd_parse_lease( + util.load_file(os.path.join(leases_d, lfile))) + return ret + + +def networkd_get_option_from_leases(keyname, leases_d=None): + if leases_d is None: + leases_d = NETWORKD_LEASES_DIR + leases = networkd_load_leases(leases_d=leases_d) + for ifindex, data in sorted(leases.items()): + if data.get(keyname): + return data[keyname] + return None + # vi: ts=4 expandtab diff --git a/cloudinit/net/tests/test_dhcp.py b/cloudinit/net/tests/test_dhcp.py index a38edaec..1c1f504a 100644 --- a/cloudinit/net/tests/test_dhcp.py +++ b/cloudinit/net/tests/test_dhcp.py @@ -6,9 +6,9 @@ from textwrap import dedent from cloudinit.net.dhcp import ( InvalidDHCPLeaseFileError, maybe_perform_dhcp_discovery, - parse_dhcp_lease_file, dhcp_discovery) + parse_dhcp_lease_file, dhcp_discovery, networkd_load_leases) from cloudinit.util import ensure_file, write_file -from cloudinit.tests.helpers import CiTestCase, wrap_and_call +from cloudinit.tests.helpers import CiTestCase, wrap_and_call, populate_dir class TestParseDHCPLeasesFile(CiTestCase): @@ -149,3 +149,112 @@ class TestDHCPDiscoveryClean(CiTestCase): [os.path.join(tmpdir, 'dhclient'), '-1', '-v', '-lf', lease_file, '-pf', os.path.join(tmpdir, 'dhclient.pid'), 'eth9', '-sf', '/bin/true'], capture=True)]) + + +class TestSystemdParseLeases(CiTestCase): + + lxd_lease = dedent("""\ + # This is private data. Do not parse. + ADDRESS=10.75.205.242 + NETMASK=255.255.255.0 + ROUTER=10.75.205.1 + SERVER_ADDRESS=10.75.205.1 + NEXT_SERVER=10.75.205.1 + BROADCAST=10.75.205.255 + T1=1580 + T2=2930 + LIFETIME=3600 + DNS=10.75.205.1 + DOMAINNAME=lxd + HOSTNAME=a1 + CLIENTID=ffe617693400020000ab110c65a6a0866931c2 + """) + + lxd_parsed = { + 'ADDRESS': '10.75.205.242', + 'NETMASK': '255.255.255.0', + 'ROUTER': '10.75.205.1', + 'SERVER_ADDRESS': '10.75.205.1', + 'NEXT_SERVER': '10.75.205.1', + 'BROADCAST': '10.75.205.255', + 'T1': '1580', + 'T2': '2930', + 'LIFETIME': '3600', + 'DNS': '10.75.205.1', + 'DOMAINNAME': 'lxd', + 'HOSTNAME': 'a1', + 'CLIENTID': 'ffe617693400020000ab110c65a6a0866931c2', + } + + azure_lease = dedent("""\ + # This is private data. Do not parse. + ADDRESS=10.132.0.5 + NETMASK=255.255.255.255 + ROUTER=10.132.0.1 + SERVER_ADDRESS=169.254.169.254 + NEXT_SERVER=10.132.0.1 + MTU=1460 + T1=43200 + T2=75600 + LIFETIME=86400 + DNS=169.254.169.254 + NTP=169.254.169.254 + DOMAINNAME=c.ubuntu-foundations.internal + DOMAIN_SEARCH_LIST=c.ubuntu-foundations.internal google.internal + HOSTNAME=tribaal-test-171002-1349.c.ubuntu-foundations.internal + ROUTES=10.132.0.1/32,0.0.0.0 0.0.0.0/0,10.132.0.1 + CLIENTID=ff405663a200020000ab11332859494d7a8b4c + OPTION_245=624c3620 + """) + + azure_parsed = { + 'ADDRESS': '10.132.0.5', + 'NETMASK': '255.255.255.255', + 'ROUTER': '10.132.0.1', + 'SERVER_ADDRESS': '169.254.169.254', + 'NEXT_SERVER': '10.132.0.1', + 'MTU': '1460', + 'T1': '43200', + 'T2': '75600', + 'LIFETIME': '86400', + 'DNS': '169.254.169.254', + 'NTP': '169.254.169.254', + 'DOMAINNAME': 'c.ubuntu-foundations.internal', + 'DOMAIN_SEARCH_LIST': 'c.ubuntu-foundations.internal google.internal', + 'HOSTNAME': 'tribaal-test-171002-1349.c.ubuntu-foundations.internal', + 'ROUTES': '10.132.0.1/32,0.0.0.0 0.0.0.0/0,10.132.0.1', + 'CLIENTID': 'ff405663a200020000ab11332859494d7a8b4c', + 'OPTION_245': '624c3620'} + + def setUp(self): + super(TestSystemdParseLeases, self).setUp() + self.lease_d = self.tmp_dir() + + def test_no_leases_returns_empty_dict(self): + """A leases dir with no lease files should return empty dictionary.""" + self.assertEqual({}, networkd_load_leases(self.lease_d)) + + def test_no_leases_dir_returns_empty_dict(self): + """A non-existing leases dir should return empty dict.""" + enodir = os.path.join(self.lease_d, 'does-not-exist') + self.assertEqual({}, networkd_load_leases(enodir)) + + def test_single_leases_file(self): + """A leases dir with one leases file.""" + populate_dir(self.lease_d, {'2': self.lxd_lease}) + self.assertEqual( + {'2': self.lxd_parsed}, networkd_load_leases(self.lease_d)) + + def test_single_azure_leases_file(self): + """On Azure, option 245 should be present, verify it specifically.""" + populate_dir(self.lease_d, {'1': self.azure_lease}) + self.assertEqual( + {'1': self.azure_parsed}, networkd_load_leases(self.lease_d)) + + def test_multiple_files(self): + """Multiple leases files on azure with one found return that value.""" + self.maxDiff = None + populate_dir(self.lease_d, {'1': self.azure_lease, + '9': self.lxd_lease}) + self.assertEqual({'1': self.azure_parsed, '9': self.lxd_parsed}, + networkd_load_leases(self.lease_d)) diff --git a/cloudinit/sources/DataSourceCloudStack.py b/cloudinit/sources/DataSourceCloudStack.py index 7e0f9bb8..9dc473fc 100644 --- a/cloudinit/sources/DataSourceCloudStack.py +++ b/cloudinit/sources/DataSourceCloudStack.py @@ -19,6 +19,7 @@ import time from cloudinit import ec2_utils as ec2 from cloudinit import log as logging +from cloudinit.net import dhcp from cloudinit import sources from cloudinit import url_helper as uhelp from cloudinit import util @@ -224,20 +225,28 @@ def get_vr_address(): # Get the address of the virtual router via dhcp leases # If no virtual router is detected, fallback on default gateway. # See http://docs.cloudstack.apache.org/projects/cloudstack-administration/en/4.8/virtual_machines/user-data.html # noqa + + # Try networkd first... + latest_address = dhcp.networkd_get_option_from_leases('SERVER_ADDRESS') + if latest_address: + LOG.debug("Found SERVER_ADDRESS '%s' via networkd_leases", + latest_address) + return latest_address + + # Try dhcp lease files next... lease_file = get_latest_lease() if not lease_file: LOG.debug("No lease file found, using default gateway") return get_default_gateway() - latest_address = None with open(lease_file, "r") as fd: for line in fd: if "dhcp-server-identifier" in line: words = line.strip(" ;\r\n").split(" ") if len(words) > 2: - dhcp = words[2] - LOG.debug("Found DHCP identifier %s", dhcp) - latest_address = dhcp + dhcptok = words[2] + LOG.debug("Found DHCP identifier %s", dhcptok) + latest_address = dhcptok if not latest_address: # No virtual router found, fallback on default gateway LOG.debug("No DHCP found, using default gateway") diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py index 28ed0ae2..959b1bda 100644 --- a/cloudinit/sources/helpers/azure.py +++ b/cloudinit/sources/helpers/azure.py @@ -8,6 +8,7 @@ import socket import struct import time +from cloudinit.net import dhcp from cloudinit import stages from cloudinit import temp_utils from contextlib import contextmanager @@ -15,7 +16,6 @@ from xml.etree import ElementTree from cloudinit import util - LOG = logging.getLogger(__name__) @@ -238,6 +238,11 @@ class WALinuxAgentShim(object): packed_bytes = unescaped_value.encode('utf-8') return socket.inet_ntoa(packed_bytes) + @staticmethod + def _networkd_get_value_from_leases(leases_d=None): + return dhcp.networkd_get_option_from_leases( + 'OPTION_245', leases_d=leases_d) + @staticmethod def _get_value_from_leases_file(fallback_lease_file): leases = [] @@ -287,12 +292,15 @@ class WALinuxAgentShim(object): @staticmethod def find_endpoint(fallback_lease_file=None): - LOG.debug('Finding Azure endpoint...') value = None - # Option-245 stored in /run/cloud-init/dhclient.hooks/.json - # a dhclient exit hook that calls cloud-init-dhclient-hook - dhcp_options = WALinuxAgentShim._load_dhclient_json() - value = WALinuxAgentShim._get_value_from_dhcpoptions(dhcp_options) + LOG.debug('Finding Azure endpoint from networkd...') + value = WALinuxAgentShim._networkd_get_value_from_leases() + if value is None: + # Option-245 stored in /run/cloud-init/dhclient.hooks/.json + # a dhclient exit hook that calls cloud-init-dhclient-hook + LOG.debug('Finding Azure endpoint from hook json...') + dhcp_options = WALinuxAgentShim._load_dhclient_json() + value = WALinuxAgentShim._get_value_from_dhcpoptions(dhcp_options) if value is None: # Fallback and check the leases file if unsuccessful LOG.debug("Unable to find endpoint in dhclient logs. " diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py index 44b99eca..b42b073f 100644 --- a/tests/unittests/test_datasource/test_azure_helper.py +++ b/tests/unittests/test_datasource/test_azure_helper.py @@ -1,10 +1,12 @@ # This file is part of cloud-init. See LICENSE file for license information. import os +from textwrap import dedent from cloudinit.sources.helpers import azure as azure_helper -from cloudinit.tests.helpers import ExitStack, mock, TestCase +from cloudinit.tests.helpers import CiTestCase, ExitStack, mock, populate_dir +from cloudinit.sources.helpers.azure import WALinuxAgentShim as wa_shim GOAL_STATE_TEMPLATE = """\ @@ -45,7 +47,7 @@ GOAL_STATE_TEMPLATE = """\ """ -class TestFindEndpoint(TestCase): +class TestFindEndpoint(CiTestCase): def setUp(self): super(TestFindEndpoint, self).setUp() @@ -56,18 +58,19 @@ class TestFindEndpoint(TestCase): mock.patch.object(azure_helper.util, 'load_file')) self.dhcp_options = patches.enter_context( - mock.patch.object(azure_helper.WALinuxAgentShim, - '_load_dhclient_json')) + mock.patch.object(wa_shim, '_load_dhclient_json')) + + self.networkd_leases = patches.enter_context( + mock.patch.object(wa_shim, '_networkd_get_value_from_leases')) + self.networkd_leases.return_value = None def test_missing_file(self): - self.assertRaises(ValueError, - azure_helper.WALinuxAgentShim.find_endpoint) + self.assertRaises(ValueError, wa_shim.find_endpoint) def test_missing_special_azure_line(self): self.load_file.return_value = '' self.dhcp_options.return_value = {'eth0': {'key': 'value'}} - self.assertRaises(ValueError, - azure_helper.WALinuxAgentShim.find_endpoint) + self.assertRaises(ValueError, wa_shim.find_endpoint) @staticmethod def _build_lease_content(encoded_address): @@ -80,8 +83,7 @@ class TestFindEndpoint(TestCase): def test_from_dhcp_client(self): self.dhcp_options.return_value = {"eth0": {"unknown_245": "5:4:3:2"}} - self.assertEqual('5.4.3.2', - azure_helper.WALinuxAgentShim.find_endpoint(None)) + self.assertEqual('5.4.3.2', wa_shim.find_endpoint(None)) def test_latest_lease_used(self): encoded_addresses = ['5:4:3:2', '4:3:2:1'] @@ -89,53 +91,38 @@ class TestFindEndpoint(TestCase): for encoded_address in encoded_addresses]) self.load_file.return_value = file_content self.assertEqual(encoded_addresses[-1].replace(':', '.'), - azure_helper.WALinuxAgentShim.find_endpoint("foobar")) + wa_shim.find_endpoint("foobar")) -class TestExtractIpAddressFromLeaseValue(TestCase): +class TestExtractIpAddressFromLeaseValue(CiTestCase): def test_hex_string(self): ip_address, encoded_address = '98.76.54.32', '62:4c:36:20' self.assertEqual( - ip_address, - azure_helper.WALinuxAgentShim.get_ip_from_lease_value( - encoded_address - )) + ip_address, wa_shim.get_ip_from_lease_value(encoded_address)) def test_hex_string_with_single_character_part(self): ip_address, encoded_address = '4.3.2.1', '4:3:2:1' self.assertEqual( - ip_address, - azure_helper.WALinuxAgentShim.get_ip_from_lease_value( - encoded_address - )) + ip_address, wa_shim.get_ip_from_lease_value(encoded_address)) def test_packed_string(self): ip_address, encoded_address = '98.76.54.32', 'bL6 ' self.assertEqual( - ip_address, - azure_helper.WALinuxAgentShim.get_ip_from_lease_value( - encoded_address - )) + ip_address, wa_shim.get_ip_from_lease_value(encoded_address)) def test_packed_string_with_escaped_quote(self): ip_address, encoded_address = '100.72.34.108', 'dH\\"l' self.assertEqual( - ip_address, - azure_helper.WALinuxAgentShim.get_ip_from_lease_value( - encoded_address - )) + ip_address, wa_shim.get_ip_from_lease_value(encoded_address)) def test_packed_string_containing_a_colon(self): ip_address, encoded_address = '100.72.58.108', 'dH:l' self.assertEqual( - ip_address, - azure_helper.WALinuxAgentShim.get_ip_from_lease_value( - encoded_address - )) + ip_address, wa_shim.get_ip_from_lease_value(encoded_address)) -class TestGoalStateParsing(TestCase): +class TestGoalStateParsing(CiTestCase): default_parameters = { 'incarnation': 1, @@ -195,7 +182,7 @@ class TestGoalStateParsing(TestCase): self.assertIsNone(certificates_xml) -class TestAzureEndpointHttpClient(TestCase): +class TestAzureEndpointHttpClient(CiTestCase): regular_headers = { 'x-ms-agent-name': 'WALinuxAgent', @@ -258,7 +245,7 @@ class TestAzureEndpointHttpClient(TestCase): self.read_file_or_url.call_args) -class TestOpenSSLManager(TestCase): +class TestOpenSSLManager(CiTestCase): def setUp(self): super(TestOpenSSLManager, self).setUp() @@ -300,7 +287,7 @@ class TestOpenSSLManager(TestCase): self.assertEqual([mock.call(manager.tmpdir)], del_dir.call_args_list) -class TestWALinuxAgentShim(TestCase): +class TestWALinuxAgentShim(CiTestCase): def setUp(self): super(TestWALinuxAgentShim, self).setUp() @@ -310,8 +297,7 @@ class TestWALinuxAgentShim(TestCase): self.AzureEndpointHttpClient = patches.enter_context( mock.patch.object(azure_helper, 'AzureEndpointHttpClient')) self.find_endpoint = patches.enter_context( - mock.patch.object( - azure_helper.WALinuxAgentShim, 'find_endpoint')) + mock.patch.object(wa_shim, 'find_endpoint')) self.GoalState = patches.enter_context( mock.patch.object(azure_helper, 'GoalState')) self.OpenSSLManager = patches.enter_context( @@ -320,7 +306,7 @@ class TestWALinuxAgentShim(TestCase): mock.patch.object(azure_helper.time, 'sleep', mock.MagicMock())) def test_http_client_uses_certificate(self): - shim = azure_helper.WALinuxAgentShim() + shim = wa_shim() shim.register_with_azure_and_fetch_data() self.assertEqual( [mock.call(self.OpenSSLManager.return_value.certificate)], @@ -328,7 +314,7 @@ class TestWALinuxAgentShim(TestCase): def test_correct_url_used_for_goalstate(self): self.find_endpoint.return_value = 'test_endpoint' - shim = azure_helper.WALinuxAgentShim() + shim = wa_shim() shim.register_with_azure_and_fetch_data() get = self.AzureEndpointHttpClient.return_value.get self.assertEqual( @@ -340,7 +326,7 @@ class TestWALinuxAgentShim(TestCase): self.GoalState.call_args_list) def test_certificates_used_to_determine_public_keys(self): - shim = azure_helper.WALinuxAgentShim() + shim = wa_shim() data = shim.register_with_azure_and_fetch_data() self.assertEqual( [mock.call(self.GoalState.return_value.certificates_xml)], @@ -351,13 +337,13 @@ class TestWALinuxAgentShim(TestCase): def test_absent_certificates_produces_empty_public_keys(self): self.GoalState.return_value.certificates_xml = None - shim = azure_helper.WALinuxAgentShim() + shim = wa_shim() data = shim.register_with_azure_and_fetch_data() self.assertEqual([], data['public-keys']) def test_correct_url_used_for_report_ready(self): self.find_endpoint.return_value = 'test_endpoint' - shim = azure_helper.WALinuxAgentShim() + shim = wa_shim() shim.register_with_azure_and_fetch_data() expected_url = 'http://test_endpoint/machine?comp=health' self.assertEqual( @@ -368,7 +354,7 @@ class TestWALinuxAgentShim(TestCase): self.GoalState.return_value.incarnation = 'TestIncarnation' self.GoalState.return_value.container_id = 'TestContainerId' self.GoalState.return_value.instance_id = 'TestInstanceId' - shim = azure_helper.WALinuxAgentShim() + shim = wa_shim() shim.register_with_azure_and_fetch_data() posted_document = ( self.AzureEndpointHttpClient.return_value.post.call_args[1]['data'] @@ -378,11 +364,11 @@ class TestWALinuxAgentShim(TestCase): self.assertIn('TestInstanceId', posted_document) def test_clean_up_can_be_called_at_any_time(self): - shim = azure_helper.WALinuxAgentShim() + shim = wa_shim() shim.clean_up() def test_clean_up_will_clean_up_openssl_manager_if_instantiated(self): - shim = azure_helper.WALinuxAgentShim() + shim = wa_shim() shim.register_with_azure_and_fetch_data() shim.clean_up() self.assertEqual( @@ -393,12 +379,12 @@ class TestWALinuxAgentShim(TestCase): pass self.AzureEndpointHttpClient.return_value.get.side_effect = ( SentinelException) - shim = azure_helper.WALinuxAgentShim() + shim = wa_shim() self.assertRaises(SentinelException, shim.register_with_azure_and_fetch_data) -class TestGetMetadataFromFabric(TestCase): +class TestGetMetadataFromFabric(CiTestCase): @mock.patch.object(azure_helper, 'WALinuxAgentShim') def test_data_from_shim_returned(self, shim): @@ -422,4 +408,65 @@ class TestGetMetadataFromFabric(TestCase): azure_helper.get_metadata_from_fabric) self.assertEqual(1, shim.return_value.clean_up.call_count) + +class TestExtractIpAddressFromNetworkd(CiTestCase): + + azure_lease = dedent("""\ + # This is private data. Do not parse. + ADDRESS=10.132.0.5 + NETMASK=255.255.255.255 + ROUTER=10.132.0.1 + SERVER_ADDRESS=169.254.169.254 + NEXT_SERVER=10.132.0.1 + MTU=1460 + T1=43200 + T2=75600 + LIFETIME=86400 + DNS=169.254.169.254 + NTP=169.254.169.254 + DOMAINNAME=c.ubuntu-foundations.internal + DOMAIN_SEARCH_LIST=c.ubuntu-foundations.internal google.internal + HOSTNAME=tribaal-test-171002-1349.c.ubuntu-foundations.internal + ROUTES=10.132.0.1/32,0.0.0.0 0.0.0.0/0,10.132.0.1 + CLIENTID=ff405663a200020000ab11332859494d7a8b4c + OPTION_245=624c3620 + """) + + def setUp(self): + super(TestExtractIpAddressFromNetworkd, self).setUp() + self.lease_d = self.tmp_dir() + + def test_no_valid_leases_is_none(self): + """No valid leases should return None.""" + self.assertIsNone( + wa_shim._networkd_get_value_from_leases(self.lease_d)) + + def test_option_245_is_found_in_single(self): + """A single valid lease with 245 option should return it.""" + populate_dir(self.lease_d, {'9': self.azure_lease}) + self.assertEqual( + '624c3620', wa_shim._networkd_get_value_from_leases(self.lease_d)) + + def test_option_245_not_found_returns_None(self): + """A valid lease, but no option 245 should return None.""" + populate_dir( + self.lease_d, + {'9': self.azure_lease.replace("OPTION_245", "OPTION_999")}) + self.assertIsNone( + wa_shim._networkd_get_value_from_leases(self.lease_d)) + + def test_multiple_returns_first(self): + """Somewhat arbitrarily return the first address when multiple. + + Most important at the moment is that this is consistent behavior + rather than changing randomly as in order of a dictionary.""" + myval = "624c3601" + populate_dir( + self.lease_d, + {'9': self.azure_lease, + '2': self.azure_lease.replace("624c3620", myval)}) + self.assertEqual( + myval, wa_shim._networkd_get_value_from_leases(self.lease_d)) + + # vi: ts=4 expandtab diff --git a/tests/unittests/test_datasource/test_cloudstack.py b/tests/unittests/test_datasource/test_cloudstack.py index 8e98e1bb..96144b64 100644 --- a/tests/unittests/test_datasource/test_cloudstack.py +++ b/tests/unittests/test_datasource/test_cloudstack.py @@ -23,13 +23,16 @@ class TestCloudStackPasswordFetching(CiTestCase): default_gw = "192.201.20.0" get_latest_lease = mock.MagicMock(return_value=None) self.patches.enter_context(mock.patch( - 'cloudinit.sources.DataSourceCloudStack.get_latest_lease', - get_latest_lease)) + mod_name + '.get_latest_lease', get_latest_lease)) get_default_gw = mock.MagicMock(return_value=default_gw) self.patches.enter_context(mock.patch( - 'cloudinit.sources.DataSourceCloudStack.get_default_gateway', - get_default_gw)) + mod_name + '.get_default_gateway', get_default_gw)) + + get_networkd_server_address = mock.MagicMock(return_value=None) + self.patches.enter_context(mock.patch( + mod_name + '.dhcp.networkd_get_option_from_leases', + get_networkd_server_address)) def _set_password_server_response(self, response_string): subp = mock.MagicMock(return_value=(response_string, '')) -- cgit v1.2.3