Skip to content

Commit

Permalink
Azure: Support for VMs without ephemeral resource disks. (#800)
Browse files Browse the repository at this point in the history
Changes:
* Only merge in default Azure cloud ephemeral disk configs
during DataSourceAzure._get_data() if the ephemeral disk
exists.
* DataSourceAzure.address_ephemeral_resize() (which is
invoked in DataSourceAzure.activate() should only set up
the ephemeral disk if the disk exists.

Azure VMs may or may not come with ephemeral resource disks
depending on the VM SKU. For VM SKUs that come with
ephemeral resource disks, the Azure platform guarantees that
the ephemeral resource disk is attached to the VM before
the VM is booted. For VM SKUs that do not come with
ephemeral resource disks, cloud-init currently attempts
to wait and set up a non-existent ephemeral resource
disk, which wastes boot time. It also causes disk setup
modules to fail (due to non-existent references to the
ephemeral resource disk).

udevadm settle is invoked by cloud-init very early in boot.
udevadm settle is invoked very early, before
DataSourceAzure's _get_data() and activate() methods.

Within DataSourceAzure's _get_data() and activate() methods,
the ephemeral resource disk path should exist if the
VM SKU comes with an ephemeral resource disk.
The ephemeral resource disk path should not exist if the
VM SKU does not come with an ephemeral resource disk.

LP: #1901011
  • Loading branch information
johnsonshi authored Feb 22, 2021
1 parent e384a54 commit a64b738
Show file tree
Hide file tree
Showing 4 changed files with 130 additions and 35 deletions.
53 changes: 31 additions & 22 deletions cloudinit/sources/DataSourceAzure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -618,8 +618,26 @@ 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
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.cfg = crawled_data['cfg']

self._metadata_imds = crawled_data['metadata']['imds']
self.metadata = util.mergemanydict(
[crawled_data['metadata'], DEFAULT_METADATA])
Expand Down Expand Up @@ -1468,26 +1486,17 @@ 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,
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.warning)
return
if not os.path.exists(devpath):
report_diagnostic_event(
"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
Expand Down
2 changes: 1 addition & 1 deletion integration-requirements.txt
Original file line number Diff line number Diff line change
@@ -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
58 changes: 58 additions & 0 deletions tests/integration_tests/bugs/test_lp1901011.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
"""Integration test for LP: #1901011
Ensure an ephemeral disk exists after boot.
See https://github.com/canonical/cloud-init/pull/800
"""
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 '/dev/disk/cloud/azure_resource' exists. "
"Merging default Azure cloud ephemeral disk configs."
)
else:
expected_log = (
"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 expected_log in log

# Verify devices
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' in dev_links
assert 'azure_resource-part1' in dev_links

# Verify mounts
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:
ephemeral_device = client.execute(
'realpath /dev/disk/cloud/azure_resource-part1'
)
assert 'NAME="{}" TYPE="part" MOUNTPOINT="/mnt"'.format(
ephemeral_device) in blks
52 changes: 40 additions & 12 deletions tests/unittests/test_datasource/test_azure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a64b738

Please sign in to comment.