Skip to content

Commit f1c519a

Browse files
Zuulopenstack-gerrit
Zuul
authored and
openstack-gerrit
committed
Merge "preserve/handle config drives on 4k block devices" into stable/zed
2 parents 256dfe4 + 5a78bf2 commit f1c519a

4 files changed

Lines changed: 340 additions & 4 deletions

File tree

‎ironic_python_agent/config.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,11 @@
326326
'cleaning from inadvertently destroying a running '
327327
'cluster which may be visible over a storage fabric '
328328
'such as FibreChannel.'),
329+
cfg.BoolOpt('config_drive_rebuild',
330+
default=False,
331+
help='If the agent should rebuild the configuration drive '
332+
'using a local filesystem, instead of letting Ironic '
333+
'determine if this action is necessary.'),
329334
]
330335

331336
CONF.register_cli_opts(cli_opts)

‎ironic_python_agent/partition_utils.py‎

Lines changed: 95 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -465,26 +465,118 @@ def create_config_drive_partition(node_uuid, device, configdrive):
465465
{'part': config_drive_part, 'node': node_uuid})
466466
utils.execute('test', '-e', config_drive_part, attempts=15,
467467
delay_on_retry=True)
468-
469-
disk_utils.dd(confdrive_file, config_drive_part)
468+
if not CONF.config_drive_rebuild:
469+
disk_utils.dd(confdrive_file, config_drive_part)
470+
if not _does_config_drive_work(config_drive_part):
471+
# If we have reached this point, we might have an
472+
# invalid configuration drive, OR the block device
473+
# layer doesn't support 2K block Logical IO (iso9660)
474+
_try_build_fat32_config_drive(config_drive_part,
475+
confdrive_file)
476+
else:
477+
LOG.info('Extracting configuration drive to write copy to disk.')
478+
_try_build_fat32_config_drive(config_drive_part, confdrive_file)
470479
LOG.info("Configdrive for node %(node)s successfully "
471480
"copied onto partition %(part)s",
472481
{'node': node_uuid, 'part': config_drive_part})
473482

483+
except exception.InstanceDeployFailure:
484+
# Since we no longer have a final action on the decorator, we need
485+
# to catch the failure, and still perform the cleanup.
486+
if confdrive_file:
487+
utils.unlink_without_raise(confdrive_file)
488+
raise
474489
except (processutils.UnknownArgumentError,
475490
processutils.ProcessExecutionError, OSError) as e:
476491
msg = ('Failed to create config drive on disk %(disk)s '
477492
'for node %(node)s. Error: %(error)s' %
478493
{'disk': device, 'node': node_uuid, 'error': e})
479494
LOG.error(msg)
480495
raise exception.InstanceDeployFailure(msg)
481-
finally:
482496
# If the configdrive was requested make sure we delete the file
483497
# after copying the content to the partition
498+
499+
finally:
484500
if confdrive_file:
485501
utils.unlink_without_raise(confdrive_file)
486502

487503

504+
def _does_config_drive_work(config_drive_part):
505+
"""Attempts to mount the config drive to validate it works.
506+
507+
:param config_drive_part: The partition to which the configuration drive
508+
was written.
509+
:returns: True if we were able to mount the configuration drive partition.
510+
"""
511+
temp_folder = tempfile.mkdtemp()
512+
try:
513+
# Why: If the filesystem is ISO9660 or vfat, and the logical sector
514+
# size which is supported is *not* something which supports 512 bytes,
515+
# i.e. a 4k Block size, then ISO9660 just will not work. Vfat also
516+
# will not work because the logical size needs to match the logical
517+
# size which is usable. If the underlying driver cannot use that size,
518+
# then the filesystem will not work and cannot be updated because
519+
# structurally it is incompaible with the block device driver.
520+
utils.execute('mount', '-o', 'ro', '-t', 'auto', config_drive_part,
521+
temp_folder)
522+
utils.execute('umount', temp_folder)
523+
except (processutils.ProcessExecutionError, OSError) as e:
524+
LOG.error('Encountered issue attempting to validate the '
525+
'supplied configuration drive. Error: %s', e)
526+
return False
527+
finally:
528+
utils.unlink_without_raise(temp_folder)
529+
530+
return True
531+
532+
533+
def _try_build_fat32_config_drive(partition, confdrive_file):
534+
conf_drive_temp = tempfile.mkdtemp()
535+
try:
536+
utils.execute('mount', '-o', 'loop,ro', '-t', 'auto',
537+
confdrive_file, conf_drive_temp)
538+
except (processutils.ProcessExecutionError, OSError) as e:
539+
# Config drive is invalid, at least to our point of view.
540+
# Bailing.
541+
LOG.warning('We were unable to examine the configuration drive, '
542+
'bypassing. Error: %s', e)
543+
return
544+
545+
new_drive_temp = tempfile.mkdtemp()
546+
try:
547+
# While creating a config drive file from scratch or on
548+
# a loopback will likely result in a 512 byte sector size,
549+
# the underlying fat filesystem utilities *automatically*
550+
# check the device block sector sizing. This *will* break
551+
# above 4k blocks, or at least might. Officially, 4k is the
552+
# *maximum* in the FAT standard. See:
553+
# https://github.com/dosfstools/dosfstools/blame/c483196dd46eab22abba756cef511d36f5f42070/src/mkfs.fat.c#L1987
554+
utils.mkfs(fs='vfat', path=partition, label='CONFIG-2')
555+
utils.execute('mount', '-t', 'auto', partition, new_drive_temp)
556+
# copytree, using copy2, copies everything in the source folder
557+
# into the destination folder, so we should be good, and metadata
558+
# is attempted to be preserved.
559+
shutil.copytree(conf_drive_temp, new_drive_temp, dirs_exist_ok=True)
560+
except (processutils.ProcessExecutionError, OSError) as e:
561+
# We failed to make the filesystem :(
562+
# This is a fairly hard error as we could not use the
563+
# config drive, nor could we recover the state.
564+
LOG.error('We were unable to make a new filesystem for the '
565+
'configuration drive. Error: %s', e)
566+
msg = ('A failure occured while attempting to format, copy, and '
567+
're-create the configuration drive in a structure which '
568+
'is compatible with the underlying hardware and Operating '
569+
'System. Due to the nature of configuration drive, it could '
570+
'have been incorrectly formatted. Operator investigation is '
571+
'required. Error: {}'.format(str(e)))
572+
raise exception.InstanceDeployFailure(msg)
573+
finally:
574+
utils.execute('umount', conf_drive_temp)
575+
utils.execute('umount', new_drive_temp)
576+
utils.unlink_without_raise(new_drive_temp)
577+
utils.unlink_without_raise(conf_drive_temp)
578+
579+
488580
def _is_disk_larger_than_max_size(device, node_uuid):
489581
"""Check if total disk size exceeds 2TB msdos limit
490582

‎ironic_python_agent/tests/unit/test_partition_utils.py‎

Lines changed: 228 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
from ironic_lib import exception
2121
from ironic_lib import utils
2222
from oslo_concurrency import processutils
23+
from oslo_config import cfg
2324
import requests
2425

2526
from ironic_python_agent import errors
@@ -28,6 +29,9 @@
2829
from ironic_python_agent.tests.unit import base
2930

3031

32+
CONF = cfg.CONF
33+
34+
3135
@mock.patch.object(shutil, 'copyfileobj', autospec=True)
3236
@mock.patch.object(requests, 'get', autospec=True)
3337
class GetConfigdriveTestCase(base.IronicAgentTest):
@@ -649,7 +653,10 @@ def test_create_partition_exists(self, mock_get_configdrive,
649653
self.config_part_label,
650654
self.node_uuid)
651655
self.assertFalse(mock_list_partitions.called)
652-
self.assertFalse(mock_execute.called)
656+
mock_execute.assert_has_calls([
657+
mock.call('mount', '-o', 'ro', '-t', 'auto',
658+
'/dev/fake-part1', mock.ANY),
659+
mock.call('umount', mock.ANY)])
653660
self.assertFalse(mock_table_type.called)
654661
mock_dd.assert_called_with(configdrive_file, configdrive_part)
655662
mock_unlink.assert_called_with(configdrive_file)
@@ -706,6 +713,135 @@ def test_create_partition_gpt(self, mock_get_configdrive,
706713
mock_dd.assert_called_with(configdrive_file, expected_part)
707714
mock_unlink.assert_called_with(configdrive_file)
708715

716+
@mock.patch('oslo_utils.uuidutils.generate_uuid', lambda: 'fake-uuid')
717+
@mock.patch.object(partition_utils, '_try_build_fat32_config_drive',
718+
autospec=True)
719+
@mock.patch.object(partition_utils, '_does_config_drive_work',
720+
autospec=True)
721+
@mock.patch.object(utils, 'execute', autospec=True)
722+
@mock.patch.object(utils, 'unlink_without_raise',
723+
autospec=True)
724+
@mock.patch.object(disk_utils, 'dd',
725+
autospec=True)
726+
@mock.patch.object(disk_utils, 'fix_gpt_partition',
727+
autospec=True)
728+
@mock.patch.object(disk_utils, 'get_partition_table_type',
729+
autospec=True)
730+
@mock.patch.object(partition_utils, 'get_partition',
731+
autospec=True)
732+
@mock.patch.object(partition_utils, 'get_labelled_partition',
733+
autospec=True)
734+
@mock.patch.object(partition_utils, 'get_configdrive',
735+
autospec=True)
736+
def test_create_partition_gpt_with_fallback(
737+
self, mock_get_configdrive,
738+
mock_get_labelled_partition,
739+
mock_get_partition_by_uuid,
740+
mock_table_type,
741+
mock_fix_gpt_partition,
742+
mock_dd, mock_unlink, mock_execute,
743+
mock_config_drive_work,
744+
mock_rebuild_config_drive):
745+
config_url = 'http://1.2.3.4/cd'
746+
configdrive_file = '/tmp/xyz'
747+
configdrive_mb = 10
748+
749+
mock_get_configdrive.return_value = (configdrive_mb, configdrive_file)
750+
mock_get_labelled_partition.return_value = None
751+
752+
mock_table_type.return_value = 'gpt'
753+
expected_part = '/dev/fake4'
754+
mock_get_partition_by_uuid.return_value = expected_part
755+
mock_config_drive_work.return_value = False
756+
757+
partition_utils.create_config_drive_partition(self.node_uuid, self.dev,
758+
config_url)
759+
mock_execute.assert_has_calls([
760+
mock.call('sgdisk', '-n', '0:-64MB:0', '-u', '0:fake-uuid',
761+
self.dev, run_as_root=True),
762+
mock.call('sync'),
763+
mock.call('udevadm', 'settle'),
764+
mock.call('partprobe', self.dev, attempts=10, run_as_root=True),
765+
mock.call('sgdisk', '-v', self.dev, run_as_root=True),
766+
767+
mock.call('udevadm', 'settle'),
768+
mock.call('test', '-e', expected_part, attempts=15,
769+
delay_on_retry=True)
770+
])
771+
772+
mock_table_type.assert_called_with(self.dev)
773+
mock_fix_gpt_partition.assert_called_with(self.dev, self.node_uuid)
774+
mock_dd.assert_called_with(configdrive_file, expected_part)
775+
mock_unlink.assert_called_with(configdrive_file)
776+
mock_config_drive_work.assert_called_once_with(expected_part)
777+
mock_rebuild_config_drive.assert_called_once_with(expected_part,
778+
configdrive_file)
779+
780+
@mock.patch('oslo_utils.uuidutils.generate_uuid', lambda: 'fake-uuid')
781+
@mock.patch.object(partition_utils, '_try_build_fat32_config_drive',
782+
autospec=True)
783+
@mock.patch.object(partition_utils, '_does_config_drive_work',
784+
autospec=True)
785+
@mock.patch.object(utils, 'execute', autospec=True)
786+
@mock.patch.object(utils, 'unlink_without_raise',
787+
autospec=True)
788+
@mock.patch.object(disk_utils, 'dd',
789+
autospec=True)
790+
@mock.patch.object(disk_utils, 'fix_gpt_partition',
791+
autospec=True)
792+
@mock.patch.object(disk_utils, 'get_partition_table_type',
793+
autospec=True)
794+
@mock.patch.object(partition_utils, 'get_partition',
795+
autospec=True)
796+
@mock.patch.object(partition_utils, 'get_labelled_partition',
797+
autospec=True)
798+
@mock.patch.object(partition_utils, 'get_configdrive',
799+
autospec=True)
800+
def test_create_partition_gpt_use_vfat(
801+
self, mock_get_configdrive,
802+
mock_get_labelled_partition,
803+
mock_get_partition_by_uuid,
804+
mock_table_type,
805+
mock_fix_gpt_partition,
806+
mock_dd, mock_unlink, mock_execute,
807+
mock_config_drive_work,
808+
mock_rebuild_config_drive):
809+
config_url = 'http://1.2.3.4/cd'
810+
configdrive_file = '/tmp/xyz'
811+
configdrive_mb = 10
812+
813+
CONF.set_override('config_drive_rebuild', True)
814+
mock_get_configdrive.return_value = (configdrive_mb, configdrive_file)
815+
mock_get_labelled_partition.return_value = None
816+
817+
mock_table_type.return_value = 'gpt'
818+
expected_part = '/dev/fake4'
819+
mock_get_partition_by_uuid.return_value = expected_part
820+
mock_config_drive_work.return_value = True
821+
822+
partition_utils.create_config_drive_partition(self.node_uuid, self.dev,
823+
config_url)
824+
mock_execute.assert_has_calls([
825+
mock.call('sgdisk', '-n', '0:-64MB:0', '-u', '0:fake-uuid',
826+
self.dev, run_as_root=True),
827+
mock.call('sync'),
828+
mock.call('udevadm', 'settle'),
829+
mock.call('partprobe', self.dev, attempts=10, run_as_root=True),
830+
mock.call('sgdisk', '-v', self.dev, run_as_root=True),
831+
832+
mock.call('udevadm', 'settle'),
833+
mock.call('test', '-e', expected_part, attempts=15,
834+
delay_on_retry=True)
835+
])
836+
837+
mock_table_type.assert_called_with(self.dev)
838+
mock_fix_gpt_partition.assert_called_with(self.dev, self.node_uuid)
839+
mock_dd.assert_not_called()
840+
mock_unlink.assert_called_with(configdrive_file)
841+
mock_config_drive_work.assert_not_called()
842+
mock_rebuild_config_drive.assert_called_once_with(expected_part,
843+
configdrive_file)
844+
709845
@mock.patch.object(disk_utils, 'count_mbr_partitions', autospec=True)
710846
@mock.patch.object(utils, 'execute', autospec=True)
711847
@mock.patch.object(partition_utils.LOG, 'warning', autospec=True)
@@ -1288,3 +1424,94 @@ def test_label(self, mock_is_md_device, mock_execute):
12881424
mock.call('lsblk', '-PbioKNAME,UUID,PARTUUID,TYPE,LABEL',
12891425
self.fake_dev)]
12901426
mock_execute.assert_has_calls(expected)
1427+
1428+
1429+
@mock.patch.object(utils, 'execute', autospec=True)
1430+
class TestConfigDriveTestRecovery(base.IronicAgentTest):
1431+
1432+
fake_dev = '/dev/fake'
1433+
configdrive_file = '/tmp/config-drive'
1434+
1435+
def test__does_config_drive_work(self, mock_execute):
1436+
self.assertTrue(partition_utils._does_config_drive_work(self.fake_dev))
1437+
mock_execute.assert_has_calls([
1438+
mock.call('mount', '-o', 'ro', '-t', 'auto', self.fake_dev,
1439+
mock.ANY),
1440+
mock.call('umount', mock.ANY)])
1441+
1442+
def test__does_config_drive_failed(self, mock_execute):
1443+
mock_execute.side_effect = processutils.ProcessExecutionError('boom')
1444+
self.assertFalse(
1445+
partition_utils._does_config_drive_work(self.fake_dev)
1446+
)
1447+
mock_execute.assert_has_calls([
1448+
mock.call('mount', '-o', 'ro', '-t', 'auto', self.fake_dev,
1449+
mock.ANY)])
1450+
1451+
@mock.patch.object(shutil, 'copytree', autospec=True)
1452+
@mock.patch.object(utils, 'mkfs', autospec=True)
1453+
def test__try_build_fat32_config_drive(self,
1454+
mock_mkfs,
1455+
mock_copy,
1456+
mock_execute):
1457+
partition_utils._try_build_fat32_config_drive(self.fake_dev,
1458+
self.configdrive_file)
1459+
mock_execute.assert_has_calls([
1460+
mock.call('mount', '-o', 'loop,ro', '-t', 'auto',
1461+
self.configdrive_file, mock.ANY),
1462+
mock.call('mount', '-t', 'auto', self.fake_dev, mock.ANY),
1463+
mock.call('umount', mock.ANY),
1464+
mock.call('umount', mock.ANY),
1465+
])
1466+
mock_mkfs.assert_called_once_with(fs='vfat', path=self.fake_dev,
1467+
label='CONFIG-2')
1468+
# Validate we called copy as we expect, both source and destination
1469+
# are temporary folders.
1470+
mock_copy.assert_called_once_with(mock.ANY, mock.ANY,
1471+
dirs_exist_ok=True)
1472+
1473+
@mock.patch.object(shutil, 'copytree', autospec=True)
1474+
@mock.patch.object(utils, 'mkfs', autospec=True)
1475+
def test__try_build_fat32_config_drive_graceful_fail(
1476+
self,
1477+
mock_mkfs,
1478+
mock_copy,
1479+
mock_execute):
1480+
mock_execute.side_effect = processutils.ProcessExecutionError('boom')
1481+
self.assertIsNone(
1482+
partition_utils._try_build_fat32_config_drive(
1483+
self.fake_dev,
1484+
self.configdrive_file)
1485+
)
1486+
mock_execute.assert_called_once_with(
1487+
'mount', '-o', 'loop,ro', '-t', 'auto',
1488+
self.configdrive_file, mock.ANY)
1489+
mock_mkfs.assert_not_called()
1490+
# Validate we called copy as we expect, both source and destination
1491+
# are temporary folders.
1492+
mock_copy.assert_not_called()
1493+
1494+
@mock.patch.object(shutil, 'copytree', autospec=True)
1495+
@mock.patch.object(utils, 'mkfs', autospec=True)
1496+
def test__try_build_fat32_config_drive_fails_once_invalid(
1497+
self,
1498+
mock_mkfs,
1499+
mock_copy,
1500+
mock_execute):
1501+
mock_mkfs.side_effect = processutils.ProcessExecutionError('boom')
1502+
self.assertRaisesRegex(
1503+
exception.InstanceDeployFailure,
1504+
'A failure occured while attempting to format.*',
1505+
partition_utils._try_build_fat32_config_drive,
1506+
self.fake_dev,
1507+
self.configdrive_file)
1508+
mock_execute.assert_has_calls([
1509+
mock.call('mount', '-o', 'loop,ro', '-t', 'auto',
1510+
self.configdrive_file, mock.ANY),
1511+
mock.call('umount', mock.ANY),
1512+
mock.call('umount', mock.ANY),
1513+
])
1514+
1515+
mock_mkfs.assert_called_once_with(fs='vfat', path=self.fake_dev,
1516+
label='CONFIG-2')
1517+
mock_copy.assert_not_called()

0 commit comments

Comments
 (0)