openstack/ironic
Code Issues Proposed changes

Remove iSCSI deploy support for IPA Mitaka

Browse Source 
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 ba83fb1bb4

Change-Id: Ib97344d96a938a550ad2c61e14d793d0f56c6a89
Fixes-Bug: #1661096
This commit is contained in:
Ruby Loo 2017-02-01 21:53:25 +00:00
parent 646a1f76a9
commit ca87811a95
3 changed files with 32 additions and 136 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

62
ironic/drivers/modules/agent_client.py
View File

@ -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):

100
ironic/tests/unit/drivers/modules/test_agent_client.py
View File

@ -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=[])

6
releasenotes/notes/remove-iscsi-deploy-ipa-mitaka-c0efa0d5c31933b6.yaml Normal file
View File

@ -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.