From 7ea02ab1d8ee0f400a84ee2d688e67ffbc449bf0 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Wed, 7 Nov 2012 21:00:33 -0800 Subject: Add a makefile yaml checking target and fix the cases where the cc yaml is not correct. --- Makefile | 9 ++++++++- doc/examples/cloud-config.txt | 3 +-- tests/configs/sample1.yaml | 1 - tools/validate-yaml.py | 20 ++++++++++++++++++++ 4 files changed, 29 insertions(+), 4 deletions(-) create mode 100755 tools/validate-yaml.py diff --git a/Makefile b/Makefile index 88c90b9b..2a6be961 100644 --- a/Makefile +++ b/Makefile @@ -2,6 +2,10 @@ CWD=$(shell pwd) PY_FILES=$(shell find cloudinit bin tests tools -name "*.py" -type f ) PY_FILES+="bin/cloud-init" +YAML_FILES=$(shell find cloudinit bin tests tools -name "*.yaml" -type f ) +YAML_FILES+=$(shell find doc/examples -name "cloud-config*.txt" -type f ) + + all: test pep8: @@ -23,11 +27,14 @@ clean: rm -rf /var/log/cloud-init.log \ /var/lib/cloud/ +yaml: + @$(CWD)/tools/validate-yaml.py $(YAML_FILES) + rpm: ./packages/brpm deb: ./packages/bddeb -.PHONY: test pylint pyflakes 2to3 clean pep8 rpm deb +.PHONY: test pylint pyflakes 2to3 clean pep8 rpm deb yaml diff --git a/doc/examples/cloud-config.txt b/doc/examples/cloud-config.txt index 04bb5df1..12bf2c91 100644 --- a/doc/examples/cloud-config.txt +++ b/doc/examples/cloud-config.txt @@ -355,8 +355,7 @@ rsyslog: - ':syslogtag, isequal, "[CLOUDINIT]" /var/log/cloud-foo.log' - content: "*.* @@192.0.2.1:10514" - filename: 01-examplecom.conf - content: | - *.* @@syslogd.example.com + content: "*.* @@syslogd.example.com" # resize_rootfs should the / filesytem be resized on first boot # this allows you to launch an instance with a larger disk / partition diff --git a/tests/configs/sample1.yaml b/tests/configs/sample1.yaml index 24e874ee..6231f293 100644 --- a/tests/configs/sample1.yaml +++ b/tests/configs/sample1.yaml @@ -50,4 +50,3 @@ runcmd: byobu_by_default: user -output: {all: '| tee -a /var/log/cloud-init-output.log'} diff --git a/tools/validate-yaml.py b/tools/validate-yaml.py new file mode 100755 index 00000000..d3218e40 --- /dev/null +++ b/tools/validate-yaml.py @@ -0,0 +1,20 @@ +#!/usr/bin/env python + +"""Try to read a YAML file and report any errors. +""" + +import sys + +import yaml + + +if __name__ == "__main__": + for fn in sys.argv[1:]: + sys.stdout.write("%s" % (fn)) + try: + fh = open(fn, 'r') + yaml.safe_load(fh.read()) + fh.close() + sys.stdout.write(" - ok\n") + except Exception, e: + sys.stdout.write(" - bad (%s)\n" % (e)) -- cgit v1.2.3 From 791598f2929a5b8b6bb380f7f16ec568db96aba6 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Wed, 7 Nov 2012 21:34:41 -0800 Subject: Start adding a 'migrator' module that can be used to aid in the moving of older versions of cloud-inits data to newer versions of cloud-inits data. 1. Move the semaphores for the current instance to there canonicalized names and use the canonicalized in the file 'locking' code --- cloudinit/config/cc_migrator.py | 53 +++++++++++++++++++++++++++++++++++++++++ cloudinit/helpers.py | 9 ++++++- config/cloud.cfg | 1 + 3 files changed, 62 insertions(+), 1 deletion(-) create mode 100644 cloudinit/config/cc_migrator.py diff --git a/cloudinit/config/cc_migrator.py b/cloudinit/config/cc_migrator.py new file mode 100644 index 00000000..b71d1d17 --- /dev/null +++ b/cloudinit/config/cc_migrator.py @@ -0,0 +1,53 @@ +# vi: ts=4 expandtab +# +# Copyright (C) 2012 Yahoo! Inc. +# +# Author: Joshua Harlow +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License version 3, as +# published by the Free Software Foundation. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +import os +import shutil + +from cloudinit import helpers +from cloudinit import util + +from cloudinit.settings import PER_ALWAYS + +frequency = PER_ALWAYS + + +def _migrate_canon_sems(cloud): + sem_path = cloud.paths.get_ipath('sem') + if not sem_path or not os.path.exists(sem_path): + return 0 + am_adjusted = 0 + for p in os.listdir(sem_path): + full_path = os.path.join(sem_path, p) + if os.path.isfile(full_path): + canon_p = helpers.canon_sem_name(p) + if canon_p != p: + new_path = os.path.join(sem_path, p) + shutil.move(full_path, new_path) + am_adjusted += 1 + return am_adjusted + + +def handle(name, cfg, cloud, log, _args): + do_migrate = util.get_cfg_option_str(cfg, "migrate", True) + if not util.translate_bool(do_migrate): + log.debug("Skipping module named %s, migration disabled", name) + return + sems_moved = _migrate_canon_sems(cloud) + log.debug("Migrated %s semaphore files to there canonicalized names", + sems_moved) diff --git a/cloudinit/helpers.py b/cloudinit/helpers.py index 985ce3e5..d26625a0 100644 --- a/cloudinit/helpers.py +++ b/cloudinit/helpers.py @@ -71,12 +71,17 @@ class FileLock(object): return "<%s using file %r>" % (util.obj_name(self), self.fn) +def canon_sem_name(name): + return name.replace("-", "_") + + class FileSemaphores(object): - def __init__(self, sem_path): + def __init__(self, sem_path): self.sem_path = sem_path @contextlib.contextmanager def lock(self, name, freq, clear_on_fail=False): + name = canon_sem_name(name) try: yield self._acquire(name, freq) except: @@ -85,6 +90,7 @@ class FileSemaphores(object): raise def clear(self, name, freq): + name = canon_sem_name(name) sem_file = self._get_path(name, freq) try: util.del_file(sem_file) @@ -119,6 +125,7 @@ class FileSemaphores(object): def has_run(self, name, freq): if not freq or freq == PER_ALWAYS: return False + name = canon_sem_name(name) sem_file = self._get_path(name, freq) # This isn't really a good atomic check # but it suffices for where and when cloudinit runs diff --git a/config/cloud.cfg b/config/cloud.cfg index 05bb4eef..ad100fff 100644 --- a/config/cloud.cfg +++ b/config/cloud.cfg @@ -23,6 +23,7 @@ preserve_hostname: false # The modules that run in the 'init' stage cloud_init_modules: + - migrator - bootcmd - write-files - resizefs -- cgit v1.2.3 From 3de3c535f37e40a79b36997a93fa218534117397 Mon Sep 17 00:00:00 2001 From: harlowja Date: Wed, 7 Nov 2012 23:16:21 -0800 Subject: 1. Check the name and not the full path when applying the canon routine. 2. Add in a function to migrate legacy semaphores to new semaphores. --- cloudinit/config/cc_migrator.py | 34 +++++++++++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 3 deletions(-) diff --git a/cloudinit/config/cc_migrator.py b/cloudinit/config/cc_migrator.py index b71d1d17..56f33d42 100644 --- a/cloudinit/config/cc_migrator.py +++ b/cloudinit/config/cc_migrator.py @@ -35,14 +35,41 @@ def _migrate_canon_sems(cloud): for p in os.listdir(sem_path): full_path = os.path.join(sem_path, p) if os.path.isfile(full_path): - canon_p = helpers.canon_sem_name(p) - if canon_p != p: - new_path = os.path.join(sem_path, p) + (name, ext) = os.path.splitext(p) + canon_name = helpers.canon_sem_name(name) + if canon_name != name: + new_path = os.path.join(sem_path, canon_name + ext) shutil.move(full_path, new_path) am_adjusted += 1 return am_adjusted +def _migrate_legacy_sems(cloud, log): + sem_path = cloud.paths.get_ipath('sem') + touch_there = { + 'apt-update-upgrade': [ + 'apt-configure', + 'package-update-upgrade-install', + ], + } + sem_helper = helpers.FileSemaphores(sem_path) + for (mod_name, migrate_to) in touch_there.items(): + possibles = [mod_name, helpers.canon_sem_name(mod_name)] + old_exists = [] + for p in os.listdir(sem_path): + (name, _ext) = os.path.splitext(p) + if name in possibles and os.path.isfile(p): + old_exists.append(p) + for p in old_exists: + util.del_file(os.path.join(sem_path, p)) + (_name, freq) = os.path.splitext(p) + for m in migrate_to: + log.debug("Migrating %s => %s with the same frequency", + p, m) + with sem_helper.lock(m, freq): + pass + + def handle(name, cfg, cloud, log, _args): do_migrate = util.get_cfg_option_str(cfg, "migrate", True) if not util.translate_bool(do_migrate): @@ -51,3 +78,4 @@ def handle(name, cfg, cloud, log, _args): sems_moved = _migrate_canon_sems(cloud) log.debug("Migrated %s semaphore files to there canonicalized names", sems_moved) + _migrate_legacy_sems(cloud, log) -- cgit v1.2.3 From 196badd4cfa5f9f76be1138fb2d073649af3e031 Mon Sep 17 00:00:00 2001 From: harlowja Date: Wed, 7 Nov 2012 23:20:40 -0800 Subject: 1. Ensure that the sem_path exists and is actually a valid value returned. 2. Adjust variable naming --- cloudinit/config/cc_migrator.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cloudinit/config/cc_migrator.py b/cloudinit/config/cc_migrator.py index 56f33d42..58232fc9 100644 --- a/cloudinit/config/cc_migrator.py +++ b/cloudinit/config/cc_migrator.py @@ -46,14 +46,16 @@ def _migrate_canon_sems(cloud): def _migrate_legacy_sems(cloud, log): sem_path = cloud.paths.get_ipath('sem') - touch_there = { + if not sem_path or not os.path.exists(sem_path): + return + legacy_adjust = { 'apt-update-upgrade': [ 'apt-configure', 'package-update-upgrade-install', ], } sem_helper = helpers.FileSemaphores(sem_path) - for (mod_name, migrate_to) in touch_there.items(): + for (mod_name, migrate_to) in legacy_adjust.items(): possibles = [mod_name, helpers.canon_sem_name(mod_name)] old_exists = [] for p in os.listdir(sem_path): -- cgit v1.2.3 From ce14139c0f94b99c8c47192620b0a9faf66a96a2 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Thu, 8 Nov 2012 09:30:51 -0500 Subject: revert old zsh fix (revno 697) --- tools/Z99-cloud-locale-test.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/Z99-cloud-locale-test.sh b/tools/Z99-cloud-locale-test.sh index 4012beeb..8ad485e8 100755 --- a/tools/Z99-cloud-locale-test.sh +++ b/tools/Z99-cloud-locale-test.sh @@ -22,7 +22,7 @@ locale_warn() { case "$w1" in locale:) bad_names="${bad_names} ${w4}";; *) - key=${w1%%\=*} + key=${w1%%=*} val=${w1#*=} val=${val#\"} val=${val%\"} @@ -31,7 +31,7 @@ locale_warn() { done for bad in $bad_names; do for var in ${vars}; do - [ "${bad}" = "${var%\=*}" ] || continue + [ "${bad}" = "${var%=*}" ] || continue value=${var#*=} [ "${bad_lcs#* ${value}}" = "${bad_lcs}" ] && bad_lcs="${bad_lcs} ${value}" -- cgit v1.2.3 From 380bd3f3417c640191bdf90f6622f8bbbe12b5c7 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Thu, 8 Nov 2012 09:31:11 -0500 Subject: remove unused variable 'cr'. fix usage of 'value' to local 'val' --- tools/Z99-cloud-locale-test.sh | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tools/Z99-cloud-locale-test.sh b/tools/Z99-cloud-locale-test.sh index 8ad485e8..97c92166 100755 --- a/tools/Z99-cloud-locale-test.sh +++ b/tools/Z99-cloud-locale-test.sh @@ -10,9 +10,7 @@ # locale_warn() { - local cr=" -" - local bad_names="" bad_lcs="" key="" value="" var="" + local bad_names="" bad_lcs="" key="" val="" var="" vars="" local w1 w2 w3 w4 remain # locale is expected to output either: # VARIABLE= @@ -32,9 +30,9 @@ locale_warn() { for bad in $bad_names; do for var in ${vars}; do [ "${bad}" = "${var%=*}" ] || continue - value=${var#*=} - [ "${bad_lcs#* ${value}}" = "${bad_lcs}" ] && - bad_lcs="${bad_lcs} ${value}" + val=${var#*=} + [ "${bad_lcs#* ${val}}" = "${bad_lcs}" ] && + bad_lcs="${bad_lcs} ${val}" break done done -- cgit v1.2.3 From 956e94d16ab9c94471d3829424e8bd5183f3a735 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Thu, 8 Nov 2012 09:31:26 -0500 Subject: work with zsh by using 'emulate -L sh'. This makes zsh act like 'sh', but only for the function local function. This way, we do not affect the user's shell, but get the behavior we want. --- tools/Z99-cloud-locale-test.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/Z99-cloud-locale-test.sh b/tools/Z99-cloud-locale-test.sh index 97c92166..3c51f22d 100755 --- a/tools/Z99-cloud-locale-test.sh +++ b/tools/Z99-cloud-locale-test.sh @@ -12,6 +12,11 @@ locale_warn() { local bad_names="" bad_lcs="" key="" val="" var="" vars="" local w1 w2 w3 w4 remain + + # if shell is zsh, act like sh only for this function (-L). + # The behavior change will not permenently affect user's shell. + [ "${ZSH_NAME+zsh}" = "zsh" ] && emulate -L sh + # locale is expected to output either: # VARIABLE= # VARIABLE="value" -- cgit v1.2.3 From d8e82fb7bc9da6259b158804ae3d8343050c67aa Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 8 Nov 2012 08:55:04 -0800 Subject: Add non-zero exit code when bad yamls are found instead of returning zero in that case. --- tools/validate-yaml.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/validate-yaml.py b/tools/validate-yaml.py index d3218e40..eda59cb8 100755 --- a/tools/validate-yaml.py +++ b/tools/validate-yaml.py @@ -9,6 +9,7 @@ import yaml if __name__ == "__main__": + bads = 0 for fn in sys.argv[1:]: sys.stdout.write("%s" % (fn)) try: @@ -18,3 +19,8 @@ if __name__ == "__main__": sys.stdout.write(" - ok\n") except Exception, e: sys.stdout.write(" - bad (%s)\n" % (e)) + bads += 1 + if bads > 0: + sys.exit(1) + else: + sys.exit(0) -- cgit v1.2.3 From 154ad87b29344ea4d29d92f8559f61bb6efe6530 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 8 Nov 2012 16:30:57 -0800 Subject: Ensure that at needed stages the local variables of the init class are reset so that when they are regenerated that they will use the updated data instead of using previous data (since they weren't reset). LP: #1076811 --- cloudinit/stages.py | 31 +++++++++++++-- tests/data/user_data.1.txt | 15 +++++++ tests/unittests/test_merging.py | 62 +++++++++++++++++++++++++++++ tests/unittests/test_runs/test_merge_run.py | 52 ++++++++++++++++++++++++ tests/unittests/test_util.py | 59 --------------------------- tools/run-pep8 | 13 +++++- 6 files changed, 169 insertions(+), 63 deletions(-) create mode 100644 tests/data/user_data.1.txt create mode 100644 tests/unittests/test_merging.py create mode 100644 tests/unittests/test_runs/test_merge_run.py diff --git a/cloudinit/stages.py b/cloudinit/stages.py index 4ed1a750..e0cf1cbe 100644 --- a/cloudinit/stages.py +++ b/cloudinit/stages.py @@ -36,6 +36,8 @@ from cloudinit.handlers import cloud_config as cc_part from cloudinit.handlers import shell_script as ss_part from cloudinit.handlers import upstart_job as up_part +from cloudinit.sources import DataSourceNone + from cloudinit import cloud from cloudinit import config from cloudinit import distros @@ -58,8 +60,16 @@ class Init(object): self._cfg = None self._paths = None self._distro = None - # Created only when a fetch occurs - self.datasource = None + # Changed only when a fetch occurs + self.datasource = DataSourceNone.DataSourceNone({}, None, None) + + def _reset(self, ds=False): + # Recreated on access + self._cfg = None + self._paths = None + self._distro = None + if ds: + self.datasource = DataSourceNone.DataSourceNone({}, None, None) @property def distro(self): @@ -236,7 +246,7 @@ class Init(object): self.datasource = ds # Ensure we adjust our path members datasource # now that we have one (thus allowing ipath to be used) - self.paths.datasource = ds + self._reset() return ds def _get_instance_subdirs(self): @@ -296,6 +306,10 @@ class Init(object): util.write_file(iid_fn, "%s\n" % iid) util.write_file(os.path.join(dp, 'previous-instance-id'), "%s\n" % (previous_iid)) + # Ensure needed components are regenerated + # after change of instance which may cause + # change of configuration + self._reset() return iid def fetch(self): @@ -409,6 +423,17 @@ class Init(object): handlers.call_end(mod, data, frequency) called.append(mod) + # Perform post-consumption adjustments so that + # modules that run during the init stage reflect + # this consumed set. + # + # They will be recreated on future access... + self._reset() + # Note(harlowja): the 'active' datasource will have + # references to the previous config, distro, paths + # objects before the load of the userdata happened, + # this is expected. + class Modules(object): def __init__(self, init, cfg_files=None): diff --git a/tests/data/user_data.1.txt b/tests/data/user_data.1.txt new file mode 100644 index 00000000..4c4543de --- /dev/null +++ b/tests/data/user_data.1.txt @@ -0,0 +1,15 @@ +#cloud-config +write_files: +- content: blah + path: /etc/blah.ini + permissions: 493 + +system_info: + package_mirrors: + - arches: [i386, amd64, blah] + failsafe: + primary: http://my.archive.mydomain.com/ubuntu + security: http://my.security.mydomain.com/ubuntu + search: + primary: [] + security: [] diff --git a/tests/unittests/test_merging.py b/tests/unittests/test_merging.py new file mode 100644 index 00000000..0037b966 --- /dev/null +++ b/tests/unittests/test_merging.py @@ -0,0 +1,62 @@ +from mocker import MockerTestCase + +from cloudinit import util + + +class TestMergeDict(MockerTestCase): + def test_simple_merge(self): + """Test simple non-conflict merge.""" + source = {"key1": "value1"} + candidate = {"key2": "value2"} + result = util.mergedict(source, candidate) + self.assertEqual({"key1": "value1", "key2": "value2"}, result) + + def test_nested_merge(self): + """Test nested merge.""" + source = {"key1": {"key1.1": "value1.1"}} + candidate = {"key1": {"key1.2": "value1.2"}} + result = util.mergedict(source, candidate) + self.assertEqual( + {"key1": {"key1.1": "value1.1", "key1.2": "value1.2"}}, result) + + def test_merge_does_not_override(self): + """Test that candidate doesn't override source.""" + source = {"key1": "value1", "key2": "value2"} + candidate = {"key1": "value2", "key2": "NEW VALUE"} + result = util.mergedict(source, candidate) + self.assertEqual(source, result) + + def test_empty_candidate(self): + """Test empty candidate doesn't change source.""" + source = {"key": "value"} + candidate = {} + result = util.mergedict(source, candidate) + self.assertEqual(source, result) + + def test_empty_source(self): + """Test empty source is replaced by candidate.""" + source = {} + candidate = {"key": "value"} + result = util.mergedict(source, candidate) + self.assertEqual(candidate, result) + + def test_non_dict_candidate(self): + """Test non-dict candidate is discarded.""" + source = {"key": "value"} + candidate = "not a dict" + result = util.mergedict(source, candidate) + self.assertEqual(source, result) + + def test_non_dict_source(self): + """Test non-dict source is not modified with a dict candidate.""" + source = "not a dict" + candidate = {"key": "value"} + result = util.mergedict(source, candidate) + self.assertEqual(source, result) + + def test_neither_dict(self): + """Test if neither candidate or source is dict source wins.""" + source = "source" + candidate = "candidate" + result = util.mergedict(source, candidate) + self.assertEqual(source, result) diff --git a/tests/unittests/test_runs/test_merge_run.py b/tests/unittests/test_runs/test_merge_run.py new file mode 100644 index 00000000..04c03730 --- /dev/null +++ b/tests/unittests/test_runs/test_merge_run.py @@ -0,0 +1,52 @@ +import os + +from tests.unittests import helpers + +from cloudinit.settings import (PER_INSTANCE) +from cloudinit import stages +from cloudinit import util + + +class TestSimpleRun(helpers.FilesystemMockingTestCase): + def _patchIn(self, root): + self.restore() + self.patchOS(root) + self.patchUtils(root) + + def test_none_ds(self): + new_root = self.makeDir() + self.replicateTestRoot('simple_ubuntu', new_root) + cfg = { + 'datasource_list': ['None'], + 'cloud_init_modules': ['write-files'], + } + ud = self.readResource('user_data.1.txt') + cloud_cfg = util.yaml_dumps(cfg) + util.ensure_dir(os.path.join(new_root, 'etc', 'cloud')) + util.write_file(os.path.join(new_root, 'etc', + 'cloud', 'cloud.cfg'), cloud_cfg) + self._patchIn(new_root) + + # Now start verifying whats created + initer = stages.Init() + initer.read_cfg() + initer.initialize() + initer.fetch() + initer.datasource.userdata_raw = ud + iid = initer.instancify() + initer.update() + initer.cloudify().run('consume_userdata', + initer.consume_userdata, + args=[PER_INSTANCE], + freq=PER_INSTANCE) + mirrors = initer.distro.get_option('package_mirrors') + self.assertEquals(1, len(mirrors)) + mirror = mirrors[0] + self.assertEquals(mirror['arches'], ['i386', 'amd64', 'blah']) + mods = stages.Modules(initer) + (which_ran, failures) = mods.run_section('cloud_init_modules') + self.assertTrue(len(failures) == 0) + self.assertTrue(os.path.exists('/etc/blah.ini')) + self.assertIn('write-files', which_ran) + contents = util.load_file('/etc/blah.ini') + self.assertEquals(contents, 'blah') diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 96962b91..02611581 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -28,65 +28,6 @@ class FakeSelinux(object): self.restored.append(path) -class TestMergeDict(MockerTestCase): - def test_simple_merge(self): - """Test simple non-conflict merge.""" - source = {"key1": "value1"} - candidate = {"key2": "value2"} - result = util.mergedict(source, candidate) - self.assertEqual({"key1": "value1", "key2": "value2"}, result) - - def test_nested_merge(self): - """Test nested merge.""" - source = {"key1": {"key1.1": "value1.1"}} - candidate = {"key1": {"key1.2": "value1.2"}} - result = util.mergedict(source, candidate) - self.assertEqual( - {"key1": {"key1.1": "value1.1", "key1.2": "value1.2"}}, result) - - def test_merge_does_not_override(self): - """Test that candidate doesn't override source.""" - source = {"key1": "value1", "key2": "value2"} - candidate = {"key1": "value2", "key2": "NEW VALUE"} - result = util.mergedict(source, candidate) - self.assertEqual(source, result) - - def test_empty_candidate(self): - """Test empty candidate doesn't change source.""" - source = {"key": "value"} - candidate = {} - result = util.mergedict(source, candidate) - self.assertEqual(source, result) - - def test_empty_source(self): - """Test empty source is replaced by candidate.""" - source = {} - candidate = {"key": "value"} - result = util.mergedict(source, candidate) - self.assertEqual(candidate, result) - - def test_non_dict_candidate(self): - """Test non-dict candidate is discarded.""" - source = {"key": "value"} - candidate = "not a dict" - result = util.mergedict(source, candidate) - self.assertEqual(source, result) - - def test_non_dict_source(self): - """Test non-dict source is not modified with a dict candidate.""" - source = "not a dict" - candidate = {"key": "value"} - result = util.mergedict(source, candidate) - self.assertEqual(source, result) - - def test_neither_dict(self): - """Test if neither candidate or source is dict source wins.""" - source = "source" - candidate = "candidate" - result = util.mergedict(source, candidate) - self.assertEqual(source, result) - - class TestGetCfgOptionListOrStr(TestCase): def test_not_found_no_default(self): """None is returned if key is not found and no default given.""" diff --git a/tools/run-pep8 b/tools/run-pep8 index ad55d420..1dfa92c3 100755 --- a/tools/run-pep8 +++ b/tools/run-pep8 @@ -21,10 +21,21 @@ else base=`pwd`/tools/ fi +IGNORE="E501" # Line too long (these are caught by pylint) + +# King Arthur: Be quiet! ... Be Quiet! I Order You to Be Quiet. +IGNORE="$IGNORE,E121" # Continuation line indentation is not a multiple of four +IGNORE="$IGNORE,E123" # Closing bracket does not match indentation of opening bracket's line +IGNORE="$IGNORE,E124" # Closing bracket missing visual indentation +IGNORE="$IGNORE,E125" # Continuation line does not distinguish itself from next logical line +IGNORE="$IGNORE,E126" # Continuation line over-indented for hanging indent +IGNORE="$IGNORE,E127" # Continuation line over-indented for visual indent +IGNORE="$IGNORE,E128" # Continuation line under-indented for visual indent + cmd=( ${base}/hacking.py - --ignore=E501 # Line too long (these are caught by pylint) + --ignore="$IGNORE" "${files[@]}" ) -- cgit v1.2.3 From b80c2401123e16b9038ff3fb6f6d660717ee68e1 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 8 Nov 2012 17:37:16 -0800 Subject: Fix the case where on a redhat based system the fully qualified domain name should end up in /etc/sysconfig/network by passing the fqdn to the update and set hostname methods and using it accordingly. LP: #1076759 --- cloudinit/config/cc_set_hostname.py | 10 ++++++---- cloudinit/config/cc_update_hostname.py | 8 +++++--- cloudinit/distros/__init__.py | 4 ++-- cloudinit/distros/debian.py | 4 ++-- cloudinit/distros/rhel.py | 25 +++++++++++++++++-------- 5 files changed, 32 insertions(+), 19 deletions(-) diff --git a/cloudinit/config/cc_set_hostname.py b/cloudinit/config/cc_set_hostname.py index b0f27ebf..2b32fc94 100644 --- a/cloudinit/config/cc_set_hostname.py +++ b/cloudinit/config/cc_set_hostname.py @@ -27,9 +27,11 @@ def handle(name, cfg, cloud, log, _args): " not setting the hostname in module %s"), name) return - (hostname, _fqdn) = util.get_hostname_fqdn(cfg, cloud) + (hostname, fqdn) = util.get_hostname_fqdn(cfg, cloud) try: - log.debug("Setting hostname to %s", hostname) - cloud.distro.set_hostname(hostname) + log.debug("Setting the hostname to %s (%s)", fqdn, hostname) + cloud.distro.set_hostname(hostname, fqdn) except Exception: - util.logexc(log, "Failed to set hostname to %s", hostname) + util.logexc(log, "Failed to set the hostname to %s (%s)", + fqdn, hostname) + raise diff --git a/cloudinit/config/cc_update_hostname.py b/cloudinit/config/cc_update_hostname.py index 1d6679ea..52225cd8 100644 --- a/cloudinit/config/cc_update_hostname.py +++ b/cloudinit/config/cc_update_hostname.py @@ -32,10 +32,12 @@ def handle(name, cfg, cloud, log, _args): " not updating the hostname in module %s"), name) return - (hostname, _fqdn) = util.get_hostname_fqdn(cfg, cloud) + (hostname, fqdn) = util.get_hostname_fqdn(cfg, cloud) try: prev_fn = os.path.join(cloud.get_cpath('data'), "previous-hostname") - cloud.distro.update_hostname(hostname, prev_fn) + log.debug("Updating hostname to %s (%s)", fqdn, hostname) + cloud.distro.update_hostname(hostname, fqdn, prev_fn) except Exception: - util.logexc(log, "Failed to set the hostname to %s", hostname) + util.logexc(log, "Failed to update the hostname to %s (%s)", + fqdn, hostname) raise diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index 869540d2..bd04ba79 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -58,11 +58,11 @@ class Distro(object): return self._cfg.get(opt_name, default) @abc.abstractmethod - def set_hostname(self, hostname): + def set_hostname(self, hostname, fqdn=None): raise NotImplementedError() @abc.abstractmethod - def update_hostname(self, hostname, prev_hostname_fn): + def update_hostname(self, hostname, fqdn, prev_hostname_fn): raise NotImplementedError() @abc.abstractmethod diff --git a/cloudinit/distros/debian.py b/cloudinit/distros/debian.py index cc7e53a0..ed4070b4 100644 --- a/cloudinit/distros/debian.py +++ b/cloudinit/distros/debian.py @@ -67,7 +67,7 @@ class Distro(distros.Distro): else: return distros.Distro._bring_up_interfaces(self, device_names) - def set_hostname(self, hostname): + def set_hostname(self, hostname, fqdn=None): self._write_hostname(hostname, "/etc/hostname") LOG.debug("Setting hostname to %s", hostname) util.subp(['hostname', hostname]) @@ -76,7 +76,7 @@ class Distro(distros.Distro): # "" gives trailing newline. util.write_file(out_fn, "%s\n" % str(hostname), 0644) - def update_hostname(self, hostname, prev_fn): + def update_hostname(self, hostname, fqdn, prev_fn): hostname_prev = self._read_hostname(prev_fn) hostname_in_etc = self._read_hostname("/etc/hostname") update_files = [] diff --git a/cloudinit/distros/rhel.py b/cloudinit/distros/rhel.py index bf3c18d2..e4c27216 100644 --- a/cloudinit/distros/rhel.py +++ b/cloudinit/distros/rhel.py @@ -146,8 +146,13 @@ class Distro(distros.Distro): lines.insert(0, _make_header()) util.write_file(fn, "\n".join(lines), 0644) - def set_hostname(self, hostname): - self._write_hostname(hostname, '/etc/sysconfig/network') + def set_hostname(self, hostname, fqdn=None): + # See: http://bit.ly/TwitgL + # Should be fqdn if we can use it + sysconfig_hostname = fqdn + if not sysconfig_hostname: + sysconfig_hostname = hostname + self._write_hostname(sysconfig_hostname, '/etc/sysconfig/network') LOG.debug("Setting hostname to %s", hostname) util.subp(['hostname', hostname]) @@ -165,28 +170,32 @@ class Distro(distros.Distro): } self._update_sysconfig_file(out_fn, host_cfg) - def update_hostname(self, hostname, prev_file): + def update_hostname(self, hostname, fqdn, prev_file): + # See: http://bit.ly/TwitgL + # Should be fqdn if we can use it + sysconfig_hostname = fqdn + if not sysconfig_hostname: + sysconfig_hostname = hostname hostname_prev = self._read_hostname(prev_file) hostname_in_sys = self._read_hostname("/etc/sysconfig/network") update_files = [] - if not hostname_prev or hostname_prev != hostname: + if not hostname_prev or hostname_prev != sysconfig_hostname: update_files.append(prev_file) if (not hostname_in_sys or (hostname_in_sys == hostname_prev - and hostname_in_sys != hostname)): + and hostname_in_sys != sysconfig_hostname)): update_files.append("/etc/sysconfig/network") for fn in update_files: try: - self._write_hostname(hostname, fn) + self._write_hostname(sysconfig_hostname, fn) except: util.logexc(LOG, "Failed to write hostname %s to %s", - hostname, fn) + sysconfig_hostname, fn) if (hostname_in_sys and hostname_prev and hostname_in_sys != hostname_prev): LOG.debug(("%s differs from /etc/sysconfig/network." " Assuming user maintained hostname."), prev_file) if "/etc/sysconfig/network" in update_files: - # Only do this if we are running in non-adjusted root mode LOG.debug("Setting hostname to %s", hostname) util.subp(['hostname', hostname]) -- cgit v1.2.3 From bd01f3466e10ca515a8e8aec42d00201f40cbd53 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Thu, 8 Nov 2012 17:41:36 -0800 Subject: Forgot the test! --- .../test_handler/test_handler_set_hostname.py | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 tests/unittests/test_handler/test_handler_set_hostname.py diff --git a/tests/unittests/test_handler/test_handler_set_hostname.py b/tests/unittests/test_handler/test_handler_set_hostname.py new file mode 100644 index 00000000..a1aba62f --- /dev/null +++ b/tests/unittests/test_handler/test_handler_set_hostname.py @@ -0,0 +1,58 @@ +from cloudinit.config import cc_set_hostname + +from cloudinit import cloud +from cloudinit import distros +from cloudinit import helpers +from cloudinit import util + +from tests.unittests import helpers as t_help + +import logging + +from StringIO import StringIO + +from configobj import ConfigObj + +LOG = logging.getLogger(__name__) + + +class TestHostname(t_help.FilesystemMockingTestCase): + def setUp(self): + super(TestHostname, self).setUp() + self.tmp = self.makeDir(prefix="unittest_") + + def _fetch_distro(self, kind): + cls = distros.fetch(kind) + paths = helpers.Paths({}) + return cls(kind, {}, paths) + + def test_write_hostname_rhel(self): + cfg = { + 'hostname': 'blah.blah.blah.yahoo.com', + } + distro = self._fetch_distro('rhel') + paths = helpers.Paths({}) + ds = None + cc = cloud.Cloud(ds, paths, {}, distro, None) + self.patchUtils(self.tmp) + self.patchOS(self.tmp) + cc_set_hostname.handle('cc_set_hostname', + cfg, cc, LOG, []) + contents = util.load_file("/etc/sysconfig/network") + n_cfg = ConfigObj(StringIO(contents)) + self.assertEquals({'HOSTNAME': 'blah.blah.blah.yahoo.com'}, + dict(n_cfg)) + + def test_write_hostname_debian(self): + cfg = { + 'hostname': 'blah.blah.blah.yahoo.com', + } + distro = self._fetch_distro('debian') + paths = helpers.Paths({}) + ds = None + cc = cloud.Cloud(ds, paths, {}, distro, None) + self.patchUtils(self.tmp) + cc_set_hostname.handle('cc_set_hostname', + cfg, cc, LOG, []) + contents = util.load_file("/etc/hostname") + self.assertEquals('blah', contents.strip()) -- cgit v1.2.3 From 8c006684034c13719171672836edfc65bf02ebe9 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 9 Nov 2012 14:40:41 -0800 Subject: Fix pep8 warnings. --- cloudinit/distros/__init__.py | 2 +- tools/run-pep8 | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index bd04ba79..3392a065 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -81,7 +81,7 @@ class Distro(object): def _get_arch_package_mirror_info(self, arch=None): mirror_info = self.get_option("package_mirrors", []) - if arch == None: + if not arch: arch = self.get_primary_arch() return _get_arch_package_mirror_info(mirror_info, arch) diff --git a/tools/run-pep8 b/tools/run-pep8 index 1dfa92c3..20e594bc 100755 --- a/tools/run-pep8 +++ b/tools/run-pep8 @@ -31,6 +31,7 @@ IGNORE="$IGNORE,E125" # Continuation line does not distinguish itself from next IGNORE="$IGNORE,E126" # Continuation line over-indented for hanging indent IGNORE="$IGNORE,E127" # Continuation line over-indented for visual indent IGNORE="$IGNORE,E128" # Continuation line under-indented for visual indent +IGNORE="$IGNORE,E502" # The backslash is redundant between brackets cmd=( ${base}/hacking.py -- cgit v1.2.3 From 1169dcc5f18fd9a5adbf353bec87e48d563550a5 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Fri, 9 Nov 2012 15:28:35 -0800 Subject: Fix the merging of group configuration when that group configuration is a dict => members. LP: #1077245 --- cloudinit/distros/__init__.py | 32 +++++++++++++++++++--- .../test_distros/test_user_data_normalize.py | 22 +++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index 3392a065..2d01efc3 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -425,12 +425,36 @@ def _get_arch_package_mirror_info(package_mirrors, arch): # is the standard form used in the rest # of cloud-init def _normalize_groups(grp_cfg): - if isinstance(grp_cfg, (str, basestring, list)): + if isinstance(grp_cfg, (str, basestring)): + grp_cfg = grp_cfg.strip().split(",") + if isinstance(grp_cfg, (list)): c_grp_cfg = {} - for i in util.uniq_merge(grp_cfg): - c_grp_cfg[i] = [] + for i in grp_cfg: + if isinstance(i, (dict)): + for k, v in i.items(): + if k not in c_grp_cfg: + if isinstance(v, (list)): + c_grp_cfg[k] = list(v) + elif isinstance(v, (basestring, str)): + c_grp_cfg[k] = [v] + else: + raise TypeError("Bad group member type %s" % + util.obj_name(v)) + else: + if isinstance(v, (list)): + c_grp_cfg[k].extend(v) + elif isinstance(v, (basestring, str)): + c_grp_cfg[k].append(v) + else: + raise TypeError("Bad group member type %s" % + util.obj_name(v)) + elif isinstance(i, (str, basestring)): + if i not in c_grp_cfg: + c_grp_cfg[i] = [] + else: + raise TypeError("Unknown group name type %s" % + util.obj_name(i)) grp_cfg = c_grp_cfg - groups = {} if isinstance(grp_cfg, (dict)): for (grp_name, grp_members) in grp_cfg.items(): diff --git a/tests/unittests/test_distros/test_user_data_normalize.py b/tests/unittests/test_distros/test_user_data_normalize.py index 8f0d8896..50400c8a 100644 --- a/tests/unittests/test_distros/test_user_data_normalize.py +++ b/tests/unittests/test_distros/test_user_data_normalize.py @@ -30,6 +30,28 @@ class TestUGNormalize(MockerTestCase): def _norm(self, cfg, distro): return distros.normalize_users_groups(cfg, distro) + def test_group_dict(self): + distro = self._make_distro('ubuntu') + g = {'groups': [ + { + 'ubuntu': ['foo', 'bar'], + 'bob': 'users', + }, + 'cloud-users', + { + 'bob': 'users2', + }, + ] + } + (users, groups) = self._norm(g, distro) + self.assertIn('ubuntu', groups) + ub_members = groups['ubuntu'] + self.assertEquals(sorted(['foo', 'bar']), sorted(ub_members)) + self.assertIn('bob', groups) + b_members = groups['bob'] + self.assertEquals(sorted(['users', 'users2']), + sorted(b_members)) + def test_basic_groups(self): distro = self._make_distro('ubuntu') ug_cfg = { -- cgit v1.2.3 From a17a69c35c1de0a6bd6f054f76d3da9e4a9c5364 Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Sat, 10 Nov 2012 10:15:16 -0800 Subject: Sudoers.d creation cleanups + tests. --- cloudinit/distros/__init__.py | 20 +++++++++++------ tests/unittests/helpers.py | 1 + tests/unittests/test_distros/test_generic.py | 32 +++++++++++++++++++++++++--- tests/unittests/test_runs/test_merge_run.py | 2 +- 4 files changed, 45 insertions(+), 10 deletions(-) diff --git a/cloudinit/distros/__init__.py b/cloudinit/distros/__init__.py index 2d01efc3..d2cb0a8b 100644 --- a/cloudinit/distros/__init__.py +++ b/cloudinit/distros/__init__.py @@ -283,8 +283,10 @@ class Distro(object): # Ensure the dir is included and that # it actually exists as a directory sudoers_contents = '' + base_exists = False if os.path.exists(sudo_base): sudoers_contents = util.load_file(sudo_base) + base_exists = True found_include = False for line in sudoers_contents.splitlines(): line = line.strip() @@ -299,18 +301,24 @@ class Distro(object): found_include = True break if not found_include: - sudoers_contents += "\n#includedir %s\n" % (path) try: - if not os.path.exists(sudo_base): + if not base_exists: + lines = [('# See sudoers(5) for more information' + ' on "#include" directives:'), '', + '# Added by cloud-init', + "#includedir %s" % (path), ''] + sudoers_contents = "\n".join(lines) util.write_file(sudo_base, sudoers_contents, 0440) else: - with open(sudo_base, 'a') as f: - f.write(sudoers_contents) - LOG.debug("added '#includedir %s' to %s" % (path, sudo_base)) + lines = ['', '# Added by cloud-init', + "#includedir %s" % (path), ''] + sudoers_contents = "\n".join(lines) + util.append_file(sudo_base, sudoers_contents) + LOG.debug("Added '#includedir %s' to %s" % (path, sudo_base)) except IOError as e: util.logexc(LOG, "Failed to write %s" % sudo_base, e) raise e - util.ensure_dir(path, 0755) + util.ensure_dir(path, 0750) def write_sudo_rules(self, user, diff --git a/tests/unittests/helpers.py b/tests/unittests/helpers.py index 2c5dcad2..e8080668 100644 --- a/tests/unittests/helpers.py +++ b/tests/unittests/helpers.py @@ -103,6 +103,7 @@ class FilesystemMockingTestCase(ResourceUsingTestCase): def patchUtils(self, new_root): patch_funcs = { util: [('write_file', 1), + ('append_file', 1), ('load_file', 1), ('ensure_dir', 1), ('chmod', 1), diff --git a/tests/unittests/test_distros/test_generic.py b/tests/unittests/test_distros/test_generic.py index 2df4c2f0..704699b5 100644 --- a/tests/unittests/test_distros/test_generic.py +++ b/tests/unittests/test_distros/test_generic.py @@ -1,6 +1,9 @@ -from mocker import MockerTestCase - from cloudinit import distros +from cloudinit import util + +from tests.unittests import helpers + +import os unknown_arch_info = { 'arches': ['default'], @@ -27,7 +30,7 @@ gpmi = distros._get_package_mirror_info # pylint: disable=W0212 gapmi = distros._get_arch_package_mirror_info # pylint: disable=W0212 -class TestGenericDistro(MockerTestCase): +class TestGenericDistro(helpers.FilesystemMockingTestCase): def return_first(self, mlist): if not mlist: @@ -52,6 +55,29 @@ class TestGenericDistro(MockerTestCase): # Make a temp directoy for tests to use. self.tmp = self.makeDir() + def test_sudoers_ensure_new(self): + cls = distros.fetch("ubuntu") + d = cls("ubuntu", {}, None) + self.patchOS(self.tmp) + self.patchUtils(self.tmp) + d.ensure_sudo_dir("/b") + contents = util.load_file("/etc/sudoers") + self.assertIn("includedir /b", contents) + self.assertTrue(os.path.isdir("/b")) + + def test_sudoers_ensure_append(self): + cls = distros.fetch("ubuntu") + d = cls("ubuntu", {}, None) + self.patchOS(self.tmp) + self.patchUtils(self.tmp) + util.write_file("/etc/sudoers", "josh, josh\n") + d.ensure_sudo_dir("/b") + contents = util.load_file("/etc/sudoers") + self.assertIn("includedir /b", contents) + self.assertTrue(os.path.isdir("/b")) + self.assertIn("josh", contents) + self.assertEquals(2, contents.count("josh")) + def test_arch_package_mirror_info_unknown(self): """for an unknown arch, we should get back that with arch 'default'.""" arch_mirrors = gapmi(package_mirrors, arch="unknown") diff --git a/tests/unittests/test_runs/test_merge_run.py b/tests/unittests/test_runs/test_merge_run.py index 04c03730..36de97ae 100644 --- a/tests/unittests/test_runs/test_merge_run.py +++ b/tests/unittests/test_runs/test_merge_run.py @@ -7,7 +7,7 @@ from cloudinit import stages from cloudinit import util -class TestSimpleRun(helpers.FilesystemMockingTestCase): +class TestMergeRun(helpers.FilesystemMockingTestCase): def _patchIn(self, root): self.restore() self.patchOS(root) -- cgit v1.2.3 From efa2dfa699bc9222105850641a2820ceda9bfe67 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Sat, 10 Nov 2012 21:57:06 -0500 Subject: update ChangeLog LP: #1076811 --- ChangeLog | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ChangeLog b/ChangeLog index 8f304db3..52f47e3c 100644 --- a/ChangeLog +++ b/ChangeLog @@ -16,6 +16,12 @@ - Remove usage of paths.join, as all code should run through util helpers - Fix pylint complaining about tests folder 'helpers.py' not being found - Add a pylintrc file that is used instead options hidden in 'run_pylint' + - fix bug where cloud-config from user-data could not affect system_info + settings [revno 703] (LP: #1076811) + - for write fqdn to system config for rh/fedora [revno 704] + - add yaml/cloud config examples checking tool [revno 706] + - Fix the merging of group configuration when that group configuration is a + dict => members. [revno 707] 0.7.0: - add a 'exception_cb' argument to 'wait_for_url'. If provided, this method will be called back with the exception received and the message. -- cgit v1.2.3