From 4fe439b9e137e0b59d00e919dc16aea9da35082a Mon Sep 17 00:00:00 2001 From: Ben Howard Date: Wed, 15 Jan 2014 17:18:32 -0700 Subject: Fix for race condition where 'captured' Windows Azure instances identify as the previous instance (LP: #1269626). --- cloudinit/sources/DataSourceAzure.py | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'cloudinit') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 97f151d6..448915d7 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -93,6 +93,12 @@ class DataSourceAzureNet(sources.DataSource): try: if cdev.startswith("/dev/"): ret = util.mount_cb(cdev, load_azure_ds_dir) + + # If we load the OVF from a device, it means that a + # new ovf-env.xml has been found. (LP: #1269626) + if os.path.exists(ddir): + LOG.info("removing old agent directory %s" % ddir) + util.del_dir(ddir) else: ret = load_azure_ds_dir(cdev) -- cgit v1.2.3 From c8bcf80d4153512450efa72b5ac2d1644a7904bd Mon Sep 17 00:00:00 2001 From: Joshua Harlow Date: Sat, 1 Feb 2014 12:03:32 -0800 Subject: Fix incorrect return get_url_settings should return a pair of max wait and timeout and not false, fix this bug by checking the max_wait <= 0 in the calling function and returning correctly from there instead. --- cloudinit/sources/DataSourceEc2.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'cloudinit') diff --git a/cloudinit/sources/DataSourceEc2.py b/cloudinit/sources/DataSourceEc2.py index f010e640..1b20ecf3 100644 --- a/cloudinit/sources/DataSourceEc2.py +++ b/cloudinit/sources/DataSourceEc2.py @@ -92,12 +92,9 @@ class DataSourceEc2(sources.DataSource): except Exception: util.logexc(LOG, "Failed to get max wait. using %s", max_wait) - if max_wait == 0: - return False - timeout = 50 try: - timeout = int(mcfg.get("timeout", timeout)) + timeout = max(0, int(mcfg.get("timeout", timeout))) except Exception: util.logexc(LOG, "Failed to get timeout, using %s", timeout) @@ -109,6 +106,8 @@ class DataSourceEc2(sources.DataSource): mcfg = {} (max_wait, timeout) = self._get_url_settings() + if max_wait <= 0: + return False # Remove addresses from the list that wont resolve. mdurls = mcfg.get("metadata_urls", DEF_MD_URLS) -- cgit v1.2.3 From ceffb35a2ddfb5f309417aaa68e7ff70199690fa Mon Sep 17 00:00:00 2001 From: Ben Howard Date: Fri, 7 Feb 2014 13:49:03 +0200 Subject: Made new ovf-env.xml handling more robust. Test cases included --- cloudinit/sources/DataSourceAzure.py | 16 ++++++----- tests/unittests/test_datasource/test_azure.py | 38 +++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) (limited to 'cloudinit') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 448915d7..eee36d22 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -84,21 +84,20 @@ class DataSourceAzureNet(sources.DataSource): candidates = [self.seed_dir] candidates.extend(list_possible_azure_ds_devs()) + previous_ovf_cfg = None if ddir: candidates.append(ddir) + previous_ovf_cfg = None + if os.path.exists("%s/ovf-env.xml" % ddir): + previous_ovf_cfg = load_azure_ds_dir(ddir) + found = None for cdev in candidates: try: if cdev.startswith("/dev/"): ret = util.mount_cb(cdev, load_azure_ds_dir) - - # If we load the OVF from a device, it means that a - # new ovf-env.xml has been found. (LP: #1269626) - if os.path.exists(ddir): - LOG.info("removing old agent directory %s" % ddir) - util.del_dir(ddir) else: ret = load_azure_ds_dir(cdev) @@ -110,6 +109,11 @@ class DataSourceAzureNet(sources.DataSource): LOG.warn("%s was not mountable" % cdev) continue + if ret != previous_ovf_cfg: + LOG.info(("instance configuration has changed, " + "removing old agent directory")) + util.del_dir(ddir) + (md, self.userdata_raw, cfg, files) = ret self.seed = cdev self.metadata = util.mergemanydict([md, DEFAULT_METADATA]) diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index aad84206..aa541a18 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -119,6 +119,10 @@ class TestAzureDataSource(MockerTestCase): {'ovf-env.xml': data['ovfcontent']}) mod = DataSourceAzure + ddir = "%s/var/lib/waagent" % self.tmp + mod.BUILTIN_DS_CONFIG['data_dir'] = ddir + if not os.path.isdir(ddir): + os.makedirs(ddir) self.apply_patches([(mod, 'list_possible_azure_ds_devs', dsdevs)]) @@ -338,6 +342,40 @@ class TestAzureDataSource(MockerTestCase): self.assertEqual(userdata, dsrc.userdata_raw) + def test_existing_ovf_same(self): + mydata = "FOOBAR" + odata = {'UserData': base64.b64encode(mydata)} + data = {'ovfcontent': construct_valid_ovf_env(data=odata)} + + with open("%s/ovf-env.xml" % self.tmp, 'w') as fp: + fp.write(construct_valid_ovf_env(data=odata)) + with open("%s/sem" % self.tmp, 'w') as fp: + fp.write("test") + + dsrc = self._get_ds(data) + ret = dsrc.get_data() + self.assertTrue(ret) + self.assertEqual(dsrc.userdata_raw, mydata) + self.assertTrue(os.path.exists("%s/sem" % self.tmp)) + + def test_existing_ovf_diff(self): + mydata = "FOOBAR" + odata = {'UserData': base64.b64encode(mydata)} + data = {'ovfcontent': construct_valid_ovf_env(data=odata)} + + data_dir = "%s/var/lib/waagent" % self.tmp + os.makedirs(data_dir) + with open("%s/ovf-env.xml" % data_dir, 'w') as fp: + fp.write(construct_valid_ovf_env()) + with open("%s/sem" % data_dir, 'w') as fp: + fp.write("test") + + dsrc = self._get_ds(data) + ret = dsrc.get_data() + self.assertTrue(ret) + self.assertEqual(dsrc.userdata_raw, mydata) + self.assertFalse(os.path.exists("%s/sem" % data_dir)) + class TestReadAzureOvf(MockerTestCase): def test_invalid_xml_raises_non_azure_ds(self): -- cgit v1.2.3 From c09e8cd6e81e478dd5276275418b70d5d946f479 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Mon, 10 Feb 2014 15:05:41 -0500 Subject: change behavior to only delete SharedConfig.xml. --- cloudinit/sources/DataSourceAzure.py | 33 ++++++---- tests/unittests/test_datasource/test_azure.py | 93 +++++++++++++++++---------- 2 files changed, 79 insertions(+), 47 deletions(-) (limited to 'cloudinit') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index eee36d22..6b48a340 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -84,14 +84,9 @@ class DataSourceAzureNet(sources.DataSource): candidates = [self.seed_dir] candidates.extend(list_possible_azure_ds_devs()) - previous_ovf_cfg = None if ddir: candidates.append(ddir) - previous_ovf_cfg = None - if os.path.exists("%s/ovf-env.xml" % ddir): - previous_ovf_cfg = load_azure_ds_dir(ddir) - found = None for cdev in candidates: @@ -109,11 +104,6 @@ class DataSourceAzureNet(sources.DataSource): LOG.warn("%s was not mountable" % cdev) continue - if ret != previous_ovf_cfg: - LOG.info(("instance configuration has changed, " - "removing old agent directory")) - util.del_dir(ddir) - (md, self.userdata_raw, cfg, files) = ret self.seed = cdev self.metadata = util.mergemanydict([md, DEFAULT_METADATA]) @@ -138,10 +128,27 @@ class DataSourceAzureNet(sources.DataSource): user_ds_cfg = util.get_cfg_by_path(self.cfg, DS_CFG_PATH, {}) self.ds_cfg = util.mergemanydict([user_ds_cfg, self.ds_cfg]) mycfg = self.ds_cfg + ddir = mycfg['data_dir'] + + if found != ddir: + cached_ovfenv = util.load_file( + os.path.join(ddir, 'ovf-env.xml'), quiet=True) + if cached_ovfenv != files['ovf-env.xml']: + # source was not walinux-agent's datadir, so we have to clean + # up so 'wait_for_files' doesn't return early due to stale data + toclean = ['SharedConfig.xml'] + cleaned = [] + for f in [os.path.join(ddir, f) for f in toclean]: + if os.path.exists(f): + util.del_file(f) + cleaned.append(f) + if cleaned: + LOG.info("removed stale file(s) in '%s': %s", + ddir, str(cleaned)) # walinux agent writes files world readable, but expects # the directory to be protected. - write_files(mycfg['data_dir'], files, dirmode=0700) + write_files(ddir, files, dirmode=0700) # handle the hostname 'publishing' try: @@ -159,13 +166,13 @@ class DataSourceAzureNet(sources.DataSource): util.logexc(LOG, "agent command '%s' failed.", mycfg['agent_command']) - shcfgxml = os.path.join(mycfg['data_dir'], "SharedConfig.xml") + shcfgxml = os.path.join(ddir, "SharedConfig.xml") wait_for = [shcfgxml] fp_files = [] for pk in self.cfg.get('_pubkeys', []): bname = str(pk['fingerprint'] + ".crt") - fp_files += [os.path.join(mycfg['data_dir'], bname)] + fp_files += [os.path.join(ddir, bname)] missing = util.log_time(logfunc=LOG.debug, msg="waiting for files", func=wait_for_files, diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index aa541a18..44c537f4 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -1,4 +1,5 @@ from cloudinit import helpers +from cloudinit.util import load_file from cloudinit.sources import DataSourceAzure from tests.unittests.helpers import populate_dir @@ -6,6 +7,7 @@ import base64 import crypt from mocker import MockerTestCase import os +import stat import yaml @@ -72,6 +74,7 @@ class TestAzureDataSource(MockerTestCase): # patch cloud_dir, so our 'seed_dir' is guaranteed empty self.paths = helpers.Paths({'cloud_dir': self.tmp}) + self.waagent_d = os.path.join(self.tmp, 'var', 'lib', 'waagent') self.unapply = [] super(TestAzureDataSource, self).setUp() @@ -92,13 +95,6 @@ class TestAzureDataSource(MockerTestCase): def _invoke_agent(cmd): data['agent_invoked'] = cmd - def _write_files(datadir, files, dirmode): - data['files'] = {} - data['datadir'] = datadir - data['datadir_mode'] = dirmode - for (fname, content) in files.items(): - data['files'][fname] = content - def _wait_for_files(flist, _maxwait=None, _naplen=None): data['waited'] = flist return [] @@ -119,15 +115,11 @@ class TestAzureDataSource(MockerTestCase): {'ovf-env.xml': data['ovfcontent']}) mod = DataSourceAzure - ddir = "%s/var/lib/waagent" % self.tmp - mod.BUILTIN_DS_CONFIG['data_dir'] = ddir - if not os.path.isdir(ddir): - os.makedirs(ddir) + mod.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d self.apply_patches([(mod, 'list_possible_azure_ds_devs', dsdevs)]) self.apply_patches([(mod, 'invoke_agent', _invoke_agent), - (mod, 'write_files', _write_files), (mod, 'wait_for_files', _wait_for_files), (mod, 'pubkeys_from_crt_files', _pubkeys_from_crt_files), @@ -151,10 +143,18 @@ class TestAzureDataSource(MockerTestCase): self.assertTrue(ret) self.assertEqual(dsrc.userdata_raw, "") self.assertEqual(dsrc.metadata['local-hostname'], odata['HostName']) - self.assertTrue('ovf-env.xml' in data['files']) - self.assertEqual(0700, data['datadir_mode']) + self.assertTrue(os.path.isfile( + os.path.join(self.waagent_d, 'ovf-env.xml'))) self.assertEqual(dsrc.metadata['instance-id'], 'i-my-azure-id') + def test_waagent_d_has_0700_perms(self): + # we expect /var/lib/waagent to be created 0700 + dsrc = self._get_ds({'ovfcontent': construct_valid_ovf_env()}) + ret = dsrc.get_data() + self.assertTrue(ret) + self.assertTrue(os.path.isdir(self.waagent_d)) + self.assertEqual(stat.S_IMODE(os.stat(self.waagent_d).st_mode), 0700) + def test_user_cfg_set_agent_command_plain(self): # set dscfg in via plaintext # we must have friendly-to-xml formatted plaintext in yaml_cfg @@ -342,39 +342,64 @@ class TestAzureDataSource(MockerTestCase): self.assertEqual(userdata, dsrc.userdata_raw) + def test_ovf_env_arrives_in_waagent_dir(self): + xml = construct_valid_ovf_env(data={}, userdata="FOODATA") + dsrc = self._get_ds({'ovfcontent': xml}) + dsrc.get_data() + + # 'data_dir' is '/var/lib/waagent' (walinux-agent's state dir) + # we expect that the ovf-env.xml file is copied there. + ovf_env_path = os.path.join(self.waagent_d, 'ovf-env.xml') + self.assertTrue(os.path.exists(ovf_env_path)) + self.assertEqual(xml, load_file(ovf_env_path)) + def test_existing_ovf_same(self): - mydata = "FOOBAR" - odata = {'UserData': base64.b64encode(mydata)} + # waagent/SharedConfig left alone if found ovf-env.xml same as cached + odata = {'UserData': base64.b64encode("SOMEUSERDATA")} data = {'ovfcontent': construct_valid_ovf_env(data=odata)} - with open("%s/ovf-env.xml" % self.tmp, 'w') as fp: - fp.write(construct_valid_ovf_env(data=odata)) - with open("%s/sem" % self.tmp, 'w') as fp: - fp.write("test") + populate_dir(self.waagent_d, + {'ovf-env.xml': data['ovfcontent'], + 'otherfile': 'otherfile-content', + 'SharedConfig.xml': 'mysharedconfig'}) dsrc = self._get_ds(data) ret = dsrc.get_data() self.assertTrue(ret) - self.assertEqual(dsrc.userdata_raw, mydata) - self.assertTrue(os.path.exists("%s/sem" % self.tmp)) + self.assertTrue(os.path.exists( + os.path.join(self.waagent_d, 'ovf-env.xml'))) + self.assertTrue(os.path.exists( + os.path.join(self.waagent_d, 'otherfile'))) + self.assertTrue(os.path.exists( + os.path.join(self.waagent_d, 'SharedConfig.xml'))) def test_existing_ovf_diff(self): - mydata = "FOOBAR" - odata = {'UserData': base64.b64encode(mydata)} - data = {'ovfcontent': construct_valid_ovf_env(data=odata)} + # waagent/SharedConfig must be removed if ovfenv is found elsewhere - data_dir = "%s/var/lib/waagent" % self.tmp - os.makedirs(data_dir) - with open("%s/ovf-env.xml" % data_dir, 'w') as fp: - fp.write(construct_valid_ovf_env()) - with open("%s/sem" % data_dir, 'w') as fp: - fp.write("test") + # 'get_data' should remove SharedConfig.xml in /var/lib/waagent + # if ovf-env.xml differs. + cached_ovfenv = construct_valid_ovf_env( + {'userdata': base64.b64encode("FOO_USERDATA")}) + new_ovfenv = construct_valid_ovf_env( + {'userdata': base64.b64encode("NEW_USERDATA")}) - dsrc = self._get_ds(data) + populate_dir(self.waagent_d, + {'ovf-env.xml': cached_ovfenv, + 'SharedConfig.xml': "mysharedconfigxml", + 'otherfile': 'otherfilecontent'}) + + dsrc = self._get_ds({'ovfcontent': new_ovfenv}) ret = dsrc.get_data() self.assertTrue(ret) - self.assertEqual(dsrc.userdata_raw, mydata) - self.assertFalse(os.path.exists("%s/sem" % data_dir)) + self.assertEqual(dsrc.userdata_raw, "NEW_USERDATA") + self.assertTrue(os.path.exists( + os.path.join(self.waagent_d, 'otherfile'))) + self.assertFalse( + os.path.exists(os.path.join(self.waagent_d, 'SharedConfig.xml'))) + self.assertTrue( + os.path.exists(os.path.join(self.waagent_d, 'ovf-env.xml'))) + self.assertEqual(new_ovfenv, + load_file(os.path.join(self.waagent_d, 'ovf-env.xml'))) class TestReadAzureOvf(MockerTestCase): -- cgit v1.2.3 From 5021612482c918c43651a2b93778df05dcabda95 Mon Sep 17 00:00:00 2001 From: Scott Moser Date: Mon, 10 Feb 2014 15:11:45 -0500 Subject: make a defined var of DATA_DIR_CLEAN_LIST, some pylint cleanups --- cloudinit/sources/DataSourceAzure.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'cloudinit') diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 6b48a340..c7331da5 100644 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -34,6 +34,7 @@ DEFAULT_METADATA = {"instance-id": "iid-AZURE-NODE"} AGENT_START = ['service', 'walinuxagent', 'start'] BOUNCE_COMMAND = ['sh', '-xc', "i=$interface; x=0; ifdown $i || x=$?; ifup $i || x=$?; exit $x"] +DATA_DIR_CLEAN_LIST = ['SharedConfig.xml'] BUILTIN_DS_CONFIG = { 'agent_command': AGENT_START, @@ -101,7 +102,7 @@ class DataSourceAzureNet(sources.DataSource): except BrokenAzureDataSource as exc: raise exc except util.MountFailedError: - LOG.warn("%s was not mountable" % cdev) + LOG.warn("%s was not mountable", cdev) continue (md, self.userdata_raw, cfg, files) = ret @@ -136,9 +137,8 @@ class DataSourceAzureNet(sources.DataSource): if cached_ovfenv != files['ovf-env.xml']: # source was not walinux-agent's datadir, so we have to clean # up so 'wait_for_files' doesn't return early due to stale data - toclean = ['SharedConfig.xml'] cleaned = [] - for f in [os.path.join(ddir, f) for f in toclean]: + for f in [os.path.join(ddir, f) for f in DATA_DIR_CLEAN_LIST]: if os.path.exists(f): util.del_file(f) cleaned.append(f) @@ -156,7 +156,7 @@ class DataSourceAzureNet(sources.DataSource): self.metadata.get('local-hostname'), mycfg['hostname_bounce']) except Exception as e: - LOG.warn("Failed publishing hostname: %s" % e) + LOG.warn("Failed publishing hostname: %s", e) util.logexc(LOG, "handling set_hostname failed") try: @@ -186,7 +186,7 @@ class DataSourceAzureNet(sources.DataSource): try: self.metadata['instance-id'] = iid_from_shared_config(shcfgxml) except ValueError as e: - LOG.warn("failed to get instance id in %s: %s" % (shcfgxml, e)) + LOG.warn("failed to get instance id in %s: %s", shcfgxml, e) pubkeys = pubkeys_from_crt_files(fp_files) @@ -267,7 +267,7 @@ def pubkeys_from_crt_files(flist): errors.append(fname) if errors: - LOG.warn("failed to convert the crt files to pubkey: %s" % errors) + LOG.warn("failed to convert the crt files to pubkey: %s", errors) return pubkeys @@ -298,7 +298,7 @@ def write_files(datadir, files, dirmode=None): def invoke_agent(cmd): # this is a function itself to simplify patching it for test if cmd: - LOG.debug("invoking agent: %s" % cmd) + LOG.debug("invoking agent: %s", cmd) util.subp(cmd, shell=(not isinstance(cmd, list))) else: LOG.debug("not invoking agent") @@ -345,7 +345,7 @@ def load_azure_ovf_pubkeys(sshnode): continue cur = {'fingerprint': "", 'path': ""} for child in pk_node.childNodes: - if (child.nodeType == text_node or not child.localName): + if child.nodeType == text_node or not child.localName: continue name = child.localName.lower() @@ -431,7 +431,7 @@ def read_azure_ovf(contents): # we accept either UserData or CustomData. If both are present # then behavior is undefined. - if (name == "userdata" or name == "customdata"): + if name == "userdata" or name == "customdata": if attrs.get('encoding') in (None, "base64"): ud = base64.b64decode(''.join(value.split())) else: -- cgit v1.2.3