From b11937faf800b0ae8054dfd64ce50dc92bbb7f80 Mon Sep 17 00:00:00 2001
From: Eduardo Otubo <otubo@redhat.com>
Date: Wed, 26 Sep 2018 13:57:41 +0200
Subject: [PATCH 3/4] azure: Add reported ready marker file.
RH-Author: Eduardo Otubo <otubo@redhat.com>
Message-id: <20180926135742.11140-4-otubo@redhat.com>
Patchwork-id: 82298
O-Subject: [RHEL-7.6 cloud-init PATCHv2 3/4] azure: Add reported ready marker file.
Bugzilla: 1560415
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