Clean up exceptions metalsmith raises
This change switches from a mix of metalsmith and SDK exceptions to consistently using metalsmith exceptions. Change-Id: I43339686a42644877dbe40d1116db1585a8a0800
This commit is contained in:
Dmitry Tantsur
parent
24d006b996
commit
97311ce7fb
|
|
@ -14,7 +14,6 @@
|
|||
# limitations under the License.
|
||||
|
||||
import logging
|
||||
import sys
|
||||
|
||||
from openstack import connection
|
||||
from openstack import exceptions as os_exc
|
||||
|
|
@ -113,7 +112,10 @@ class Provisioner(object):
|
|||
candidates, predicate):
|
||||
"""Build a list of candidate nodes for allocation."""
|
||||
if candidates:
|
||||
nodes = [self._get_node(node) for node in candidates]
|
||||
try:
|
||||
nodes = [self._get_node(node) for node in candidates]
|
||||
except os_exc.ResourceNotFound as exc:
|
||||
raise exceptions.InvalidNode(str(exc))
|
||||
else:
|
||||
nodes = list(self.connection.baremetal.nodes(
|
||||
details=True,
|
||||
|
|
@ -149,9 +151,14 @@ class Provisioner(object):
|
|||
'with traits %(traits)s and candidate nodes %(candidates)s',
|
||||
{'rsc': resource_class, 'traits': traits,
|
||||
'candidates': candidates})
|
||||
allocation = self.connection.baremetal.create_allocation(
|
||||
name=hostname, candidate_nodes=candidates,
|
||||
resource_class=resource_class, traits=traits)
|
||||
try:
|
||||
allocation = self.connection.baremetal.create_allocation(
|
||||
name=hostname, candidate_nodes=candidates,
|
||||
resource_class=resource_class, traits=traits)
|
||||
except os_exc.SDKException as exc:
|
||||
# Re-raise the expected exception class
|
||||
raise exceptions.ReservationFailed(
|
||||
'Failed to create an allocation: %s' % exc)
|
||||
|
||||
node = None
|
||||
try:
|
||||
|
|
@ -171,18 +178,15 @@ class Provisioner(object):
|
|||
node = self._patch_reserved_node(node, allocation, hostname,
|
||||
capabilities)
|
||||
except Exception as exc:
|
||||
exc_info = sys.exc_info()
|
||||
|
||||
try:
|
||||
with _utils.reraise_os_exc(
|
||||
exceptions.ReservationFailed,
|
||||
'Failed to delete failed allocation') as expected:
|
||||
LOG.error('Processing allocation %(alloc)s for node %(node)s '
|
||||
'failed: %(exc)s; deleting allocation',
|
||||
{'alloc': _utils.log_res(allocation),
|
||||
'node': _utils.log_res(node), 'exc': exc})
|
||||
'node': _utils.log_res(node), 'exc': exc},
|
||||
exc_info=not expected)
|
||||
self.connection.baremetal.delete_allocation(allocation)
|
||||
except Exception:
|
||||
LOG.exception('Failed to delete failed allocation')
|
||||
|
||||
six.reraise(*exc_info)
|
||||
|
||||
LOG.debug('Reserved node: %s', node)
|
||||
return node, allocation
|
||||
|
|
@ -427,17 +431,12 @@ class Provisioner(object):
|
|||
self.connection.baremetal.set_node_provision_state(
|
||||
node, 'active', config_drive=cd)
|
||||
except Exception:
|
||||
exc_info = sys.exc_info()
|
||||
|
||||
if clean_up_on_failure:
|
||||
try:
|
||||
with _utils.reraise_os_exc(
|
||||
exceptions.DeploymentFailed) as expected:
|
||||
if clean_up_on_failure:
|
||||
LOG.error('Deploy attempt failed on node %s, cleaning up',
|
||||
_utils.log_res(node))
|
||||
_utils.log_res(node), exc_info=not expected)
|
||||
self._clean_up(node, nics=nics)
|
||||
except Exception:
|
||||
LOG.exception('Clean up failed')
|
||||
|
||||
six.reraise(*exc_info)
|
||||
|
||||
LOG.info('Provisioning started on node %s', _utils.log_res(node))
|
||||
|
||||
|
|
@ -466,14 +465,20 @@ class Provisioner(object):
|
|||
(more precisely, until the operation times out on server side).
|
||||
:return: List of updated :py:class:`metalsmith.Instance` objects if
|
||||
all succeeded.
|
||||
:raises: `openstack.exceptions.ResourceTimeout` if deployment times
|
||||
out for any node.
|
||||
:raises: `openstack.exceptions.SDKException` if deployment fails
|
||||
for any node.
|
||||
:raises: :py:class:`metalsmith.exceptions.DeploymentFailed`
|
||||
if deployment fails or times out.
|
||||
:raises: :py:class:`metalsmith.exceptions.InstanceNotFound`
|
||||
if requested nodes cannot be found.
|
||||
"""
|
||||
nodes = [self._find_node_and_allocation(n)[0] for n in nodes]
|
||||
nodes = self.connection.baremetal.wait_for_nodes_provision_state(
|
||||
nodes, 'active', timeout=timeout)
|
||||
try:
|
||||
nodes = self.connection.baremetal.wait_for_nodes_provision_state(
|
||||
nodes, 'active', timeout=timeout)
|
||||
except os_exc.ResourceTimeout as exc:
|
||||
raise exceptions.DeploymentTimeout(str(exc))
|
||||
except os_exc.SDKException as exc:
|
||||
raise exceptions.DeploymentFailed(str(exc))
|
||||
|
||||
# Using _get_instance in case the deployment started by something
|
||||
# external that uses allocations.
|
||||
return [self._get_instance(node) for node in nodes]
|
||||
|
|
@ -533,6 +538,12 @@ class Provisioner(object):
|
|||
:param wait: How many seconds to wait for the process to finish,
|
||||
None to return immediately.
|
||||
:return: the latest `Node` object.
|
||||
:raises: :py:class:`metalsmith.exceptions.DeploymentFailed`
|
||||
if undeployment fails.
|
||||
:raises: :py:class:`metalsmith.exceptions.DeploymentTimeout`
|
||||
if undeployment times out.
|
||||
:raises: :py:class:`metalsmith.exceptions.InstanceNotFound`
|
||||
if requested node cannot be found.
|
||||
"""
|
||||
node = self._find_node_and_allocation(node)[0]
|
||||
if self._dry_run:
|
||||
|
|
@ -540,16 +551,23 @@ class Provisioner(object):
|
|||
return
|
||||
|
||||
self._clean_up(node)
|
||||
node = self.connection.baremetal.set_node_provision_state(
|
||||
node, 'deleted', wait=False)
|
||||
try:
|
||||
node = self.connection.baremetal.set_node_provision_state(
|
||||
node, 'deleted', wait=False)
|
||||
|
||||
LOG.info('Deleting started for node %s', _utils.log_res(node))
|
||||
LOG.info('Deleting started for node %s', _utils.log_res(node))
|
||||
|
||||
if wait is None:
|
||||
return node
|
||||
|
||||
if wait is not None:
|
||||
node = self.connection.baremetal.wait_for_nodes_provision_state(
|
||||
[node], 'available', timeout=wait)[0]
|
||||
LOG.info('Node %s undeployed successfully', _utils.log_res(node))
|
||||
except os_exc.ResourceTimeout as exc:
|
||||
raise exceptions.DeploymentTimeout(str(exc))
|
||||
except os_exc.SDKException as exc:
|
||||
raise exceptions.DeploymentFailed(str(exc))
|
||||
|
||||
LOG.info('Node %s undeployed successfully', _utils.log_res(node))
|
||||
return node
|
||||
|
||||
def show_instance(self, instance_id):
|
||||
|
|
@ -557,7 +575,7 @@ class Provisioner(object):
|
|||
|
||||
:param instance_id: hostname, UUID or node name.
|
||||
:return: :py:class:`metalsmith.Instance` object.
|
||||
:raises: :py:class:`metalsmith.exceptions.InvalidInstance`
|
||||
:raises: :py:class:`metalsmith.exceptions.InstanceNotFound`
|
||||
if the instance is not a valid instance.
|
||||
"""
|
||||
return self.show_instances([instance_id])[0]
|
||||
|
|
@ -571,8 +589,9 @@ class Provisioner(object):
|
|||
:param instances: list of hostnames, UUIDs or node names.
|
||||
:return: list of :py:class:`metalsmith.Instance` objects in the same
|
||||
order as ``instances``.
|
||||
:raises: :py:class:`metalsmith.exceptions.InvalidInstance`
|
||||
if one of the instances are not valid instances.
|
||||
:raises: :py:class:`metalsmith.exceptions.InstanceNotFound`
|
||||
if one of the instances cannot be found or the found node is
|
||||
not a valid instance.
|
||||
"""
|
||||
result = [self._get_instance(inst) for inst in instances]
|
||||
# NOTE(dtantsur): do not accept node names as valid instances if they
|
||||
|
|
@ -580,7 +599,7 @@ class Provisioner(object):
|
|||
missing = [inst for (res, inst) in zip(result, instances)
|
||||
if res.state == _instance.InstanceState.UNKNOWN]
|
||||
if missing:
|
||||
raise exceptions.InvalidInstance(
|
||||
raise exceptions.InstanceNotFound(
|
||||
"Node(s)/instance(s) %s are not valid instances"
|
||||
% ', '.join(map(str, missing)))
|
||||
return result
|
||||
|
|
@ -611,28 +630,41 @@ class Provisioner(object):
|
|||
return node
|
||||
|
||||
def _find_node_and_allocation(self, node, refresh=False):
|
||||
if (not isinstance(node, six.string_types)
|
||||
or not _utils.is_hostname_safe(node)):
|
||||
return self._get_node(node, refresh=refresh), None
|
||||
|
||||
try:
|
||||
allocation = self.connection.baremetal.get_allocation(node)
|
||||
except os_exc.ResourceNotFound:
|
||||
return self._get_node(node, refresh=refresh), None
|
||||
else:
|
||||
if allocation.node_id:
|
||||
if (not isinstance(node, six.string_types)
|
||||
or not _utils.is_hostname_safe(node)):
|
||||
return self._get_node(node, refresh=refresh), None
|
||||
|
||||
try:
|
||||
allocation = self.connection.baremetal.get_allocation(node)
|
||||
except os_exc.ResourceNotFound:
|
||||
return self._get_node(node, refresh=refresh), None
|
||||
except os_exc.ResourceNotFound as exc:
|
||||
raise exceptions.InstanceNotFound(str(exc))
|
||||
|
||||
if allocation.node_id:
|
||||
try:
|
||||
return (self.connection.baremetal.get_node(allocation.node_id),
|
||||
allocation)
|
||||
else:
|
||||
raise exceptions.InvalidInstance(
|
||||
'Allocation %s exists but is not associated '
|
||||
'with a node' % node)
|
||||
except os_exc.ResourceNotFound:
|
||||
raise exceptions.InstanceNotFound(
|
||||
'Node %(node)s associated with allocation '
|
||||
'%(alloc)s was not found' %
|
||||
{'node': allocation.node_id,
|
||||
'alloc': allocation.id})
|
||||
else:
|
||||
raise exceptions.InstanceNotFound(
|
||||
'Allocation %s exists but is not associated '
|
||||
'with a node' % node)
|
||||
|
||||
def _get_instance(self, ident):
|
||||
node, allocation = self._find_node_and_allocation(ident)
|
||||
if allocation is None and node.allocation_id:
|
||||
allocation = self.connection.baremetal.get_allocation(
|
||||
node.allocation_id)
|
||||
try:
|
||||
allocation = self.connection.baremetal.get_allocation(
|
||||
node.allocation_id)
|
||||
except os_exc.ResourceNotFound as exc:
|
||||
raise exceptions.InstanceNotFound(str(exc))
|
||||
|
||||
return _instance.Instance(self.connection, node,
|
||||
allocation=allocation)
|
||||
|
|
|
|||
|
|
@ -13,13 +13,20 @@
|
|||
# See the License for the specific language governing permissions and
|
||||
# limitations under the License.
|
||||
|
||||
import contextlib
|
||||
import logging
|
||||
import re
|
||||
import sys
|
||||
|
||||
from openstack import exceptions as os_exc
|
||||
import six
|
||||
|
||||
from metalsmith import exceptions
|
||||
|
||||
|
||||
LOG = logging.getLogger(__name__)
|
||||
|
||||
|
||||
def log_res(res):
|
||||
if res is None:
|
||||
return None
|
||||
|
|
@ -122,3 +129,19 @@ def hostname_for(node, allocation=None):
|
|||
return allocation.name
|
||||
else:
|
||||
return default_hostname(node)
|
||||
|
||||
|
||||
@contextlib.contextmanager
|
||||
def reraise_os_exc(reraise_as, failure_message='Clean up failed'):
|
||||
exc_info = sys.exc_info()
|
||||
is_expected = isinstance(exc_info[1], os_exc.SDKException)
|
||||
|
||||
try:
|
||||
yield is_expected
|
||||
except Exception:
|
||||
LOG.exception(failure_message)
|
||||
|
||||
if is_expected:
|
||||
raise reraise_as(str(exc_info[1]))
|
||||
else:
|
||||
six.reraise(*exc_info)
|
||||
|
|
|
|||
|
|
@ -67,28 +67,6 @@ class CapabilitiesNotFound(ReservationFailed):
|
|||
super(CapabilitiesNotFound, self).__init__(message)
|
||||
|
||||
|
||||
class TraitsNotFound(ReservationFailed):
|
||||
"""DEPRECATED."""
|
||||
|
||||
def __init__(self, message, traits):
|
||||
self.requested_traits = traits
|
||||
super(TraitsNotFound, self).__init__(message)
|
||||
|
||||
|
||||
class ValidationFailed(ReservationFailed):
|
||||
"""Validation failed for all requested nodes."""
|
||||
|
||||
|
||||
class NoNodesReserved(ReservationFailed):
|
||||
"""DEPRECATED."""
|
||||
|
||||
def __init__(self, nodes):
|
||||
self.nodes = nodes
|
||||
message = ('All the candidate nodes are already reserved '
|
||||
'or failed validation')
|
||||
super(NoNodesReserved, self).__init__(message)
|
||||
|
||||
|
||||
class InvalidImage(Error):
|
||||
"""Requested image is invalid and cannot be used."""
|
||||
|
||||
|
|
@ -105,13 +83,18 @@ class InvalidNode(Error):
|
|||
"""This node cannot be deployed onto."""
|
||||
|
||||
|
||||
class DeploymentFailure(Error):
|
||||
"""DEPRECATED."""
|
||||
|
||||
def __init__(self, message, nodes):
|
||||
self.nodes = nodes
|
||||
super(DeploymentFailure, self).__init__(message)
|
||||
class DeploymentFailed(Error):
|
||||
"""Deployment failed."""
|
||||
|
||||
|
||||
class InvalidInstance(Error):
|
||||
"""The node(s) does not have a metalsmith instance associated."""
|
||||
class DeploymentTimeout(DeploymentFailed):
|
||||
"""Timeout during deployment."""
|
||||
|
||||
|
||||
class InstanceNotFound(Error):
|
||||
"""Instance not found or node doesn't have an instance associated."""
|
||||
|
||||
|
||||
# Deprecated aliases
|
||||
DeploymentFailure = DeploymentFailed
|
||||
InvalidInstance = InstanceNotFound
|
||||
|
|
|
|||
|
|
@ -21,6 +21,7 @@ import testtools
|
|||
|
||||
from metalsmith import _instance
|
||||
from metalsmith import _provisioner
|
||||
from metalsmith import _utils
|
||||
from metalsmith import exceptions
|
||||
from metalsmith import instance_config
|
||||
from metalsmith import sources
|
||||
|
|
@ -133,6 +134,13 @@ class TestGetFindNode(testtools.TestCase):
|
|||
self.assertIs(result, node)
|
||||
self.assertIsNone(alloc)
|
||||
|
||||
def test__find_node_and_allocation_by_node_not_found(self):
|
||||
node = mock.Mock(spec=['id', 'name'])
|
||||
self.api.baremetal.get_node.side_effect = os_exc.ResourceNotFound
|
||||
self.assertRaises(exceptions.InstanceNotFound,
|
||||
self.pr._find_node_and_allocation, node,
|
||||
refresh=True)
|
||||
|
||||
def test__find_node_and_allocation_by_hostname(self):
|
||||
result, alloc = self.pr._find_node_and_allocation('node')
|
||||
self.assertIs(result, self.api.baremetal.get_node.return_value)
|
||||
|
|
@ -148,9 +156,16 @@ class TestGetFindNode(testtools.TestCase):
|
|||
self.assertIsNone(alloc)
|
||||
self.api.baremetal.get_node.assert_called_once_with('node')
|
||||
|
||||
def test__find_node_and_allocation_by_hostname_node_in_allocation(self):
|
||||
self.api.baremetal.get_node.side_effect = os_exc.ResourceNotFound
|
||||
self.assertRaises(exceptions.InstanceNotFound,
|
||||
self.pr._find_node_and_allocation, 'node')
|
||||
self.api.baremetal.get_node.assert_called_once_with(
|
||||
self.api.baremetal.get_allocation.return_value.node_id)
|
||||
|
||||
def test__find_node_and_allocation_by_hostname_bad_allocation(self):
|
||||
self.api.baremetal.get_allocation.return_value.node_id = None
|
||||
self.assertRaises(exceptions.InvalidInstance,
|
||||
self.assertRaises(exceptions.InstanceNotFound,
|
||||
self.pr._find_node_and_allocation, 'node')
|
||||
self.assertFalse(self.api.baremetal.get_node.called)
|
||||
|
||||
|
|
@ -195,6 +210,19 @@ class TestReserveNode(Base):
|
|||
self.assertFalse(self.api.baremetal.patch_node.called)
|
||||
self.assertFalse(self.api.baremetal.delete_allocation.called)
|
||||
|
||||
def test_create_allocation_failed(self):
|
||||
self.api.baremetal.create_allocation.side_effect = (
|
||||
os_exc.SDKException('boom'))
|
||||
|
||||
self.assertRaisesRegex(exceptions.ReservationFailed, 'boom',
|
||||
self.pr.reserve_node, self.RSC)
|
||||
|
||||
self.api.baremetal.create_allocation.assert_called_once_with(
|
||||
name=None, candidate_nodes=None,
|
||||
resource_class=self.RSC, traits=None)
|
||||
self.assertFalse(self.api.baremetal.delete_allocation.called)
|
||||
self.assertFalse(self.api.baremetal.patch_node.called)
|
||||
|
||||
def test_allocation_failed(self):
|
||||
self.api.baremetal.wait_for_allocation.side_effect = (
|
||||
os_exc.SDKException('boom'))
|
||||
|
|
@ -209,7 +237,7 @@ class TestReserveNode(Base):
|
|||
self.api.baremetal.create_allocation.return_value)
|
||||
self.assertFalse(self.api.baremetal.patch_node.called)
|
||||
|
||||
@mock.patch.object(_provisioner.LOG, 'exception', autospec=True)
|
||||
@mock.patch.object(_utils.LOG, 'exception', autospec=True)
|
||||
def test_allocation_failed_clean_up_failed(self, mock_log):
|
||||
self.api.baremetal.delete_allocation.side_effect = RuntimeError()
|
||||
self.api.baremetal.wait_for_allocation.side_effect = (
|
||||
|
|
@ -269,7 +297,27 @@ class TestReserveNode(Base):
|
|||
self.api.baremetal.nodes.return_value = [expected]
|
||||
self.api.baremetal.patch_node.side_effect = os_exc.SDKException('boom')
|
||||
|
||||
self.assertRaisesRegex(os_exc.SDKException, 'boom',
|
||||
self.assertRaisesRegex(exceptions.ReservationFailed, 'boom',
|
||||
self.pr.reserve_node, self.RSC,
|
||||
capabilities={'answer': '42'})
|
||||
|
||||
self.api.baremetal.create_allocation.assert_called_once_with(
|
||||
name=None, candidate_nodes=[expected.id],
|
||||
resource_class=self.RSC, traits=None)
|
||||
self.api.baremetal.delete_allocation.assert_called_once_with(
|
||||
self.api.baremetal.wait_for_allocation.return_value)
|
||||
self.api.baremetal.patch_node.assert_called_once_with(
|
||||
expected, [{'path': '/instance_info/capabilities',
|
||||
'op': 'add', 'value': {'answer': '42'}}])
|
||||
|
||||
def test_node_update_unexpected_exception(self):
|
||||
expected = self._node(properties={'local_gb': 100,
|
||||
'capabilities': {'answer': '42'}})
|
||||
self.api.baremetal.get_node.return_value = expected
|
||||
self.api.baremetal.nodes.return_value = [expected]
|
||||
self.api.baremetal.patch_node.side_effect = RuntimeError('boom')
|
||||
|
||||
self.assertRaisesRegex(RuntimeError, 'boom',
|
||||
self.pr.reserve_node, self.RSC,
|
||||
capabilities={'answer': '42'})
|
||||
|
||||
|
|
@ -352,6 +400,16 @@ class TestReserveNode(Base):
|
|||
self.api.baremetal.wait_for_allocation.return_value.node_id)
|
||||
self.assertFalse(self.api.baremetal.patch_node.called)
|
||||
|
||||
def test_provided_node_not_found(self):
|
||||
self.mock_get_node.side_effect = os_exc.ResourceNotFound
|
||||
|
||||
self.assertRaises(exceptions.InvalidNode, self.pr.reserve_node,
|
||||
self.RSC, candidates=['node1'])
|
||||
|
||||
self.assertFalse(self.api.baremetal.nodes.called)
|
||||
self.assertFalse(self.api.baremetal.create_allocation.called)
|
||||
self.assertFalse(self.api.baremetal.patch_node.called)
|
||||
|
||||
def test_nodes_filtered(self):
|
||||
nodes = [self._node(resource_class='banana'),
|
||||
self._node(resource_class='compute'),
|
||||
|
|
@ -1776,6 +1834,16 @@ class TestUnprovisionNode(Base):
|
|||
wait_mock.assert_called_once_with([self.node], 'available',
|
||||
timeout=3600)
|
||||
|
||||
def test_with_wait_failed(self):
|
||||
for caught, expected in [(os_exc.ResourceTimeout,
|
||||
exceptions.DeploymentTimeout),
|
||||
(os_exc.SDKException,
|
||||
exceptions.DeploymentFailed)]:
|
||||
self.api.baremetal.wait_for_nodes_provision_state.side_effect = (
|
||||
caught)
|
||||
self.assertRaises(expected, self.pr.unprovision_node,
|
||||
self.node, wait=3600)
|
||||
|
||||
def test_without_allocation(self):
|
||||
self.node.allocation_id = None
|
||||
# Check that unrelated extra fields are not touched.
|
||||
|
|
@ -1867,7 +1935,7 @@ class TestShowInstance(testtools.TestCase):
|
|||
self.node.provision_state = 'manageable'
|
||||
self.api.baremetal.get_allocation.side_effect = (
|
||||
os_exc.ResourceNotFound())
|
||||
self.assertRaises(exceptions.InvalidInstance,
|
||||
self.assertRaises(exceptions.InstanceNotFound,
|
||||
self.pr.show_instance, 'id1')
|
||||
self.api.baremetal.get_node.assert_called_once_with('id1')
|
||||
|
||||
|
|
@ -1881,6 +1949,17 @@ class TestWaitForProvisioning(Base):
|
|||
self.assertEqual([node], [inst.node for inst in result])
|
||||
self.assertIsInstance(result[0], _instance.Instance)
|
||||
|
||||
def test_exceptions(self):
|
||||
node = mock.Mock(spec=NODE_FIELDS)
|
||||
|
||||
for caught, expected in [(os_exc.ResourceTimeout,
|
||||
exceptions.DeploymentTimeout),
|
||||
(os_exc.SDKException,
|
||||
exceptions.DeploymentFailed)]:
|
||||
self.api.baremetal.wait_for_nodes_provision_state.side_effect = (
|
||||
caught)
|
||||
self.assertRaises(expected, self.pr.wait_for_provisioning, [node])
|
||||
|
||||
|
||||
class TestListInstances(Base):
|
||||
def setUp(self):
|
||||
|
|
|
|||
|
|
@ -0,0 +1,8 @@
|
|||
---
|
||||
upgrade:
|
||||
- |
|
||||
Changes to consistently using exceptions from ``metalsmith.exceptions``
|
||||
rathen than exposing OpenStackSDK exceptions.
|
||||
deprecations:
|
||||
- |
|
||||
The exception ``InvalidInstance`` has been renamed to ``InstanceNotFound``.
|
||||
Loading…
Reference in New Issue