Azure: get instance id from dmi instead of SharedConfig

Replace the use of SharedConfig.xml in both the walinuxagent case,
and the case where we communicate with the Azure fabric ourselves.

The instance id present in the dmi data is unfortunately different
that that in the SharedConfig.  This means that something needs
to handle migration so that a reboot after newer version is installed
will not re-run first instance things.

In Ubuntu this is being handled in packaging.

LP: #1506187

5 files changed, 24 insertions, 155 deletions

diff --git a/ChangeLog b/ChangeLog
index 8657486f..f41412fa 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -68,6 +68,7 @@
  - Ubuntu templates: modify sources.list template to provide same sources
    as install from server or desktop ISO. (LP: #1177432)
  - cc_mounts: use 'nofail' if system uses systemd. (LP: #1514485)
+ - Azure: get instance id from dmi instead of SharedConfig (LP: #1506187)
 0.7.6:
  - open 0.7.6
  - Enable vendordata on CloudSigma datasource (LP: #1303986)
diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index c6228e6c..bd80a8a6 100644
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -31,8 +31,7 @@ from cloudinit import log as logging
 from cloudinit.settings import PER_ALWAYS
 from cloudinit import sources
 from cloudinit import util
-from cloudinit.sources.helpers.azure import (
-    get_metadata_from_fabric, iid_from_shared_config_content)
+from cloudinit.sources.helpers.azure import get_metadata_from_fabric
 
 LOG = logging.getLogger(__name__)
 
@@ -41,7 +40,6 @@ 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,
@@ -144,8 +142,6 @@ class DataSourceAzureNet(sources.DataSource):
                             self.ds_cfg['agent_command'])
 
             ddir = self.ds_cfg['data_dir']
-            shcfgxml = os.path.join(ddir, "SharedConfig.xml")
-            wait_for = [shcfgxml]
 
             fp_files = []
             key_value = None
@@ -160,19 +156,11 @@ class DataSourceAzureNet(sources.DataSource):
 
             missing = util.log_time(logfunc=LOG.debug, msg="waiting for files",
                                     func=wait_for_files,
-                                    args=(wait_for + fp_files,))
+                                    args=(fp_files,))
         if len(missing):
             LOG.warn("Did not find files, but going on: %s", missing)
 
         metadata = {}
-        if shcfgxml in missing:
-            LOG.warn("SharedConfig.xml missing, using static instance-id")
-        else:
-            try:
-                metadata['instance-id'] = iid_from_shared_config(shcfgxml)
-            except ValueError as e:
-                LOG.warn("failed to get instance id in %s: %s", shcfgxml, e)
-
         metadata['public-keys'] = key_value or pubkeys_from_crt_files(fp_files)
         return metadata
 
@@ -229,21 +217,6 @@ 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])
 
-        if found != ddir:
-            cached_ovfenv = util.load_file(
-                os.path.join(ddir, 'ovf-env.xml'), quiet=True, decode=False)
-            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
-                cleaned = []
-                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)
-                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(ddir, files, dirmode=0o700)
@@ -259,6 +232,7 @@ class DataSourceAzureNet(sources.DataSource):
                      " on Azure.", exc_info=True)
             return False
 
+        self.metadata['instance-id'] = util.read_dmi_data('system-uuid')
         self.metadata.update(fabric_data)
 
         found_ephemeral = find_fabric_formatted_ephemeral_disk()
@@ -649,12 +623,6 @@ def load_azure_ds_dir(source_dir):
     return (md, ud, cfg, {'ovf-env.xml': contents})
 
 
-def iid_from_shared_config(path):
-    with open(path, "rb") as fp:
-        content = fp.read()
-    return iid_from_shared_config_content(content)
-
-
 class BrokenAzureDataSource(Exception):
     pass
 
diff --git a/cloudinit/sources/helpers/azure.py b/cloudinit/sources/helpers/azure.py
index 281d733e..d90c22fd 100644
--- a/cloudinit/sources/helpers/azure.py
+++ b/cloudinit/sources/helpers/azure.py
@@ -79,12 +79,6 @@ class GoalState(object):
             './Container/RoleInstanceList/RoleInstance/InstanceId')
 
     @property
-    def shared_config_xml(self):
-        url = self._text_from_xpath('./Container/RoleInstanceList/RoleInstance'
-                                    '/Configuration/SharedConfig')
-        return self.http_client.get(url).contents
-
-    @property
     def certificates_xml(self):
         if self._certificates_xml is None:
             url = self._text_from_xpath(
@@ -172,19 +166,6 @@ class OpenSSLManager(object):
         return keys
 
 
-def iid_from_shared_config_content(content):
-    """
-    find INSTANCE_ID in:
-    <?xml version="1.0" encoding="utf-8"?>
-    <SharedConfig version="1.0.0.0" goalStateIncarnation="1">
-    <Deployment name="INSTANCE_ID" guid="{...}" incarnation="0">
-        <Service name="..." guid="{00000000-0000-0000-0000-000000000000}"/>
-    """
-    root = ElementTree.fromstring(content)
-    depnode = root.find('Deployment')
-    return depnode.get('name')
-
-
 class WALinuxAgentShim(object):
 
     REPORT_READY_XML_TEMPLATE = '\n'.join([
@@ -263,8 +244,6 @@ class WALinuxAgentShim(object):
             public_keys = self.openssl_manager.parse_certificates(
                 goal_state.certificates_xml)
         data = {
-            'instance-id': iid_from_shared_config_content(
-                goal_state.shared_config_xml),
             'public-keys': public_keys,
         }
         self._report_ready(goal_state, http_client)
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index ec0435f5..3933794f 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -115,10 +115,6 @@ class TestAzureDataSource(TestCase):
             data['pubkey_files'] = flist
             return ["pubkey_from: %s" % f for f in flist]
 
-        def _iid_from_shared_config(path):
-            data['iid_from_shared_cfg'] = path
-            return 'i-my-azure-id'
-
         if data.get('ovfcontent') is not None:
             populate_dir(os.path.join(self.paths.seed_dir, "azure"),
                          {'ovf-env.xml': data['ovfcontent']})
@@ -127,20 +123,22 @@ class TestAzureDataSource(TestCase):
         mod.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d
 
         self.get_metadata_from_fabric = mock.MagicMock(return_value={
-            'instance-id': 'i-my-azure-id',
             'public-keys': [],
         })
 
+        self.instance_id = 'test-instance-id'
+
         self.apply_patches([
             (mod, 'list_possible_azure_ds_devs', dsdevs),
             (mod, 'invoke_agent', _invoke_agent),
             (mod, 'wait_for_files', _wait_for_files),
             (mod, 'pubkeys_from_crt_files', _pubkeys_from_crt_files),
-            (mod, 'iid_from_shared_config', _iid_from_shared_config),
             (mod, 'perform_hostname_bounce', mock.MagicMock()),
             (mod, 'get_hostname', mock.MagicMock()),
             (mod, 'set_hostname', mock.MagicMock()),
             (mod, 'get_metadata_from_fabric', self.get_metadata_from_fabric),
+            (mod.util, 'read_dmi_data', mock.MagicMock(
+                return_value=self.instance_id)),
         ])
 
         dsrc = mod.DataSourceAzureNet(
@@ -193,7 +191,6 @@ class TestAzureDataSource(TestCase):
         self.assertEqual(dsrc.metadata['local-hostname'], odata['HostName'])
         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
@@ -345,7 +342,6 @@ class TestAzureDataSource(TestCase):
         for mypk in mypklist:
             self.assertIn(mypk['value'], dsrc.metadata['public-keys'])
 
-
     def test_default_ephemeral(self):
         # make sure the ephemeral device works
         odata = {}
@@ -434,54 +430,6 @@ class TestAzureDataSource(TestCase):
         dsrc = self._get_ds({'ovfcontent': xml})
         dsrc.get_data()
 
-    def test_existing_ovf_same(self):
-        # waagent/SharedConfig left alone if found ovf-env.xml same as cached
-        odata = {'UserData': b64e("SOMEUSERDATA")}
-        data = {'ovfcontent': construct_valid_ovf_env(data=odata)}
-
-        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.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):
-        # waagent/SharedConfig must be removed if ovfenv is found elsewhere
-
-        # 'get_data' should remove SharedConfig.xml in /var/lib/waagent
-        # if ovf-env.xml differs.
-        cached_ovfenv = construct_valid_ovf_env(
-            {'userdata': b64e("FOO_USERDATA")})
-        new_ovfenv = construct_valid_ovf_env(
-            {'userdata': b64e("NEW_USERDATA")})
-
-        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, b"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')))
-        new_xml = load_file(os.path.join(self.waagent_d, 'ovf-env.xml'))
-        self.xml_equals(new_ovfenv, new_xml)
-
     def test_exception_fetching_fabric_data_doesnt_propagate(self):
         ds = self._get_ds({'ovfcontent': construct_valid_ovf_env()})
         ds.ds_cfg['agent_command'] = '__builtin__'
@@ -496,6 +444,17 @@ class TestAzureDataSource(TestCase):
         self.assertTrue(ret)
         self.assertEqual('value', ds.metadata['test'])
 
+    def test_instance_id_from_dmidecode_used(self):
+        ds = self._get_ds({'ovfcontent': construct_valid_ovf_env()})
+        ds.get_data()
+        self.assertEqual(self.instance_id, ds.metadata['instance-id'])
+
+    def test_instance_id_from_dmidecode_used_for_builtin(self):
+        ds = self._get_ds({'ovfcontent': construct_valid_ovf_env()})
+        ds.ds_cfg['agent_command'] = '__builtin__'
+        ds.get_data()
+        self.assertEqual(self.instance_id, ds.metadata['instance-id'])
+
 
 class TestAzureBounce(TestCase):
 
@@ -505,9 +464,6 @@ class TestAzureBounce(TestCase):
         self.patches.enter_context(
             mock.patch.object(DataSourceAzure, 'wait_for_files'))
         self.patches.enter_context(
-            mock.patch.object(DataSourceAzure, 'iid_from_shared_config',
-                              mock.MagicMock(return_value='i-my-azure-id')))
-        self.patches.enter_context(
             mock.patch.object(DataSourceAzure, 'list_possible_azure_ds_devs',
                               mock.MagicMock(return_value=[])))
         self.patches.enter_context(
@@ -521,6 +477,9 @@ class TestAzureBounce(TestCase):
         self.patches.enter_context(
             mock.patch.object(DataSourceAzure, 'get_metadata_from_fabric',
                               mock.MagicMock(return_value={})))
+        self.patches.enter_context(
+            mock.patch.object(DataSourceAzure.util, 'read_dmi_data',
+                              mock.MagicMock(return_value='test-instance-id')))
 
     def setUp(self):
         super(TestAzureBounce, self).setUp()
diff --git a/tests/unittests/test_datasource/test_azure_helper.py b/tests/unittests/test_datasource/test_azure_helper.py
index a5228870..0638c974 100644
--- a/tests/unittests/test_datasource/test_azure_helper.py
+++ b/tests/unittests/test_datasource/test_azure_helper.py
@@ -40,7 +40,7 @@ GOAL_STATE_TEMPLATE = """\
           <HostingEnvironmentConfig>
             http://100.86.192.70:80/...hostingEnvironmentConfig...
           </HostingEnvironmentConfig>
-          <SharedConfig>{shared_config_url}</SharedConfig>
+          <SharedConfig>http://100.86.192.70:80/..SharedConfig..</SharedConfig>
           <ExtensionsConfig>
             http://100.86.192.70:80/...extensionsConfig...
           </ExtensionsConfig>
@@ -55,21 +55,6 @@ GOAL_STATE_TEMPLATE = """\
 """
 
 
-class TestReadAzureSharedConfig(unittest.TestCase):
-
-    def test_valid_content(self):
-        xml = """<?xml version="1.0" encoding="utf-8"?>
-            <SharedConfig>
-             <Deployment name="MY_INSTANCE_ID">
-              <Service name="myservice"/>
-              <ServiceInstance name="INSTANCE_ID.0" guid="{abcd-uuid}" />
-             </Deployment>
-            <Incarnation number="1"/>
-            </SharedConfig>"""
-        ret = azure_helper.iid_from_shared_config_content(xml)
-        self.assertEqual("MY_INSTANCE_ID", ret)
-
-
 class TestFindEndpoint(TestCase):
 
     def setUp(self):
@@ -140,7 +125,6 @@ class TestGoalStateParsing(TestCase):
         'incarnation': 1,
         'container_id': 'MyContainerId',
         'instance_id': 'MyInstanceId',
-        'shared_config_url': 'MySharedConfigUrl',
         'certificates_url': 'MyCertificatesUrl',
     }
 
@@ -174,20 +158,9 @@ class TestGoalStateParsing(TestCase):
         goal_state = self._get_goal_state(instance_id=instance_id)
         self.assertEqual(instance_id, goal_state.instance_id)
 
-    def test_shared_config_xml_parsed_and_fetched_correctly(self):
-        http_client = mock.MagicMock()
-        shared_config_url = 'TestSharedConfigUrl'
-        goal_state = self._get_goal_state(
-            http_client=http_client, shared_config_url=shared_config_url)
-        shared_config_xml = goal_state.shared_config_xml
-        self.assertEqual(1, http_client.get.call_count)
-        self.assertEqual(shared_config_url, http_client.get.call_args[0][0])
-        self.assertEqual(http_client.get.return_value.contents,
-                         shared_config_xml)
-
     def test_certificates_xml_parsed_and_fetched_correctly(self):
         http_client = mock.MagicMock()
-        certificates_url = 'TestSharedConfigUrl'
+        certificates_url = 'TestCertificatesUrl'
         goal_state = self._get_goal_state(
             http_client=http_client, certificates_url=certificates_url)
         certificates_xml = goal_state.certificates_xml
@@ -324,8 +297,6 @@ class TestWALinuxAgentShim(TestCase):
                 azure_helper.WALinuxAgentShim, 'find_endpoint'))
         self.GoalState = patches.enter_context(
             mock.patch.object(azure_helper, 'GoalState'))
-        self.iid_from_shared_config_content = patches.enter_context(
-            mock.patch.object(azure_helper, 'iid_from_shared_config_content'))
         self.OpenSSLManager = patches.enter_context(
             mock.patch.object(azure_helper, 'OpenSSLManager'))
         patches.enter_context(
@@ -367,15 +338,6 @@ class TestWALinuxAgentShim(TestCase):
         data = shim.register_with_azure_and_fetch_data()
         self.assertEqual([], data['public-keys'])
 
-    def test_instance_id_returned_in_data(self):
-        shim = azure_helper.WALinuxAgentShim()
-        data = shim.register_with_azure_and_fetch_data()
-        self.assertEqual(
-            [mock.call(self.GoalState.return_value.shared_config_xml)],
-            self.iid_from_shared_config_content.call_args_list)
-        self.assertEqual(self.iid_from_shared_config_content.return_value,
-                         data['instance-id'])
-
     def test_correct_url_used_for_report_ready(self):
         self.find_endpoint.return_value = 'test_endpoint'
         shim = azure_helper.WALinuxAgentShim()