From 719fbcd15614253ccaa0e77dfb460ee0a991f830 Mon Sep 17 00:00:00 2001 From: Johnson Shi Date: Mon, 7 Dec 2020 23:58:47 +0000 Subject: [PATCH 1/9] Decrease Azure ephemeral disk wait time --- cloudinit/sources/DataSourceAzure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 748a971678e..e4816a1800d 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -1468,7 +1468,7 @@ def count_files(mp): @azure_ds_telemetry_reporter -def address_ephemeral_resize(devpath=RESOURCE_DISK_PATH, maxwait=120, +def address_ephemeral_resize(devpath=RESOURCE_DISK_PATH, maxwait=5, is_new_instance=False, preserve_ntfs=False): # wait for ephemeral disk to come up naplen = .2 From fdd65ad82fcda047b986d4841f70f0fa2eb86ca4 Mon Sep 17 00:00:00 2001 From: Johnson Shi Date: Tue, 8 Dec 2020 01:49:40 +0000 Subject: [PATCH 2/9] Use LOG.debug for logging ephemeral disk not appearing --- cloudinit/sources/DataSourceAzure.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index e4816a1800d..31bd23f100b 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -1486,7 +1486,7 @@ def address_ephemeral_resize(devpath=RESOURCE_DISK_PATH, maxwait=5, report_diagnostic_event( "ephemeral device '%s' did not appear after %d seconds." % (devpath, maxwait), - logger_func=LOG.warning) + logger_func=LOG.debug) return result = False From 05cd49db952b35f13e461bde684cd83e16b1a2c7 Mon Sep 17 00:00:00 2001 From: Johnson Shi Date: Wed, 3 Feb 2021 21:25:25 +0000 Subject: [PATCH 3/9] Azure: Only process ephemeral disk if exists --- cloudinit/sources/DataSourceAzure.py | 36 +++++++++++----------------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 31bd23f100b..35ef1fe408a 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -270,7 +270,7 @@ def get_resource_disk_on_freebsd(port_id): } # RELEASE_BLOCKER: Xenial and earlier apply_network_config default is False -BUILTIN_CLOUD_CONFIG = { +BUILTIN_CLOUD_EPHEMERAL_DISK_CONFIG = { 'disk_setup': { 'ephemeral0': {'table_type': 'gpt', 'layout': [100], @@ -618,8 +618,13 @@ def _get_data(self): maybe_remove_ubuntu_network_config_scripts() # Process crawled data and augment with various config defaults - self.cfg = util.mergemanydict( - [crawled_data['cfg'], BUILTIN_CLOUD_CONFIG]) + + # Only merge in default cloud config related to the ephemeral disk + # if the ephemeral disk exists + if os.path.exists(RESOURCE_DISK_PATH): + self.cfg = util.mergemanydict( + [crawled_data['cfg'], BUILTIN_CLOUD_EPHEMERAL_DISK_CONFIG]) + self._metadata_imds = crawled_data['metadata']['imds'] self.metadata = util.mergemanydict( [crawled_data['metadata'], DEFAULT_METADATA]) @@ -1468,26 +1473,13 @@ def count_files(mp): @azure_ds_telemetry_reporter -def address_ephemeral_resize(devpath=RESOURCE_DISK_PATH, maxwait=5, +def address_ephemeral_resize(devpath=RESOURCE_DISK_PATH, is_new_instance=False, preserve_ntfs=False): - # wait for ephemeral disk to come up - naplen = .2 - with events.ReportEventStack( - name="wait-for-ephemeral-disk", - description="wait for ephemeral disk", - parent=azure_ds_reporter - ): - missing = util.wait_for_files([devpath], - maxwait=maxwait, - naplen=naplen, - log_pre="Azure ephemeral disk: ") - - if missing: - report_diagnostic_event( - "ephemeral device '%s' did not appear after %d seconds." % - (devpath, maxwait), - logger_func=LOG.debug) - return + if not os.path.exists(devpath): + report_diagnostic_event( + "ephemeral resource disk '%s' does not exist." % devpath, + logger_func=LOG.debug) + return result = False msg = None From feecb2ec1fb9d242f32705f4cd5fb9fe4f1158cc Mon Sep 17 00:00:00 2001 From: Johnson Shi Date: Wed, 3 Feb 2021 21:40:06 +0000 Subject: [PATCH 4/9] Azure: Update log msg on presence of ephemeral disk --- cloudinit/sources/DataSourceAzure.py | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 35ef1fe408a..5d8e8dc0d16 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -621,9 +621,21 @@ def _get_data(self): # Only merge in default cloud config related to the ephemeral disk # if the ephemeral disk exists - if os.path.exists(RESOURCE_DISK_PATH): + devpath = RESOURCE_DISK_PATH + if os.path.exists(devpath): + report_diagnostic_event( + "Ephemeral resource disk '%s' exists. " + "Merging default Azure cloud ephemeral disk configs." + % devpath, + logger_func=LOG.debug) self.cfg = util.mergemanydict( [crawled_data['cfg'], BUILTIN_CLOUD_EPHEMERAL_DISK_CONFIG]) + else: + report_diagnostic_event( + "Ephemeral resource disk '%s' does not exist. " + "Not merging default Azure cloud ephemeral disk configs." + % devpath, + logger_func=LOG.debug) self._metadata_imds = crawled_data['metadata']['imds'] self.metadata = util.mergemanydict( @@ -1477,9 +1489,13 @@ def address_ephemeral_resize(devpath=RESOURCE_DISK_PATH, is_new_instance=False, preserve_ntfs=False): if not os.path.exists(devpath): report_diagnostic_event( - "ephemeral resource disk '%s' does not exist." % devpath, + "Ephemeral resource disk '%s' does not exist." % devpath, logger_func=LOG.debug) return + else: + report_diagnostic_event( + "Ephemeral resource disk '%s' exists." % devpath, + logger_func=LOG.debug) result = False msg = None From 12e3ea03f1eaaae6da5f2c17936e172543f1a605 Mon Sep 17 00:00:00 2001 From: Johnson Shi Date: Wed, 3 Feb 2021 22:42:33 +0000 Subject: [PATCH 5/9] Azure: fix _get_data bug with undefined self.cfg ref --- cloudinit/sources/DataSourceAzure.py | 1 + 1 file changed, 1 insertion(+) diff --git a/cloudinit/sources/DataSourceAzure.py b/cloudinit/sources/DataSourceAzure.py index 5d8e8dc0d16..cee630f703c 100755 --- a/cloudinit/sources/DataSourceAzure.py +++ b/cloudinit/sources/DataSourceAzure.py @@ -636,6 +636,7 @@ def _get_data(self): "Not merging default Azure cloud ephemeral disk configs." % devpath, logger_func=LOG.debug) + self.cfg = crawled_data['cfg'] self._metadata_imds = crawled_data['metadata']['imds'] self.metadata = util.mergemanydict( From 625d20fc2088e7ff8de25d4416f96a67f1abc8e6 Mon Sep 17 00:00:00 2001 From: Johnson Shi Date: Wed, 17 Feb 2021 21:09:22 +0000 Subject: [PATCH 6/9] Add UTs for ephemeral present and absent --- tests/unittests/test_datasource/test_azure.py | 52 ++++++++++++++----- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/tests/unittests/test_datasource/test_azure.py b/tests/unittests/test_datasource/test_azure.py index 152a2e1ae6e..f597c7236ae 100644 --- a/tests/unittests/test_datasource/test_azure.py +++ b/tests/unittests/test_datasource/test_azure.py @@ -1354,23 +1354,51 @@ def test_cfg_has_no_fingerprint_has_value(self): for mypk in mypklist: self.assertIn(mypk['value'], dsrc.metadata['public-keys']) - def test_default_ephemeral(self): - # make sure the ephemeral device works + def test_default_ephemeral_configs_ephemeral_exists(self): + # make sure the ephemeral configs are correct if disk present odata = {} data = {'ovfcontent': construct_valid_ovf_env(data=odata), 'sys_cfg': {}} - dsrc = self._get_ds(data) - ret = dsrc.get_data() - self.assertTrue(ret) - cfg = dsrc.get_config_obj() + orig_exists = dsaz.os.path.exists + + def changed_exists(path): + return True if path == dsaz.RESOURCE_DISK_PATH else orig_exists( + path) + + with mock.patch(MOCKPATH + 'os.path.exists', new=changed_exists): + dsrc = self._get_ds(data) + ret = dsrc.get_data() + self.assertTrue(ret) + cfg = dsrc.get_config_obj() + + self.assertEqual(dsrc.device_name_to_device("ephemeral0"), + dsaz.RESOURCE_DISK_PATH) + assert 'disk_setup' in cfg + assert 'fs_setup' in cfg + self.assertIsInstance(cfg['disk_setup'], dict) + self.assertIsInstance(cfg['fs_setup'], list) + + def test_default_ephemeral_configs_ephemeral_does_not_exist(self): + # make sure the ephemeral configs are correct if disk not present + odata = {} + data = {'ovfcontent': construct_valid_ovf_env(data=odata), + 'sys_cfg': {}} + + orig_exists = dsaz.os.path.exists + + def changed_exists(path): + return False if path == dsaz.RESOURCE_DISK_PATH else orig_exists( + path) + + with mock.patch(MOCKPATH + 'os.path.exists', new=changed_exists): + dsrc = self._get_ds(data) + ret = dsrc.get_data() + self.assertTrue(ret) + cfg = dsrc.get_config_obj() - self.assertEqual(dsrc.device_name_to_device("ephemeral0"), - dsaz.RESOURCE_DISK_PATH) - assert 'disk_setup' in cfg - assert 'fs_setup' in cfg - self.assertIsInstance(cfg['disk_setup'], dict) - self.assertIsInstance(cfg['fs_setup'], list) + assert 'disk_setup' not in cfg + assert 'fs_setup' not in cfg def test_provide_disk_aliases(self): # Make sure that user can affect disk aliases From 4cefabbbd12ae8fa72cc78dad9fcb05348049f63 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Fri, 19 Feb 2021 17:37:31 -0500 Subject: [PATCH 7/9] integration test for ephemeral instance --- integration-requirements.txt | 2 +- .../integration_tests/bugs/test_lp1901011.py | 36 +++++++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) create mode 100644 tests/integration_tests/bugs/test_lp1901011.py diff --git a/integration-requirements.txt b/integration-requirements.txt index c64b3b26e68..6b5964260ed 100644 --- a/integration-requirements.txt +++ b/integration-requirements.txt @@ -1,5 +1,5 @@ # PyPI requirements for cloud-init integration testing # https://cloudinit.readthedocs.io/en/latest/topics/integration_tests.html # -pycloudlib @ git+https://github.com/canonical/pycloudlib.git@3a6c668fed769f00d83d1e6bea7d68953787cc38 +pycloudlib @ git+https://github.com/canonical/pycloudlib.git@da8445325875674394ffd85aaefaa3d2d0e0020d pytest diff --git a/tests/integration_tests/bugs/test_lp1901011.py b/tests/integration_tests/bugs/test_lp1901011.py new file mode 100644 index 00000000000..b666334a73c --- /dev/null +++ b/tests/integration_tests/bugs/test_lp1901011.py @@ -0,0 +1,36 @@ +"""Integration test for LP: #1901011 + +Ensure an ephemeral disk exists after boot. + +See https://github.com/canonical/cloud-init/pull/800 +""" +import re + +import pytest + +from tests.integration_tests.clouds import IntegrationCloud + + +@pytest.mark.azure +@pytest.mark.parametrize('instance_type,is_ephemeral', [ + ('Standard_DS1_v2', True), + ('Standard_D2s_v4', False), +]) +def test_ephemeral(instance_type, is_ephemeral, + session_cloud: IntegrationCloud, setup_image): + if is_ephemeral: + expected_log = ( + 'Ephemeral resource disk .* exists. Merging default Azure cloud ' + 'ephemeral disk configs.' + ) + else: + expected_log = ( + 'Ephemeral resource disk .* does not exist. Not merging ' + 'default Azure cloud ephemeral disk configs.' + ) + + with session_cloud.launch( + launch_kwargs={'instance_type': instance_type} + ) as client: + log = client.read_from_file('/var/log/cloud-init.log') + assert re.search(expected_log, log) is not None From a8192b2ecbe4d7833e33b01a4e2ca25c971e4b39 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Mon, 22 Feb 2021 15:42:52 -0500 Subject: [PATCH 8/9] Added checking devices/mounts to test --- .../integration_tests/bugs/test_lp1901011.py | 26 +++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/tests/integration_tests/bugs/test_lp1901011.py b/tests/integration_tests/bugs/test_lp1901011.py index b666334a73c..5c3e517328f 100644 --- a/tests/integration_tests/bugs/test_lp1901011.py +++ b/tests/integration_tests/bugs/test_lp1901011.py @@ -20,17 +20,33 @@ def test_ephemeral(instance_type, is_ephemeral, session_cloud: IntegrationCloud, setup_image): if is_ephemeral: expected_log = ( - 'Ephemeral resource disk .* exists. Merging default Azure cloud ' - 'ephemeral disk configs.' + "Ephemeral resource disk '/dev/disk/cloud/azure_resource' exists. " + "Merging default Azure cloud ephemeral disk configs." ) else: expected_log = ( - 'Ephemeral resource disk .* does not exist. Not merging ' - 'default Azure cloud ephemeral disk configs.' + "Ephemeral resource disk '/dev/disk/cloud/azure_resource' does " + "not exist. Not merging default Azure cloud ephemeral disk " + "configs." ) with session_cloud.launch( launch_kwargs={'instance_type': instance_type} ) as client: + # Verify log file log = client.read_from_file('/var/log/cloud-init.log') - assert re.search(expected_log, log) is not None + assert expected_log in log + + # Verify devices + dev_links = client.execute('ls -l /dev/disk/cloud') + assert 'azure_root -> ../../sda' in dev_links + assert 'azure_root-part1 -> ../../sda1' in dev_links + if is_ephemeral: + assert 'azure_resource -> ../../sdb' in dev_links + assert 'azure_resource-part1 -> ../../sdb1' in dev_links + + # Verify mounts + blks = client.execute('lsblk') + assert re.search('sda1.*part /', blks) is not None + if is_ephemeral: + assert re.search('sdb1.*part /mnt', blks) is not None From 1d25e980992458376ce1e74a369c5d27ecca2c19 Mon Sep 17 00:00:00 2001 From: James Falcon Date: Mon, 22 Feb 2021 16:16:38 -0500 Subject: [PATCH 9/9] updated test based on comments --- .../integration_tests/bugs/test_lp1901011.py | 26 ++++++++++++------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/tests/integration_tests/bugs/test_lp1901011.py b/tests/integration_tests/bugs/test_lp1901011.py index 5c3e517328f..2b47f0a8509 100644 --- a/tests/integration_tests/bugs/test_lp1901011.py +++ b/tests/integration_tests/bugs/test_lp1901011.py @@ -4,8 +4,6 @@ See https://github.com/canonical/cloud-init/pull/800 """ -import re - import pytest from tests.integration_tests.clouds import IntegrationCloud @@ -38,15 +36,23 @@ def test_ephemeral(instance_type, is_ephemeral, assert expected_log in log # Verify devices - dev_links = client.execute('ls -l /dev/disk/cloud') - assert 'azure_root -> ../../sda' in dev_links - assert 'azure_root-part1 -> ../../sda1' in dev_links + dev_links = client.execute('ls /dev/disk/cloud') + assert 'azure_root' in dev_links + assert 'azure_root-part1' in dev_links if is_ephemeral: - assert 'azure_resource -> ../../sdb' in dev_links - assert 'azure_resource-part1 -> ../../sdb1' in dev_links + assert 'azure_resource' in dev_links + assert 'azure_resource-part1' in dev_links # Verify mounts - blks = client.execute('lsblk') - assert re.search('sda1.*part /', blks) is not None + blks = client.execute('lsblk -pPo NAME,TYPE,MOUNTPOINT') + root_device = client.execute( + 'realpath /dev/disk/cloud/azure_root-part1' + ) + assert 'NAME="{}" TYPE="part" MOUNTPOINT="/"'.format( + root_device) in blks if is_ephemeral: - assert re.search('sdb1.*part /mnt', blks) is not None + ephemeral_device = client.execute( + 'realpath /dev/disk/cloud/azure_resource-part1' + ) + assert 'NAME="{}" TYPE="part" MOUNTPOINT="/mnt"'.format( + ephemeral_device) in blks