Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:aws-local-dhcp into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:137c2881ed2d330f9b2f26a267680bbe2f945149 https://jenkins.ubuntu.com/server/job/cloud-init-ci/116/ Executed test runs: SUCCESS: Checkout FAILED: Unit & Style Tests Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/116/rebuild -- https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/328241 Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:aws-local-dhcp 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:aws-local-dhcp into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:a1268952438c26bb949b7799019b1a8fbb1fe6e6 https://jenkins.ubuntu.com/server/job/cloud-init-ci/114/ Executed test runs: SUCCESS: Checkout FAILED: Unit & Style Tests Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/114/rebuild -- https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/328241 Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:aws-local-dhcp 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:aws-local-dhcp into cloud-init:master
Diff comments: > diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py > index 46cb9c8..d38ea8b 100644 > --- a/cloudinit/net/__init__.py > +++ b/cloudinit/net/__init__.py > @@ -233,15 +231,24 @@ def generate_fallback_config(blacklist_drivers=None, > config_driver=None): > if DEFAULT_PRIMARY_INTERFACE in names: > names.remove(DEFAULT_PRIMARY_INTERFACE) > names.insert(0, DEFAULT_PRIMARY_INTERFACE) > -target_name = None > -target_mac = None > + > +# pick the first that has a address Updated the comment here. this is a mac-address we are looking at no an ip address, so we aren't looking for the first nic that is "up" we are looking for the first nic with a configured mac address. > for name in names: > -mac = read_sys_net_safe(name, 'address') > -if mac: > -target_name = name > -target_mac = mac > -break > -if target_mac and target_name: > +if read_sys_net_safe(name, 'address'): > +return name > +return None > + > + > +def generate_fallback_config(blacklist_drivers=None, config_driver=None): > +"""Determine which attached net dev is most likely to have a connection > and > + generate network state to run dhcp on that interface""" > + > +if not config_driver: > +config_driver = False > + > +target_name = find_fallback_nic(blacklist_drivers=blacklist_drivers) > +if target_name: > +target_mac = read_sys_net_safe(target_name, 'address') > nconf = {'config': [], 'version': 1} > cfg = {'type': 'physical', 'name': target_name, > 'mac_address': target_mac, 'subnets': [{'type': 'dhcp'}]} -- https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/328241 Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:aws-local-dhcp 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:aws-local-dhcp into cloud-init:master
theres nothing huge in my comments. thanks. Diff comments: > diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py > index 46cb9c8..d38ea8b 100644 > --- a/cloudinit/net/__init__.py > +++ b/cloudinit/net/__init__.py > @@ -175,12 +175,10 @@ def is_disabled_cfg(cfg): > return cfg.get('config') == "disabled" > > > -def generate_fallback_config(blacklist_drivers=None, config_driver=None): > -"""Determine which attached net dev is most likely to have a connection > and > - generate network state to run dhcp on that interface""" > - > -if not config_driver: > -config_driver = False > +def find_fallback_nic(blacklist_drivers=None): > +"""Return the name of the 'fallback' network device.""" > +if not blacklist_drivers: > +blacklist_drivers = [] > > if not blacklist_drivers: > blacklist_drivers = [] if not blacklist_drivers: ... if not blacklist_drivers: am i seeing double? > @@ -233,15 +231,24 @@ def generate_fallback_config(blacklist_drivers=None, > config_driver=None): > if DEFAULT_PRIMARY_INTERFACE in names: > names.remove(DEFAULT_PRIMARY_INTERFACE) > names.insert(0, DEFAULT_PRIMARY_INTERFACE) > -target_name = None > -target_mac = None > + > +# pick the first that has a address 'address' is mac address. unlikely that it changes. possibly we could make a helper like 'valid_mac()' and use that above in the 'for interface in potential_intefaces' loop. i say 'valid_mac' because we have knowledge elsewhere to avoid a mac like '00:00:00:00:00:00' (bug 1692028) > for name in names: > -mac = read_sys_net_safe(name, 'address') > -if mac: > -target_name = name > -target_mac = mac > -break > -if target_mac and target_name: > +if read_sys_net_safe(name, 'address'): > +return name > +return None > + > + > +def generate_fallback_config(blacklist_drivers=None, config_driver=None): > +"""Determine which attached net dev is most likely to have a connection > and > + generate network state to run dhcp on that interface""" > + > +if not config_driver: > +config_driver = False > + > +target_name = find_fallback_nic(blacklist_drivers=blacklist_drivers) > +if target_name: > +target_mac = read_sys_net_safe(target_name, 'address') > nconf = {'config': [], 'version': 1} > cfg = {'type': 'physical', 'name': target_name, > 'mac_address': target_mac, 'subnets': [{'type': 'dhcp'}]} > diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py > new file mode 100644 > index 000..4d59bd0 > --- /dev/null > +++ b/cloudinit/net/dhcp.py > @@ -0,0 +1,118 @@ > +# Copyright (C) 2017 Canonical Ltd. > +# > +# Author: Chad Smith > +# > +# This file is part of cloud-init. See LICENSE file for license information. > + > +import logging > +import os > +import re > + > +from cloudinit.net import find_fallback_nic, get_devicelist > +from cloudinit import util > + > +LOG = logging.getLogger(__name__) > + > + > +class InvalidDHCPLeaseFileError(Exception): > +"""Raised when parsing an empty or invalid dhcp.leases file. > + > +Current uses are DataSourceAzure and DataSourceEc2 during ephemeral > +boot to scrape metadata. > +""" > +pass > + > + > +def maybe_dhcp_clean_discovery(nic=None): > +"""Create a temporary working directory and find the nic if needed. clean here is "without side affects". see dhcp_clean_discovery . this is just "maybe" do that. it does say that "If the device is already connected, do nothing." but i dont see that. > + > +If the device is already connected, do nothing. > + > +@param nic: Name of the network interface we want to run dhclient on. > +@return: A dict of dhcp options from the dhclient discovery, otherwise an > +empty dict. > +""" > +if nic is None: > +nic = find_fallback_nic() > +if nic is None: > +LOG.debug( > +'Skip dhcp_clean_discovery: Unable to find network > interface.') > +return {} > +elif nic not in get_devicelist(): > +LOG.debug( > +'Skip dhcp_clean_discovery: Invalid interface name %s.', nic) > +return {} > +with util.tempdir(prefix='cloud-init-dhcp-') as tmpdir: > +return dhcp_clean_discovery(nic, tmpdir) > + > + > +def parse_dhcp_lease_file(lease_file): > +"""Parse the given dhcp lease file for the most recent lease. > + > +Return a dict of dhcp options as key value pairs for the most recent > lease > +block. > + > +@raises: InvalidDHCPLeaseFileError on empty of unparseable leasefile > +content. > +""" > +lease_regex = re.compile(r"lease {(?P[^}]*)}\n") > +dhcp_leases = [] > +with open(lease_file) as stream: > +lease_content = stream.read() > +if len(lease_content) == 0: > +raise InvalidDHCPLeaseFileError( > +
Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:aws-local-dhcp into cloud-init:master
Review: Needs Fixing continuous-integration FAILED: Continuous integration, rev:f08dc1ea917cbd86a5ee4ab9e343800a6d904104 https://jenkins.ubuntu.com/server/job/cloud-init-ci/113/ Executed test runs: SUCCESS: Checkout FAILED: Unit & Style Tests Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/113/rebuild -- https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/328241 Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:aws-local-dhcp 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] ~smoser/cloud-init:cleanup/get_by_mac_use_get_interfaces into cloud-init:master
Review: Approve continuous-integration PASSED: Continuous integration, rev:9d923c1ab9c4556b980509513ece4a414269b5b9 https://jenkins.ubuntu.com/server/job/cloud-init-ci/112/ Executed test runs: SUCCESS: Checkout SUCCESS: Unit & Style Tests SUCCESS: Ubuntu LTS: Build SUCCESS: Ubuntu LTS: Integration SUCCESS: CentOS 6 & 7: Build & Test IN_PROGRESS: Declarative: Post Actions Click here to trigger a rebuild: https://jenkins.ubuntu.com/server/job/cloud-init-ci/112/rebuild -- https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/328542 Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:cleanup/get_by_mac_use_get_interfaces 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:aws-local-dhcp into cloud-init:master
Diff comments: > diff --git a/cloudinit/sources/DataSourceEc2.py > b/cloudinit/sources/DataSourceEc2.py > index 4ec9592..ae0fe26 100644 > --- a/cloudinit/sources/DataSourceEc2.py > +++ b/cloudinit/sources/DataSourceEc2.py > @@ -73,21 +77,25 @@ 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") But, I'd like to leave the writing of dhclient.conf out of this branch and tackle BSD another time if we could postpone it. > 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_dhcp_clean_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 self._crawl_metadata() > +else: > +return self._crawl_metadata() > > @property > def launch_index(self): -- https://code.launchpad.net/~chad.smith/cloud-init/+git/cloud-init/+merge/328241 Your team cloud-init commiters is requested to review the proposed merge of ~chad.smith/cloud-init:aws-local-dhcp 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:aws-local-dhcp into cloud-init:master
Diff comments: > diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py > new file mode 100644 > index 000..4d59bd0 > --- /dev/null > +++ b/cloudinit/net/dhcp.py > @@ -0,0 +1,118 @@ > +# Copyright (C) 2017 Canonical Ltd. > +# > +# Author: Chad Smith > +# > +# This file is part of cloud-init. See LICENSE file for license information. > + > +import logging > +import os > +import re > + > +from cloudinit.net import find_fallback_nic, get_devicelist > +from cloudinit import util > + > +LOG = logging.getLogger(__name__) > + > + > +class InvalidDHCPLeaseFileError(Exception): > +"""Raised when parsing an empty or invalid dhcp.leases file. > + > +Current uses are DataSourceAzure and DataSourceEc2 during ephemeral > +boot to scrape metadata. > +""" > +pass > + > + > +def maybe_dhcp_clean_discovery(nic=None): > +"""Create a temporary working directory and find the nic if needed. Good point, fixed this and dhcp_clean_discovery > + > +If the device is already connected, do nothing. > + > +@param nic: Name of the network interface we want to run dhclient on. > +@return: A dict of dhcp options from the dhclient discovery, otherwise an > +empty dict. > +""" > +if nic is None: > +nic = find_fallback_nic() > +if nic is None: > +LOG.debug( > +'Skip dhcp_clean_discovery: Unable to find network > interface.') > +return {} > +elif nic not in get_devicelist(): > +LOG.debug( > +'Skip dhcp_clean_discovery: Invalid interface name %s.', nic) done. > +return {} > +with util.tempdir(prefix='cloud-init-dhcp-') as tmpdir: > +return dhcp_clean_discovery(nic, tmpdir) > + > + > +def parse_dhcp_lease_file(lease_file): > +"""Parse the given dhcp lease file for the most recent lease. > + > +Return a dict of dhcp options as key value pairs for the most recent > lease > +block. > + > +@raises: InvalidDHCPLeaseFileError on empty of unparseable leasefile > +content. > +""" > +lease_regex = re.compile(r"lease {(?P[^}]*)}\n") > +dhcp_leases = [] > +with open(lease_file) as stream: > +lease_content = stream.read() done > +if len(lease_content) == 0: > +raise InvalidDHCPLeaseFileError( > +'Cannot parse empty dhcp lease file {0}'.format(lease_file)) > +for lease in lease_regex.findall(lease_content): > +lease_options = [] > +for line in lease.split(';'): > +# Strip newlines, double-quotes and option prefix > +line = line.strip().replace('"', '').replace('option ', '') > +if not line: > +continue > +lease_options.append(line.split(' ', 1)) > +dhcp_leases.append(dict(lease_options)) > +if not dhcp_leases: > +raise InvalidDHCPLeaseFileError( > +'Cannot parse dhcp lease file {0}. No leases found'.format( > +lease_file)) > +return dhcp_leases > + > + > +def dhcp_clean_discovery(interface, cleandir): > +"""Start up dhclient on the provided inteface without side-effects. done. > + > +@param interface: Name of the network inteface on which to dhclient. > +@param cleandir: The directory from which to run dhclient as well as > store > +dhcp leases. > + > +@return: A dict of dhcp options parsed from the dhcp.leases file or empty > +dict. > +""" > +dhclient_script = util.which('dhclient') > +if not dhclient_script: > +LOG.debug('Skip dhclient configuration: No dhclient found.') > +return {} > +LOG.debug('Performing a clean dhcp discovery on %s', interface) > + > +# XXX We copy dhclient out of /sbin/dhclient to avoid dealing with strict > +# app armor profiles which disallow running dhclient -sf > . enabled SELinux doesn't have rules currrently which prevent dhclient from running using a separate dhclient-script, so SELinux passes this case. > +# We want to avoid running /sbin/dhclient-script because of side-effects > in > +# /etc/resolv.conf any any other vendor specific scripts in > +# /etc/dhcp/dhclient*hooks.d. > +dhclient_cmd = os.path.join(cleandir, 'dhclient') > +util.copy(dhclient_script, dhclient_cmd) > +pid_file = os.path.join(cleandir, 'dhclient.pid') > +lease_file = os.path.join(cleandir, 'dhcp.leases') > + > +# dhclient needs the interface up to send initial discovery packets. done > +# Generally dhclient relies on dhclient-script PREINIT action to bring > the > +# link up before attempting discovery. Since we are using -sf /bin/true, > +# we need to do that "link up" ourselves first. > +util.subp(['ip', 'link', 'set', 'dev', interface, 'up'], capture=True) > +dhclient_cmd = [dhclient_cmd, '-1', '-v', '-lf', lease_file, '-pf', > +pid_file, interface, '-sf', '/bin/true'] > +util.subp(dhclient_cmd, capture=True) > +retu
Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:ntp-configure-timesyncd-fallback-lp-1686485 into cloud-init:master
Thanks for the review, I'll fix the issues you raised and push in an update. Diff comments: > diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py > index 31ed64e..0b92a40 100644 > --- a/cloudinit/config/cc_ntp.py > +++ b/cloudinit/config/cc_ntp.py > @@ -185,19 +217,25 @@ def write_ntp_config_template(cfg, cloud): > 'pools': pools, > } > > -template_fn = cloud.get_template_filename('ntp.conf.%s' % > - (cloud.distro.name)) > +if template is None: > +template = 'ntp.conf.%s' % cloud.distro.name > + > +if target is None: OK > +target = NTP_CONF > + > +template_fn = cloud.get_template_filename(template) > if not template_fn: > template_fn = cloud.get_template_filename('ntp.conf') > if not template_fn: > raise RuntimeError(("No template found, " > -"not rendering %s"), NTP_CONF) > +"not rendering %s"), target) > > -templater.render_to_file(template_fn, NTP_CONF, params) > +templater.render_to_file(template_fn, target, params) > > > -def reload_ntp(systemd=False): > -service = 'ntp' > +def reload_ntp(systemd=False, service=None): > +if service is None: > +service = 'ntp' OK > if systemd: > cmd = ['systemctl', 'reload-or-restart', service] > else: -- https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/328427 Your team cloud-init commiters is requested to review the proposed merge of ~raharper/cloud-init:ntp-configure-timesyncd-fallback-lp-1686485 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] ~smoser/cloud-init:cleanup/get_by_mac_use_get_interfaces into cloud-init:master
Scott Moser has proposed merging ~smoser/cloud-init:cleanup/get_by_mac_use_get_interfaces into cloud-init:master. Commit message: net: Reduce duplicate code. Have get_interfaces_by_mac use get_interfaces. get_interfaces_by_mac and get_interfaces just looked much alike. This makes get_interfaces_by_mac call get_interfaces. Requested reviews: cloud-init commiters (cloud-init-dev) For more details, see: https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/328542 -- Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:cleanup/get_by_mac_use_get_interfaces into cloud-init:master. diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py index 46cb9c8..1ff8fae 100644 --- a/cloudinit/net/__init__.py +++ b/cloudinit/net/__init__.py @@ -511,21 +511,7 @@ def get_interfaces_by_mac(): Bridges and any devices that have a 'stolen' mac are excluded.""" ret = {} -devs = get_devicelist() -empty_mac = '00:00:00:00:00:00' -for name in devs: -if not interface_has_own_mac(name): -continue -if is_bridge(name): -continue -if is_vlan(name): -continue -mac = get_interface_mac(name) -# some devices may not have a mac (tun0) -if not mac: -continue -if mac == empty_mac and name != 'lo': -continue +for name, mac, _driver, _devid in get_interfaces(): if mac in ret: raise RuntimeError( "duplicate mac found! both '%s' and '%s' have mac '%s'" % ___ 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] ~raharper/cloud-init:ntp-configure-timesyncd-fallback-lp-1686485 into cloud-init:master
some small things. i assume you were as much trying to avoid test changes as anything else, but the defaults we have there seem simplistic. Diff comments: > diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py > index 31ed64e..0b92a40 100644 > --- a/cloudinit/config/cc_ntp.py > +++ b/cloudinit/config/cc_ntp.py > @@ -185,19 +217,25 @@ def write_ntp_config_template(cfg, cloud): > 'pools': pools, > } > > -template_fn = cloud.get_template_filename('ntp.conf.%s' % > - (cloud.distro.name)) > +if template is None: > +template = 'ntp.conf.%s' % cloud.distro.name > + > +if target is None: lets just make this a required variable. and can we name it 'path'? target is just used other places to mean "root of filesystem". > +target = NTP_CONF > + > +template_fn = cloud.get_template_filename(template) > if not template_fn: > template_fn = cloud.get_template_filename('ntp.conf') > if not template_fn: > raise RuntimeError(("No template found, " > -"not rendering %s"), NTP_CONF) > +"not rendering %s"), target) > > -templater.render_to_file(template_fn, NTP_CONF, params) > +templater.render_to_file(template_fn, target, params) > > > -def reload_ntp(systemd=False): > -service = 'ntp' > +def reload_ntp(systemd=False, service=None): > +if service is None: > +service = 'ntp' i think since you change all the callers, just change the signature too. it requires a service name. > if systemd: > cmd = ['systemctl', 'reload-or-restart', service] > else: -- https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/328427 Your team cloud-init commiters is requested to review the proposed merge of ~raharper/cloud-init:ntp-configure-timesyncd-fallback-lp-1686485 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:aws-local-dhcp into cloud-init:master
This looks really good. Just some questions and clarifications in inline comments. Diff comments: > diff --git a/cloudinit/net/__init__.py b/cloudinit/net/__init__.py > index 46cb9c8..d38ea8b 100644 > --- a/cloudinit/net/__init__.py > +++ b/cloudinit/net/__init__.py > @@ -233,15 +231,24 @@ def generate_fallback_config(blacklist_drivers=None, > config_driver=None): > if DEFAULT_PRIMARY_INTERFACE in names: > names.remove(DEFAULT_PRIMARY_INTERFACE) > names.insert(0, DEFAULT_PRIMARY_INTERFACE) > -target_name = None > -target_mac = None > + > +# pick the first that has a address Hrm; this seems risky; in that it may return a different answer between subsequent calls if more than one interface is up. It seems somewhat wasteful to build a list of names with 'address' value so we can sort it but I don't like the variability in the return value. > for name in names: > -mac = read_sys_net_safe(name, 'address') > -if mac: > -target_name = name > -target_mac = mac > -break > -if target_mac and target_name: > +if read_sys_net_safe(name, 'address'): > +return name > +return None > + > + > +def generate_fallback_config(blacklist_drivers=None, config_driver=None): > +"""Determine which attached net dev is most likely to have a connection > and > + generate network state to run dhcp on that interface""" > + > +if not config_driver: > +config_driver = False > + > +target_name = find_fallback_nic(blacklist_drivers=blacklist_drivers) > +if target_name: > +target_mac = read_sys_net_safe(target_name, 'address') > nconf = {'config': [], 'version': 1} > cfg = {'type': 'physical', 'name': target_name, > 'mac_address': target_mac, 'subnets': [{'type': 'dhcp'}]} > diff --git a/cloudinit/net/dhcp.py b/cloudinit/net/dhcp.py > new file mode 100644 > index 000..4d59bd0 > --- /dev/null > +++ b/cloudinit/net/dhcp.py > @@ -0,0 +1,118 @@ > +# Copyright (C) 2017 Canonical Ltd. > +# > +# Author: Chad Smith > +# > +# This file is part of cloud-init. See LICENSE file for license information. > + > +import logging > +import os > +import re > + > +from cloudinit.net import find_fallback_nic, get_devicelist > +from cloudinit import util > + > +LOG = logging.getLogger(__name__) > + > + > +class InvalidDHCPLeaseFileError(Exception): > +"""Raised when parsing an empty or invalid dhcp.leases file. > + > +Current uses are DataSourceAzure and DataSourceEc2 during ephemeral > +boot to scrape metadata. > +""" > +pass > + > + > +def maybe_dhcp_clean_discovery(nic=None): > +"""Create a temporary working directory and find the nic if needed. The function name and docstring don't seem clear to me. What's the 'clean' part? It doesn't always create a tmpdir, only if we have a nic and it's not up. I think the "clean" is an implementation detail we don't need to expose in the function name. maybe_perform_dhcp_discovery() is't alot better name-wise (I'm happy to help bikeshed), but I think we can say something like: Determine if we need to perform a DHCP DISCOVERY operation on ``nic`` if it's not up. Utilize a tempdir to prevent dhclient from disturbing the host filesystem. Something like that. > + > +If the device is already connected, do nothing. > + > +@param nic: Name of the network interface we want to run dhclient on. > +@return: A dict of dhcp options from the dhclient discovery, otherwise an > +empty dict. > +""" > +if nic is None: > +nic = find_fallback_nic() > +if nic is None: > +LOG.debug( > +'Skip dhcp_clean_discovery: Unable to find network > interface.') > +return {} > +elif nic not in get_devicelist(): > +LOG.debug( > +'Skip dhcp_clean_discovery: Invalid interface name %s.', nic) Since this is a debug message, then maybe something like nic '%s' not found in get_devicelist() or system network device list. > +return {} > +with util.tempdir(prefix='cloud-init-dhcp-') as tmpdir: > +return dhcp_clean_discovery(nic, tmpdir) > + > + > +def parse_dhcp_lease_file(lease_file): > +"""Parse the given dhcp lease file for the most recent lease. > + > +Return a dict of dhcp options as key value pairs for the most recent > lease > +block. > + > +@raises: InvalidDHCPLeaseFileError on empty of unparseable leasefile > +content. > +""" > +lease_regex = re.compile(r"lease {(?P[^}]*)}\n") > +dhcp_leases = [] > +with open(lease_file) as stream: > +lease_content = stream.read() util.load_file() > +if len(lease_content) == 0: > +raise InvalidDHCPLeaseFileError( > +'Cannot parse empty dhcp lease file {0}'.format(lease_file)) > +for lease in lease_regex.findall(lease_content): > +lease_options = [] > +for line in