some inline comments, questions. Diff comments:
> diff --git a/cloudinit/sources/DataSourceIBMCloud.py > b/cloudinit/sources/DataSourceIBMCloud.py > new file mode 100644 > index 0000000..99ec4c2 > --- /dev/null > +++ b/cloudinit/sources/DataSourceIBMCloud.py > @@ -0,0 +1,275 @@ > +# This file is part of cloud-init. See LICENSE file for license information. > +"""Datasource for IBMCloud. > + > +IBMCloud is also know as SoftLayer or BlueMix. > +IBMCloud hypervisor is xen (2018-03-10). > + > +There are 2 different api exposed launch methods. > + * template: This is the legacy method of launching instances. > + When booting from an image template, the system boots first into > + a "provisioning" mode. There, host <-> guest mechanisms are utilized > + to execute code in the guest and provision it. > + > + Cloud-init will disables itself when it detects that it is in the > + provisioning mode. It detects this by the presence of > + a file '/root/provisioningConfiguration.cfg'. > + > + When provided with user-data, the "first boot" will contain a > + ConfigDrive-like disk labeled with 'METADATA'. If there is no user-data > + provided, then there is no data-source. > + > + Cloud-init never does any network configuration in this mode. > + > + * os_code: Essentially "launch by OS Code" (Operating System Code). > + This is a more modern approach. There is no specific "provisioning" boot. > + Instead, cloud-init does all the customization. With or without > + user-data provided, an OpenStack ConfigDrive is attached. > + > + This disk will have only a 'latest' version, and will have the UUID > + of 9796-932E. > + > +TODO: > + * is uuid (/sys/hypervisor/uuid) stable for life of an instance? > + it seems it is not the same as data's uuid in the os_code case > + but is in the template case. > + > +""" > +import os > + > +from cloudinit import log as logging > +from cloudinit import sources > +from cloudinit.sources.helpers import openstack > +from cloudinit import util > + > +LOG = logging.getLogger(__name__) > + > +IBM_CONFIG_UUID = "9796-932E" > + > + > +class Platforms(object): > + TEMPLATE_LIVE_METADATA = "Template/Live/Metadata" > + TEMPLATE_LIVE_NODATA = "UNABLE TO BE IDENTIFIED." > + TEMPLATE_PROVISIONING_METADATA = "Template/Provisioning/Metadata" > + TEMPLATE_PROVISIONING_NODATA = "Template/Provisioning/No-Metadata" > + OS_CODE = "OS-Code/Live" > + > + > +PROVISIONING = ( > + Platforms.TEMPLATE_PROVISIONING_METADATA, > + Platforms.TEMPLATE_PROVISIONING_NODATA) > + > + > +class DataSourceIBMCloud(sources.DataSource): > + > + dsname = 'IBMCloud' > + system_uuid = None did we want to initilize this to "unset" instead of None w.r.t restoring objects? > + > + def __init__(self, sys_cfg, distro, paths): > + super(DataSourceIBMCloud, self).__init__(sys_cfg, distro, paths) > + self.source = None > + self._network_config = None > + self.network_json = None > + self.platform = None > + > + def __str__(self): > + root = sources.DataSource.__str__(self) > + mstr = "%s [%s %s]" % (root, self.platform, self.source) > + return mstr > + > + def _get_data(self): > + results = read_md() > + if results is None: > + return False > + > + self.source = results['source'] > + self.platform = results['platform'] > + self.metadata = results['metadata'] > + self.userdata_raw = results.get('userdata') > + self.version = results['version'] > + self.network_json = results.get('networkdata') > + vd = results.get('vendordata') > + self.vendordata_pure = vd > + self.system_uuid = results['system-uuid'] > + try: > + self.vendordata_raw = sources.convert_vendordata(vd) > + except ValueError as e: > + LOG.warning("Invalid content in vendor-data: %s", e) > + self.vendordata_raw = None > + > + return True > + > + def check_instance_id(self, sys_cfg): > + """quickly (local check only) if self.instance_id is still valid > + > + in Template mode, the system uuid (/sys/hypervisor/uuid) is the > + same as found in the METADATA disk. But that is not true in OS_CODE > + mode. So we read the system_uuid and keep that for later compare.""" > + if self.system_uuid is None: > + return False > + return self.system_uuid == _read_system_uuid() > + > + @property > + def network_config(self): > + if self.platform != Platforms.OS_CODE: > + # If deployed from template, an agent in the provisioning > + # environment handles networking configuration. Not cloud-init. > + return {'config': 'disabled', 'version': 1} > + if self._network_config is None: > + if self.network_json is not None: > + LOG.debug("network config provided via network_json") > + self._network_config = openstack.convert_net_json( > + self.network_json, known_macs=None) > + else: > + LOG.debug("no network configuration available.") > + return self._network_config > + > + > +def _read_system_uuid(): > + uuid_path = "/sys/hypervisor/uuid" > + if not os.path.isfile(uuid_path): > + return None > + return util.load_file(uuid_path).strip().lower() > + > + > +def _is_xen(): > + return os.path.exists("/sys/hypervisor") > + > + > +def _is_ibm_provisioning(): > + return os.path.exists("/root/provisioningConfiguration.cfg") > + > + > +def get_ibm_platform(): > + """Return a tuple (Platform, path) > + > + If this is Not IBM cloud, then the return value is (None, None). > + An instance in provisioning mode is considered running on IBM cloud.""" > + label_mdata = "METADATA" > + label_cfg2 = "CONFIG-2" > + not_found = (None, None) > + > + if not _is_xen(): > + return not_found > + > + # fslabels contains only the first entry with a given label. > + fslabels = {} > + try: > + devs = util.blkid() > + except util.ProcessExecutionError as e: > + LOG.warning("Failed to run blkid: %s", e) > + return (None, None) > + > + for dev in sorted(devs.keys()): > + data = devs[dev] > + label = data.get("LABEL", "").upper() > + uuid = data.get("UUID", "").upper() > + if label not in (label_mdata, label_cfg2): > + continue > + if label in fslabels: > + LOG.warning("Duplicate fslabel '%s'. existing=%s current=%s", > + label, fslabels[label], data) > + continue > + if label == label_cfg2 and uuid != IBM_CONFIG_UUID: > + LOG.debug("Skipping %s with LABEL=%s due to uuid != %s: %s", > + dev, label, uuid, data) > + continue > + fslabels[label] = data > + > + metadata_path = fslabels.get(label_mdata, {}).get('DEVNAME') > + cfg2_path = fslabels.get(label_cfg2, {}).get('DEVNAME') > + > + if cfg2_path: > + return (Platforms.OS_CODE, cfg2_path) > + elif metadata_path: > + if _is_ibm_provisioning(): > + return (Platforms.TEMPLATE_PROVISIONING_METADATA, metadata_path) > + else: > + return (Platforms.TEMPLATE_LIVE_METADATA, metadata_path) > + elif _is_ibm_provisioning(): > + return (Platforms.TEMPLATE_PROVISIONING_NODATA, None) > + return not_found > + > + > +def read_md(): > + """Read data from IBM Cloud. > + > + @return: None if not running on IBM Cloud. > + dictionary with guaranteed fields: metadata, version > + and optional fields: userdata, vendordata, networkdata. > + Also includes the system uuid from /sys/hypervisor/uuid.""" > + platform, path = get_ibm_platform() > + if platform is None: > + LOG.debug("This is not an IBMCloud platform.") > + return None > + elif platform in PROVISIONING: > + LOG.debug("Cloud-init is disabled during provisioning: %s.", > + platform) > + return None > + > + ret = {'platform': platform, 'source': path} > + > + try: > + if os.path.isdir(path): > + results = read_config_drive(path) > + else: > + results = util.mount_cb(path, read_config_drive) > + except Exception as e: > + raise RuntimeError( > + "Failed mounting IBM config disk. platform=%s path=%s: %s" % > + (platform, path, e)) > + > + ret['system-uuid'] = _read_system_uuid() > + > + # rename public_keys so it can be read by base class get_public_ssh_keys > + md = results['metadata'] > + renames = (('public_keys', 'public-keys'), ('hostname', > 'local-hostname')) > + for mdname, newname in renames: > + if mdname in md: > + md[newname] = md[mdname] > + del md[mdname] > + for drop in ('name', 'uuid', 'files', 'network_config'): > + if drop in md: > + del md[drop] > + > + if results.get('userdata') == "": > + # the read_v2 incorrectly puts user-data in as ''. > + # this work around does cuase issue if user provided user-data as "". > + results['userdata'] = None > + > + # In order to avoid additional functionality being utilized from the > + # copied openstack interface we specifically pull out only some fields. > + # in the read_v2 response. Specifically avoided are > + # 'files', 'ec2-metadata', 'dsmode', 'network_config' > + # network_config is ENI formatted data that is legacy. It may be > + # present in a Template environment, but we assume the provision > + # stage handles that configuration. > + allowed = ('metadata', 'userdata', 'vendordata', 'version', > 'networkdata') > + ret.update(dict([(k, v) for k, v in results.items() if k in allowed])) > + return ret > + > + > +def read_config_drive(source_dir): > + reader = openstack.ConfigDriveReader(source_dir, versions=["latest"]) > + return reader.read_v2() > + > + > +# Used to match classes to dependencies > +datasources = [ > + (DataSourceIBMCloud, (sources.DEP_FILESYSTEM,)), > +] > + > + > +# Return a list of data sources that match this set of dependencies > +def get_datasource_list(depends): > + return sources.list_from_depends(depends, datasources) > + > + > +if __name__ == "__main__": > + import argparse > + > + parser = argparse.ArgumentParser(description='Query IBM Cloud Metadata') > + args = parser.parse_args() > + data = read_md() > + print(util.json_dumps(data)) > + > +# vi: ts=4 expandtab > diff --git a/cloudinit/util.py b/cloudinit/util.py > index cae8b19..7ae62cc 100644 > --- a/cloudinit/util.py > +++ b/cloudinit/util.py > @@ -1237,6 +1237,30 @@ def find_devs_with(criteria=None, oformat='device', > return entries > > > +def blkid(devs=None, disable_cache=False): Shouldn't we update util.find_devs_with() to make use of this? or some other refactoring since we now have to callers to blkid? > + if devs is None: > + devs = [] > + else: > + devs = list(devs) > + > + cmd = ['blkid', '-o', 'full'] > + if disable_cache: > + cmd.extend(['-c', '/dev/null']) > + cmd.extend(devs) > + > + # we have to decode with 'replace' as shelx.split (called by > + # load_shell_content) can't take bytes. So this is potentially > + # lossy of non-utf-8 chars in blkid output. > + out, _ = subp(cmd, capture=True, decode="replace") > + ret = {} > + for line in out.splitlines(): > + dev, _, data = line.partition(":") > + ret[dev] = load_shell_content(data) > + ret[dev]["DEVNAME"] = dev > + > + return ret > + > + > def peek_file(fname, max_bytes): > LOG.debug("Peeking at %s (max_bytes=%s)", fname, max_bytes) > with open(fname, 'rb') as ifh: > diff --git a/tests/unittests/test_datasource/test_ibmcloud.py > b/tests/unittests/test_datasource/test_ibmcloud.py > new file mode 100644 > index 0000000..f6f5419 > --- /dev/null > +++ b/tests/unittests/test_datasource/test_ibmcloud.py Don't we want to put this in cloudinit/sources/tests/test_ibmcloud.py ? > @@ -0,0 +1,258 @@ > +# This file is part of cloud-init. See LICENSE file for license information. > + > +from cloudinit.sources import DataSourceIBMCloud as ibm > +from cloudinit.tests import helpers as test_helpers > + > +import base64 > +import copy > +import json > +import mock > +from textwrap import dedent > + > +D_PATH = "cloudinit.sources.DataSourceIBMCloud." > + > + > +class TestIBMCloud(test_helpers.CiTestCase): > + """Test the datasource.""" > + def setUp(self): > + super(TestIBMCloud, self).setUp() > + pass > + > + > +@mock.patch(D_PATH + "_is_xen", return_value=True) > +@mock.patch(D_PATH + "_is_ibm_provisioning") > +@mock.patch(D_PATH + "util.blkid") > +class TestGetIBMPlatform(test_helpers.CiTestCase): > + """Test the get_ibm_platform helper.""" > + > + blkid_base = { > + "/dev/xvda1": { > + "DEVNAME": "/dev/xvda1", "LABEL": "cloudimg-bootfs", > + "TYPE": "ext3"}, > + "/dev/xvda2": { > + "DEVNAME": "/dev/xvda2", "LABEL": "cloudimg-rootfs", > + "TYPE": "ext4"}, > + } > + > + blkid_metadata_disk = { > + "/dev/xvdh1": { > + "DEVNAME": "/dev/xvdh1", "LABEL": "METADATA", "TYPE": "vfat", > + "SEC_TYPE": "msdos", "UUID": "681B-8C5D", > + "PARTUUID": "3d631e09-01"}, > + } > + > + blkid_oscode_disk = { > + "/dev/xvdh": { > + "DEVNAME": "/dev/xvdh", "LABEL": "config-2", "TYPE": "vfat", > + "SEC_TYPE": "msdos", "UUID": ibm.IBM_CONFIG_UUID} > + } > + > + def setUp(self): > + self.blkid_metadata = copy.deepcopy(self.blkid_base) aren't the copies only needed in the test-cases that modify the base config? You could defer the copy to those unittests > + self.blkid_metadata.update(copy.deepcopy(self.blkid_metadata_disk)) > + > + self.blkid_oscode = copy.deepcopy(self.blkid_base) > + self.blkid_oscode.update(copy.deepcopy(self.blkid_oscode_disk)) > + > + def test_id_template_live_metadata(self, m_blkid, m_is_prov, _m_xen): > + """identify TEMPLATE_LIVE_METADATA.""" > + m_blkid.return_value = self.blkid_metadata > + m_is_prov.return_value = False > + self.assertEqual( > + (ibm.Platforms.TEMPLATE_LIVE_METADATA, "/dev/xvdh1"), > + ibm.get_ibm_platform()) > + > + def test_id_template_prov_metadata(self, m_blkid, m_is_prov, _m_xen): > + """identify TEMPLATE_PROVISIONING_METADATA.""" > + m_blkid.return_value = self.blkid_metadata > + m_is_prov.return_value = True > + self.assertEqual( > + (ibm.Platforms.TEMPLATE_PROVISIONING_METADATA, "/dev/xvdh1"), > + ibm.get_ibm_platform()) > + > + def test_id_template_prov_nodata(self, m_blkid, m_is_prov, _m_xen): > + """identify TEMPLATE_PROVISIONING_NODATA.""" > + m_blkid.return_value = self.blkid_base > + m_is_prov.return_value = True > + self.assertEqual( > + (ibm.Platforms.TEMPLATE_PROVISIONING_NODATA, None), > + ibm.get_ibm_platform()) > + > + def test_id_os_code(self, m_blkid, m_is_prov, _m_xen): > + """Identify OS_CODE.""" > + m_blkid.return_value = self.blkid_oscode > + m_is_prov.return_value = False > + self.assertEqual((ibm.Platforms.OS_CODE, "/dev/xvdh"), > + ibm.get_ibm_platform()) > + > + def test_id_os_code_must_match_uuid(self, m_blkid, m_is_prov, _m_xen): > + """Test against false positive on openstack with non-ibm UUID.""" > + blkid = self.blkid_oscode > + blkid["/dev/xvdh"]["UUID"] = "9999-9999" > + m_blkid.return_value = blkid > + m_is_prov.return_value = False > + self.assertEqual((None, None), ibm.get_ibm_platform()) > + > + > +@mock.patch(D_PATH + "_read_system_uuid", return_value=None) > +@mock.patch(D_PATH + "get_ibm_platform") > +class TestReadMD(test_helpers.CiTestCase): > + """Test the read_datasource helper.""" > + > + template_md = { > + "files": [], > + "network_config": {"content_path": "/content/interfaces"}, > + "hostname": "ci-fond-ram", > + "name": "ci-fond-ram", > + "domain": "testing.ci.cloud-init.org", > + "meta": {"dsmode": "net"}, > + "uuid": "8e636730-9f5d-c4a5-327c-d7123c46e82f", > + "public_keys": {"1091307": "ssh-rsa AAAAB3NzaC1...Hw== ci-pubkey"}, > + } > + > + oscode_md = { > + "hostname": "ci-grand-gannet.testing.ci.cloud-init.org", > + "name": "ci-grand-gannet", > + "uuid": "2f266908-8e6c-4818-9b5c-42e9cc66a785", > + "random_seed": "bm90LXJhbmRvbQo=", > + "crypt_key": "ssh-rsa AAAAB3NzaC1yc2..n6z/", > + "configuration_token": "eyJhbGciOi..M3ZA", > + "public_keys": {"1091307": "ssh-rsa AAAAB3N..Hw== ci-pubkey"}, > + } > + > + content_interfaces = dedent("""\ > + auto lo > + iface lo inet loopback > + > + auto eth0 > + allow-hotplug eth0 > + iface eth0 inet static > + address 10.82.43.5 > + netmask 255.255.255.192 > + """) > + > + userdata = b"#!/bin/sh\necho hi mom\n" > + # meta.js file gets json encoded userdata as a list. > + meta_js = '["#!/bin/sh\necho hi mom\n"]' > + vendor_data = { > + "cloud-init": "#!/bin/bash\necho 'root:$6$5ab01p1m1' | chpasswd -e"} > + > + network_data = { > + "links": [ > + {"id": "interface_29402281", "name": "eth0", "mtu": None, > + "type": "phy", "ethernet_mac_address": "06:00:f1:bd:da:25"}, > + {"id": "interface_29402279", "name": "eth1", "mtu": None, > + "type": "phy", "ethernet_mac_address": "06:98:5e:d0:7f:86"} > + ], > + "networks": [ > + {"id": "network_109887563", "link": "interface_29402281", > + "type": "ipv4", "ip_address": "10.82.43.2", > + "netmask": "255.255.255.192", > + "routes": [ > + {"network": "10.0.0.0", "netmask": "255.0.0.0", > + "gateway": "10.82.43.1"}, > + {"network": "161.26.0.0", "netmask": "255.255.0.0", > + "gateway": "10.82.43.1"}]}, > + {"id": "network_109887551", "link": "interface_29402279", > + "type": "ipv4", "ip_address": "108.168.194.252", > + "netmask": "255.255.255.248", > + "routes": [ > + {"network": "0.0.0.0", "netmask": "0.0.0.0", > + "gateway": "108.168.194.249"}]} > + ], > + "services": [ > + {"type": "dns", "address": "10.0.80.11"}, > + {"type": "dns", "address": "10.0.80.12"} > + ], > + } > + > + sysuuid = '7f79ebf5-d791-43c3-a723-854e8389d59f' > + > + def test_provisioning_md(self, m_platform, m_sysuuid): > + m_platform.return_value = ( These tests need docstrings > + ibm.Platforms.TEMPLATE_PROVISIONING_METADATA, "/dev/xvdh") > + self.assertIsNone(ibm.read_md()) > + > + def test_provisioning_no_metadata(self, m_platform, m_sysuuid): > + m_platform.return_value = ( > + ibm.Platforms.TEMPLATE_PROVISIONING_NODATA, None) > + self.assertIsNone(ibm.read_md()) > + > + def test_provisioning_not_ibm(self, m_platform, m_sysuuid): > + m_platform.return_value = (None, None) > + self.assertIsNone(ibm.read_md()) > + > + def _get_expected_metadata(self, os_md): > + """return expected 'metadata' for data loaded fro meta_data.json.""" > + os_md = copy.deepcopy(os_md) > + copies = ('meta', 'domain', 'configuration_token', 'crypt_key') > + renames = ( > + ('hostname', 'local-hostname'), > + ('uuid', 'instance-id'), > + ('public_keys', 'public-keys')) > + ret = {} > + ret.update(dict([(k, os_md[k]) for k in copies if k in os_md])) > + for osname, mdname in renames: > + if osname in os_md: > + ret[mdname] = os_md[osname] > + if 'random_seed' in os_md: > + ret['random_seed'] = base64.b64decode(os_md['random_seed']) > + > + return ret > + > + def test_template_live(self, m_platform, m_sysuuid): > + tmpdir = self.tmp_dir() > + m_platform.return_value = ( > + ibm.Platforms.TEMPLATE_LIVE_METADATA, tmpdir) > + m_sysuuid.return_value = self.sysuuid > + > + test_helpers.populate_dir(tmpdir, { > + 'openstack/latest/meta_data.json': json.dumps(self.template_md), > + 'openstack/latest/user_data': self.userdata, > + 'openstack/content/interfaces': self.content_interfaces, > + 'meta.js': self.meta_js}) > + > + ret = ibm.read_md() > + self.assertEqual(ibm.Platforms.TEMPLATE_LIVE_METADATA, > + ret['platform']) > + self.assertEqual(tmpdir, ret['source']) > + self.assertEqual(self.userdata, ret['userdata']) > + self.assertEqual(self._get_expected_metadata(self.template_md), > + ret['metadata']) > + self.assertEqual(self.sysuuid, ret['system-uuid']) > + > + def test_os_code_live(self, m_platform, m_sysuuid): > + """verify an os_code metadata path.""" > + tmpdir = self.tmp_dir() > + m_platform.return_value = (ibm.Platforms.OS_CODE, tmpdir) > + test_helpers.populate_dir(tmpdir, { > + 'openstack/latest/meta_data.json': json.dumps(self.oscode_md), > + 'openstack/latest/user_data': self.userdata, > + 'openstack/latest/vendor_data.json': > json.dumps(self.vendor_data), > + }) > + > + ret = ibm.read_md() > + self.assertEqual(ibm.Platforms.OS_CODE, ret['platform']) > + self.assertEqual(tmpdir, ret['source']) > + self.assertEqual(self.userdata, ret['userdata']) > + self.assertEqual(self._get_expected_metadata(self.oscode_md), > + ret['metadata']) > + > + def test_os_code_live_no_userdata(self, m_platform, m_sysuuid): > + """Verify os_code without user-data.""" > + tmpdir = self.tmp_dir() > + m_platform.return_value = (ibm.Platforms.OS_CODE, tmpdir) > + test_helpers.populate_dir(tmpdir, { > + 'openstack/latest/meta_data.json': json.dumps(self.oscode_md), > + 'openstack/latest/vendor_data.json': > json.dumps(self.vendor_data), > + }) > + > + ret = ibm.read_md() > + self.assertEqual(ibm.Platforms.OS_CODE, ret['platform']) > + self.assertEqual(tmpdir, ret['source']) > + self.assertIsNone(ret['userdata']) > + self.assertEqual(self._get_expected_metadata(self.oscode_md), > + ret['metadata']) > + > + > +# vi: ts=4 expandtab > diff --git a/tests/unittests/test_ds_identify.py > b/tests/unittests/test_ds_identify.py > index 9c5628e..f3f0b4c 100644 > --- a/tests/unittests/test_ds_identify.py > +++ b/tests/unittests/test_ds_identify.py > @@ -37,8 +39,8 @@ BLKID_UEFI_UBUNTU = [ > > POLICY_FOUND_ONLY = "search,found=all,maybe=none,notfound=disabled" > POLICY_FOUND_OR_MAYBE = "search,found=all,maybe=all,notfound=disabled" > -DI_DEFAULT_POLICY = "search,found=all,maybe=all,notfound=enabled" > -DI_DEFAULT_POLICY_NO_DMI = "search,found=all,maybe=all,notfound=disabled" > +DI_DEFAULT_POLICY = "search,found=all,maybe=all,notfound=disabled" > +DI_DEFAULT_POLICY_NO_DMI = "search,found=all,maybe=all,notfound=enabled" why is this changing? Should we read this from the source tree? > DI_EC2_STRICT_ID_DEFAULT = "true" > OVF_MATCH_STRING = 'http://schemas.dmtf.org/ovf/environment/1' > -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/341774 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:feature/datasource-ibmcloud into cloud-init:master. _______________________________________________ Mailing list: https://launchpad.net/~cloud-init-dev Post to : cloud-init-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~cloud-init-dev More help : https://help.launchpad.net/ListHelp