Re: [Cloud-init-dev] [Merge] ~wesley-wiedenmeier/cloud-init:integration-testing-invocation-cleanup into cloud-init:master
Timeouts will no longer be an issue for bddeb as dep installation is now handled after boot. I think this should be ready to merge, I've tested again with tree_collect, tree_run, and just with a normal 'run --ppa' -- https://code.launchpad.net/~wesley-wiedenmeier/cloud-init/+git/cloud-init/+merge/314496 Your team cloud init development team is requested to review the proposed merge of ~wesley-wiedenmeier/cloud-init:integration-testing-invocation-cleanup 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] ~msaikia/cloud-init:topic-msaikia-vmware into cloud-init:master
Diff comments:
> diff --git a/cloudinit/sources/helpers/vmware/imc/config.py
> b/cloudinit/sources/helpers/vmware/imc/config.py
> index d645c49..711dc0b 100644
> --- a/cloudinit/sources/helpers/vmware/imc/config.py
> +++ b/cloudinit/sources/helpers/vmware/imc/config.py
> @@ -28,11 +30,15 @@ class Config(object):
>
> DNS = 'DNS|NAMESERVER|'
> SUFFIX = 'DNS|SUFFIX|'
> -PASS = 'PASSWORD|-PASS'
> TIMEZONE = 'DATETIME|TIMEZONE'
> UTC = 'DATETIME|UTC'
> HOSTNAME = 'NETWORK|HOSTNAME'
> DOMAINNAME = 'NETWORK|DOMAINNAME'
> +CUSTOM_SCRIPT = 'CUSTOM-SCRIPT|SCRIPT-NAME'
> +PASS = 'PASSWORD|-PASS'
> +RESETPASS = 'PASSWORD|RESET'
> +MARKERID = 'MISC|MARKER-ID'
> +POST_GC = 'MISC|POST-GC-STATUS'
Thanks. I just wanted to keep functions related to a certain product together.
But sorted sounds good too.
>
> def __init__(self, configFile):
> self._configFile = configFile
> @@ -93,3 +94,34 @@ class Config(object):
> res.append(Nic(nic, self._configFile))
>
> return res
> +
> +@property
> +def admin_password(self):
> +"""Return the root password to be set."""
> +return self._configFile.get(Config.PASS, None)
The reason i moved this was just to keep all the methods associated to a
particular product together. But it is nothing critical. So I can move it back
to where it was.
> +
> +@property
> +def reset_password(self):
> +"""Retreives if the root password needs to be reset."""
> +resetPass = self._configFile.get(Config.RESETPASS, None)
> +if resetPass and not re.match('yes$|no$', resetPass.lower()):
> +raise ValueError('ResetPassword value should be yes/no')
> +return resetPass
> +
> +@property
> +def marker_id(self):
> +"""Returns marker id."""
> +return self._configFile.get(Config.MARKERID, None)
> +
> +@property
> +def post_gc_status(self):
> +"""Retreives if customization status needs to be posted."""
> +postGcStatus = self._configFile.get(Config.POST_GC, None)
> +if postGcStatus and not re.match('yes$|no$', postGcStatus.lower()):
> +raise ValueError('GC status value should be yes/no')
> +return postGcStatus
> +
> +@property
> +def custom_script_name(self):
> +"""Return the name of custom (pre/post) script."""
> +return self._configFile.get(Config.CUSTOM_SCRIPT, None)
--
https://code.launchpad.net/~msaikia/cloud-init/+git/cloud-init/+merge/305427
Your team cloud init development team is requested to review the proposed merge
of ~msaikia/cloud-init:topic-msaikia-vmware 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] ~msaikia/cloud-init:topic-msaikia-vmware into cloud-init:master
> Please set a combined commit message ('Set commit message' above.
>
> Can you explain where you're going also?
> I'm concerned about adding more "vmware" paths for configuring things that are
> done elsewhere (or not elsewhere) in cloud-init. We'd like have consistent
> paths for doing things as much as possible.
>
> I understand that you're trying to have cloud-init take over another more
> solution, and I'm not entirely opposed to that, but I want to integrate as
> much as possible rather than having specific paths and function on vmware.
Thanks for your input Scott. I have updated the commit message.
This changeset is just to retrieve data from customization spec, for
implementing some other customization functionalities like setting password,
running post customization scripts etc. I shall post another changeset with the
password configurator, which manipulates the values retrieved from these
methods to be used by cloud-init's cc_set_password. We will be using as much
cloud-init code as possible. For other functionalities like running pre and
post customization specs that will be uploaded by the user, I dont think there
is existing support. Please let me know if that sounds ok. Also for functions
that are "vmware" specific, shall we still keep it in the helper path that we
have created for vmware, or do we want to move them to some common code area?
Thanks
Maitreyee
--
https://code.launchpad.net/~msaikia/cloud-init/+git/cloud-init/+merge/305427
Your team cloud init development team is requested to review the proposed merge
of ~msaikia/cloud-init:topic-msaikia-vmware 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] ~msaikia/cloud-init:topic-msaikia-vmware into cloud-init:master
The proposal to merge ~msaikia/cloud-init:topic-msaikia-vmware into cloud-init:master has been updated. Commit Message changed to: Guest Customization support for product vcloud director - part 1 This changeset is to retrieve data from customization spec, for performing certain customization functions. For more details, see: https://code.launchpad.net/~msaikia/cloud-init/+git/cloud-init/+merge/305427 -- Your team cloud init development team is requested to review the proposed merge of ~msaikia/cloud-init:topic-msaikia-vmware 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] ~wesley-wiedenmeier/cloud-init:integration-testing-invocation-cleanup into cloud-init:master
Review: Approve LGTM with two comments: 1) Had to increase the timeout as 120 seconds was not enough. I bumped to 600. 2) the tox target "cittest_run" actually does a "tree_run" In order to not be confused between run versus tree_run I think this should really be "cittest_tree_run" Tested by running the following on my local system with pylxd 2.1.3: # lint $ tox # runs with options $ python3 -m tests.cloud_tests run -v -n trusty $ python3 -m tests.cloud_tests run -v -n xenial $ python3 -m tests.cloud_tests run -v -n yakkety --repo 'deb http://archive.ubuntu.com/ubuntu/ yakkety main' $ python3 -m tests.cloud_tests run -v -n xenial --deb cloud-init_0.7.9-0ubuntu1_all.deb # new tox based runs + tree_run $ tox -e citest_run $ tox -e citest -- tree_run -n zesty -t tests/cloud_tests/configs/modules/write_files.yaml $ python3 -m tests.cloud_tests tree_run -v -n zesty -t tests/cloud_tests/configs/modules/timezone.yaml lxc list was clean afterwards as expected. I believe I only missed a ppa test. The only warning message I got other than those for pylxd's 2.2 deprecation was: /home/powersj/Work/repos/cloud-init-wesley/tests/cloud_tests/collect.py:48: DeprecationWarning: The 'warn' method is deprecated, use 'warning' instead -- https://code.launchpad.net/~wesley-wiedenmeier/cloud-init/+git/cloud-init/+merge/314496 Your team cloud init development team is requested to review the proposed merge of ~wesley-wiedenmeier/cloud-init:integration-testing-invocation-cleanup 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] ~harlowja/cloud-init:no-cheetah into cloud-init:master
that could be a solution; i'd be ok with that, other option is to release cloud-init 1.0 and switch it then. -- https://code.launchpad.net/~harlowja/cloud-init/+git/cloud-init/+merge/308304 Your team cloud init development team is requested to review the proposed merge of ~harlowja/cloud-init:no-cheetah 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] ~raharper/cloud-init:bug-lp-1645644-ntp into cloud-init:master
The proposal to merge ~raharper/cloud-init:bug-lp-1645644-ntp into cloud-init:master has been updated. Description changed to: cc_ntp: write template before installing and add service restart On systems which installed ntp and specified servers or pools in the config ntpd didn't notice the updated configuration file and didn't use the correct configuration. Resolve this by rendering the template first which allows the package install to use the existing configuration. Additionally add a service restart to handle the case where ntp does not need to be installed but it may not have started. Add an integration test to confirm that cc_ntp enables ntp to use the specific servers and pools in the cloud-config. LP: #1645644 For more details, see: https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/314539 -- Your team cloud init development team is requested to review the proposed merge of ~raharper/cloud-init:bug-lp-1645644-ntp 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] ~raharper/cloud-init:bug-lp-1645644-ntp into cloud-init:master
Ryan Harper has proposed merging ~raharper/cloud-init:bug-lp-1645644-ntp into
cloud-init:master.
Requested reviews:
cloud init development team (cloud-init-dev)
Related bugs:
Bug #1645644 in cloud-init: "ntp not using expected servers"
https://bugs.launchpad.net/cloud-init/+bug/1645644
For more details, see:
https://code.launchpad.net/~raharper/cloud-init/+git/cloud-init/+merge/314539
cc_ntp: write template before installing and add service restart
On systems which installed ntp and specified servers or pools in the config
ntpd didn't notice the updated configuration file and didn't use the
correct configuration. Resolve this by rendering the template first which
allows the package install to use the existing configuration. Additionally
add a service restart to handle the case where ntp does not need to be installed
but it may not have started.
Add an integration test to confirm that cc_ntp enables ntp to use the specific
servers and pools in the cloud-config.
LP: #1645644
--
Your team cloud init development team is requested to review the proposed merge
of ~raharper/cloud-init:bug-lp-1645644-ntp into cloud-init:master.
diff --git a/cloudinit/config/cc_ntp.py b/cloudinit/config/cc_ntp.py
index e33032f..15813bf 100644
--- a/cloudinit/config/cc_ntp.py
+++ b/cloudinit/config/cc_ntp.py
@@ -74,10 +74,18 @@ def handle(name, cfg, cloud, log, _args):
"not present or disabled by cfg", name)
return True
-install_ntp(cloud.distro.install_packages, packages=['ntp'],
-check_exe="ntpd")
rename_ntp_conf()
+# ensure when ntp is installed it has a configuration file
+# to use instead of starting up with packaged defaults
write_ntp_config_template(ntp_cfg, cloud)
+install_ntp(cloud.distro.install_packages, packages=['ntp'],
+check_exe="ntpd")
+
+# if ntp was already installed, it may not have started
+try:
+reload_ntp(systemd=cloud.distro.uses_systemd())
+except util.ProcessExecutionError as e:
+log.warn("Failed to reload/start ntp service", e)
def install_ntp(install_func, packages=None, check_exe="ntpd"):
@@ -90,6 +98,7 @@ def install_ntp(install_func, packages=None, check_exe="ntpd"):
def rename_ntp_conf(config=NTP_CONF):
+""" Rename any existing ntp.conf file and render from template """
if os.path.exists(config):
util.rename(config, config + ".dist")
@@ -125,4 +134,14 @@ def write_ntp_config_template(cfg, cloud):
templater.render_to_file(template_fn, NTP_CONF, params)
+
+def reload_ntp(systemd=False):
+service = 'ntp'
+if systemd:
+cmd = ['systemctl', 'reload-or-restart', service]
+else:
+cmd = ['service', service, 'restart']
+util.subp(cmd, capture=True)
+
+
# vi: ts=4 expandtab
diff --git a/tests/cloud_tests/configs/modules/ntp_pools.yaml b/tests/cloud_tests/configs/modules/ntp_pools.yaml
index bd0ac29..e040cc3 100644
--- a/tests/cloud_tests/configs/modules/ntp_pools.yaml
+++ b/tests/cloud_tests/configs/modules/ntp_pools.yaml
@@ -5,10 +5,9 @@ cloud_config: |
#cloud-config
ntp:
pools:
-- 0.pool.ntp.org
-- 1.pool.ntp.org
-- 2.pool.ntp.org
-- 3.pool.ntp.org
+- 0.cloud-init.mypool
+- 1.cloud-init.mypool
+- 172.16.15.14
collect_scripts:
ntp_installed_pools: |
#!/bin/bash
@@ -19,5 +18,8 @@ collect_scripts:
ntp_conf_pools: |
#!/bin/bash
grep '^pool' /etc/ntp.conf
+ ntpq_servers: |
+#!/bin/sh
+ntpq -p -w
# vi: ts=4 expandtab
diff --git a/tests/cloud_tests/configs/modules/ntp_servers.yaml b/tests/cloud_tests/configs/modules/ntp_servers.yaml
index 934b9c5..e0564a0 100644
--- a/tests/cloud_tests/configs/modules/ntp_servers.yaml
+++ b/tests/cloud_tests/configs/modules/ntp_servers.yaml
@@ -5,16 +5,20 @@ cloud_config: |
#cloud-config
ntp:
servers:
-- pool.ntp.org
+- 172.16.15.14
+- 172.16.17.18
collect_scripts:
ntp_installed_servers: |
-#!/bin/bash
-dpkg -l | grep ntp | wc -l
+#!/bin/sh
+dpkg -l | grep -c ntp
ntp_conf_dist_servers: |
-#!/bin/bash
-ls /etc/ntp.conf.dist | wc -l
+#!/bin/sh
+cat /etc/ntp.conf.dist | wc -l
ntp_conf_servers: |
-#!/bin/bash
+#!/bin/sh
grep '^server' /etc/ntp.conf
+ ntpq_servers: |
+#!/bin/sh
+ntpq -p -w
# vi: ts=4 expandtab
diff --git a/tests/cloud_tests/testcases/modules/ntp.py b/tests/cloud_tests/testcases/modules/ntp.py
index b111925..82d3288 100644
--- a/tests/cloud_tests/testcases/modules/ntp.py
+++ b/tests/cloud_tests/testcases/modules/ntp.py
@@ -13,9 +13,9 @@ class TestNtp(base.CloudTestCase):
self.assertEqual(1, int(out))
def test_ntp_dist_entries(self):
-"""Test dist config file has one entry"""
+"""Test dist config file is empty"""
out = self.get_data_file('ntp_conf_dist_empty')
-self.assertEqual(1, int(out))
+self.assertEqual(0,
Re: [Cloud-init-dev] [Merge] ~smoser/cloud-init:bug/noise-on-cmdline-url-fail into cloud-init:master
Thanks for the review, your comments seem sane.
I really do want to get this in, as currently this is a common path for failure
in maas (node can't reach region controller) and there isn't much path as to
what is going on there.
Diff comments:
> diff --git a/cloudinit/cmd/main.py b/cloudinit/cmd/main.py
> index 26c0240..5005d69 100644
> --- a/cloudinit/cmd/main.py
> +++ b/cloudinit/cmd/main.py
> @@ -140,23 +141,97 @@ def apply_reporting_cfg(cfg):
> reporting.update_configuration(cfg.get('reporting'))
>
>
> +def parse_cmdline_url(cmdline, names=('cloud-config-url', 'url')):
> +data = util.keyval_str_to_dict(cmdline)
> +for key in names:
> +if key in data:
> +return data[key]
> +return None
> +
> +
> +def attempt_cmdline_url(path, network=True, cmdline=None):
> +"""Write data from url referenced in command line to path.
> +
> +path: a file to write content to if downloaded.
> +network: should network access be assumed.
> +cmdline: the cmdline to parse for cloud-config-url.
> +
> +This is used in MAAS datasource, in "ephemeral" (read-only root)
> +environment where the instance netboots to iscsi ro root.
> +and the entity that controls the pxe config has to configure
> +the maas datasource.
> +
> +An attempt is made on network urls even in local datasource
> +for case of network set up in initramfs.
> +
> +Return value is a tuple of a logger function (logging.DEBUG)
> +and a message indicating what happened.
> +"""
> +
> +if cmdline is None:
> +cmdline = util.get_cmdline()
> +
> +url = parse_cmdline_url(cmdline, names=('cloud-config-url', 'url'))
yeah, i guess we can drop the names= here.
> +if not url:
> +return (None, None)
true, we could return debug on that.
> +
> +path_is_local = url.startswith("file://") or url.startswith("/")
> +
> +if path_is_local and os.path.exists(path):
> +if network:
> +m = ("file '%s' existed, possibly from local stage download"
> + " of command line url '%s'. Not re-writing." % (path, url))
> +level = logging.INFO
> +if path_is_local:
> +level = logging.DEBUG
> +else:
> +m = ("file '%s' existed, possibly from previous boot download"
> + " of command line url '%s'. Not re-writing." % (path, url))
> +level = logging.WARN
> +
> +return (level, m)
> +
> +kwargs = {'url': url, 'timeout': 10, 'retries': 2}
> +if network or path_is_local:
> +level = logging.WARN
> +kwargs['sec_between'] = 1
> +else:
> +level = logging.DEBUG
> +kwargs['sec_between'] = .1
> +
> +data = None
> +header = b'#cloud-config'
> +try:
> +resp = util.read_file_or_url(**kwargs)
> +if resp.ok():
> +data = resp.contents
> +if not resp.contents.startswith(header):
> +return (
> +logging.INFO,
I guess this could be made to be an error. One reason for 'info' only would be
that url=http:// is not 100% obviously for cloud-init. So this is not
really an error if something else would consume that. Perhaps this is an error
if cloud-config-url= but only info if url=
> +"contents of '%s' did not start with %s" % (url, header))
> +else:
> +return (level,
> +"url '%s' returned code %s. Ignoring." % (url,
> resp.code))
> +
> +except url_helper.UrlError as e:
> +return (level, "retrieving url '%s' failed: %s" % (url, e))
> +
> +util.write_file(path, data, mode=0o600)
> +return (logging.INFO,
> +"wrote cloud-config data from '%s' to %s" % (url, path))
> +
> +
> def main_init(name, args):
> deps = [sources.DEP_FILESYSTEM, sources.DEP_NETWORK]
> if args.local:
> deps = [sources.DEP_FILESYSTEM]
>
> -if not args.local:
> -# See doc/kernel-cmdline.txt
> -#
> -# This is used in maas datasource, in "ephemeral" (read-only root)
> -# environment where the instance netboots to iscsi ro root.
> -# and the entity that controls the pxe config has to configure
> -# the maas datasource.
> -#
> -# Could be used elsewhere, only works on network based (not local).
> -root_name = "%s.d" % (CLOUD_CONFIG)
> -target_fn = os.path.join(root_name, "91_kernel_cmdline_url.cfg")
> -util.read_write_cmdline_url(target_fn)
> +early_logs = []
> +_lvl, _msg = attempt_cmdline_url(
> +path=os.path.join("%.d" % CLOUD_CONFIG, "91_kernel_cmdline_url.cfg"),
> +network=not args.local)
> +if _lvl:
> +early_logs.extend(_lvl, _msg)
>
> # Cloud-init 'init' stage is broken up into the following sub-stages
> # 1. Ensure that the init object fetches its config without errors
--
