From ca87811a950ff0da597386ee5d1ad1086112a155 Mon Sep 17 00:00:00 2001 From: Ruby Loo Date: Wed, 1 Feb 2017 21:53:25 +0000 Subject: [PATCH] Remove iSCSI deploy support for IPA Mitaka The Newton (6.0.0) release included a fix [1] to support iSCSI deploys to IPA ramdisks < 1.3 (Mitaka). These older versions of IPA did not handle wiping disk metadata or customized portal ports. We are in the Ocata cycle and ironic no longer needs to support IPA Mitaka or earlier, so we can remove this fix. [1] Commit ba83fb1bb45b63e056c87c3bbf42cabaade7865c Change-Id: Ib97344d96a938a550ad2c61e14d793d0f56c6a89 Fixes-Bug: #1661096 --- ironic/drivers/modules/agent_client.py | 62 ++--------- .../unit/drivers/modules/test_agent_client.py | 100 ++++-------------- ...si-deploy-ipa-mitaka-c0efa0d5c31933b6.yaml | 6 ++ 3 files changed, 32 insertions(+), 136 deletions(-) create mode 100644 releasenotes/notes/remove-iscsi-deploy-ipa-mitaka-c0efa0d5c31933b6.yaml diff --git a/ironic/drivers/modules/agent_client.py b/ironic/drivers/modules/agent_client.py index 7e0d92a93f..e908bd646c 100644 --- a/ironic/drivers/modules/agent_client.py +++ b/ironic/drivers/modules/agent_client.py @@ -18,7 +18,7 @@ from oslo_serialization import jsonutils import requests from ironic.common import exception -from ironic.common.i18n import _, _LE, _LW +from ironic.common.i18n import _ from ironic.conf import CONF LOG = log.getLogger(__name__) @@ -138,59 +138,13 @@ class AgentClient(object): disk magic strings like the partition table, RAID or filesystem signature. """ - params = {'iqn': iqn} - # This is to workaround passing default values to an old ramdisk - # TODO(vdrok): remove this workaround in Ocata release - if portal_port != DEFAULT_IPA_PORTAL_PORT: - params['portal_port'] = portal_port - if wipe_disk_metadata: - params['wipe_disk_metadata'] = wipe_disk_metadata - while True: - result = self._command(node=node, - method='iscsi.start_iscsi_target', - params=params, - wait=True) - if (result['command_status'] == 'FAILED' and - result['command_error']['type'] == 'TypeError'): - message = result['command_error']['message'] - if 'wipe_disk_metadata' in message: - # wipe_disk_metadata was introduced after portal_port, so - # portal_port might still work, retry - LOG.warning(_LW( - "The ironic python agent in the ramdisk on node " - "%(node)s failed to start the iSCSI target because " - "it doesn't support wipe_disk_metadata parameter, " - "retrying without passing it. If you need to have " - "node's root disk wiped before exposing it via iSCSI, " - "or because https://bugs.launchpad.net/bugs/1550604 " - "affects you, please update the ramdisk to use " - "version >= 1.3 (Newton, or higher) of ironic python " - "agent."), {'node': node.uuid}) - # NOTE(vdrok): This is needed to make unit test's - # assert_has_calls work, otherwise it will report it was - # called without wipe_disk_metadata both times as "params" - # dictionary is stored by reference in mock - params = params.copy() - del params['wipe_disk_metadata'] - continue - elif 'portal_port' in message: - # It means that ironic is configured in a way that the - # deploy driver has requested some things not available - # on the old ramdisk. Since the user specified a - # non-default portal_port, we do not try again with the - # default value. Instead, the user needs to take some - # explicit action. - LOG.error(_LE( - "The ironic python agent in the ramdisk on node " - "%(node)s failed to start the iSCSI target because " - "the agent doesn't support portal_port parameter. " - "Please update the ramdisk to use version >= 1.3 " - "(Newton, or higher) of ironic python agent, or use " - "the default value of [iscsi]portal_port config " - "option."), {'node': node.uuid}) - # In all the other cases, it is a usual error, no additional action - # required, break from the loop returning the result - return result + 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, + wait=True) @METRICS.timer('AgentClient.install_bootloader') def install_bootloader(self, node, root_uuid, efi_system_part_uuid=None): diff --git a/ironic/tests/unit/drivers/modules/test_agent_client.py b/ironic/tests/unit/drivers/modules/test_agent_client.py index a61c3066f4..5f81decaa0 100644 --- a/ironic/tests/unit/drivers/modules/test_agent_client.py +++ b/ironic/tests/unit/drivers/modules/test_agent_client.py @@ -173,12 +173,12 @@ class TestAgentClient(base.TestCase): def test_start_iscsi_target(self): self.client._command = mock.MagicMock(spec_set=[]) iqn = 'fake-iqn' - port = 3260 - wipe_disk_metadata = True - params = {'iqn': iqn, 'wipe_disk_metadata': True} + port = agent_client.DEFAULT_IPA_PORTAL_PORT + wipe_disk_metadata = False + params = {'iqn': iqn, 'portal_port': port, + 'wipe_disk_metadata': wipe_disk_metadata} - self.client.start_iscsi_target(self.node, iqn, portal_port=port, - wipe_disk_metadata=wipe_disk_metadata) + self.client.start_iscsi_target(self.node, iqn) self.client._command.assert_called_once_with( node=self.node, method='iscsi.start_iscsi_target', params=params, wait=True) @@ -188,91 +188,27 @@ class TestAgentClient(base.TestCase): iqn = 'fake-iqn' port = 3261 wipe_disk_metadata = False - params = {'iqn': iqn, 'portal_port': port} + params = {'iqn': iqn, 'portal_port': port, + 'wipe_disk_metadata': wipe_disk_metadata} - self.client.start_iscsi_target(self.node, iqn, portal_port=port, - wipe_disk_metadata=wipe_disk_metadata) + self.client.start_iscsi_target(self.node, iqn, portal_port=port) self.client._command.assert_called_once_with( node=self.node, method='iscsi.start_iscsi_target', params=params, wait=True) - @mock.patch.object(agent_client, 'LOG') - def test_start_iscsi_target_old_agent_only_wipe(self, log_mock): - self.client._command = mock.MagicMock( - spec_set=[], - side_effect=[ - { - 'command_status': 'FAILED', - 'command_error': { - 'message': u"wipe_disk_metadata doesn't exist", - 'type': u'TypeError' - } - }, - { - 'command_status': 'SUCCESS', - } - ] - ) + def test_start_iscsi_target_wipe_disk_metadata(self): + self.client._command = mock.MagicMock(spec_set=[]) iqn = 'fake-iqn' - port = 3260 + port = agent_client.DEFAULT_IPA_PORTAL_PORT wipe_disk_metadata = True - params_first_try = {'iqn': iqn, - 'wipe_disk_metadata': wipe_disk_metadata} - params_second_try = {'iqn': iqn} + params = {'iqn': iqn, 'portal_port': port, + 'wipe_disk_metadata': wipe_disk_metadata} - ret = self.client.start_iscsi_target( - self.node, iqn, portal_port=port, - wipe_disk_metadata=wipe_disk_metadata) - self.client._command.assert_has_calls([ - mock.call(node=self.node, method='iscsi.start_iscsi_target', - params=params_first_try, wait=True), - mock.call(node=self.node, method='iscsi.start_iscsi_target', - params=params_second_try, wait=True) - ]) - self.assertEqual('SUCCESS', ret['command_status']) - self.assertTrue(log_mock.warning.called) - self.assertFalse(log_mock.error.called) - - @mock.patch.object(agent_client, 'LOG') - def test_start_iscsi_target_old_agent_custom_options(self, log_mock): - self.client._command = mock.MagicMock( - spec_set=[], - side_effect=[ - { - 'command_status': 'FAILED', - 'command_error': { - 'message': u"wipe_disk_metadata doesn't exist", - 'type': u'TypeError' - } - }, - { - 'command_status': 'FAILED', - 'command_error': { - 'message': u"portal_port doesn't exist", - 'type': u'TypeError' - } - } - ] - ) - iqn = 'fake-iqn' - port = 3261 - wipe_disk_metadata = True - params_first_try = {'iqn': iqn, 'portal_port': port, - 'wipe_disk_metadata': wipe_disk_metadata} - params_second_try = {'iqn': iqn, 'portal_port': port} - - ret = self.client.start_iscsi_target( - self.node, iqn, portal_port=port, - wipe_disk_metadata=wipe_disk_metadata) - self.client._command.assert_has_calls([ - mock.call(node=self.node, method='iscsi.start_iscsi_target', - params=params_first_try, wait=True), - mock.call(node=self.node, method='iscsi.start_iscsi_target', - params=params_second_try, wait=True) - ]) - self.assertEqual('FAILED', ret['command_status']) - self.assertTrue(log_mock.warning.called) - self.assertTrue(log_mock.error.called) + self.client.start_iscsi_target(self.node, iqn, + wipe_disk_metadata=wipe_disk_metadata) + self.client._command.assert_called_once_with( + node=self.node, method='iscsi.start_iscsi_target', + params=params, wait=True) def test_install_bootloader(self): self.client._command = mock.MagicMock(spec_set=[]) diff --git a/releasenotes/notes/remove-iscsi-deploy-ipa-mitaka-c0efa0d5c31933b6.yaml b/releasenotes/notes/remove-iscsi-deploy-ipa-mitaka-c0efa0d5c31933b6.yaml new file mode 100644 index 0000000000..3fa3642dd8 --- /dev/null +++ b/releasenotes/notes/remove-iscsi-deploy-ipa-mitaka-c0efa0d5c31933b6.yaml @@ -0,0 +1,6 @@ +--- +upgrade: + - | + There is no longer any support for doing an iSCSI deploy on ironic + python agent (IPA) ramdisks with versions < 1.3 (Mitaka or earlier). + Please upgrade ironic python agent to a newer version.