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