From 1a4bf23ce5773726fc776369b483254d13d54f2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gon=C3=A9ri=20Le=20Bouder?= Date: Fri, 8 Apr 2016 16:26:17 -0400 Subject: [PATCH] iscsi: wipe the disk before deployment If the remote disk has already a partition table, it must be clean up before the disk is exposed through iscsi. Otherwise this disk partition can create a conflict during the grub installation. start_iscsi_target() has now a new paramter wipe_disk_metadata to request the agent to wipe the disk before exposing it. Now, disk without preserve_ephemeral are cleaned up before new deployment. References: - IPA: Ie68cb6296c782e904d40f6e9de0faa52ab2af2bf - ironic-lib: 042aa9ab5a27e251c8fb2f1855695cf5e791ecf5 - https://bugzilla.redhat.com/show_bug.cgi?id=1310883 Change-Id: I4f2f754ab0ff01046768e73fff789807d307b829 Closes-Bug: 1550604 --- ironic/drivers/modules/agent_client.py | 17 ++++++++-- ironic/drivers/modules/iscsi_deploy.py | 10 +++--- .../unit/drivers/modules/test_agent_client.py | 2 +- .../unit/drivers/modules/test_iscsi_deploy.py | 32 +++++++++++++++++-- ...sk-before-deployment-0a8b9cede4a659e9.yaml | 9 ++++++ 5 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/wipe-disk-before-deployment-0a8b9cede4a659e9.yaml diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py index 544bf47688..eccbb6c81e 100644 --- a/ironic/drivers/modules/agent_client.py +++ b/ironic/drivers/modules/agent_client.py @@ -130,9 +130,20 @@ class AgentClient(object): params=params, wait=wait) - def start_iscsi_target(self, node, iqn, portal_port=3260): - """Expose the node's disk as an ISCSI target.""" - params = {'iqn': iqn, 'portal_port': portal_port} + def start_iscsi_target(self, node, iqn, + portal_port=3260, wipe_disk_metadata=False): + """Expose the node's disk as an ISCSI target. + + :param node: an Ironic node object + :param iqn: iSCSI target IQN + :param portal_port: iSCSI portal port + :param wipe_disk_metadata: True if the agent should wipe first the + disk magic strings like the partition table, RAID or filesystem + signature. + """ + params = {'iqn': iqn, + 'portal_port': portal_port, + 'wipe_disk_metadata': wipe_disk_metadata} return self._command(node=node, method='iscsi.start_iscsi_target', params=params, diff --git a/ironic/drivers/modules/iscsi_deploy.py b/ironic/drivers/modules/iscsi_deploy.py index 9853e7bd83..3bfd3b01a4 100644 --- a/ironic/drivers/modules/iscsi_deploy.py +++ b/ironic/drivers/modules/iscsi_deploy.py @@ -339,13 +339,15 @@ def do_agent_iscsi_deploy(task, agent_client): """ node = task.node iscsi_options = build_deploy_ramdisk_options(node) + i_info = deploy_utils.parse_instance_info(node) + wipe_disk_metadata = not i_info['preserve_ephemeral'] iqn = iscsi_options['iscsi_target_iqn'] portal_port = iscsi_options['iscsi_portal_port'] - - result = agent_client.start_iscsi_target(node, iqn, - portal_port) - + result = agent_client.start_iscsi_target( + node, iqn, + portal_port, + wipe_disk_metadata=wipe_disk_metadata) if result['command_status'] == 'FAILED': msg = (_("Failed to start the iSCSI target to deploy the " "node %(node)s. Error: %(error)s") % diff --git a/ironic/tests/unit/drivers/modules/test_agent_client.py b/ironic/tests/unit/drivers/modules/test_agent_client.py index ff41df22b8..be81927201 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_client.py +++ b/ironic/tests/unit/drivers/modules/test_agent_client.py @@ -179,7 +179,7 @@ class TestAgentClient(base.TestCase): self.client._command = mock.MagicMock(spec_set=[]) iqn = 'fake-iqn' port = 3260 - params = {'iqn': iqn, 'portal_port': port} + params = {'iqn': iqn, 'portal_port': port, 'wipe_disk_metadata': False} self.client.start_iscsi_target(self.node, iqn, port) self.client._command.assert_called_once_with( diff --git a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py index 662c7f6feb..1511d8ae93 100644 --- a/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py +++ b/ironic/tests/unit/drivers/modules/test_iscsi_deploy.py @@ -522,7 +522,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): task, agent_client_mock) build_options_mock.assert_called_once_with(task.node) agent_client_mock.start_iscsi_target.assert_called_once_with( - task.node, 'iqn-qweqwe', 3260) + task.node, 'iqn-qweqwe', 3260, wipe_disk_metadata=True) continue_deploy_mock.assert_called_once_with( task, error=None, iqn='iqn-qweqwe', key='abcdef', address='1.2.3.4') @@ -531,6 +531,34 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): task.node.driver_internal_info['root_uuid_or_disk_id']) self.assertEqual(ret_val, uuid_dict_returned) + @mock.patch.object(iscsi_deploy, 'continue_deploy', autospec=True) + @mock.patch.object(iscsi_deploy, 'build_deploy_ramdisk_options', + autospec=True) + def test_do_agent_iscsi_deploy_preserve_ephemeral(self, build_options_mock, + continue_deploy_mock): + """Ensure the disk is not wiped if preserve_ephemeral is True.""" + build_options_mock.return_value = {'deployment_key': 'abcdef', + 'iscsi_target_iqn': 'iqn-qweqwe', + 'iscsi_portal_port': 3260} + agent_client_mock = mock.MagicMock(spec_set=agent_client.AgentClient) + agent_client_mock.start_iscsi_target.return_value = { + 'command_status': 'SUCCESS', 'command_error': None} + driver_internal_info = { + 'agent_url': 'http://1.2.3.4:1234'} + self.node.driver_internal_info = driver_internal_info + self.node.save() + uuid_dict_returned = {'root uuid': 'some-root-uuid'} + continue_deploy_mock.return_value = uuid_dict_returned + + with task_manager.acquire(self.context, self.node.uuid, + shared=False) as task: + task.node.instance_info['preserve_ephemeral'] = True + iscsi_deploy.do_agent_iscsi_deploy( + task, agent_client_mock) + build_options_mock.assert_called_once_with(task.node) + agent_client_mock.start_iscsi_target.assert_called_once_with( + task.node, 'iqn-qweqwe', 3260, wipe_disk_metadata=False) + @mock.patch.object(iscsi_deploy, 'build_deploy_ramdisk_options', autospec=True) def test_do_agent_iscsi_deploy_start_iscsi_failure(self, @@ -552,7 +580,7 @@ class IscsiDeployMethodsTestCase(db_base.DbTestCase): task, agent_client_mock) build_options_mock.assert_called_once_with(task.node) agent_client_mock.start_iscsi_target.assert_called_once_with( - task.node, 'iqn-qweqwe', 3260) + task.node, 'iqn-qweqwe', 3260, wipe_disk_metadata=True) self.node.refresh() self.assertEqual(states.DEPLOYFAIL, self.node.provision_state) self.assertEqual(states.ACTIVE, self.node.target_provision_state) diff --git a/releasenotes/notes/wipe-disk-before-deployment-0a8b9cede4a659e9.yaml b/releasenotes/notes/wipe-disk-before-deployment-0a8b9cede4a659e9.yaml new file mode 100644 index 0000000000..f1631f5690 --- /dev/null +++ b/releasenotes/notes/wipe-disk-before-deployment-0a8b9cede4a659e9.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - Fixed a bug that was causing grub installation failure. If the disk + was already coming with a partition table, the conductor was not able + to wipe it properly and the new partition table would conflict with + the old one. The issue was only impacting new nodes and installations + with automated_clean disabled in the configuration. + A disk instance without preserve_ephemeral is now purged before new deployment. + See https://bugs.launchpad.net/ironic-lib/+bug/1550604