Merge "iscsi: wipe the disk before deployment"

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):
+    def start_iscsi_target(self, node, iqn,
-        """Expose the node's disk as an ISCSI target."""
+                           portal_port=3260, wipe_disk_metadata=False):
-        params = {'iqn': iqn, 'portal_port': portal_port}
+        """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,

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(
-    result = agent_client.start_iscsi_target(node, iqn,
+        node, iqn,
-                                             portal_port)
+        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") %

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(

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)

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