Merge "Gracefully degrade start_iscsi_target for Mitaka ramdisk"
This commit is contained in:
Jenkins
Gerrit Code Review
commit
a155f30c88
@ -19,6 +19,8 @@ import requests
|
||||
|
||||
from ironic.common import exception
|
||||
from ironic.common.i18n import _
|
||||
from ironic.common.i18n import _LE
|
||||
from ironic.common.i18n import _LW
|
||||
|
||||
agent_opts = [
|
||||
cfg.StrOpt('agent_api_version',
|
||||
@ -32,6 +34,8 @@ CONF.register_opts(agent_opts, group='agent')
|
||||
|
||||
LOG = log.getLogger(__name__)
|
||||
|
||||
DEFAULT_IPA_PORTAL_PORT = 3260
|
||||
|
||||
|
||||
class AgentClient(object):
|
||||
"""Client for interacting with nodes via a REST API."""
|
||||
@ -127,7 +131,8 @@ class AgentClient(object):
|
||||
wait=wait)
|
||||
|
||||
def start_iscsi_target(self, node, iqn,
|
||||
portal_port=3260, wipe_disk_metadata=False):
|
||||
portal_port=DEFAULT_IPA_PORTAL_PORT,
|
||||
wipe_disk_metadata=False):
|
||||
"""Expose the node's disk as an ISCSI target.
|
||||
|
||||
:param node: an Ironic node object
|
||||
@ -137,13 +142,59 @@ class AgentClient(object):
|
||||
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,
|
||||
wait=True)
|
||||
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
|
||||
|
||||
def install_bootloader(self, node, root_uuid, efi_system_part_uuid=None):
|
||||
"""Install a boot loader on the image."""
|
||||
|
||||
@ -141,7 +141,6 @@ class TestAgentClient(base.TestCase):
|
||||
mock_get.return_value = res
|
||||
self.assertEqual([], self.client.get_commands_status(self.node))
|
||||
|
||||
@mock.patch('uuid.uuid4', mock.MagicMock(spec_set=[], return_value='uuid'))
|
||||
def test_prepare_image(self):
|
||||
self.client._command = mock.MagicMock(spec_set=[])
|
||||
image_info = {'image_id': 'image'}
|
||||
@ -154,7 +153,6 @@ class TestAgentClient(base.TestCase):
|
||||
node=self.node, method='standby.prepare_image',
|
||||
params=params, wait=False)
|
||||
|
||||
@mock.patch('uuid.uuid4', mock.MagicMock(spec_set=[], return_value='uuid'))
|
||||
def test_prepare_image_with_configdrive(self):
|
||||
self.client._command = mock.MagicMock(spec_set=[])
|
||||
configdrive_url = 'http://swift/configdrive'
|
||||
@ -172,19 +170,110 @@ class TestAgentClient(base.TestCase):
|
||||
node=self.node, method='standby.prepare_image',
|
||||
params=params, wait=False)
|
||||
|
||||
@mock.patch('uuid.uuid4', mock.MagicMock(spec_set=[], return_value='uuid'))
|
||||
def test_start_iscsi_target(self):
|
||||
self.client._command = mock.MagicMock(spec_set=[])
|
||||
iqn = 'fake-iqn'
|
||||
port = 3260
|
||||
params = {'iqn': iqn, 'portal_port': port, 'wipe_disk_metadata': False}
|
||||
wipe_disk_metadata = True
|
||||
params = {'iqn': iqn, 'wipe_disk_metadata': True}
|
||||
|
||||
self.client.start_iscsi_target(self.node, iqn, port)
|
||||
self.client.start_iscsi_target(self.node, iqn, portal_port=port,
|
||||
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)
|
||||
|
||||
@mock.patch('uuid.uuid4', mock.MagicMock(spec_set=[], return_value='uuid'))
|
||||
def test_start_iscsi_target_custom_port(self):
|
||||
self.client._command = mock.MagicMock(spec_set=[])
|
||||
iqn = 'fake-iqn'
|
||||
port = 3261
|
||||
wipe_disk_metadata = False
|
||||
params = {'iqn': iqn, 'portal_port': port}
|
||||
|
||||
self.client.start_iscsi_target(self.node, iqn, portal_port=port,
|
||||
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)
|
||||
|
||||
@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',
|
||||
}
|
||||
]
|
||||
)
|
||||
iqn = 'fake-iqn'
|
||||
port = 3260
|
||||
wipe_disk_metadata = True
|
||||
params_first_try = {'iqn': iqn,
|
||||
'wipe_disk_metadata': wipe_disk_metadata}
|
||||
params_second_try = {'iqn': iqn}
|
||||
|
||||
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)
|
||||
|
||||
def test_install_bootloader(self):
|
||||
self.client._command = mock.MagicMock(spec_set=[])
|
||||
root_uuid = 'fake-root-uuid'
|
||||
|
||||
8
releasenotes/notes/fix-mitaka-ipa-iscsi.yaml
Normal file
8
releasenotes/notes/fix-mitaka-ipa-iscsi.yaml
Normal file
@ -0,0 +1,8 @@
|
||||
---
|
||||
upgrade:
|
||||
- Fixed Mitaka ironic python agent ramdisk iSCSI deploy compatibility with
|
||||
newer versions of ironic by logging the warning and retrying the deploy if
|
||||
wiping root disk metadata before exposing it over iSCSI fails. If custom
|
||||
iSCSI port is requested, an error clarifying the issue is logged and the
|
||||
operator is requested either to use the default iSCSI portal port, or to
|
||||
upgrade ironic python agent ramdisk to version >= 1.3 (Newton).
|
||||
Loading…
Reference in New Issue
Block a user