Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:feature/ec2-secondary-nics into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:047a654e9b7ca8dd8115aab2467c55f25ba312ca https://jenkins.ubuntu.com/server/job/cloud-init-ci/762/ Executed test runs: SUCCESS: Checkout FAILED: Unit & Style Tests Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/762/rebuild -- https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/369792 Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:feature/ec2-secondary-nics 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
[Cloud-init-dev] [Merge] ~chad.smith/cloud-init:feature/ec2-secondary-nics into cloud-init:master
The proposal to merge ~chad.smith/cloud-init:feature/ec2-secondary-nics into cloud-init:master has been updated. Commit message changed to: ec2: render secondary IPs on primary nic when present in metadata Parse local-ipv4s and subnet-ipv4-cidr-block on EC2 metadata version 2018-09-24 to obtain secondary nic private IPs and network mask for the primary nic. In adding this feature, convert DataSourceEc2.network_config to emit network version 2 instead of version 1. To allow for retaining original network config behavior on earlier distribution series, surface a datasource config option configure_secondary_ips which defaults to True on tip. Older/stable distribution series will set configure_secondary_ips default to False. For more details, see: https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/369792 -- Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:feature/ec2-secondary-nics 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
[Cloud-init-dev] [Merge] ~chad.smith/cloud-init:feature/ec2-secondary-nics into cloud-init:master
The proposal to merge ~chad.smith/cloud-init:feature/ec2-secondary-nics into cloud-init:master has been updated. Commit message changed to: ec2: render secondary IPs on primary nic when present in metadata Parse local-ipv4s and subnet-ipv4-cidr-block on EC2 metadata version 2018-09-24 to obtain secondary nic private IPs and network mask for the primary nic. In adding this feature, convert DataSourceEc2.network_config to emit network version 2 instead of version 1. To allow for retaining original network config behavior on earlier distribution series, surface a datasource config option configure_secondary_ips which defaults to True on tip. Older distribution series will set configure_secondary_ips default to False. For more details, see: https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/369792 -- Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:feature/ec2-secondary-nics 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:feature/ec2-secondary-nics into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:db953566a9e6df91b4eaeebb813e45726136316c https://jenkins.ubuntu.com/server/job/cloud-init-ci/761/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/761/rebuild -- https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/369792 Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:feature/ec2-secondary-nics 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:feature/ec2-secondary-nics into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:f6b0d9eb283917f4c2a5a5f863ad07c048346199 https://jenkins.ubuntu.com/server/job/cloud-init-ci/759/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/759/rebuild -- https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/369792 Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:feature/ec2-secondary-nics 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
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:feature/ec2-secondary-nics into cloud-init:master
Diff comments:
> diff --git a/cloudinit/sources/DataSourceEc2.py
> b/cloudinit/sources/DataSourceEc2.py
> index 5c017bf..9a5ed43 100644
> --- a/cloudinit/sources/DataSourceEc2.py
> +++ b/cloudinit/sources/DataSourceEc2.py
> @@ -536,24 +536,43 @@ def convert_ec2_metadata_network_config(network_md,
> macs_to_nics=None,
I wonder if we should keep both versions of the convert here and specify the
version in the signature?
> @param: fallback_nic: Optionally provide the primary nic interface name.
> This nic will be guaranteed to minimally have a dhcp4 configuration.
>
> -@return A dict of network config version 1 based on the metadata and
> macs.
> +@return A dict of network config version 2 based on the metadata and
> macs.
> """
> -netcfg = {'version': 1, 'config': []}
> +netcfg = {'version': 2, 'ethernets': {}}
> if not macs_to_nics:
> macs_to_nics = net.get_interfaces_by_mac()
> macs_metadata = network_md['interfaces']['macs']
> for mac, nic_name in macs_to_nics.items():
> +dev_config = {}
> 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
> +local_ipv4s = nic_metadata.get('local-ipv4s')
> if (nic_name == fallback_nic or nic_metadata.get('public-ipv4s') or
> -nic_metadata.get('local-ipv4s')):
> -nic_cfg['subnets'].append({'type': 'dhcp4'})
> +local_ipv4s):
> +dev_config['dhcp4'] = True
> +# In version < 2018-09-24 local_ipvs is a str with a single IP
> +if isinstance(local_ipv4s, list) and len(local_ipv4s) > 1:
> +if dev_config.get('addresses') is None:
> +dev_config['addresses'] = []
> +subnet_cidr = nic_metadata.get('subnet-ipv4-cidr-block')
> +if not subnet_cidr or '/' not in subnet_cidr:
> +LOG.warning(
> +'Could not parse subnet-ipv4-cidr-block %s.'
> +' Network config for Secondary IPs default to /32',
> +subnet_cidr)
> +prefix = '32'
> +else:
> +_ip, prefix = subnet_cidr.split('/')
> +for secondary_ip in local_ipv4s[1:]:
> +dev_config['addresses'].append(
> +'{ip}/{prefix}'.format(ip=secondary_ip,
> prefix=prefix))
> if nic_metadata.get('ipv6s'):
> -nic_cfg['subnets'].append({'type': 'dhcp6'})
> -netcfg['config'].append(nic_cfg)
> +dev_config['dhcp6'] = True
> +dev_config.update({
> +'match': {'macaddress': mac.lower()},
> +'set-name': nic_name})
> +netcfg['ethernets'][nic_name] = dev_config
> return netcfg
>
>
> diff --git a/tests/unittests/test_datasource/test_ec2.py
> b/tests/unittests/test_datasource/test_ec2.py
> index 20d59bf..e031fe9 100644
> --- a/tests/unittests/test_datasource/test_ec2.py
> +++ b/tests/unittests/test_datasource/test_ec2.py
> @@ -277,10 +365,9 @@ class TestEc2(test_helpers.HttprettyTestCase):
> 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'}]}
> +expected = {'version': 2, 'ethernets': {'eth9': {
> +'match': {'macaddress': '06:17:04:d7:26:09'}, 'set-name': 'eth9',
> +'dhcp4': True, 'dhcp6': True}}}
If we keep both versions of the parser, then we can add a variant to the test
calling with with v1 and v2?
> patch_path = (
> 'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac')
> get_interface_mac_path = (
> @@ -309,10 +396,40 @@ class TestEc2(test_helpers.HttprettyTestCase):
> ds.get_data()
>
> mac1 = '06:17:04:d7:26:0A' # IPv4 only in DEFAULT_METADATA
> -expected = {'version': 1, 'config': [
> -{'mac_address': '06:17:04:d7:26:0A', 'name': 'eth9',
> - 'subnets': [{'type': 'dhcp4'}],
> - 'type': 'physical'}]}
> +expected = {'version': 2, 'ethernets': {'eth9': {
> +'match': {'macaddress': '06:17:04:d7:26:0a'}, 'set-name': 'eth9',
> +'dhcp4': True}}}
> +patch_path = (
> +'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac')
I suggest a class attribute or global for the prefix here:
net_path = 'cloudinit.sources.DataSourceEc2.net'
by_mac = mock.patch(net_path + '.get_interfaces_by_mac'
> +get_interface_mac_path = (
> +
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:feature/ec2-secondary-nics into cloud-init:master
Thanks! A couple of follow-ups, and a couple of nits I missed first time
around.
Diff comments:
> diff --git a/cloudinit/sources/DataSourceEc2.py
> b/cloudinit/sources/DataSourceEc2.py
> index 5c017bf..9a5ed43 100644
> --- a/cloudinit/sources/DataSourceEc2.py
> +++ b/cloudinit/sources/DataSourceEc2.py
> @@ -536,24 +536,43 @@ def convert_ec2_metadata_network_config(network_md,
> macs_to_nics=None,
> @param: fallback_nic: Optionally provide the primary nic interface name.
> This nic will be guaranteed to minimally have a dhcp4 configuration.
>
> -@return A dict of network config version 1 based on the metadata and
> macs.
> +@return A dict of network config version 2 based on the metadata and
> macs.
> """
> -netcfg = {'version': 1, 'config': []}
> +netcfg = {'version': 2, 'ethernets': {}}
> if not macs_to_nics:
> macs_to_nics = net.get_interfaces_by_mac()
> macs_metadata = network_md['interfaces']['macs']
> for mac, nic_name in macs_to_nics.items():
> +dev_config = {}
> 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
> +local_ipv4s = nic_metadata.get('local-ipv4s')
> if (nic_name == fallback_nic or nic_metadata.get('public-ipv4s') or
> -nic_metadata.get('local-ipv4s')):
> -nic_cfg['subnets'].append({'type': 'dhcp4'})
> +local_ipv4s):
> +dev_config['dhcp4'] = True
> +# In version < 2018-09-24 local_ipvs is a str with a single IP
Thanks for this comment! I have the context of the commit message informing me
that this about secondary IP addresses, but I wonder if in future people might
wonder why we aren't doing anything with the one str IP address; is it worth
explicitly mentioning that the next stanza is handling secondary interfaces?
> +if isinstance(local_ipv4s, list) and len(local_ipv4s) > 1:
> +if dev_config.get('addresses') is None:
If we get here, dev_config is, I think, always going to be {'dhcp4': True}. So
do we need this to be conditional any longer?
> +dev_config['addresses'] = []
> +subnet_cidr = nic_metadata.get('subnet-ipv4-cidr-block')
> +if not subnet_cidr or '/' not in subnet_cidr:
> +LOG.warning(
> +'Could not parse subnet-ipv4-cidr-block %s.'
> +' Network config for Secondary IPs default to /32',
> +subnet_cidr)
> +prefix = '32'
> +else:
> +_ip, prefix = subnet_cidr.split('/')
If subnet_cidr has more than one / in it, this will fail with "ValueError: too
many values to unpack".
(Sorry I didn't catch this the first time around!)
> +for secondary_ip in local_ipv4s[1:]:
> +dev_config['addresses'].append(
> +'{ip}/{prefix}'.format(ip=secondary_ip,
> prefix=prefix))
> if nic_metadata.get('ipv6s'):
> -nic_cfg['subnets'].append({'type': 'dhcp6'})
> -netcfg['config'].append(nic_cfg)
> +dev_config['dhcp6'] = True
> +dev_config.update({
> +'match': {'macaddress': mac.lower()},
> +'set-name': nic_name})
> +netcfg['ethernets'][nic_name] = dev_config
> return netcfg
>
>
> diff --git a/tests/unittests/test_datasource/test_ec2.py
> b/tests/unittests/test_datasource/test_ec2.py
> index 20d59bf..e031fe9 100644
> --- a/tests/unittests/test_datasource/test_ec2.py
> +++ b/tests/unittests/test_datasource/test_ec2.py
> @@ -309,10 +396,40 @@ class TestEc2(test_helpers.HttprettyTestCase):
> ds.get_data()
>
> mac1 = '06:17:04:d7:26:0A' # IPv4 only in DEFAULT_METADATA
> -expected = {'version': 1, 'config': [
> -{'mac_address': '06:17:04:d7:26:0A', 'name': 'eth9',
> - 'subnets': [{'type': 'dhcp4'}],
> - 'type': 'physical'}]}
> +expected = {'version': 2, 'ethernets': {'eth9': {
> +'match': {'macaddress': '06:17:04:d7:26:0a'}, 'set-name': 'eth9',
> +'dhcp4': True}}}
> +patch_path = (
> +'cloudinit.sources.DataSourceEc2.net.get_interfaces_by_mac')
> +get_interface_mac_path = (
> +'cloudinit.sources.DataSourceEc2.net.get_interface_mac')
> +with mock.patch(patch_path) as m_get_interfaces_by_mac:
> +with mock.patch(find_fallback_path) as m_find_fallback:
> +with mock.patch(get_interface_mac_path) as m_get_mac:
> +m_get_interfaces_by_mac.return_value = {mac1: 'eth9'}
> +m_find_fallback.return_value = 'eth9'
> +m_get_mac.return_value
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:feature/ec2-secondary-nics into cloud-init:master
Diff comments:
> diff --git a/cloudinit/sources/DataSourceEc2.py
> b/cloudinit/sources/DataSourceEc2.py
> index 5c017bf..aef5d80 100644
> --- a/cloudinit/sources/DataSourceEc2.py
> +++ b/cloudinit/sources/DataSourceEc2.py
> @@ -536,24 +536,35 @@ def convert_ec2_metadata_network_config(network_md,
> macs_to_nics=None,
> @param: fallback_nic: Optionally provide the primary nic interface name.
> This nic will be guaranteed to minimally have a dhcp4 configuration.
>
> -@return A dict of network config version 1 based on the metadata and
> macs.
> +@return A dict of network config version 2 based on the metadata and
> macs.
> """
> -netcfg = {'version': 1, 'config': []}
> +netcfg = {'version': 2, 'ethernets': {}}
> if not macs_to_nics:
> macs_to_nics = net.get_interfaces_by_mac()
> macs_metadata = network_md['interfaces']['macs']
> for mac, nic_name in macs_to_nics.items():
> +dev_config = {}
> 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
> +local_ipv4s = nic_metadata.get('local-ipv4s')
> if (nic_name == fallback_nic or nic_metadata.get('public-ipv4s') or
> -nic_metadata.get('local-ipv4s')):
> -nic_cfg['subnets'].append({'type': 'dhcp4'})
> +local_ipv4s):
> +dev_config['dhcp4'] = True
> +if isinstance(local_ipv4s, list) and len(local_ipv4s) > 1:
Added the comment # In version < 2018-09-24 local_ipvs is a str
with a single IP
> +subnet_cidr = nic_metadata.get('subnet-ipv4-cidr-block')
> +_ip, prefix = subnet_cidr.split('/')
checking now and wawrning if we see it + default to /32 secondary IP setting
which will at least preserve a strict route and the desired secondary IP.
default routes should still have proper connectivity on preferred primary IP.
> +for secondary_ip in local_ipv4s[1:]:
> +if dev_config.get('addresses') is None:
yes and done.
> +dev_config['addresses'] = []
> +dev_config['addresses'].append(
> +'{ip}/{prefix}'.format(ip=secondary_ip,
> prefix=prefix))
> if nic_metadata.get('ipv6s'):
> -nic_cfg['subnets'].append({'type': 'dhcp6'})
> -netcfg['config'].append(nic_cfg)
> +dev_config['dhcp6'] = True
> +dev_config.update({
> +'match': {'macaddress': nic_metadata.get('mac').lower()},
oops, don't really even need that as we already have mac local variable from
our outer macs_to_nics for loop.
> +'set-name': nic_name})
> +netcfg['ethernets'][nic_name] = dev_config
> return netcfg
>
>
> diff --git a/tests/unittests/test_datasource/test_ec2.py
> b/tests/unittests/test_datasource/test_ec2.py
> index 20d59bf..c99f58f 100644
> --- a/tests/unittests/test_datasource/test_ec2.py
> +++ b/tests/unittests/test_datasource/test_ec2.py
> @@ -112,6 +112,94 @@ DEFAULT_METADATA = {
> "services": {"domain": "amazonaws.com", "partition": "aws"},
> }
>
> +# collected from api version 2016-09-02/ with
Correct, fixed to 2018-09-24.
> +# python3 -c 'import json
> +# from cloudinit.ec2_utils import get_instance_metadata as gm
> +# print(json.dumps(gm("2018-09-24"), indent=1, sort_keys=True))'
> +DEFAULT_METADATA_2018_09_24 = {
Good point, changed it to SECONDARY_IP_METADATA_2018_09_24
> +"ami-id": "ami-0986c2ac728528ac2",
> +"ami-launch-index": "0",
> +"ami-manifest-path": "(unknown)",
> +"block-device-mapping": {
> +"ami": "/dev/sda1",
> +"root": "/dev/sda1"
> +},
> +"events": {
> +"maintenance": {
> +"history": "[]",
> +"scheduled": "[]"
> +}
> +},
> +"hostname": "ip-172-31-44-13.us-east-2.compute.internal",
> +"identity-credentials": {
> +"ec2": {
> +"info": {
> +"AccountId": "329910648901",
> +"Code": "Success",
> +"LastUpdated": "2019-07-06T14:22:56Z"
> +}
> +}
> +},
> +"instance-action": "none",
> +"instance-id": "i-069e01e8cc43732f8",
> +"instance-type": "t2.micro",
> +"local-hostname": "ip-172-31-44-13.us-east-2.compute.internal",
> +"local-ipv4": "172.31.44.13",
> +"mac": "0a:07:84:3d:6e:38",
> +"metrics": {
> +"vhostmd": ""
> +},
> +"network": {
> +"interfaces": {
> +"macs": {
> +"0a:07:84:3d:6e:38": {
> +"device-number": "0",
> +"interface-id": "eni-0d6335689899ce9cc",
> +"ipv4-associations": {
> +"18.218.219.181": "172.31.44.13"
> +
Re: [Cloud-init-dev] [Merge] ~bitfehler/cloud-init:bitfehler/load_seed into cloud-init:master
Hi, I previously wrote a message to the mailing list, but without any response so far, so I figured I'd write some more code and try this way. Happy about any suggestions for improvements or guidance for other approaches. Some notes right away: - The "xenial" tox env does not work for me, even on clean master, is this a known issue? - I thought it would be nice to make a holistic change (i.e. changing all callers to the new function), but in fact I do not have access to a test setup for the OVH or CloudStack data sources. How is this usually handled? Is it preferable to not touch any data sources you can not test? Or are there means for this to be tested before it lands in a release? Thanks a bunch, Conrad -- https://code.launchpad.net/~bitfehler/cloud-init/+git/cloud-init/+merge/369814 Your team cloud-init commiters is requested to review the proposed merge of ~bitfehler/cloud-init:bitfehler/load_seed 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
Re: [Cloud-init-dev] [Merge] ~tribaal/cloud-init:feat/datasource-exoscale into cloud-init:master
A first pass fixing most review comments is done.
One open point here is the extra_config overwriting user-data - it seems to me
that perhaps we should be using some other mechanism instead (my assumption was
that extra_config would be overwritten by user data - but that doesn't seem to
be the case).
Should we use "vendordata" instead for this particular case?
Diff comments:
> diff --git a/cloudinit/sources/DataSourceExoscale.py
> b/cloudinit/sources/DataSourceExoscale.py
> new file mode 100644
> index 000..4588c9d
> --- /dev/null
> +++ b/cloudinit/sources/DataSourceExoscale.py
> @@ -0,0 +1,126 @@
> +import time
> +
> +from cloudinit import ec2_utils as ec2
> +from cloudinit import log as logging
> +from cloudinit import sources
> +from cloudinit import url_helper
> +
> +API_VERSION = "latest"
Excellent point - I switched the version to "1.0", which happens to be an alias
for "latest" for now. At least this way the version won't change under our
user's feet as you pointed out.
> +LOG = logging.getLogger(__name__)
> +SERVICE_ADDRESS = "http://169.254.169.254;
> +
> +
> +class DataSourceExoscale(sources.DataSource):
> +
> +dsname = 'Exoscale'
> +url_timeout = 10
> +url_retries = 6
> +url_max_wait = 60
> +
> +def __init__(self, sys_cfg, distro, paths):
> +sources.DataSource.__init__(self, sys_cfg, distro, paths)
> +LOG.info("Initializing the Exoscale datasource")
> +self.extra_config = {}
> +
> +def get_password(self):
We would rather not reuse any code from the Cloudstack datasource, if it isn't
a blocker.
In particular: we prefer using python (via read_file_or_url()) to perform HTTP
requests, instead of shelling out to wget as the Cloudstack source currently
does.
> +"""Return the VM's passwords."""
> +LOG.info("Fetching password from metadata service")
Done.
> +password_url = "{}:8080".format(SERVICE_ADDRESS)
> +response = url_helper.read_file_or_url(
> +password_url,
> +ssl_details=None,
> +headers={"DomU_Request": "send_my_password"},
> +timeout=self.url_timeout,
> +retries=self.url_retries)
> +password = response.contents.decode('utf-8')
> +# the password is empty or already saved
> +if password in ['', 'saved_password']:
> +LOG.info("Password is missing or already saved")
> +return None
> +LOG.info("Found the password in metadata service")
> +# save the password
> +url_helper.read_file_or_url(
> +password_url,
> +ssl_details=None,
> +headers={"DomU_Request": "saved_password"},
> +timeout=self.url_timeout,
> +retries=self.url_retries)
> +LOG.info("password saved")
> +return password
> +
> +def wait_for_metadata_service(self):
> +"""Wait for the metadata service to be reachable."""
> +LOG.info("waiting for the metadata service")
> +start_time = time.time()
> +
> +metadata_url = "{}/{}/meta-data/instance-id".format(
> +SERVICE_ADDRESS,
> +API_VERSION)
> +
> +start_time = time.time()
Done.
This was tackled by letting url_helper.readurl() do the logging when timing out
(it was redundant).
The general time it took for us to get the metadata is now logged in
_get_data() (using util.log_time() as recommended).
> +url = url_helper.wait_for_url(
> +urls=[metadata_url],
> +max_wait=self.url_max_wait,
> +timeout=self.url_timeout,
> +status_cb=LOG.critical)
> +
> +if url:
> +LOG.info("metadata service ok")
Done.
> +return True
> +else:
> +wait_time = int(time.time() - start_time)
> +LOG.critical(("Giving up on waiting for the metadata from %s"
> + " after %s seconds"),
> + url,
> + wait_time)
> +return False
> +
> +def _get_data(self):
> +"""Fetch the user data, the metadata and the VM password
> +from the metadata service."""
> +LOG.info("fetching data")
Done.
> +if not self.wait_for_metadata_service():
> +return False
> +start_time = time.time()
> +self.userdata_raw = ec2.get_instance_userdata(API_VERSION,
Done.
The logic now goes:
self._get_data() -> self.crawl_metadata() -> read_data() -> get_password()
Added a simple __main__ to call read_data() directly (that performs a
json.dumps on stdout)
> + SERVICE_ADDRESS,
> +
> timeout=self.url_timeout,
> +
> retries=self.url_retries)
> +self.metadata = ec2.get_instance_metadata(API_VERSION,
> +
[Cloud-init-dev] [Merge] ~bitfehler/cloud-init:bitfehler/load_seed into cloud-init:master
The proposal to merge ~bitfehler/cloud-init:bitfehler/load_seed into cloud-init:master has been updated. Commit message changed to: Support network-config in nocloud(-net) data source This proposal adds a new function `load_seed` that can be used as a replacement for `read_seeded` and `read_optional_seed`, but has support for additional files (network-config, vendor-data). The nocloud(-net) data source is updated to make use of it, adding support for network-config in user-supplied seeds. This addresses the following bugs: https://bugs.launchpad.net/cloud-init/+bug/1809180 https://bugs.launchpad.net/cloud-init/+bug/1809277 The OVH and CloudStack datasources are also updated to use the new function, but without changing the data source semantics. Finally, as all previous callers have been updated, the old functions `read_seeded` and `read_optional_seed` are removed. For more details, see: https://code.launchpad.net/~bitfehler/cloud-init/+git/cloud-init/+merge/369814 -- Your team cloud-init commiters is requested to review the proposed merge of ~bitfehler/cloud-init:bitfehler/load_seed 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
[Cloud-init-dev] [Merge] ~bitfehler/cloud-init:bitfehler/load_seed into cloud-init:master
The proposal to merge ~bitfehler/cloud-init:bitfehler/load_seed into cloud-init:master has been updated. Commit message changed to: Support network-config if nocloud(-net) data source This proposal adds a new function `load_seed` that can be used as a replacement for `read_seeded` and `read_optional_seed`, but has support for additional files (network-config, vendor-data). The nocloud(-net) data source is updated to make use of it, adding support for network-config in user-supplied seeds. This addresses the following bugs: https://bugs.launchpad.net/cloud-init/+bug/1809180 https://bugs.launchpad.net/cloud-init/+bug/1809277 The OVH and CloudStack datasources are also updated to use the new function, but without changing the data source semantics. Finally, as all previous callers have been updated, the old functions `read_seeded` and `read_optional_seed` are removed. For more details, see: https://code.launchpad.net/~bitfehler/cloud-init/+git/cloud-init/+merge/369814 -- Your team cloud-init commiters is requested to review the proposed merge of ~bitfehler/cloud-init:bitfehler/load_seed 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
[Cloud-init-dev] [Merge] ~bitfehler/cloud-init:bitfehler/load_seed into cloud-init:master
Conrad Hoffmann has proposed merging ~bitfehler/cloud-init:bitfehler/load_seed
into cloud-init:master.
Requested reviews:
cloud-init commiters (cloud-init-dev)
For more details, see:
https://code.launchpad.net/~bitfehler/cloud-init/+git/cloud-init/+merge/369814
--
Your team cloud-init commiters is requested to review the proposed merge of
~bitfehler/cloud-init:bitfehler/load_seed into cloud-init:master.
diff --git a/cloudinit/sources/DataSourceCloudStack.py b/cloudinit/sources/DataSourceCloudStack.py
index f185dc7..3605076 100644
--- a/cloudinit/sources/DataSourceCloudStack.py
+++ b/cloudinit/sources/DataSourceCloudStack.py
@@ -110,12 +110,15 @@ class DataSourceCloudStack(sources.DataSource):
return self.cfg
def _get_data(self):
-seed_ret = {}
-if util.read_optional_seed(seed_ret, base=(self.seed_dir + "/")):
-self.userdata_raw = seed_ret['user-data']
-self.metadata = seed_ret['meta-data']
+try:
+# Using `required` wrapped in try because we want both or none
+seed = util.load_seed(self.seed_dir, ['meta-data', 'user-data'])
+self.userdata_raw = seed['user-data']
+self.metadata = seed['meta-data']
LOG.debug("Using seeded cloudstack data from: %s", self.seed_dir)
return True
+except Exception:
+pass
try:
if not self.wait_for_metadata_service():
return False
diff --git a/cloudinit/sources/DataSourceNoCloud.py b/cloudinit/sources/DataSourceNoCloud.py
index 8a9e5dd..da92875 100644
--- a/cloudinit/sources/DataSourceNoCloud.py
+++ b/cloudinit/sources/DataSourceNoCloud.py
@@ -165,13 +165,11 @@ class DataSourceNoCloud(sources.DataSource):
# This could throw errors, but the user told us to do it
# so if errors are raised, let them raise
-(md_seed, ud) = util.read_seeded(seedfrom, timeout=None)
+seed = util.load_seed(seedfrom, timeout=None, **pp2d_kwargs)
LOG.debug("Using seeded cache data from %s", seedfrom)
# Values in the command line override those from the seed
-mydata['meta-data'] = util.mergemanydict([mydata['meta-data'],
- md_seed])
-mydata['user-data'] = ud
+mydata = _merge_new_seed(mydata, seed)
found.append(seedfrom)
# Now that we have exhausted any other places merge in the defaults
@@ -357,8 +355,10 @@ def _merge_new_seed(cur, seeded):
ret['meta-data'] = util.mergemanydict([cur['meta-data'], newmd])
if seeded.get('network-config'):
-ret['network-config'] = _maybe_remove_top_network(
-util.load_yaml(seeded.get('network-config')))
+newnc = seeded['network-config']
+if not isinstance(newnc, dict):
+newnc = util.load_yaml(seeded['network-config'])
+ret['network-config'] = _maybe_remove_top_network(newnc)
if 'user-data' in seeded:
ret['user-data'] = seeded['user-data']
diff --git a/cloudinit/sources/DataSourceOVF.py b/cloudinit/sources/DataSourceOVF.py
index 70e7a5c..dd9b994 100644
--- a/cloudinit/sources/DataSourceOVF.py
+++ b/cloudinit/sources/DataSourceOVF.py
@@ -262,10 +262,10 @@ class DataSourceOVF(sources.DataSource):
seedfrom, self)
return False
-(md_seed, ud) = util.read_seeded(seedfrom, timeout=None)
+seed = util.load_seed(seedfrom, ['meta-data'], timeout=None)
LOG.debug("Using seeded cache data from %s", seedfrom)
-md = util.mergemanydict([md, md_seed])
+md = util.mergemanydict([md, seed.get('meta-data', {})])
found.append(seedfrom)
# Now that we have exhausted any other places merge in the defaults
diff --git a/cloudinit/util.py b/cloudinit/util.py
index aa23b3f..43bb0e8 100644
--- a/cloudinit/util.py
+++ b/cloudinit/util.py
@@ -892,22 +892,6 @@ def runparts(dirp, skip_no_exist=True, exe_prefix=None):
% (len(failed), len(attempted)))
-# read_optional_seed
-# returns boolean indicating success or failure (presense of files)
-# if files are present, populates 'fill' dictionary with 'user-data' and
-# 'meta-data' entries
-def read_optional_seed(fill, base="", ext="", timeout=5):
-try:
-(md, ud) = read_seeded(base, ext, timeout)
-fill['user-data'] = ud
-fill['meta-data'] = md
-return True
-except url_helper.UrlError as e:
-if e.code == url_helper.NOT_FOUND:
-return False
-raise
-
-
def fetch_ssl_details(paths=None):
ssl_details = {}
# Lookup in these locations for ssl key/cert files
@@ -974,34 +958,31 @@ def load_yaml(blob, default=None, allowed=(dict,)):
return loaded
-def read_seeded(base="", ext="", timeout=5, retries=10, file_retries=0):
+def load_seed(base='', required=None,
