From d9105e4062dfd8f9b5dca3926868890d086744d8 Mon Sep 17 00:00:00 2001 From: Julia Kreger Date: Wed, 11 Nov 2020 17:31:05 -0800 Subject: [PATCH] Fix default disk label with partition images Partition images through the agent have the unfortunate side effect of being executed without full node context by default. Luckilly we've had a similar problem and cache the node. This patch changes the lookup from a default of msdos partitions to use the cached node object. Change-Id: I002816c9372fdf1cc32f3c67f420073551479fd9 (cherry picked from commit cb6c0059b5f5bd428623c461bd69afc793441f7e) --- ironic_python_agent/extensions/standby.py | 7 ++- ironic_python_agent/hardware.py | 4 ++ .../tests/unit/extensions/test_standby.py | 58 +++++++++++++++++++ ...partition-table-type-3c78bf78266e8cef.yaml | 6 ++ 4 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/fix-agent-determination-of-partition-table-type-3c78bf78266e8cef.yaml diff --git a/ironic_python_agent/extensions/standby.py b/ironic_python_agent/extensions/standby.py index 0d42c9292..4f9c378b3 100644 --- a/ironic_python_agent/extensions/standby.py +++ b/ironic_python_agent/extensions/standby.py @@ -151,12 +151,17 @@ def _write_partition_image(image, image_info, device): provided image. :raises: ImageWriteError if writing the image to disk encounters any error. """ + # Retrieve the cached node as it has the latest information + # and allows us to also sanity check the deployment so we don't end + # up writing MBR when we're in UEFI mode. + cached_node = hardware.get_cached_node() + node_uuid = image_info.get('node_uuid') preserve_ep = image_info['preserve_ephemeral'] configdrive = image_info['configdrive'] boot_option = image_info.get('boot_option', 'netboot') boot_mode = image_info.get('deploy_boot_mode', 'bios') - disk_label = image_info.get('disk_label', 'msdos') + disk_label = utils.get_partition_table_type_from_specs(cached_node) root_mb = image_info['root_mb'] cpu_arch = hardware.dispatch_to_managers('get_cpus').architecture diff --git a/ironic_python_agent/hardware.py b/ironic_python_agent/hardware.py index e0df214d7..ac9bb1ae8 100644 --- a/ironic_python_agent/hardware.py +++ b/ironic_python_agent/hardware.py @@ -2334,6 +2334,10 @@ def cache_node(node): expected root device to appear. :param node: Ironic node object + :param wait_for_disks: Default True switch to wait for disk setup to be + completed so the node information can be aligned + with the physical storage devices of the host. + This is likely to be used in unit testing. """ global NODE new_node = NODE is None or NODE['uuid'] != node['uuid'] diff --git a/ironic_python_agent/tests/unit/extensions/test_standby.py b/ironic_python_agent/tests/unit/extensions/test_standby.py index 9ca0f244e..5fe7ca581 100644 --- a/ironic_python_agent/tests/unit/extensions/test_standby.py +++ b/ironic_python_agent/tests/unit/extensions/test_standby.py @@ -25,6 +25,7 @@ from ironic_python_agent import errors from ironic_python_agent.extensions import standby from ironic_python_agent import hardware from ironic_python_agent.tests.unit import base +from ironic_python_agent import utils def _build_fake_image_info(url='http://example.org'): @@ -66,6 +67,11 @@ class TestStandbyExtension(base.IronicAgentTest): count=1, architecture='generic', flags='') + with mock.patch('ironic_python_agent.hardware.dispatch_to_managers', + autospec=True): + hardware.cache_node( + {'uuid': '1-2-3-4', + 'instance_info': {}}) def test_validate_image_info_success(self): standby._validate_image_info(None, _build_fake_image_info()) @@ -1358,6 +1364,58 @@ class TestStandbyExtension(base.IronicAgentTest): result = self.agent_extension.get_partition_uuids() self.assertEqual({'1': '2'}, result.serialize()['command_result']) + @mock.patch.object(utils, 'get_partition_table_type_from_specs', + lambda self: 'gpt') + @mock.patch.object(hardware, 'dispatch_to_managers', autospec=True) + @mock.patch('builtins.open', autospec=True) + @mock.patch('ironic_python_agent.utils.execute', autospec=True) + @mock.patch('ironic_lib.disk_utils.get_image_mb', autospec=True) + @mock.patch('ironic_lib.disk_utils.work_on_disk', autospec=True) + def test_write_partition_image_no_node_uuid_uefi( + self, work_on_disk_mock, + image_mb_mock, + execute_mock, open_mock, + dispatch_mock): + image_info = _build_fake_partition_image_info() + image_info['node_uuid'] = None + device = '/dev/sda' + root_mb = image_info['root_mb'] + swap_mb = image_info['swap_mb'] + ephemeral_mb = image_info['ephemeral_mb'] + ephemeral_format = image_info['ephemeral_format'] + node_uuid = image_info['node_uuid'] + pr_ep = image_info['preserve_ephemeral'] + configdrive = image_info['configdrive'] + boot_mode = image_info['deploy_boot_mode'] + boot_option = image_info['boot_option'] + cpu_arch = self.fake_cpu.architecture + + image_path = standby._image_location(image_info) + + image_mb_mock.return_value = 1 + dispatch_mock.return_value = self.fake_cpu + uuids = {'root uuid': 'root_uuid'} + expected_uuid = {'root uuid': 'root_uuid'} + image_mb_mock.return_value = 1 + work_on_disk_mock.return_value = uuids + + standby._write_image(image_info, device) + image_mb_mock.assert_called_once_with(image_path) + work_on_disk_mock.assert_called_once_with(device, root_mb, swap_mb, + ephemeral_mb, + ephemeral_format, + image_path, + node_uuid, + configdrive=configdrive, + preserve_ephemeral=pr_ep, + boot_mode=boot_mode, + boot_option=boot_option, + disk_label='gpt', + cpu_arch=cpu_arch) + + self.assertEqual(expected_uuid, work_on_disk_mock.return_value) + self.assertIsNone(node_uuid) + @mock.patch('hashlib.md5', autospec=True) @mock.patch('requests.get', autospec=True) diff --git a/releasenotes/notes/fix-agent-determination-of-partition-table-type-3c78bf78266e8cef.yaml b/releasenotes/notes/fix-agent-determination-of-partition-table-type-3c78bf78266e8cef.yaml new file mode 100644 index 000000000..dde41296d --- /dev/null +++ b/releasenotes/notes/fix-agent-determination-of-partition-table-type-3c78bf78266e8cef.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixes the agent process for determining what partition label type to + utilize when writing partition images. In many cases, this could fallback + to ``msdos`` if the instance flavor was not properly labeled.