From c3428fd4f0f0b098a2323d270067f64456577b8e Mon Sep 17 00:00:00 2001 From: Pointbr8ker-123 Date: Sun, 2 Nov 2025 08:53:08 +0100 Subject: [PATCH] Move configdrive code to configdrive_utils Moves configdrive utility functions to the more appropriate configdrive dedicated utils module to improve code organization and separation of concern. Closes-Bug: #2113892 Change-Id: I5851e84fe8f15de05dcddca773b1f28f639dc617 Signed-off-by: David Nwosu Signed-off-by: Afonne-CID --- ironic/conductor/configdrive_utils.py | 45 ++++++++++++ ironic/conductor/deployments.py | 3 +- ironic/conductor/utils.py | 47 ------------- ironic/drivers/modules/agent.py | 5 +- ironic/drivers/modules/ansible/deploy.py | 3 +- ironic/drivers/modules/redfish/boot.py | 3 +- .../unit/conductor/test_configdrive_utils.py | 68 +++++++++++++++++++ .../tests/unit/conductor/test_deployments.py | 8 ++- ironic/tests/unit/conductor/test_utils.py | 68 ------------------- .../drivers/modules/ansible/test_deploy.py | 3 +- .../unit/drivers/modules/redfish/test_boot.py | 3 +- .../tests/unit/drivers/modules/test_agent.py | 3 +- 12 files changed, 133 insertions(+), 126 deletions(-) diff --git a/ironic/conductor/configdrive_utils.py b/ironic/conductor/configdrive_utils.py index b140f1e1ca..0322f5d9f5 100644 --- a/ironic/conductor/configdrive_utils.py +++ b/ironic/conductor/configdrive_utils.py @@ -16,9 +16,11 @@ import json import os import tempfile +from openstack.baremetal import configdrive as os_configdrive from oslo_concurrency import processutils from oslo_config import cfg from oslo_log import log +from oslo_serialization import jsonutils import pycdlib from ironic.common import exception @@ -484,3 +486,46 @@ def check_and_fix_configdrive(task, configdrive): # Always return configdrive content, otherwise we hand None back # which causes the overall process to fail. return configdrive + + +def build_configdrive(node, configdrive): + """Build a configdrive from provided meta_data, network_data and user_data. + + If uuid or name are not provided in the meta_data, they're defaulted to the + node's uuid and name accordingly. + + :param node: an Ironic node object. + :param configdrive: A configdrive as a dict with keys ``meta_data``, + ``network_data``, ``user_data`` and ``vendor_data`` (all optional). + :returns: A gzipped and base64 encoded configdrive as a string. + """ + meta_data = configdrive.setdefault('meta_data', {}) + meta_data.setdefault('uuid', node.uuid) + if node.name: + meta_data.setdefault('name', node.name) + + user_data = configdrive.get('user_data') + if isinstance(user_data, (dict, list)): + user_data = jsonutils.dump_as_bytes(user_data) + elif user_data: + user_data = user_data.encode('utf-8') + + LOG.debug('Building a configdrive for node %s', node.uuid) + return os_configdrive.build(meta_data, user_data=user_data, + network_data=configdrive.get('network_data'), + vendor_data=configdrive.get('vendor_data')) + + +def get_configdrive_image(node): + """Get configdrive as an ISO image or a URL. + + Converts the JSON representation into an image. URLs and raw contents + are returned unchanged. + + :param node: an Ironic node object. + :returns: A gzipped and base64 encoded configdrive as a string. + """ + configdrive = node.instance_info.get('configdrive') + if isinstance(configdrive, dict): + configdrive = build_configdrive(node, configdrive) + return configdrive diff --git a/ironic/conductor/deployments.py b/ironic/conductor/deployments.py index 84a950daba..0a9fe87506 100644 --- a/ironic/conductor/deployments.py +++ b/ironic/conductor/deployments.py @@ -555,7 +555,8 @@ def _store_configdrive(node, configdrive): if CONF.deploy.configdrive_use_object_store: # Don't store the JSON source in swift. if isinstance(configdrive, dict): - configdrive = utils.build_configdrive(node, configdrive) + configdrive = configdrive_utils.build_configdrive( + node, configdrive) # NOTE(lucasagomes): No reason to use a different timeout than # the one used for deploying the node diff --git a/ironic/conductor/utils.py b/ironic/conductor/utils.py index ccdb778ffc..65cb52bdfc 100644 --- a/ironic/conductor/utils.py +++ b/ironic/conductor/utils.py @@ -20,10 +20,8 @@ import os import secrets import time -from openstack.baremetal import configdrive as os_configdrive from oslo_config import cfg from oslo_log import log -from oslo_serialization import jsonutils from oslo_service import loopingcall from oslo_utils import excutils from oslo_utils import secretutils @@ -1181,51 +1179,6 @@ def power_state_for_network_configuration(task): restore_power_state_if_needed(task, previous) -# NOTE(TheJulia): Move this to configdrive_utils at some point in the future. -def build_configdrive(node, configdrive): - """Build a configdrive from provided meta_data, network_data and user_data. - - If uuid or name are not provided in the meta_data, they're defaulted to the - node's uuid and name accordingly. - - :param node: an Ironic node object. - :param configdrive: A configdrive as a dict with keys ``meta_data``, - ``network_data``, ``user_data`` and ``vendor_data`` (all optional). - :returns: A gzipped and base64 encoded configdrive as a string. - """ - meta_data = configdrive.setdefault('meta_data', {}) - meta_data.setdefault('uuid', node.uuid) - if node.name: - meta_data.setdefault('name', node.name) - - user_data = configdrive.get('user_data') - if isinstance(user_data, (dict, list)): - user_data = jsonutils.dump_as_bytes(user_data) - elif user_data: - user_data = user_data.encode('utf-8') - - LOG.debug('Building a configdrive for node %s', node.uuid) - return os_configdrive.build(meta_data, user_data=user_data, - network_data=configdrive.get('network_data'), - vendor_data=configdrive.get('vendor_data')) - - -# NOTE(TheJulia): Move this to configdrive_utils at some point in the future. -def get_configdrive_image(node): - """Get configdrive as an ISO image or a URL. - - Converts the JSON representation into an image. URLs and raw contents - are returned unchanged. - - :param node: an Ironic node object. - :returns: A gzipped and base64 encoded configdrive as a string. - """ - configdrive = node.instance_info.get('configdrive') - if isinstance(configdrive, dict): - configdrive = build_configdrive(node, configdrive) - return configdrive - - def fast_track_able(task): """Checks if the operation can be a streamlined deployment sequence. diff --git a/ironic/drivers/modules/agent.py b/ironic/drivers/modules/agent.py index 6c93abebfc..994095898e 100644 --- a/ironic/drivers/modules/agent.py +++ b/ironic/drivers/modules/agent.py @@ -33,6 +33,7 @@ from ironic.common import oci_registry as oci from ironic.common import raid from ironic.common import states from ironic.common import utils +from ironic.conductor import configdrive_utils from ironic.conductor import deployments from ironic.conductor import steps as conductor_steps from ironic.conductor import task_manager @@ -760,7 +761,7 @@ class AgentDeploy(CustomAgentDeploy): if disk_label is not None: image_info['disk_label'] = disk_label - configdrive = manager_utils.get_configdrive_image(node) + configdrive = configdrive_utils.get_configdrive_image(node) if configdrive: # FIXME(dtantsur): remove this duplication once IPA is ready: # https://review.opendev.org/c/openstack/ironic-python-agent/+/790471 @@ -1001,7 +1002,7 @@ class BootcAgentDeploy(CustomAgentDeploy): # FIXME(TheJulia): We likely, either need to grab/collect creds # and pass them along in the step call, or initialize the client. # bootc runs in the target container as well, so ... hmmm - configdrive = manager_utils.get_configdrive_image(node) + configdrive = configdrive_utils.get_configdrive_image(node) img_auth = image_service.get_image_service_auth_override(task.node) diff --git a/ironic/drivers/modules/ansible/deploy.py b/ironic/drivers/modules/ansible/deploy.py index 77e25550f0..8dfef749eb 100644 --- a/ironic/drivers/modules/ansible/deploy.py +++ b/ironic/drivers/modules/ansible/deploy.py @@ -35,6 +35,7 @@ from ironic.common import images from ironic.common import metrics_utils from ironic.common import states from ironic.common import utils +from ironic.conductor import configdrive_utils from ironic.conductor import steps as conductor_steps from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils @@ -283,7 +284,7 @@ def _prepare_variables(task): image['checksum'] = 'md5:%s' % checksum _add_ssl_image_options(image) variables = {'image': image} - configdrive = manager_utils.get_configdrive_image(task.node) + configdrive = configdrive_utils.get_configdrive_image(task.node) if configdrive: if urlparse.urlparse(configdrive).scheme in ('http', 'https'): cfgdrv_type = 'url' diff --git a/ironic/drivers/modules/redfish/boot.py b/ironic/drivers/modules/redfish/boot.py index b0d6b0b00c..a9d85c2754 100644 --- a/ironic/drivers/modules/redfish/boot.py +++ b/ironic/drivers/modules/redfish/boot.py @@ -22,6 +22,7 @@ from ironic.common import exception from ironic.common.i18n import _ from ironic.common import states from ironic.common import utils as common_utils +from ironic.conductor import configdrive_utils from ironic.conductor import utils as manager_utils from ironic.conf import CONF from ironic.drivers import base @@ -931,7 +932,7 @@ class RedfishVirtualMediaBoot(base.BootInterface): 'device': boot_devices.CDROM}) def _attach_configdrive(self, task, managers): - configdrive = manager_utils.get_configdrive_image(task.node) + configdrive = configdrive_utils.get_configdrive_image(task.node) if not configdrive: return diff --git a/ironic/tests/unit/conductor/test_configdrive_utils.py b/ironic/tests/unit/conductor/test_configdrive_utils.py index ae4806e786..3eb1d069d8 100644 --- a/ironic/tests/unit/conductor/test_configdrive_utils.py +++ b/ironic/tests/unit/conductor/test_configdrive_utils.py @@ -687,3 +687,71 @@ class PatchConfigDriveTestCase(db_base.DbTestCase): mock_invalid.assert_not_called() mock_gen.assert_not_called() self.assertEqual('https://bifrost/url', res) + + +class GetConfigDriveImageTestCase(db_base.DbTestCase): + + def setUp(self): + super(GetConfigDriveImageTestCase, self).setUp() + self.node = obj_utils.create_test_node( + self.context, + uuid=uuidutils.generate_uuid(), + instance_info={}) + + def test_no_configdrive(self): + self.assertIsNone(cd_utils.get_configdrive_image(self.node)) + + def test_string(self): + self.node.instance_info['configdrive'] = 'data' + self.assertEqual('data', + cd_utils.get_configdrive_image(self.node)) + + @mock.patch('openstack.baremetal.configdrive.build', autospec=True) + def test_build_empty(self, mock_cd): + self.node.instance_info['configdrive'] = {} + self.assertEqual(mock_cd.return_value, + cd_utils.get_configdrive_image(self.node)) + mock_cd.assert_called_once_with({'uuid': self.node.uuid}, + network_data=None, + user_data=None, + vendor_data=None) + + @mock.patch('openstack.baremetal.configdrive.build', autospec=True) + def test_build_populated(self, mock_cd): + configdrive = { + 'meta_data': {'uuid': uuidutils.generate_uuid(), + 'name': 'new-name', + 'hostname': 'example.com'}, + 'network_data': {'links': []}, + 'vendor_data': {'foo': 'bar'}, + } + self.node.instance_info['configdrive'] = configdrive + self.assertEqual(mock_cd.return_value, + cd_utils.get_configdrive_image(self.node)) + mock_cd.assert_called_once_with( + configdrive['meta_data'], + network_data=configdrive['network_data'], + user_data=None, + vendor_data=configdrive['vendor_data']) + + @mock.patch('openstack.baremetal.configdrive.build', autospec=True) + def test_build_user_data_as_string(self, mock_cd): + self.node.instance_info['configdrive'] = {'user_data': 'abcd'} + self.assertEqual(mock_cd.return_value, + cd_utils.get_configdrive_image(self.node)) + mock_cd.assert_called_once_with({'uuid': self.node.uuid}, + network_data=None, + user_data=b'abcd', + vendor_data=None) + + @mock.patch('openstack.baremetal.configdrive.build', autospec=True) + def test_build_user_data_as_dict(self, mock_cd): + self.node.instance_info['configdrive'] = { + 'user_data': {'user': 'data'} + } + self.assertEqual(mock_cd.return_value, + cd_utils.get_configdrive_image(self.node)) + mock_cd.assert_called_once_with({'uuid': self.node.uuid}, + network_data=None, + user_data=b'{"user": "data"}', + vendor_data=None) diff --git a/ironic/tests/unit/conductor/test_deployments.py b/ironic/tests/unit/conductor/test_deployments.py index 541fe5c995..44c718db6c 100644 --- a/ironic/tests/unit/conductor/test_deployments.py +++ b/ironic/tests/unit/conductor/test_deployments.py @@ -1260,15 +1260,17 @@ class StoreConfigDriveTestCase(db_base.DbTestCase): self.node.refresh() self.assertEqual(expected_instance_info, self.node.instance_info) - @mock.patch.object(conductor_utils, 'build_configdrive', autospec=True) - def test_store_configdrive_swift_build(self, mock_cd, mock_swift): + @mock.patch('ironic.conductor.configdrive_utils.build_configdrive', + autospec=True) + def test_store_configdrive_swift_build( + self, mock_cd, mock_swift): container_name = 'foo_container' timeout = 123 expected_obj_name = 'configdrive-%s' % self.node.uuid expected_obj_header = {'X-Delete-After': str(timeout)} expected_instance_info = {'configdrive': 'http://1.2.3.4'} - mock_cd.return_value = 'fake' + mock_cd.return_value = 'fake_configdrive_content' # set configs and mocks CONF.set_override('configdrive_use_object_store', True, diff --git a/ironic/tests/unit/conductor/test_utils.py b/ironic/tests/unit/conductor/test_utils.py index c168f6769c..5ae3674bf8 100644 --- a/ironic/tests/unit/conductor/test_utils.py +++ b/ironic/tests/unit/conductor/test_utils.py @@ -3026,74 +3026,6 @@ class CacheBootModeTestCase(db_base.DbTestCase): self.assertTrue(self.node.secure_boot) -class GetConfigDriveImageTestCase(db_base.DbTestCase): - - def setUp(self): - super(GetConfigDriveImageTestCase, self).setUp() - self.node = obj_utils.create_test_node( - self.context, - uuid=uuidutils.generate_uuid(), - instance_info={}) - - def test_no_configdrive(self): - self.assertIsNone(conductor_utils.get_configdrive_image(self.node)) - - def test_string(self): - self.node.instance_info['configdrive'] = 'data' - self.assertEqual('data', - conductor_utils.get_configdrive_image(self.node)) - - @mock.patch('openstack.baremetal.configdrive.build', autospec=True) - def test_build_empty(self, mock_cd): - self.node.instance_info['configdrive'] = {} - self.assertEqual(mock_cd.return_value, - conductor_utils.get_configdrive_image(self.node)) - mock_cd.assert_called_once_with({'uuid': self.node.uuid}, - network_data=None, - user_data=None, - vendor_data=None) - - @mock.patch('openstack.baremetal.configdrive.build', autospec=True) - def test_build_populated(self, mock_cd): - configdrive = { - 'meta_data': {'uuid': uuidutils.generate_uuid(), - 'name': 'new-name', - 'hostname': 'example.com'}, - 'network_data': {'links': []}, - 'vendor_data': {'foo': 'bar'}, - } - self.node.instance_info['configdrive'] = configdrive - self.assertEqual(mock_cd.return_value, - conductor_utils.get_configdrive_image(self.node)) - mock_cd.assert_called_once_with( - configdrive['meta_data'], - network_data=configdrive['network_data'], - user_data=None, - vendor_data=configdrive['vendor_data']) - - @mock.patch('openstack.baremetal.configdrive.build', autospec=True) - def test_build_user_data_as_string(self, mock_cd): - self.node.instance_info['configdrive'] = {'user_data': 'abcd'} - self.assertEqual(mock_cd.return_value, - conductor_utils.get_configdrive_image(self.node)) - mock_cd.assert_called_once_with({'uuid': self.node.uuid}, - network_data=None, - user_data=b'abcd', - vendor_data=None) - - @mock.patch('openstack.baremetal.configdrive.build', autospec=True) - def test_build_user_data_as_dict(self, mock_cd): - self.node.instance_info['configdrive'] = { - 'user_data': {'user': 'data'} - } - self.assertEqual(mock_cd.return_value, - conductor_utils.get_configdrive_image(self.node)) - mock_cd.assert_called_once_with({'uuid': self.node.uuid}, - network_data=None, - user_data=b'{"user": "data"}', - vendor_data=None) - - class NodeHistoryRecordTestCase(db_base.DbTestCase): def setUp(self): diff --git a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py index 4545e955e7..07275ce995 100644 --- a/ironic/tests/unit/drivers/modules/ansible/test_deploy.py +++ b/ironic/tests/unit/drivers/modules/ansible/test_deploy.py @@ -495,7 +495,8 @@ class TestAnsibleMethods(AnsibleDeployTestCaseBase): mock.call().write('fake-content'), mock.call().__exit__(None, None, None))) - @mock.patch.object(utils, 'build_configdrive', autospec=True) + @mock.patch('ironic.conductor.configdrive_utils.build_configdrive', + autospec=True) def test__prepare_variables_configdrive_json(self, mock_build_configdrive): i_info = self.node.instance_info i_info['configdrive'] = {'meta_data': {}} diff --git a/ironic/tests/unit/drivers/modules/redfish/test_boot.py b/ironic/tests/unit/drivers/modules/redfish/test_boot.py index 1bf25091b1..ee03a9ecaf 100644 --- a/ironic/tests/unit/drivers/modules/redfish/test_boot.py +++ b/ironic/tests/unit/drivers/modules/redfish/test_boot.py @@ -20,6 +20,7 @@ import sushy from ironic.common import boot_devices from ironic.common import exception from ironic.common import states +from ironic.conductor import configdrive_utils from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils from ironic.drivers.modules import boot_mode_utils @@ -1210,7 +1211,7 @@ class RedfishVirtualMediaBootTestCase(db_base.DbTestCase): mock_boot_mode_utils.sync_boot_mode.assert_called_once_with(task) - @mock.patch.object(redfish_boot.manager_utils, 'build_configdrive', + @mock.patch.object(configdrive_utils, 'build_configdrive', autospec=True) @mock.patch.object(redfish_boot.RedfishVirtualMediaBoot, '_eject_all', autospec=True) diff --git a/ironic/tests/unit/drivers/modules/test_agent.py b/ironic/tests/unit/drivers/modules/test_agent.py index 64b26335bb..e82d1b3cb8 100644 --- a/ironic/tests/unit/drivers/modules/test_agent.py +++ b/ironic/tests/unit/drivers/modules/test_agent.py @@ -25,6 +25,7 @@ from ironic.common import image_service from ironic.common import images from ironic.common import raid from ironic.common import states +from ironic.conductor import configdrive_utils from ironic.conductor import task_manager from ironic.conductor import utils as manager_utils from ironic.drivers import base as drivers_base @@ -1604,7 +1605,7 @@ class TestAgentDeploy(CommonTestsMixin, db_base.DbTestCase): self.assertEqual(states.ACTIVE, task.node.target_provision_state) - @mock.patch.object(manager_utils, 'build_configdrive', autospec=True) + @mock.patch.object(configdrive_utils, 'build_configdrive', autospec=True) def test_write_image_render_configdrive(self, mock_build_configdrive): self.node.provision_state = states.DEPLOYWAIT self.node.target_provision_state = states.ACTIVE