From a7489106b4f6f5e0b9019631775a44438a16df60 Mon Sep 17 00:00:00 2001 From: Dmitry Tantsur Date: Tue, 4 Sep 2018 10:48:19 +0200 Subject: [PATCH] baremetal: implement validate_node Change-Id: I2cc71931e0352f70dacb479512cdb4fb7cb011dc --- doc/source/user/proxies/baremetal.rst | 1 + .../user/resources/baremetal/v1/node.rst | 8 +++ openstack/baremetal/v1/_proxy.py | 17 ++++++ openstack/baremetal/v1/node.py | 56 ++++++++++++++++++ openstack/cloud/openstackcloud.py | 17 +----- openstack/exceptions.py | 4 ++ .../baremetal/test_baremetal_node.py | 8 +++ .../tests/unit/baremetal/v1/test_node.py | 57 ++++++++++++++++++ .../tests/unit/cloud/test_baremetal_node.py | 58 +++++++++---------- .../baremetal-validate-ccce2a37d2a20d96.yaml | 4 ++ 10 files changed, 186 insertions(+), 44 deletions(-) create mode 100644 releasenotes/notes/baremetal-validate-ccce2a37d2a20d96.yaml diff --git a/doc/source/user/proxies/baremetal.rst b/doc/source/user/proxies/baremetal.rst index b3bacb035..e6faffe4e 100644 --- a/doc/source/user/proxies/baremetal.rst +++ b/doc/source/user/proxies/baremetal.rst @@ -24,6 +24,7 @@ Node Operations .. automethod:: openstack.baremetal.v1._proxy.Proxy.nodes .. automethod:: openstack.baremetal.v1._proxy.Proxy.set_node_provision_state .. automethod:: openstack.baremetal.v1._proxy.Proxy.wait_for_nodes_provision_state + .. automethod:: openstack.baremetal.v1._proxy.Proxy.validate_node Port Operations ^^^^^^^^^^^^^^^ diff --git a/doc/source/user/resources/baremetal/v1/node.rst b/doc/source/user/resources/baremetal/v1/node.rst index 37d563f7d..bf5f8a694 100644 --- a/doc/source/user/resources/baremetal/v1/node.rst +++ b/doc/source/user/resources/baremetal/v1/node.rst @@ -10,3 +10,11 @@ The ``Node`` class inherits from :class:`~openstack.resource.Resource`. .. autoclass:: openstack.baremetal.v1.node.Node :members: + +The ValidationResult Class +^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The ``ValidationResult`` class represents the result of a validation. + +.. autoclass:: openstack.baremetal.v1.node.ValidationResult + :members: diff --git a/openstack/baremetal/v1/_proxy.py b/openstack/baremetal/v1/_proxy.py index abf02dc4d..df71e216d 100644 --- a/openstack/baremetal/v1/_proxy.py +++ b/openstack/baremetal/v1/_proxy.py @@ -325,6 +325,23 @@ class Proxy(proxy.Proxy): {'nodes': ', '.join(n.id for n in remaining), 'target': expected_state}) + def validate_node(self, node, required=('boot', 'deploy', 'power')): + """Validate required information on a node. + + :param node: The value can be either the name or ID of a node or + a :class:`~openstack.baremetal.v1.node.Node` instance. + :param required: List of interfaces that are required to pass + validation. The default value is the list of minimum required + interfaces for provisioning. + + :return: dict mapping interface names to + :class:`~openstack.baremetal.v1.node.ValidationResult` objects. + :raises: :exc:`~openstack.exceptions.ValidationException` if validation + fails for a required interface. + """ + res = self._get_resource(_node.Node, node) + return res.validate(self, required=required) + def delete_node(self, node, ignore_missing=True): """Delete a node. diff --git a/openstack/baremetal/v1/node.py b/openstack/baremetal/v1/node.py index fafdfd5d2..0c312d94a 100644 --- a/openstack/baremetal/v1/node.py +++ b/openstack/baremetal/v1/node.py @@ -21,6 +21,20 @@ from openstack import utils _logger = _log.setup_logging('openstack') +class ValidationResult(object): + """Result of a single interface validation. + + :ivar result: Result of a validation, ``True`` for success, ``False`` for + failure, ``None`` for unsupported interface. + :ivar reason: If ``result`` is ``False`` or ``None``, explanation of + the result. + """ + + def __init__(self, result, reason): + self.result = result + self.reason = reason + + class Node(resource.Resource): resources_key = 'nodes' @@ -443,6 +457,48 @@ class Node(resource.Resource): exceptions.raise_from_response(response, error_message=msg) return [vif['id'] for vif in response.json()['vifs']] + def validate(self, session, required=('boot', 'deploy', 'power')): + """Validate required information on a node. + + :param session: The session to use for making this request. + :type session: :class:`~keystoneauth1.adapter.Adapter` + :param required: List of interfaces that are required to pass + validation. The default value is the list of minimum required + interfaces for provisioning. + + :return: dict mapping interface names to :class:`ValidationResult` + objects. + :raises: :exc:`~openstack.exceptions.ValidationException` if validation + fails for a required interface. + """ + session = self._get_session(session) + version = self._get_microversion_for(session, 'fetch') + + request = self._prepare_request(requires_id=True) + request.url = utils.urljoin(request.url, 'validate') + response = session.get(request.url, headers=request.headers, + microversion=version) + + msg = ("Failed to validate node {node}".format(node=self.id)) + exceptions.raise_from_response(response, error_message=msg) + result = response.json() + + if required: + failed = [ + '%s (%s)' % (key, value.get('reason', 'no reason')) + for key, value in result.items() + if key in required and not value.get('result') + ] + + if failed: + raise exceptions.ValidationException( + 'Validation failed for required interfaces of node {node}:' + ' {failures}'.format(node=self.id, + failures=', '.join(failed))) + + return {key: ValidationResult(value.get('result'), value.get('reason')) + for key, value in result.items()} + class NodeDetail(Node): diff --git a/openstack/cloud/openstackcloud.py b/openstack/cloud/openstackcloud.py index 0144d71e0..e75664df8 100755 --- a/openstack/cloud/openstackcloud.py +++ b/openstack/cloud/openstackcloud.py @@ -9856,20 +9856,9 @@ class OpenStackCloud(_normalize.Normalizer): return [self.get_port(vif) for vif in vif_ids] def validate_node(self, uuid): - # TODO(TheJulia): There are soooooo many other interfaces - # that we can support validating, while these are essential, - # we should support more. - # TODO(TheJulia): Add a doc string :( - msg = ("Failed to query the API for validation status of " - "node {node_id}").format(node_id=uuid) - url = '/nodes/{node_id}/validate'.format(node_id=uuid) - ifaces = self._baremetal_client.get(url, error_message=msg) - - if not ifaces['deploy'] or not ifaces['power']: - raise exc.OpenStackCloudException( - "ironic node %s failed to validate. " - "(deploy: %s, power: %s)" % (ifaces['deploy'], - ifaces['power'])) + # TODO(dtantsur): deprecate this short method in favor of a fully + # written validate_machine call. + self.baremetal.validate_node(uuid) def node_set_provision_state(self, name_or_id, diff --git a/openstack/exceptions.py b/openstack/exceptions.py index c6687e080..9e5a76152 100644 --- a/openstack/exceptions.py +++ b/openstack/exceptions.py @@ -222,3 +222,7 @@ class ConfigException(SDKException): class NotSupported(SDKException): """Request cannot be performed by any supported API version.""" + + +class ValidationException(SDKException): + """Validation failed for resource.""" diff --git a/openstack/tests/functional/baremetal/test_baremetal_node.py b/openstack/tests/functional/baremetal/test_baremetal_node.py index 04c5d8acc..45c4f2c94 100644 --- a/openstack/tests/functional/baremetal/test_baremetal_node.py +++ b/openstack/tests/functional/baremetal/test_baremetal_node.py @@ -82,6 +82,14 @@ class TestBareMetalNode(base.BaseBaremetalTest): wait=True) self.assertEqual(node.provision_state, 'available') + def test_node_validate(self): + node = self.create_node() + # Fake hardware passes validation for all interfaces + result = self.conn.baremetal.validate_node(node) + for iface in ('boot', 'deploy', 'management', 'power'): + self.assertTrue(result[iface].result) + self.assertFalse(result[iface].reason) + def test_node_negative_non_existing(self): uuid = "5c9dcd04-2073-49bc-9618-99ae634d8971" self.assertRaises(exceptions.ResourceNotFound, diff --git a/openstack/tests/unit/baremetal/v1/test_node.py b/openstack/tests/unit/baremetal/v1/test_node.py index e965590f9..7756f2ec8 100644 --- a/openstack/tests/unit/baremetal/v1/test_node.py +++ b/openstack/tests/unit/baremetal/v1/test_node.py @@ -432,3 +432,60 @@ class TestNodeVif(base.TestCase): self.assertRaises(exceptions.NotSupported, self.node.list_vifs, self.session) + + +@mock.patch.object(exceptions, 'raise_from_response', mock.Mock()) +@mock.patch.object(node.Node, '_get_session', lambda self, x: x) +class TestNodeValidate(base.TestCase): + + def setUp(self): + super(TestNodeValidate, self).setUp() + self.session = mock.Mock(spec=adapter.Adapter) + self.session.default_microversion = '1.28' + self.node = node.Node(**FAKE) + + def test_validate_ok(self): + self.session.get.return_value.json.return_value = { + 'boot': {'result': True}, + 'console': {'result': False, 'reason': 'Not configured'}, + 'deploy': {'result': True}, + 'inspect': {'result': None, 'reason': 'Not supported'}, + 'power': {'result': True} + } + result = self.node.validate(self.session) + for iface in ('boot', 'deploy', 'power'): + self.assertTrue(result[iface].result) + self.assertFalse(result[iface].reason) + for iface in ('console', 'inspect'): + self.assertIsNot(True, result[iface].result) + self.assertTrue(result[iface].reason) + + def test_validate_failed(self): + self.session.get.return_value.json.return_value = { + 'boot': {'result': False}, + 'console': {'result': False, 'reason': 'Not configured'}, + 'deploy': {'result': False, 'reason': 'No deploy for you'}, + 'inspect': {'result': None, 'reason': 'Not supported'}, + 'power': {'result': True} + } + self.assertRaisesRegex(exceptions.ValidationException, + 'No deploy for you', + self.node.validate, self.session) + + def test_validate_no_failure(self): + self.session.get.return_value.json.return_value = { + 'boot': {'result': False}, + 'console': {'result': False, 'reason': 'Not configured'}, + 'deploy': {'result': False, 'reason': 'No deploy for you'}, + 'inspect': {'result': None, 'reason': 'Not supported'}, + 'power': {'result': True} + } + result = self.node.validate(self.session, required=None) + self.assertTrue(result['power'].result) + self.assertFalse(result['power'].reason) + for iface in ('deploy', 'console', 'inspect'): + self.assertIsNot(True, result[iface].result) + self.assertTrue(result[iface].reason) + # Reason can be empty + self.assertFalse(result['boot'].result) + self.assertIsNone(result['boot'].reason) diff --git a/openstack/tests/unit/cloud/test_baremetal_node.py b/openstack/tests/unit/cloud/test_baremetal_node.py index da1ae81c1..769d0fc16 100644 --- a/openstack/tests/unit/cloud/test_baremetal_node.py +++ b/openstack/tests/unit/cloud/test_baremetal_node.py @@ -22,6 +22,7 @@ import uuid from testscenarios import load_tests_apply_scenarios as load_tests # noqa from openstack.cloud import exc +from openstack import exceptions from openstack.tests import fakes from openstack.tests.unit import base @@ -119,36 +120,33 @@ class TestBaremetalNode(base.IronicTestCase): self.assert_calls() - # FIXME(TheJulia): So, this doesn't presently fail, but should fail. - # Placing the test here, so we can sort out the issue in the actual - # method later. - # def test_validate_node_raises_exception(self): - # validate_return = { - # 'deploy': { - # 'result': False, - # 'reason': 'error!', - # }, - # 'power': { - # 'result': False, - # 'reason': 'meow!', - # }, - # 'foo': { - # 'result': True - # }} - # self.register_uris([ - # dict(method='GET', - # uri=self.get_mock_url( - # resource='nodes', - # append=[self.fake_baremetal_node['uuid'], - # 'validate']), - # json=validate_return), - # ]) - # self.assertRaises( - # Exception, - # self.cloud.validate_node, - # self.fake_baremetal_node['uuid']) - # - # self.assert_calls() + def test_validate_node_raises_exception(self): + validate_return = { + 'deploy': { + 'result': False, + 'reason': 'error!', + }, + 'power': { + 'result': False, + 'reason': 'meow!', + }, + 'foo': { + 'result': True + }} + self.register_uris([ + dict(method='GET', + uri=self.get_mock_url( + resource='nodes', + append=[self.fake_baremetal_node['uuid'], + 'validate']), + json=validate_return), + ]) + self.assertRaises( + exceptions.ValidationException, + self.cloud.validate_node, + self.fake_baremetal_node['uuid']) + + self.assert_calls() def test_patch_machine(self): test_patch = [{ diff --git a/releasenotes/notes/baremetal-validate-ccce2a37d2a20d96.yaml b/releasenotes/notes/baremetal-validate-ccce2a37d2a20d96.yaml new file mode 100644 index 000000000..7783c2fb9 --- /dev/null +++ b/releasenotes/notes/baremetal-validate-ccce2a37d2a20d96.yaml @@ -0,0 +1,4 @@ +--- +features: + - | + Adds support for bare metal node validation to the bare metal proxy.