change behavior to only delete SharedConfig.xml.

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,

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'] = self.waagent_d
-        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)])
 
         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.assertTrue(os.path.isfile(
-        self.assertEqual(0700, data['datadir_mode'])
+            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"
+        # waagent/SharedConfig left alone if found ovf-env.xml same as cached
-        odata = {'UserData': base64.b64encode(mydata)}
+        odata = {'UserData': base64.b64encode("SOMEUSERDATA")}
         data = {'ovfcontent': construct_valid_ovf_env(data=odata)}
 
-        with open("%s/ovf-env.xml" % self.tmp, 'w') as fp:
+        populate_dir(self.waagent_d, 
-            fp.write(construct_valid_ovf_env(data=odata))
+            {'ovf-env.xml': data['ovfcontent'],
-        with open("%s/sem" % self.tmp, 'w') as fp:
+             'otherfile': 'otherfile-content',
-            fp.write("test")
+             '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(
-        self.assertTrue(os.path.exists("%s/sem" % self.tmp))
+            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"
+        # waagent/SharedConfig must be removed if ovfenv is found elsewhere
-        odata = {'UserData': base64.b64encode(mydata)}
-        data = {'ovfcontent': construct_valid_ovf_env(data=odata)}
 
-        data_dir = "%s/var/lib/waagent" % self.tmp
+        # 'get_data' should remove SharedConfig.xml in /var/lib/waagent
-        os.makedirs(data_dir)
+        # if ovf-env.xml differs.
-        with open("%s/ovf-env.xml" % data_dir, 'w') as fp:
+        cached_ovfenv = construct_valid_ovf_env(
-            fp.write(construct_valid_ovf_env())
+            {'userdata': base64.b64encode("FOO_USERDATA")})
-        with open("%s/sem" % data_dir, 'w') as fp:
+        new_ovfenv = construct_valid_ovf_env(
-            fp.write("test")
+            {'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.assertEqual(dsrc.userdata_raw, "NEW_USERDATA")
-        self.assertFalse(os.path.exists("%s/sem" % data_dir))
+        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):