diff --git a/nova/api/openstack/compute/baremetal_nodes.py b/nova/api/openstack/compute/baremetal_nodes.py index a9d304b4597f..d380e30b13e9 100644 --- a/nova/api/openstack/compute/baremetal_nodes.py +++ b/nova/api/openstack/compute/baremetal_nodes.py @@ -14,110 +14,100 @@ # License for the specific language governing permissions and limitations # under the License. -"""The bare-metal admin extension.""" +"""The baremetal admin extension.""" -from oslo_utils import importutils +from openstack import exceptions as sdk_exc import webob from nova.api.openstack.api_version_request \ import MAX_PROXY_API_SUPPORT_VERSION -from nova.api.openstack import common from nova.api.openstack import wsgi import nova.conf from nova.i18n import _ from nova.policies import baremetal_nodes as bn_policies - -ironic_client = importutils.try_import('ironicclient.client') -ironic_exc = importutils.try_import('ironicclient.exc') +from nova import utils CONF = nova.conf.CONF -def _check_ironic_client_enabled(): - """Check whether Ironic is installed or not.""" - if ironic_client is None: - common.raise_feature_not_supported() - - -def _get_ironic_client(): - """return an Ironic client.""" - # TODO(NobodyCam): Fix insecure setting - # NOTE(efried): This should all be replaced by ksa adapter options; but the - # nova-to-baremetal API is deprecated, so not changing it. - # https://docs.openstack.org/api-ref/compute/#bare-metal-nodes-os-baremetal-nodes-deprecated # noqa - kwargs = {'os_username': CONF.ironic.admin_username, - 'os_password': CONF.ironic.admin_password, - 'os_auth_url': CONF.ironic.admin_url, - 'os_tenant_name': CONF.ironic.admin_tenant_name, - 'os_service_type': 'baremetal', - 'os_endpoint_type': 'public', - 'insecure': 'true', - 'endpoint': CONF.ironic.endpoint_override} - # NOTE(mriedem): The 1 api_version arg here is the only valid value for - # the client, but it's not even used so it doesn't really matter. The - # ironic client wrapper in the virt driver actually uses a hard-coded - # microversion via the os_ironic_api_version kwarg. - icli = ironic_client.get_client(1, **kwargs) - return icli - - def _no_ironic_proxy(cmd): - raise webob.exc.HTTPBadRequest( - explanation=_("Command Not supported. Please use Ironic " - "command %(cmd)s to perform this " - "action.") % {'cmd': cmd}) + msg = _( + "Command Not supported. Please use Ironic " + "command %(cmd)s to perform this action." + ) + raise webob.exc.HTTPBadRequest(explanation=msg % {'cmd': cmd}) class BareMetalNodeController(wsgi.Controller): """The Bare-Metal Node API controller for the OpenStack API.""" + def __init__(self): + super().__init__() + + self._ironic_connection = None + + @property + def ironic_connection(self): + if self._ironic_connection is None: + # Ask get_sdk_adapter to raise ServiceUnavailable if the baremetal + # service isn't ready yet. Consumers of ironic_connection are set + # up to handle this and raise VirtDriverNotReady as appropriate. + self._ironic_connection = utils.get_sdk_adapter( + 'baremetal', + check_service=True, + ) + return self._ironic_connection + @wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION) @wsgi.expected_errors((404, 501)) def index(self, req): context = req.environ['nova.context'] - context.can(bn_policies.BASE_POLICY_NAME % 'list', - target={}) + context.can(bn_policies.BASE_POLICY_NAME % 'list', target={}) + nodes = [] # proxy command to Ironic - _check_ironic_client_enabled() - icli = _get_ironic_client() - ironic_nodes = icli.node.list(detail=True) - for inode in ironic_nodes: - node = {'id': inode.uuid, - 'interfaces': [], - 'host': 'IRONIC MANAGED', - 'task_state': inode.provision_state, - 'cpus': inode.properties.get('cpus', 0), - 'memory_mb': inode.properties.get('memory_mb', 0), - 'disk_gb': inode.properties.get('local_gb', 0)} - nodes.append(node) - return {'nodes': nodes} - - @wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION) - @wsgi.expected_errors((404, 501)) - def show(self, req, id): - context = req.environ['nova.context'] - context.can(bn_policies.BASE_POLICY_NAME % 'show', - target={}) - # proxy command to Ironic - _check_ironic_client_enabled() - icli = _get_ironic_client() - try: - inode = icli.node.get(id) - except ironic_exc.NotFound: - msg = _("Node %s could not be found.") % id - raise webob.exc.HTTPNotFound(explanation=msg) - iports = icli.node.list_ports(id) - node = {'id': inode.uuid, + inodes = self.ironic_connection.nodes(details=True) + for inode in inodes: + node = { + 'id': inode.id, 'interfaces': [], 'host': 'IRONIC MANAGED', 'task_state': inode.provision_state, 'cpus': inode.properties.get('cpus', 0), 'memory_mb': inode.properties.get('memory_mb', 0), 'disk_gb': inode.properties.get('local_gb', 0), - 'instance_uuid': inode.instance_uuid} + } + nodes.append(node) + + return {'nodes': nodes} + + @wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION) + @wsgi.expected_errors((404, 501)) + def show(self, req, id): + context = req.environ['nova.context'] + context.can(bn_policies.BASE_POLICY_NAME % 'show', target={}) + + # proxy command to Ironic + try: + inode = self.ironic_connection.get_node(id) + except sdk_exc.NotFoundException: + msg = _("Node %s could not be found.") % id + raise webob.exc.HTTPNotFound(explanation=msg) + + iports = self.ironic_connection.ports(node=id) + node = { + 'id': inode.id, + 'interfaces': [], + 'host': 'IRONIC MANAGED', + 'task_state': inode.provision_state, + 'cpus': inode.properties.get('cpus', 0), + 'memory_mb': inode.properties.get('memory_mb', 0), + 'disk_gb': inode.properties.get('local_gb', 0), + 'instance_uuid': inode.instance_id, + } for port in iports: node['interfaces'].append({'address': port.address}) + return {'node': node} @wsgi.Controller.api_version("2.1", MAX_PROXY_API_SUPPORT_VERSION) diff --git a/nova/tests/functional/api_sample_tests/test_baremetal_nodes.py b/nova/tests/functional/api_sample_tests/test_baremetal_nodes.py index 569df728e31a..0896be0158cf 100644 --- a/nova/tests/functional/api_sample_tests/test_baremetal_nodes.py +++ b/nova/tests/functional/api_sample_tests/test_baremetal_nodes.py @@ -14,51 +14,65 @@ from unittest import mock +from openstack.baremetal.v1 import _proxy as baremetal_proxy +from openstack.baremetal.v1 import node + from nova.tests.functional.api_sample_tests import api_sample_base -class FakeNode(object): - def __init__(self, uuid='058d27fa-241b-445a-a386-08c04f96db43'): - self.uuid = uuid - self.provision_state = 'active' - self.properties = {'cpus': '2', - 'memory_mb': '1024', - 'local_gb': '10'} - self.instance_uuid = '1ea4e53e-149a-4f02-9515-590c9fb2315a' +fake_nodes = [ + node.Node( + id=id_, + provision_state='active', + properties={ + 'cpus': '2', + 'memory_mb': '1024', + 'local_gb': '10', + }, + instance_id='1ea4e53e-149a-4f02-9515-590c9fb2315a', + ) for id_ in ( + '058d27fa-241b-445a-a386-08c04f96db43', + 'e2025409-f3ce-4d6a-9788-c565cf3b1b1c', + ) +] -class NodeManager(object): - def list(self, detail=False): - return [FakeNode(), FakeNode('e2025409-f3ce-4d6a-9788-c565cf3b1b1c')] - - def get(self, id): - return FakeNode(id) - - def list_ports(self, id): - return [] +def nodes(*args, **kwargs): + for fake_node in fake_nodes: + yield fake_node -class fake_client(object): - node = NodeManager() +def get_node(*args, **kwargs): + return fake_nodes[0] +def ports(*args, **kwargs): + # return an empty generator + return + yield + + +fake_client = mock.create_autospec(baremetal_proxy.Proxy) +fake_client.nodes.side_effect = nodes +fake_client.get_node.side_effect = get_node +fake_client.ports.side_effect = ports + + +@mock.patch( + "nova.api.openstack.compute.baremetal_nodes" + ".BareMetalNodeController.ironic_connection", + new_callable=mock.PropertyMock, + return_value=fake_client, +) class BareMetalNodesSampleJsonTest(api_sample_base.ApiSampleTestBaseV21): ADMIN_API = True sample_dir = "os-baremetal-nodes" - @mock.patch("nova.api.openstack.compute.baremetal_nodes" - "._get_ironic_client") def test_baremetal_nodes_list(self, mock_get_irc): - mock_get_irc.return_value = fake_client() - response = self._do_get('os-baremetal-nodes') self._verify_response('baremetal-node-list-resp', {}, response, 200) - @mock.patch("nova.api.openstack.compute.baremetal_nodes" - "._get_ironic_client") def test_baremetal_nodes_get(self, mock_get_irc): - mock_get_irc.return_value = fake_client() - response = self._do_get('os-baremetal-nodes/' '058d27fa-241b-445a-a386-08c04f96db43') self._verify_response('baremetal-node-get-resp', {}, response, 200) diff --git a/nova/tests/unit/api/openstack/compute/test_baremetal_nodes.py b/nova/tests/unit/api/openstack/compute/test_baremetal_nodes.py index c8ad907b1028..0749310c4e45 100644 --- a/nova/tests/unit/api/openstack/compute/test_baremetal_nodes.py +++ b/nova/tests/unit/api/openstack/compute/test_baremetal_nodes.py @@ -13,12 +13,11 @@ # License for the specific language governing permissions and limitations # under the License. -from unittest import mock - -from ironicclient import exc as ironic_exc +import fixtures +from openstack import exceptions as sdk_exc from webob import exc -from nova.api.openstack.compute import baremetal_nodes as b_nodes_v21 +from nova.api.openstack.compute import baremetal_nodes from nova import context from nova import exception from nova import test @@ -26,105 +25,69 @@ from nova.tests.unit.api.openstack import fakes from nova.tests.unit.virt.ironic import utils as ironic_utils -def fake_node(**updates): - node = { - 'id': 1, - 'service_host': "host", - 'cpus': 8, - 'memory_mb': 8192, - 'local_gb': 128, - 'pm_address': "10.1.2.3", - 'pm_user': "pm_user", - 'pm_password': "pm_pass", - 'terminal_port': 8000, - 'interfaces': [], - 'instance_uuid': 'fake-instance-uuid', - } - if updates: - node.update(updates) - return node - - -def fake_node_ext_status(**updates): - node = fake_node(uuid='fake-uuid', - task_state='fake-task-state', - updated_at='fake-updated-at', - pxe_config_path='fake-pxe-config-path') - if updates: - node.update(updates) - return node - - -FAKE_IRONIC_CLIENT = ironic_utils.FakeClient() - - -@mock.patch.object(b_nodes_v21, '_get_ironic_client', - lambda *_: FAKE_IRONIC_CLIENT) -class BareMetalNodesTestV21(test.NoDBTestCase): - mod = b_nodes_v21 +class BareMetalNodesTest(test.NoDBTestCase): + mod = baremetal_nodes def setUp(self): - super(BareMetalNodesTestV21, self).setUp() + super().setUp() self._setup() self.context = context.get_admin_context() self.request = fakes.HTTPRequest.blank('', use_admin_context=True) - def _setup(self): - self.controller = b_nodes_v21.BareMetalNodeController() + # stub out openstacksdk + self.mock_conn = self.useFixture( + fixtures.MockPatchObject(self.controller, '_ironic_connection'), + ).mock - @mock.patch.object(FAKE_IRONIC_CLIENT.node, 'list') - def test_index_ironic(self, mock_list): + def _setup(self): + self.controller = baremetal_nodes.BareMetalNodeController() + + def test_index_ironic(self): properties = {'cpus': 2, 'memory_mb': 1024, 'local_gb': 20} node = ironic_utils.get_test_node(properties=properties) - mock_list.return_value = [node] + self.mock_conn.nodes.return_value = iter([node]) res_dict = self.controller.index(self.request) + expected_output = {'nodes': [{'memory_mb': properties['memory_mb'], 'host': 'IRONIC MANAGED', 'disk_gb': properties['local_gb'], 'interfaces': [], 'task_state': None, - 'id': node.uuid, + 'id': node.id, 'cpus': properties['cpus']}]} self.assertEqual(expected_output, res_dict) - mock_list.assert_called_once_with(detail=True) + self.mock_conn.nodes.assert_called_once_with(details=True) - @mock.patch.object(FAKE_IRONIC_CLIENT.node, 'list') - def test_index_ironic_missing_properties(self, mock_list): + def test_index_ironic_missing_properties(self): properties = {'cpus': 2} node = ironic_utils.get_test_node(properties=properties) - mock_list.return_value = [node] + self.mock_conn.nodes.return_value = iter([node]) res_dict = self.controller.index(self.request) + expected_output = {'nodes': [{'memory_mb': 0, 'host': 'IRONIC MANAGED', 'disk_gb': 0, 'interfaces': [], 'task_state': None, - 'id': node.uuid, + 'id': node.id, 'cpus': properties['cpus']}]} self.assertEqual(expected_output, res_dict) - mock_list.assert_called_once_with(detail=True) + self.mock_conn.nodes.assert_called_once_with(details=True) - def test_index_ironic_not_implemented(self): - with mock.patch.object(self.mod, 'ironic_client', None): - self.assertRaises(exc.HTTPNotImplemented, - self.controller.index, - self.request) - - @mock.patch.object(FAKE_IRONIC_CLIENT.node, 'list_ports') - @mock.patch.object(FAKE_IRONIC_CLIENT.node, 'get') - def test_show_ironic(self, mock_get, mock_list_ports): + def test_show_ironic(self): properties = {'cpus': 1, 'memory_mb': 512, 'local_gb': 10} node = ironic_utils.get_test_node(properties=properties) port = ironic_utils.get_test_port() - mock_get.return_value = node - mock_list_ports.return_value = [port] + self.mock_conn.get_node.return_value = node + self.mock_conn.ports.return_value = iter([port]) + + res_dict = self.controller.show(self.request, node.id) - res_dict = self.controller.show(self.request, node.uuid) expected_output = {'node': {'memory_mb': properties['memory_mb'], 'instance_uuid': None, @@ -132,22 +95,21 @@ class BareMetalNodesTestV21(test.NoDBTestCase): 'disk_gb': properties['local_gb'], 'interfaces': [{'address': port.address}], 'task_state': None, - 'id': node.uuid, + 'id': node.id, 'cpus': properties['cpus']}} self.assertEqual(expected_output, res_dict) - mock_get.assert_called_once_with(node.uuid) - mock_list_ports.assert_called_once_with(node.uuid) + self.mock_conn.get_node.assert_called_once_with(node.id) + self.mock_conn.ports.assert_called_once_with(node=node.id) - @mock.patch.object(FAKE_IRONIC_CLIENT.node, 'list_ports') - @mock.patch.object(FAKE_IRONIC_CLIENT.node, 'get') - def test_show_ironic_no_properties(self, mock_get, mock_list_ports): + def test_show_ironic_no_properties(self): properties = {} node = ironic_utils.get_test_node(properties=properties) port = ironic_utils.get_test_port() - mock_get.return_value = node - mock_list_ports.return_value = [port] + self.mock_conn.get_node.return_value = node + self.mock_conn.ports.return_value = iter([port]) + + res_dict = self.controller.show(self.request, node.id) - res_dict = self.controller.show(self.request, node.uuid) expected_output = {'node': {'memory_mb': 0, 'instance_uuid': None, @@ -155,39 +117,35 @@ class BareMetalNodesTestV21(test.NoDBTestCase): 'disk_gb': 0, 'interfaces': [{'address': port.address}], 'task_state': None, - 'id': node.uuid, + 'id': node.id, 'cpus': 0}} self.assertEqual(expected_output, res_dict) - mock_get.assert_called_once_with(node.uuid) - mock_list_ports.assert_called_once_with(node.uuid) + self.mock_conn.get_node.assert_called_once_with(node.id) + self.mock_conn.ports.assert_called_once_with(node=node.id) - @mock.patch.object(FAKE_IRONIC_CLIENT.node, 'list_ports') - @mock.patch.object(FAKE_IRONIC_CLIENT.node, 'get') - def test_show_ironic_no_interfaces(self, mock_get, mock_list_ports): + def test_show_ironic_no_interfaces(self): properties = {'cpus': 1, 'memory_mb': 512, 'local_gb': 10} node = ironic_utils.get_test_node(properties=properties) - mock_get.return_value = node - mock_list_ports.return_value = [] + self.mock_conn.get_node.return_value = node + self.mock_conn.ports.return_value = iter([]) + + res_dict = self.controller.show(self.request, node.id) - res_dict = self.controller.show(self.request, node.uuid) self.assertEqual([], res_dict['node']['interfaces']) - mock_get.assert_called_once_with(node.uuid) - mock_list_ports.assert_called_once_with(node.uuid) + self.mock_conn.get_node.assert_called_once_with(node.id) + self.mock_conn.ports.assert_called_once_with(node=node.id) + + def test_show_ironic_node_not_found(self): + self.mock_conn.get_node.side_effect = sdk_exc.NotFoundException() + + error = self.assertRaises( + exc.HTTPNotFound, + self.controller.show, + self.request, 'fake-uuid', + ) - @mock.patch.object(FAKE_IRONIC_CLIENT.node, 'get', - side_effect=ironic_exc.NotFound()) - def test_show_ironic_node_not_found(self, mock_get): - error = self.assertRaises(exc.HTTPNotFound, self.controller.show, - self.request, 'fake-uuid') self.assertIn('fake-uuid', str(error)) - def test_show_ironic_not_implemented(self): - with mock.patch.object(self.mod, 'ironic_client', None): - properties = {'cpus': 1, 'memory_mb': 512, 'local_gb': 10} - node = ironic_utils.get_test_node(properties=properties) - self.assertRaises(exc.HTTPNotImplemented, self.controller.show, - self.request, node.uuid) - def test_create_ironic_not_supported(self): self.assertRaises(exc.HTTPBadRequest, self.controller.create, @@ -209,11 +167,11 @@ class BareMetalNodesTestV21(test.NoDBTestCase): self.request, 'fake-id', 'fake-body') -class BareMetalNodesTestDeprecation(test.NoDBTestCase): +class BareMetalNodesTestV236(test.NoDBTestCase): def setUp(self): - super(BareMetalNodesTestDeprecation, self).setUp() - self.controller = b_nodes_v21.BareMetalNodeController() + super().setUp() + self.controller = baremetal_nodes.BareMetalNodeController() self.req = fakes.HTTPRequest.blank('', version='2.36') def test_all_apis_return_not_found(self): diff --git a/nova/tests/unit/policies/test_baremetal_nodes.py b/nova/tests/unit/policies/test_baremetal_nodes.py index 68f02087c4a2..9cc9b3b9f396 100644 --- a/nova/tests/unit/policies/test_baremetal_nodes.py +++ b/nova/tests/unit/policies/test_baremetal_nodes.py @@ -10,8 +10,7 @@ # License for the specific language governing permissions and limitations # under the License. -from unittest import mock - +import fixtures from oslo_utils.fixture import uuidsentinel as uuids from nova.api.openstack.compute import baremetal_nodes @@ -22,9 +21,6 @@ from nova.tests.unit.policies import base from nova.tests.unit.virt.ironic import utils as ironic_utils -FAKE_IRONIC_CLIENT = ironic_utils.FakeClient() - - class BaremetalNodesPolicyTest(base.BasePolicyTest): """Test Baremetal Nodes APIs policies with all possible context. @@ -35,32 +31,38 @@ class BaremetalNodesPolicyTest(base.BasePolicyTest): """ def setUp(self): - super(BaremetalNodesPolicyTest, self).setUp() + super().setUp() + self.controller = baremetal_nodes.BareMetalNodeController() self.req = fakes.HTTPRequest.blank('') - self.stub_out('nova.api.openstack.compute.' - 'baremetal_nodes._get_ironic_client', - lambda *_: FAKE_IRONIC_CLIENT) + + # stub out openstacksdk + self.mock_conn = self.useFixture( + fixtures.MockPatchObject(self.controller, '_ironic_connection'), + ).mock + # With legacy rule and scope check disabled by default, system admin, # legacy admin, and project admin will be able to get baremetal nodes. self.project_admin_authorized_contexts = [ - self.legacy_admin_context, self.system_admin_context, - self.project_admin_context] + self.legacy_admin_context, + self.system_admin_context, + self.project_admin_context, + ] def test_index_nodes_policy(self): rule_name = "os_compute_api:os-baremetal-nodes:list" + self.mock_conn.nodes.return_value = iter([]) + self.common_policy_auth(self.project_admin_authorized_contexts, rule_name, self.controller.index, self.req) - @mock.patch.object(FAKE_IRONIC_CLIENT.node, 'list_ports') - @mock.patch.object(FAKE_IRONIC_CLIENT.node, 'get') - def test_show_node_policy(self, mock_get, mock_port): + def test_show_node_policy(self): rule_name = "os_compute_api:os-baremetal-nodes:show" properties = {'cpus': 1, 'memory_mb': 512, 'local_gb': 10} node = ironic_utils.get_test_node(properties=properties) - mock_get.return_value = node - mock_port.return_value = [] + self.mock_conn.get_node.return_value = node + self.mock_conn.ports.return_value = iter([]) self.common_policy_auth(self.project_admin_authorized_contexts, rule_name,