Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:aws-local-dhcp into cloud-init:master

[Server Team CI bot]

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

[Server Team CI bot]

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

[Chad Smith]

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

[Scott Moser]

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 

Re: [Cloud-init-dev] [Merge] ~chad.smith/cloud-init:aws-local-dhcp into cloud-init:master

[Server Team CI bot]

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

[Server Team CI bot]

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

[Chad Smith]

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

[Chad Smith]

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']
> +

Re: [Cloud-init-dev] [Merge] ~raharper/cloud-init:ntp-configure-timesyncd-fallback-lp-1686485 into cloud-init:master

[Ryan Harper]

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]

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

[Scott Moser]

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