openstack/ironic
Code Issues Proposed changes

Merge "Add retries to ssh._get_hosts_name_for_node"

Browse Source
This commit is contained in:
Jenkins 2015-10-08 11:17:23 +00:00 committed by Gerrit Code Review
commit 28f9f1275a
3 changed files with 129 additions and 79 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
etc/ironic/ironic.conf.sample
View File

@ -1779,9 +1779,18 @@
# Options defined in ironic.drivers.modules.ssh # Options defined in ironic.drivers.modules.ssh
# #
# libvirt URI (string value) # libvirt URI. (string value)
#libvirt_uri=qemu:///system #libvirt_uri=qemu:///system
# Number of attempts to try to get VM name used by the host
# that corresponds to a node's MAC address. (integer value)
#get_vm_name_attempts=3
# Number of seconds to wait between attempts to get VM name
# used by the host that corresponds to a node's MAC address.
# (integer value)
#get_vm_name_retry_interval=3
[swift] [swift]

148
ironic/drivers/modules/ssh.py
View File

@ -33,6 +33,7 @@ from oslo_concurrency import processutils
from oslo_config import cfg from oslo_config import cfg
from oslo_log import log as logging from oslo_log import log as logging
from oslo_utils import excutils from oslo_utils import excutils
import retrying
from ironic.common import boot_devices from ironic.common import boot_devices
from ironic.common import exception from ironic.common import exception
@ -48,7 +49,16 @@ from ironic.drivers import utils as driver_utils
libvirt_opts = [ libvirt_opts = [
cfg.StrOpt('libvirt_uri', cfg.StrOpt('libvirt_uri',
default='qemu:///system', default='qemu:///system',
help=_('libvirt URI')) help=_('libvirt URI.')),
cfg.IntOpt('get_vm_name_attempts',
default=3,
help=_("Number of attempts to try to get VM name used by the "
"host that corresponds to a node's MAC address.")),
cfg.IntOpt('get_vm_name_retry_interval',
default=3,
help=_("Number of seconds to wait between attempts to get "
"VM name used by the host that corresponds to a "
"node's MAC address.")),
] ]
CONF = cfg.CONF CONF = cfg.CONF
@ -236,6 +246,8 @@ def _get_boot_device(ssh_obj, driver_info):
:raises: SSHCommandFailed on an error from ssh. :raises: SSHCommandFailed on an error from ssh.
:raises: NotImplementedError if the virt_type does not support :raises: NotImplementedError if the virt_type does not support
getting the boot device. getting the boot device.
:raises: NodeNotFound if could not find a VM corresponding to any
of the provided MACs.
""" """
cmd_to_exec = driver_info['cmd_set'].get('get_boot_device') cmd_to_exec = driver_info['cmd_set'].get('get_boot_device')
@ -261,6 +273,8 @@ def _set_boot_device(ssh_obj, driver_info, device):
:raises: SSHCommandFailed on an error from ssh. :raises: SSHCommandFailed on an error from ssh.
:raises: NotImplementedError if the virt_type does not support :raises: NotImplementedError if the virt_type does not support
setting the boot device. setting the boot device.
:raises: NodeNotFound if could not find a VM corresponding to any
of the provided MACs.
""" """
cmd_to_exec = driver_info['cmd_set'].get('set_boot_device') cmd_to_exec = driver_info['cmd_set'].get('set_boot_device')
@ -363,39 +377,33 @@ def _get_power_status(ssh_obj, driver_info):
:param ssh_obj: paramiko.SSHClient, an active ssh connection. :param ssh_obj: paramiko.SSHClient, an active ssh connection.
:param driver_info: information for accessing the node. :param driver_info: information for accessing the node.
:returns: one of ironic.common.states POWER_OFF, POWER_ON. :returns: one of ironic.common.states POWER_OFF, POWER_ON.
:raises: NodeNotFound :raises: NodeNotFound if could not find a VM corresponding to any
of the provided MACs.
""" """
power_state = None power_state = None
node_name = _get_hosts_name_for_node(ssh_obj, driver_info) node_name = _get_hosts_name_for_node(ssh_obj, driver_info)
if node_name: # Get a list of vms running on the host. If the command supports
# Get a list of vms running on the host. If the command supports # it, explicitly specify the desired node."
# it, explicitly specify the desired node." cmd_to_exec = "%s %s" % (driver_info['cmd_set']['base_cmd'],
cmd_to_exec = "%s %s" % (driver_info['cmd_set']['base_cmd'], driver_info['cmd_set']['list_running'])
driver_info['cmd_set']['list_running']) cmd_to_exec = cmd_to_exec.replace('{_NodeName_}', node_name)
cmd_to_exec = cmd_to_exec.replace('{_NodeName_}', node_name) running_list = _ssh_execute(ssh_obj, cmd_to_exec)
running_list = _ssh_execute(ssh_obj, cmd_to_exec)
# Command should return a list of running vms. If the current node is # Command should return a list of running vms. If the current node is
# not listed then we can assume it is not powered on. # not listed then we can assume it is not powered on.
quoted_node_name = '"%s"' % node_name quoted_node_name = '"%s"' % node_name
for node in running_list: for node in running_list:
if not node: if not node:
continue continue
# 'node' here is a formatted output from the virt cli's. The # 'node' here is a formatted output from the virt cli's. The
# node name is either an exact match or quoted (optionally with # node name is either an exact match or quoted (optionally with
# other information, e.g. vbox returns '"NodeName" {<uuid>}') # other information, e.g. vbox returns '"NodeName" {<uuid>}')
if (quoted_node_name in node) or (node_name == node): if (quoted_node_name in node) or (node_name == node):
power_state = states.POWER_ON power_state = states.POWER_ON
break break
if not power_state: if not power_state:
power_state = states.POWER_OFF power_state = states.POWER_OFF
else:
err_msg = _LE('Node "%(host)s" with MAC address %(mac)s not found.')
LOG.error(err_msg, {'host': driver_info['host'],
'mac': driver_info['macs']})
raise exception.NodeNotFound(node=driver_info['host'])
return power_state return power_state
@ -415,41 +423,56 @@ def _get_hosts_name_for_node(ssh_obj, driver_info):
:param ssh_obj: paramiko.SSHClient, an active ssh connection. :param ssh_obj: paramiko.SSHClient, an active ssh connection.
:param driver_info: information for accessing the node. :param driver_info: information for accessing the node.
:returns: the name or None if not found. :returns: the name of the node.
:raises: NodeNotFound if could not find a VM corresponding to any of
the provided MACs
""" """
matched_name = None
cmd_to_exec = "%s %s" % (driver_info['cmd_set']['base_cmd'], @retrying.retry(
driver_info['cmd_set']['list_all']) retry_on_result=lambda v: v is None,
full_node_list = _ssh_execute(ssh_obj, cmd_to_exec) retry_on_exception=lambda _: False, # Do not retry on SSHCommandFailed
LOG.debug("Retrieved Node List: %s" % repr(full_node_list)) stop_max_attempt_number=CONF.ssh.get_vm_name_attempts,
# for each node check Mac Addresses wait_fixed=CONF.ssh.get_vm_name_retry_interval * 1000)
for node in full_node_list: def _with_retries():
if not node: matched_name = None
continue
LOG.debug("Checking Node: %s's Mac address." % node)
cmd_to_exec = "%s %s" % (driver_info['cmd_set']['base_cmd'], cmd_to_exec = "%s %s" % (driver_info['cmd_set']['base_cmd'],
driver_info['cmd_set']['get_node_macs']) driver_info['cmd_set']['list_all'])
cmd_to_exec = cmd_to_exec.replace('{_NodeName_}', node) full_node_list = _ssh_execute(ssh_obj, cmd_to_exec)
hosts_node_mac_list = _ssh_execute(ssh_obj, cmd_to_exec) LOG.debug("Retrieved Node List: %s" % repr(full_node_list))
# for each node check Mac Addresses
for host_mac in hosts_node_mac_list: for node in full_node_list:
if not host_mac: if not node:
continue continue
for node_mac in driver_info['macs']: LOG.debug("Checking Node: %s's Mac address." % node)
if not node_mac: cmd_to_exec = "%s %s" % (driver_info['cmd_set']['base_cmd'],
continue driver_info['cmd_set']['get_node_macs'])
if _normalize_mac(host_mac) in _normalize_mac(node_mac): cmd_to_exec = cmd_to_exec.replace('{_NodeName_}', node)
LOG.debug("Found Mac address: %s" % node_mac) hosts_node_mac_list = _ssh_execute(ssh_obj, cmd_to_exec)
matched_name = node
break
for host_mac in hosts_node_mac_list:
if not host_mac:
continue
for node_mac in driver_info['macs']:
if _normalize_mac(host_mac) in _normalize_mac(node_mac):
LOG.debug("Found Mac address: %s" % node_mac)
matched_name = node
break
if matched_name:
break
if matched_name: if matched_name:
break break
if matched_name:
break
return matched_name return matched_name
try:
return _with_retries()
except retrying.RetryError:
raise exception.NodeNotFound(
_("SSH driver was not able to find a VM with any of the "
"specified MACs: %(macs)s for node %(node)s.") %
{'macs': driver_info['macs'], 'node': driver_info['uuid']})
def _power_on(ssh_obj, driver_info): def _power_on(ssh_obj, driver_info):
@ -549,7 +572,8 @@ class SSHPower(base.PowerInterface):
:raises: InvalidParameterValue if any connection parameters are :raises: InvalidParameterValue if any connection parameters are
incorrect. incorrect.
:raises: MissingParameterValue when a required parameter is missing :raises: MissingParameterValue when a required parameter is missing
:raises: NodeNotFound. :raises: NodeNotFound if could not find a VM corresponding to any
of the provided MACs.
:raises: SSHCommandFailed on an error from ssh. :raises: SSHCommandFailed on an error from ssh.
:raises: SSHConnectFailed if ssh failed to connect to the node. :raises: SSHConnectFailed if ssh failed to connect to the node.
""" """
@ -570,7 +594,8 @@ class SSHPower(base.PowerInterface):
:raises: InvalidParameterValue if any connection parameters are :raises: InvalidParameterValue if any connection parameters are
incorrect, or if the desired power state is invalid. incorrect, or if the desired power state is invalid.
:raises: MissingParameterValue when a required parameter is missing :raises: MissingParameterValue when a required parameter is missing
:raises: NodeNotFound. :raises: NodeNotFound if could not find a VM corresponding to any
of the provided MACs.
:raises: PowerStateFailure if it failed to set power state to pstate. :raises: PowerStateFailure if it failed to set power state to pstate.
:raises: SSHCommandFailed on an error from ssh. :raises: SSHCommandFailed on an error from ssh.
:raises: SSHConnectFailed if ssh failed to connect to the node. :raises: SSHConnectFailed if ssh failed to connect to the node.
@ -601,7 +626,8 @@ class SSHPower(base.PowerInterface):
:raises: InvalidParameterValue if any connection parameters are :raises: InvalidParameterValue if any connection parameters are
incorrect. incorrect.
:raises: MissingParameterValue when a required parameter is missing :raises: MissingParameterValue when a required parameter is missing
:raises: NodeNotFound. :raises: NodeNotFound if could not find a VM corresponding to any
of the provided MACs.
:raises: PowerStateFailure if it failed to set power state to POWER_ON. :raises: PowerStateFailure if it failed to set power state to POWER_ON.
:raises: SSHCommandFailed on an error from ssh. :raises: SSHCommandFailed on an error from ssh.
:raises: SSHConnectFailed if ssh failed to connect to the node. :raises: SSHConnectFailed if ssh failed to connect to the node.
@ -664,6 +690,8 @@ class SSHManagement(base.ManagementInterface):
:raises: SSHCommandFailed on an error from ssh. :raises: SSHCommandFailed on an error from ssh.
:raises: NotImplementedError if the virt_type does not support :raises: NotImplementedError if the virt_type does not support
setting the boot device. setting the boot device.
:raises: NodeNotFound if could not find a VM corresponding to any
of the provided MACs.
""" """
node = task.node node = task.node
@ -697,6 +725,8 @@ class SSHManagement(base.ManagementInterface):
:raises: MissingParameterValue if a required parameter is missing :raises: MissingParameterValue if a required parameter is missing
:raises: SSHConnectFailed if ssh failed to connect to the node. :raises: SSHConnectFailed if ssh failed to connect to the node.
:raises: SSHCommandFailed on an error from ssh. :raises: SSHCommandFailed on an error from ssh.
:raises: NodeNotFound if could not find a VM corresponding to any
of the provided MACs.
:returns: a dictionary containing: :returns: a dictionary containing:
:boot_device: the boot device, one of :boot_device: the boot device, one of

49
ironic/tests/unit/drivers/modules/test_ssh.py
View File

@ -287,22 +287,6 @@ class SSHPrivateMethodsTestCase(db_base.DbTestCase):
exec_ssh_mock.assert_called_once_with(self.sshclient, ssh_cmd) exec_ssh_mock.assert_called_once_with(self.sshclient, ssh_cmd)
get_hosts_name_mock.assert_called_once_with(self.sshclient, info) get_hosts_name_mock.assert_called_once_with(self.sshclient, info)
@mock.patch.object(processutils, 'ssh_execute', autospec=True)
@mock.patch.object(ssh, '_get_hosts_name_for_node', autospec=True)
def test__get_power_status_error(self, get_hosts_name_mock, exec_ssh_mock):
info = ssh._parse_driver_info(self.node)
info['macs'] = ["11:11:11:11:11:11", "52:54:00:cf:2d:31"]
get_hosts_name_mock.return_value = None
self.assertRaises(exception.NodeNotFound,
ssh._get_power_status,
self.sshclient,
info)
get_hosts_name_mock.assert_called_once_with(self.sshclient, info)
self.assertFalse(exec_ssh_mock.called)
@mock.patch.object(processutils, 'ssh_execute', autospec=True) @mock.patch.object(processutils, 'ssh_execute', autospec=True)
def test__get_power_status_exception(self, exec_ssh_mock): def test__get_power_status_exception(self, exec_ssh_mock):
info = ssh._parse_driver_info(self.node) info = ssh._parse_driver_info(self.node)
@ -352,10 +336,12 @@ class SSHPrivateMethodsTestCase(db_base.DbTestCase):
@mock.patch.object(processutils, 'ssh_execute', autospec=True) @mock.patch.object(processutils, 'ssh_execute', autospec=True)
def test__get_hosts_name_for_node_no_match(self, exec_ssh_mock): def test__get_hosts_name_for_node_no_match(self, exec_ssh_mock):
self.config(group='ssh', get_vm_name_attempts=2)
self.config(group='ssh', get_vm_name_retry_interval=0)
info = ssh._parse_driver_info(self.node) info = ssh._parse_driver_info(self.node)
info['macs'] = ["11:11:11:11:11:11", "22:22:22:22:22:22"] info['macs'] = ["11:11:11:11:11:11", "22:22:22:22:22:22"]
exec_ssh_mock.side_effect = iter([('NodeName', ''), exec_ssh_mock.side_effect = iter([('NodeName', ''),
('52:54:00:cf:2d:31', '')]) ('52:54:00:cf:2d:31', '')] * 2)
ssh_cmd = "%s %s" % (info['cmd_set']['base_cmd'], ssh_cmd = "%s %s" % (info['cmd_set']['base_cmd'],
info['cmd_set']['list_all']) info['cmd_set']['list_all'])
@ -365,11 +351,36 @@ class SSHPrivateMethodsTestCase(db_base.DbTestCase):
cmd_to_exec = cmd_to_exec.replace('{_NodeName_}', 'NodeName') cmd_to_exec = cmd_to_exec.replace('{_NodeName_}', 'NodeName')
expected = [mock.call(self.sshclient, ssh_cmd), expected = [mock.call(self.sshclient, ssh_cmd),
mock.call(self.sshclient, cmd_to_exec)] mock.call(self.sshclient, cmd_to_exec)] * 2
self.assertRaises(exception.NodeNotFound,
ssh._get_hosts_name_for_node, self.sshclient, info)
self.assertEqual(expected, exec_ssh_mock.call_args_list)
@mock.patch.object(processutils, 'ssh_execute', autospec=True)
def test__get_hosts_name_for_node_match_after_retry(self, exec_ssh_mock):
self.config(group='ssh', get_vm_name_attempts=2)
self.config(group='ssh', get_vm_name_retry_interval=0)
info = ssh._parse_driver_info(self.node)
info['macs'] = ["11:11:11:11:11:11", "22:22:22:22:22:22"]
exec_ssh_mock.side_effect = iter([('NodeName', ''),
('', ''),
('NodeName', ''),
('11:11:11:11:11:11', '')])
ssh_cmd = "%s %s" % (info['cmd_set']['base_cmd'],
info['cmd_set']['list_all'])
cmd_to_exec = "%s %s" % (info['cmd_set']['base_cmd'],
info['cmd_set']['get_node_macs'])
cmd_to_exec = cmd_to_exec.replace('{_NodeName_}', 'NodeName')
expected = [mock.call(self.sshclient, ssh_cmd),
mock.call(self.sshclient, cmd_to_exec)] * 2
found_name = ssh._get_hosts_name_for_node(self.sshclient, info) found_name = ssh._get_hosts_name_for_node(self.sshclient, info)
self.assertIsNone(found_name) self.assertEqual('NodeName', found_name)
self.assertEqual(expected, exec_ssh_mock.call_args_list) self.assertEqual(expected, exec_ssh_mock.call_args_list)
@mock.patch.object(processutils, 'ssh_execute', autospec=True) @mock.patch.object(processutils, 'ssh_execute', autospec=True)