azure: Add reported ready marker file.

This change is for Azure VM Preprovisioning. A bug was found when after
azure VMs report ready the first time, during the time when VM is polling
indefinitely for the new ovf-env.xml from Instance Metadata Service
(IMDS), if a reboot happens, we send another report ready signal to the
fabric, which deletes the reprovisioning data on the node.

This marker file is used to fix this issue so that we will only send a
report ready signal to the fabric when no marker file is present. Then,
create a marker file so that when a reboot does occur, we check if a
marker file has been created and decide whether we would like to send the
repot ready signal.

LP: #1765214

2 files changed, 134 insertions, 57 deletions

diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py
index a71197a6..1b03d460 100644
--- a/cloudinit/sources/DataSourceAzure.py
+++ b/cloudinit/sources/DataSourceAzure.py
@@ -48,6 +48,7 @@ DEFAULT_FS = 'ext4'
 # DMI chassis-asset-tag is set static for all azure instances
 AZURE_CHASSIS_ASSET_TAG = '7783-7084-3265-9085-8269-3286-77'
 REPROVISION_MARKER_FILE = "/var/lib/cloud/data/poll_imds"
+REPORTED_READY_MARKER_FILE = "/var/lib/cloud/data/reported_ready"
 IMDS_URL = "http://169.254.169.254/metadata/reprovisiondata"
 
 
@@ -436,11 +437,12 @@ class DataSourceAzure(sources.DataSource):
             LOG.debug("negotiating already done for %s",
                       self.get_instance_id())
 
-    def _poll_imds(self, report_ready=True):
+    def _poll_imds(self):
         """Poll IMDS for the new provisioning data until we get a valid
         response. Then return the returned JSON object."""
         url = IMDS_URL + "?api-version=2017-04-02"
         headers = {"Metadata": "true"}
+        report_ready = bool(not os.path.isfile(REPORTED_READY_MARKER_FILE))
         LOG.debug("Start polling IMDS")
 
         def exc_cb(msg, exception):
@@ -450,13 +452,17 @@ class DataSourceAzure(sources.DataSource):
             # call DHCP and setup the ephemeral network to acquire the new IP.
             return False
 
-        need_report = report_ready
         while True:
             try:
                 with EphemeralDHCPv4() as lease:
-                    if need_report:
+                    if report_ready:
+                        path = REPORTED_READY_MARKER_FILE
+                        LOG.info(
+                            "Creating a marker file to report ready: %s", path)
+                        util.write_file(path, "{pid}: {time}\n".format(
+                            pid=os.getpid(), time=time()))
                         self._report_ready(lease=lease)
-                        need_report = False
+                        report_ready = False
                     return readurl(url, timeout=1, headers=headers,
                                    exception_cb=exc_cb, infinite=True).contents
             except UrlError:
@@ -490,8 +496,10 @@ class DataSourceAzure(sources.DataSource):
         if (cfg.get('PreprovisionedVm') is True or
                 os.path.isfile(path)):
             if not os.path.isfile(path):
-                LOG.info("Creating a marker file to poll imds")
-                util.write_file(path, "%s: %s\n" % (os.getpid(), time()))
+                LOG.info("Creating a marker file to poll imds: %s",
+                         path)
+                util.write_file(path, "{pid}: {time}\n".format(
+                    pid=os.getpid(), time=time()))
             return True
         return False
 
@@ -526,6 +534,7 @@ class DataSourceAzure(sources.DataSource):
                 "Error communicating with Azure fabric; You may experience."
                 "connectivity issues.", exc_info=True)
             return False
+        util.del_file(REPORTED_READY_MARKER_FILE)
         util.del_file(REPROVISION_MARKER_FILE)
         return fabric_data
 
diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py
index 88fe76c7..26e8d7d3 100644
--- a/tests/unittests/test_datasource/test_azure.py
+++ b/tests/unittests/test_datasource/test_azure.py
@@ -1125,19 +1125,9 @@ class TestAzureNetExists(CiTestCase):
         self.assertTrue(hasattr(dsaz, "DataSourceAzureNet"))
 
 
-@mock.patch('cloudinit.sources.DataSourceAzure.util.subp')
-@mock.patch.object(dsaz, 'get_hostname')
-@mock.patch.object(dsaz, 'set_hostname')
-class TestAzureDataSourcePreprovisioning(CiTestCase):
-
-    def setUp(self):
-        super(TestAzureDataSourcePreprovisioning, self).setUp()
-        tmp = self.tmp_dir()
-        self.waagent_d = self.tmp_path('/var/lib/waagent', tmp)
-        self.paths = helpers.Paths({'cloud_dir': tmp})
-        dsaz.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d
+class TestPreprovisioningReadAzureOvfFlag(CiTestCase):
 
-    def test_read_azure_ovf_with_true_flag(self, *args):
+    def test_read_azure_ovf_with_true_flag(self):
         """The read_azure_ovf method should set the PreprovisionedVM
            cfg flag if the proper setting is present."""
         content = construct_valid_ovf_env(
@@ -1146,7 +1136,7 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
         cfg = ret[2]
         self.assertTrue(cfg['PreprovisionedVm'])
 
-    def test_read_azure_ovf_with_false_flag(self, *args):
+    def test_read_azure_ovf_with_false_flag(self):
         """The read_azure_ovf method should set the PreprovisionedVM
            cfg flag to false if the proper setting is false."""
         content = construct_valid_ovf_env(
@@ -1155,7 +1145,7 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
         cfg = ret[2]
         self.assertFalse(cfg['PreprovisionedVm'])
 
-    def test_read_azure_ovf_without_flag(self, *args):
+    def test_read_azure_ovf_without_flag(self):
         """The read_azure_ovf method should not set the
            PreprovisionedVM cfg flag."""
         content = construct_valid_ovf_env()
@@ -1163,12 +1153,121 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
         cfg = ret[2]
         self.assertFalse(cfg['PreprovisionedVm'])
 
-    @mock.patch('cloudinit.sources.DataSourceAzure.util.is_FreeBSD')
-    @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network')
-    @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery')
-    @mock.patch('requests.Session.request')
+
+@mock.patch('os.path.isfile')
+class TestPreprovisioningShouldReprovision(CiTestCase):
+
+    def setUp(self):
+        super(TestPreprovisioningShouldReprovision, self).setUp()
+        tmp = self.tmp_dir()
+        self.waagent_d = self.tmp_path('/var/lib/waagent', tmp)
+        self.paths = helpers.Paths({'cloud_dir': tmp})
+        dsaz.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d
+
+    @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file')
+    def test__should_reprovision_with_true_cfg(self, isfile, write_f):
+        """The _should_reprovision method should return true with config
+           flag present."""
+        isfile.return_value = False
+        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        self.assertTrue(dsa._should_reprovision(
+            (None, None, {'PreprovisionedVm': True}, None)))
+
+    def test__should_reprovision_with_file_existing(self, isfile):
+        """The _should_reprovision method should return True if the sentinal
+           exists."""
+        isfile.return_value = True
+        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        self.assertTrue(dsa._should_reprovision(
+            (None, None, {'preprovisionedvm': False}, None)))
+
+    def test__should_reprovision_returns_false(self, isfile):
+        """The _should_reprovision method should return False
+           if config and sentinal are not present."""
+        isfile.return_value = False
+        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        self.assertFalse(dsa._should_reprovision((None, None, {}, None)))
+
+    @mock.patch('cloudinit.sources.DataSourceAzure.DataSourceAzure._poll_imds')
+    def test_reprovision_calls__poll_imds(self, _poll_imds, isfile):
+        """_reprovision will poll IMDS."""
+        isfile.return_value = False
+        hostname = "myhost"
+        username = "myuser"
+        odata = {'HostName': hostname, 'UserName': username}
+        _poll_imds.return_value = construct_valid_ovf_env(data=odata)
+        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        dsa._reprovision()
+        _poll_imds.assert_called_with()
+
+
+@mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network')
+@mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery')
+@mock.patch('requests.Session.request')
+@mock.patch(
+    'cloudinit.sources.DataSourceAzure.DataSourceAzure._report_ready')
+class TestPreprovisioningPollIMDS(CiTestCase):
+
+    def setUp(self):
+        super(TestPreprovisioningPollIMDS, self).setUp()
+        self.tmp = self.tmp_dir()
+        self.waagent_d = self.tmp_path('/var/lib/waagent', self.tmp)
+        self.paths = helpers.Paths({'cloud_dir': self.tmp})
+        dsaz.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d
+
+    @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file')
+    def test_poll_imds_calls_report_ready(self, write_f, report_ready_func,
+                                          fake_resp, m_dhcp, m_net):
+        """The poll_imds will call report_ready after creating marker file."""
+        report_marker = self.tmp_path('report_marker', self.tmp)
+        lease = {
+            'interface': 'eth9', 'fixed-address': '192.168.2.9',
+            'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0',
+            'unknown-245': '624c3620'}
+        m_dhcp.return_value = [lease]
+        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        mock_path = (
+            'cloudinit.sources.DataSourceAzure.REPORTED_READY_MARKER_FILE')
+        with mock.patch(mock_path, report_marker):
+            dsa._poll_imds()
+        self.assertEqual(report_ready_func.call_count, 1)
+        report_ready_func.assert_called_with(lease=lease)
+
+    def test_poll_imds_report_ready_false(self, report_ready_func,
+                                          fake_resp, m_dhcp, m_net):
+        """The poll_imds should not call reporting ready
+           when flag is false"""
+        report_marker = self.tmp_path('report_marker', self.tmp)
+        write_file(report_marker, content='dont run report_ready :)')
+        m_dhcp.return_value = [{
+            'interface': 'eth9', 'fixed-address': '192.168.2.9',
+            'routers': '192.168.2.1', 'subnet-mask': '255.255.255.0',
+            'unknown-245': '624c3620'}]
+        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
+        mock_path = (
+            'cloudinit.sources.DataSourceAzure.REPORTED_READY_MARKER_FILE')
+        with mock.patch(mock_path, report_marker):
+            dsa._poll_imds()
+        self.assertEqual(report_ready_func.call_count, 0)
+
+
+@mock.patch('cloudinit.sources.DataSourceAzure.util.subp')
+@mock.patch('cloudinit.sources.DataSourceAzure.util.write_file')
+@mock.patch('cloudinit.sources.DataSourceAzure.util.is_FreeBSD')
+@mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network')
+@mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery')
+@mock.patch('requests.Session.request')
+class TestAzureDataSourcePreprovisioning(CiTestCase):
+
+    def setUp(self):
+        super(TestAzureDataSourcePreprovisioning, self).setUp()
+        tmp = self.tmp_dir()
+        self.waagent_d = self.tmp_path('/var/lib/waagent', tmp)
+        self.paths = helpers.Paths({'cloud_dir': tmp})
+        dsaz.BUILTIN_DS_CONFIG['data_dir'] = self.waagent_d
+
     def test_poll_imds_returns_ovf_env(self, fake_resp, m_dhcp, m_net,
-                                       m_is_bsd, *args):
+                                       m_is_bsd, write_f, subp):
         """The _poll_imds method should return the ovf_env.xml."""
         m_is_bsd.return_value = False
         m_dhcp.return_value = [{
@@ -1194,12 +1293,8 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
             prefix_or_mask='255.255.255.0', router='192.168.2.1')
         self.assertEqual(m_net.call_count, 1)
 
-    @mock.patch('cloudinit.sources.DataSourceAzure.util.is_FreeBSD')
-    @mock.patch('cloudinit.net.dhcp.EphemeralIPv4Network')
-    @mock.patch('cloudinit.net.dhcp.maybe_perform_dhcp_discovery')
-    @mock.patch('requests.Session.request')
     def test__reprovision_calls__poll_imds(self, fake_resp, m_dhcp, m_net,
-                                           m_is_bsd, *args):
+                                           m_is_bsd, write_f, subp):
         """The _reprovision method should call poll IMDS."""
         m_is_bsd.return_value = False
         m_dhcp.return_value = [{
@@ -1231,32 +1326,5 @@ class TestAzureDataSourcePreprovisioning(CiTestCase):
             prefix_or_mask='255.255.255.0', router='192.168.2.1')
         self.assertEqual(m_net.call_count, 1)
 
-    @mock.patch('cloudinit.sources.DataSourceAzure.util.write_file')
-    @mock.patch('os.path.isfile')
-    def test__should_reprovision_with_true_cfg(self, isfile, write_f, *args):
-        """The _should_reprovision method should return true with config
-           flag present."""
-        isfile.return_value = False
-        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
-        self.assertTrue(dsa._should_reprovision(
-            (None, None, {'PreprovisionedVm': True}, None)))
-
-    @mock.patch('os.path.isfile')
-    def test__should_reprovision_with_file_existing(self, isfile, *args):
-        """The _should_reprovision method should return True if the sentinal
-           exists."""
-        isfile.return_value = True
-        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
-        self.assertTrue(dsa._should_reprovision(
-            (None, None, {'preprovisionedvm': False}, None)))
-
-    @mock.patch('os.path.isfile')
-    def test__should_reprovision_returns_false(self, isfile, *args):
-        """The _should_reprovision method should return False
-           if config and sentinal are not present."""
-        isfile.return_value = False
-        dsa = dsaz.DataSourceAzure({}, distro=None, paths=self.paths)
-        self.assertFalse(dsa._should_reprovision((None, None, {}, None)))
-
 
 # vi: ts=4 expandtab