From b03bfae6c032a2590e094e9bceeedd47525ca057 Mon Sep 17 00:00:00 2001

From: Eduardo Otubo <otubo@redhat.com>

Date: Fri, 5 Oct 2018 09:53:02 +0200

Subject: [PATCH 3/4] azure: Add reported ready marker file.

RH-Author: Eduardo Otubo <otubo@redhat.com>

Message-id: <20181005095303.20597-4-otubo@redhat.com>

Patchwork-id: 82386

O-Subject: [RHEL-8.0 cloud-init PATCH 3/4] azure: Add reported ready marker file.

Bugzilla: 1615599

RH-Acked-by: Cathy Avery <cavery@redhat.com>

RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>

RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>

commit aae494c39f4c6f625e7409ca262e657d085dd5d1

Author: Joshua Chan <jocha@microsoft.com>

Date:   Thu May 3 14:50:16 2018 -0600

    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

Signed-off-by: Eduardo Otubo <otubo@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 cloudinit/sources/DataSourceAzure.py          |  21 +++-

 tests/unittests/test_datasource/test_azure.py | 170 ++++++++++++++++++--------

 2 files changed, 134 insertions(+), 57 deletions(-)

diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py

index 7e49455..46d5744 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"

@@ -439,11 +440,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):

@@ -453,13 +455,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:

@@ -493,8 +499,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

@@ -529,6 +537,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 af2c93a..ed810d2 100644

--- a/tests/unittests/test_datasource/test_azure.py

+++ b/tests/unittests/test_datasource/test_azure.py

@@ -1196,19 +1196,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(

@@ -1217,7 +1207,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(

@@ -1226,7 +1216,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()

@@ -1234,12 +1224,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 = [{

@@ -1265,12 +1364,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 = [{

@@ -1302,32 +1397,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

-- 

1.8.3.1