openstack/ironic
Code Issues Proposed changes

Merge "Convert nodes endpoint to plain JSON"

Browse Source
This commit is contained in:
Zuul 2020-11-18 16:19:04 +00:00 committed by Gerrit Code Review
commit b95478937d
8 changed files with 645 additions and 1014 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1238
ironic/api/controllers/v1/node.py
View File

File diff suppressed because it is too large Load Diff

13
ironic/api/controllers/v1/ramdisk.py
View File

@ -35,8 +35,8 @@ from ironic import objects
CONF = cfg.CONF
LOG = log.getLogger(__name__)
_LOOKUP_RETURN_FIELDS = ('uuid', 'properties', 'instance_info',
'driver_internal_info')
_LOOKUP_RETURN_FIELDS = ['uuid', 'properties', 'instance_info',
'driver_internal_info']
def config(token):
@ -64,21 +64,16 @@ def config(token):
class LookupResult(base.APIBase):
"""API representation of the node lookup result."""
node = node_ctl.Node
node = None
"""The short node representation."""
config = {str: types.jsontype}
"""The configuration to pass to the ramdisk."""
@classmethod
def sample(cls):
return cls(node=node_ctl.Node.sample(),
config={'heartbeat_timeout': 600})
@classmethod
def convert_with_links(cls, node):
token = node.driver_internal_info.get('agent_secret_token')
node = node_ctl.Node.convert_with_links(node, _LOOKUP_RETURN_FIELDS)
node = node_ctl.node_convert_with_links(node, _LOOKUP_RETURN_FIELDS)
return cls(node=node, config=config(token))

30
ironic/api/controllers/v1/types.py
View File

@ -393,33 +393,3 @@ class LocalLinkConnectionType(atypes.UserType):
locallinkconnectiontype = LocalLinkConnectionType()
class VifType(JsonType):
basetype = str
name = 'viftype'
mandatory_fields = {'id'}
@staticmethod
def validate(value):
super(VifType, VifType).validate(value)
keys = set(value)
# Check all mandatory fields are present
missing = VifType.mandatory_fields - keys
if missing:
msg = _('Missing mandatory keys: %s') % ', '.join(list(missing))
raise exception.Invalid(msg)
UuidOrNameType.validate(value['id'])
return value
@staticmethod
def frombasetype(value):
if value is None:
return None
return VifType.validate(value)
viftype = VifType()

21
ironic/api/controllers/v1/utils.py
View File

@ -343,27 +343,6 @@ def validate_sort_dir(sort_dir):
return sort_dir
def validate_trait(trait, error_prefix=_('Invalid trait')):
# TODO(sbaker) remove when all trait validation is jsonschema based
error = exception.ClientSideError(
_('%(error_prefix)s. A valid trait must be no longer than 255 '
'characters. Standard traits are defined in the os_traits library. '
'A custom trait must start with the prefix CUSTOM_ and use '
'the following characters: A-Z, 0-9 and _') %
{'error_prefix': error_prefix})
if not isinstance(trait, str):
raise error
if len(trait) > 255 or len(trait) < 1:
raise error
if trait in STANDARD_TRAITS:
return
if CUSTOM_TRAIT_REGEX.match(trait) is None:
raise error
def apply_jsonpatch(doc, patch):
"""Apply a JSON patch, one operation at a time.

73
ironic/tests/unit/api/controllers/v1/test_node.py
View File

@ -32,7 +32,6 @@ from ironic.api.controllers.v1 import node as api_node
from ironic.api.controllers.v1 import notification_utils
from ironic.api.controllers.v1 import utils as api_utils
from ironic.api.controllers.v1 import versions
from ironic.api import types as atypes
from ironic.common import boot_devices
from ironic.common import components
from ironic.common import driver_factory
@ -57,15 +56,6 @@ with open(
NETWORK_DATA = json.load(fl)
class TestNodeObject(base.TestCase):
def test_node_init(self):
node_dict = test_api_utils.node_post_data()
del node_dict['instance_uuid']
node = api_node.Node(**node_dict)
self.assertEqual(atypes.Unset, node.instance_uuid)
class TestListNodes(test_api_base.BaseApiTest):
def setUp(self):
@ -1359,11 +1349,11 @@ class TestListNodes(test_api_base.BaseApiTest):
self.assertEqual(http_client.NOT_FOUND, response.status_int)
def test_ports_subresource_invalid_ident(self):
invalid_ident = '123~123'
invalid_ident = '123 123'
response = self.get_json('/nodes/%s/ports' % invalid_ident,
expect_errors=True)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
self.assertIn('Expected a logical name or UUID',
self.assertIn('Expected UUID or name for node',
response.json['error_message'])
def test_ports_subresource_via_portgroups_subres_not_allowed(self):
@ -2798,9 +2788,8 @@ class TestPatch(test_api_base.BaseApiTest):
node_dict = self.node.as_dict()
node_dict['conductor_group'] = 'NEW-GROUP'
node_obj = api_node.Node(**node_dict)
controller._update_changed_fields(node_obj, self.node)
controller._update_changed_fields(node_dict, self.node)
self.assertEqual('new-group', self.node.conductor_group)
@mock.patch("ironic.api.request")
@ -2810,9 +2799,8 @@ class TestPatch(test_api_base.BaseApiTest):
node_dict = self.node.as_dict()
del node_dict['chassis_id']
node_no_chassis = api_node.Node(**node_dict)
controller._update_changed_fields(node_no_chassis, self.node)
controller._update_changed_fields(node_dict, self.node)
self.assertIsNone(self.node.chassis_id)
def test_add_chassis_id(self):
@ -2876,7 +2864,7 @@ class TestPatch(test_api_base.BaseApiTest):
response = self.patch_json('/nodes/%s' % self.node.uuid,
[{'path': '/maintenance', 'op': 'replace',
'value': 'true'}])
'value': True}])
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
@ -2889,7 +2877,7 @@ class TestPatch(test_api_base.BaseApiTest):
response = self.patch_json(
'/nodes/%s' % self.node.name,
[{'path': '/maintenance', 'op': 'replace',
'value': 'true'}],
'value': True}],
headers={api_base.Version.string: "1.5"})
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
@ -3387,6 +3375,18 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
def test_update_protected_remove(self):
node = obj_utils.create_test_node(self.context,
uuid=uuidutils.generate_uuid(),
provision_state='active')
self.mock_update_node.return_value = node
headers = {api_base.Version.string: '1.48'}
response = self.patch_json('/nodes/%s' % node.uuid,
[{"op": "remove", "path": "/protected"}],
headers=headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
def test_update_protected_with_reason(self):
node = obj_utils.create_test_node(self.context,
uuid=uuidutils.generate_uuid(),
@ -3634,6 +3634,18 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
def test_update_retired_remove(self):
node = obj_utils.create_test_node(self.context,
uuid=uuidutils.generate_uuid(),
provision_state='active')
self.mock_update_node.return_value = node
headers = {api_base.Version.string: '1.61'}
response = self.patch_json('/nodes/%s' % node.uuid,
[{"op": "remove", "path": "/retired"}],
headers=headers)
self.assertEqual('application/json', response.content_type)
self.assertEqual(http_client.OK, response.status_code)
def test_update_retired_with_reason(self):
node = obj_utils.create_test_node(self.context,
uuid=uuidutils.generate_uuid(),
@ -4132,14 +4144,13 @@ class TestPost(test_api_base.BaseApiTest):
def test_create_node_valid_driver_info(self):
self._test_jsontype_attributes('driver_info')
def test_create_node_valid_instance_info(self):
self._test_jsontype_attributes('instance_info')
def _test_vendor_passthru_ok(self, mock_vendor, return_value=None,
is_async=True):
expected_status = http_client.ACCEPTED if is_async else http_client.OK
expected_return_value = json.dumps(return_value)
expected_return_value = expected_return_value.encode('utf-8')
if return_value is None:
expected_return_value = b''
else:
expected_return_value = json.dumps(return_value).encode('utf-8')
node = obj_utils.create_test_node(self.context)
info = {'foo': 'bar'}
@ -4156,8 +4167,10 @@ class TestPost(test_api_base.BaseApiTest):
def _test_vendor_passthru_ok_by_name(self, mock_vendor, return_value=None,
is_async=True):
expected_status = http_client.ACCEPTED if is_async else http_client.OK
expected_return_value = json.dumps(return_value)
expected_return_value = expected_return_value.encode('utf-8')
if return_value is None:
expected_return_value = b''
else:
expected_return_value = json.dumps(return_value).encode('utf-8')
node = obj_utils.create_test_node(self.context, name='node-109')
info = {'foo': 'bar'}
@ -4191,7 +4204,7 @@ class TestPost(test_api_base.BaseApiTest):
'/nodes/%s/vendor_passthru/do_test' % node.uuid,
{'test_key': 'test_value'})
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(return_value['return'], response.json)
self.assertEqual(b'', response.body)
@mock.patch.object(rpcapi.ConductorAPI, 'vendor_passthru')
def test_vendor_passthru_by_name(self, mock_vendor):
@ -4214,7 +4227,7 @@ class TestPost(test_api_base.BaseApiTest):
response = self.delete(
'/nodes/%s/vendor_passthru/do_test' % node.uuid)
self.assertEqual(http_client.ACCEPTED, response.status_int)
self.assertEqual(return_value['return'], response.json)
self.assertEqual(b'', response.body)
def test_vendor_passthru_no_such_method(self):
node = obj_utils.create_test_node(self.context)
@ -4245,7 +4258,7 @@ class TestPost(test_api_base.BaseApiTest):
pdict['node_uuid'] = node.uuid
response = self.post_json('/nodes/ports', pdict,
expect_errors=True)
self.assertEqual(http_client.BAD_REQUEST, response.status_int)
self.assertEqual(http_client.NOT_FOUND, response.status_int)
def test_post_ports_subresource(self):
node = obj_utils.create_test_node(self.context)
@ -4475,7 +4488,8 @@ class TestPost(test_api_base.BaseApiTest):
def test_create_node_protected_not_allowed(self):
headers = {api_base.Version.string: '1.48'}
ndict = test_api_utils.post_get_test_node(protected=True)
ndict = test_api_utils.post_get_test_node()
ndict['protected'] = True
response = self.post_json('/nodes', ndict, headers=headers,
expect_errors=True)
self.assertEqual('application/json', response.content_type)
@ -6038,6 +6052,7 @@ class TestAttachDetachVif(test_api_base.BaseApiTest):
@mock.patch.object(rpcapi.ConductorAPI, 'vif_list')
def test_vif_list(self, mock_list, mock_get):
mock_get.return_value = self.node
mock_list.return_value = []
self.get_json('/nodes/%s/vifs' % self.node.uuid,
headers={api_base.Version.string:
self.vif_version})

25
ironic/tests/unit/api/controllers/v1/test_types.py
View File

@ -386,28 +386,3 @@ class TestLocalLinkConnectionType(base.TestCase):
v = types.locallinkconnectiontype
value = {'network_type': 'invalid'}
self.assertRaises(exception.Invalid, v.validate, value)
@mock.patch("ironic.api.request", mock.Mock(version=mock.Mock(minor=10)))
class TestVifType(base.TestCase):
def test_vif_type(self):
v = types.viftype
value = {'id': 'foo'}
self.assertCountEqual(value, v.validate(value))
def test_vif_type_missing_mandatory_key(self):
v = types.viftype
value = {'foo': 'bar'}
self.assertRaisesRegex(exception.Invalid, 'Missing mandatory',
v.validate, value)
def test_vif_type_optional_key(self):
v = types.viftype
value = {'id': 'foo', 'misc': 'something'}
self.assertCountEqual(value, v.frombasetype(value))
def test_vif_type_bad_id(self):
v = types.viftype
self.assertRaises(exception.InvalidUuidOrName,
v.frombasetype, {'id': 5678})

52
ironic/tests/unit/api/controllers/v1/test_utils.py
View File

@ -19,7 +19,6 @@ from http import client as http_client
import io
from unittest import mock
import os_traits
from oslo_config import cfg
from oslo_utils import uuidutils
@ -27,7 +26,6 @@ from ironic import api
from ironic.api.controllers.v1 import node as api_node
from ironic.api.controllers.v1 import utils
from ironic.api import types as atypes
from ironic.common import args
from ironic.common import exception
from ironic.common import policy
from ironic.common import states
@ -64,56 +62,6 @@ class TestApiUtils(base.TestCase):
utils.validate_sort_dir,
'fake-sort')
def test_validate_trait(self):
utils.validate_trait(os_traits.HW_CPU_X86_AVX2)
utils.validate_trait("CUSTOM_1")
utils.validate_trait("CUSTOM_TRAIT_GOLD")
self.assertRaises(exception.ClientSideError,
utils.validate_trait, "A" * 256)
self.assertRaises(exception.ClientSideError,
utils.validate_trait, "CuSTOM_1")
self.assertRaises(exception.ClientSideError,
utils.validate_trait, "")
self.assertRaises(exception.ClientSideError,
utils.validate_trait, "CUSTOM_bob")
self.assertRaises(exception.ClientSideError,
utils.validate_trait, "CUSTOM_1-BOB")
self.assertRaises(exception.ClientSideError,
utils.validate_trait, "aCUSTOM_1a")
large = "CUSTOM_" + ("1" * 248)
self.assertEqual(255, len(large))
utils.validate_trait(large)
self.assertRaises(exception.ClientSideError,
utils.validate_trait, large + "1")
# Check custom error prefix.
self.assertRaisesRegex(exception.ClientSideError,
"spongebob",
utils.validate_trait, "invalid", "spongebob")
def test_validate_trait_jsonschema(self):
validate_trait = args.schema(utils.TRAITS_SCHEMA)
validate_trait('foo', os_traits.HW_CPU_X86_AVX2)
validate_trait('foo', "CUSTOM_1")
validate_trait('foo', "CUSTOM_TRAIT_GOLD")
self.assertRaises(exception.InvalidParameterValue,
validate_trait, 'foo', "A" * 256)
self.assertRaises(exception.InvalidParameterValue,
validate_trait, 'foo', "CuSTOM_1")
self.assertRaises(exception.InvalidParameterValue,
validate_trait, 'foo', "")
self.assertRaises(exception.InvalidParameterValue,
validate_trait, 'foo', "CUSTOM_bob")
self.assertRaises(exception.InvalidParameterValue,
validate_trait, 'foo', "CUSTOM_1-BOB")
self.assertRaises(exception.InvalidParameterValue,
validate_trait, 'foo', "aCUSTOM_1a")
large = "CUSTOM_" + ("1" * 248)
self.assertEqual(255, len(large))
validate_trait('foo', large)
self.assertRaises(exception.InvalidParameterValue,
validate_trait, 'foo', large + "1")
def test_apply_jsonpatch(self):
doc = {"foo": {"bar": "baz"}}
patch = [{"op": "add", "path": "/foo/answer", "value": 42}]

11
ironic/tests/unit/api/utils.py
View File

@ -100,13 +100,6 @@ def remove_other_fields(values, allowed_fields):
def node_post_data(**kw):
node = db_utils.get_test_node(**kw)
# These values are not part of the API object
node.pop('version')
node.pop('conductor_affinity')
node.pop('chassis_id')
node.pop('tags')
node.pop('traits')
node.pop('allocation_id')
# NOTE(jroll): pop out fields that were introduced in later API versions,
# unless explicitly requested. Otherwise, these will cause tests using
@ -115,8 +108,8 @@ def node_post_data(**kw):
if field not in kw:
node.pop(field, None)
internal = node_controller.NodePatchType.internal_attrs()
return remove_internal(node, internal)
return remove_other_fields(
node, node_controller.NODE_SCHEMA['properties'])
def port_post_data(**kw):