openstack
/
ironic
Code Issues Proposed changes

Merge "Convert portgroups endpoint to plain JSON"

This commit is contained in:
Zuul 2020-11-18 19:24:07 +00:00 committed by Gerrit Code Review
parent 2b96ef429d 989064a742
commit 6b5222c466
4 changed files with 169 additions and 275 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

424
ironic/api/controllers/v1/portgroup.py
View File

@ -10,7 +10,7 @@
# License for the specific language governing permissions and limitations
# under the License.
import datetime
import copy
from http import client as http_client
from ironic_lib import metrics_utils
@ -18,15 +18,13 @@ from oslo_utils import uuidutils
import pecan
from ironic import api
from ironic.api.controllers import base
from ironic.api.controllers import link
from ironic.api.controllers.v1 import collection
from ironic.api.controllers.v1 import notification_utils as notify
from ironic.api.controllers.v1 import port
from ironic.api.controllers.v1 import types
from ironic.api.controllers.v1 import utils as api_utils
from ironic.api import expose
from ironic.api import types as atypes
from ironic.api import method
from ironic.common import args
from ironic.common import exception
from ironic.common.i18n import _
from ironic.common import policy
@ -35,217 +33,103 @@ from ironic import objects
METRICS = metrics_utils.get_metrics_logger(__name__)
_DEFAULT_RETURN_FIELDS = ('uuid', 'address', 'name')
_DEFAULT_RETURN_FIELDS = ['uuid', 'address', 'name']
PORTGROUP_SCHEMA = {
'type': 'object',
'properties': {
'address': {'type': ['string', 'null']},
'extra': {'type': ['object', 'null']},
'mode': {'type': ['string', 'null']},
'name': {'type': ['string', 'null']},
'node_uuid': {'type': 'string'},
'properties': {'type': ['object', 'null']},
'standalone_ports_supported': {'type': ['string', 'boolean', 'null']},
'uuid': {'type': ['string', 'null']},
},
'required': ['node_uuid'],
'additionalProperties': False,
}
PORTGROUP_PATCH_SCHEMA = copy.deepcopy(PORTGROUP_SCHEMA)
# patch supports patching some internal_info values
PORTGROUP_PATCH_SCHEMA['properties']['internal_info'] = {
'type': ['null', 'object']}
PORTGROUP_VALIDATOR_EXTRA = args.dict_valid(
address=args.mac_address,
node_uuid=args.uuid,
standalone_ports_supported=args.boolean,
uuid=args.uuid
)
PORTGROUP_VALIDATOR = args.and_valid(
args.schema(PORTGROUP_SCHEMA),
PORTGROUP_VALIDATOR_EXTRA
)
PORTGROUP_PATCH_VALIDATOR = args.and_valid(
args.schema(PORTGROUP_PATCH_SCHEMA),
PORTGROUP_VALIDATOR_EXTRA
)
PATCH_ALLOWED_FIELDS = [
'address',
'extra',
'mode',
'name',
'node_uuid',
'properties',
'standalone_ports_supported'
]
class Portgroup(base.APIBase):
"""API representation of a portgroup.
def convert_with_links(rpc_portgroup, fields=None, sanitize=True):
"""Add links to the portgroup."""
portgroup = api_utils.object_to_dict(
rpc_portgroup,
link_resource='portgroups',
fields=(
'address',
'extra',
'internal_info',
'mode',
'name',
'properties',
'standalone_ports_supported'
)
)
api_utils.populate_node_uuid(rpc_portgroup, portgroup)
url = api.request.public_url
portgroup['ports'] = [
link.make_link('self', url, 'portgroups',
rpc_portgroup.uuid + "/ports"),
link.make_link('bookmark', url, 'portgroups',
rpc_portgroup.uuid + "/ports", bookmark=True)
]
This class enforces type checking and value constraints, and converts
between the internal object model and the API representation of a
portgroup.
"""
if fields is not None:
api_utils.check_for_invalid_fields(fields, portgroup)
_node_uuid = None
def _get_node_uuid(self):
return self._node_uuid
def _set_node_uuid(self, value):
if value and self._node_uuid != value:
if not api_utils.allow_portgroups():
self._node_uuid = atypes.Unset
return
try:
node = objects.Node.get(api.request.context, value)
self._node_uuid = node.uuid
# NOTE: Create the node_id attribute on-the-fly
# to satisfy the api -> rpc object
# conversion.
self.node_id = node.id
except exception.NodeNotFound as e:
# Change error code because 404 (NotFound) is inappropriate
# response for a POST request to create a Portgroup
e.code = http_client.BAD_REQUEST
raise e
elif value == atypes.Unset:
self._node_uuid = atypes.Unset
uuid = types.uuid
"""Unique UUID for this portgroup"""
address = atypes.wsattr(types.macaddress)
"""MAC Address for this portgroup"""
extra = {str: types.jsontype}
"""This portgroup's meta data"""
internal_info = atypes.wsattr({str: types.jsontype}, readonly=True)
"""This portgroup's internal info"""
node_uuid = atypes.wsproperty(types.uuid, _get_node_uuid, _set_node_uuid,
mandatory=True)
"""The UUID of the node this portgroup belongs to"""
name = atypes.wsattr(str)
"""The logical name for this portgroup"""
links = None
"""A list containing a self link and associated portgroup links"""
standalone_ports_supported = types.boolean
"""Indicates whether ports of this portgroup may be used as
single NIC ports"""
mode = atypes.wsattr(str)
"""The mode for this portgroup. See linux bonding
documentation for details:
https://www.kernel.org/doc/Documentation/networking/bonding.txt"""
properties = {str: types.jsontype}
"""This portgroup's properties"""
ports = None
"""Links to the collection of ports of this portgroup"""
def __init__(self, **kwargs):
self.fields = []
fields = list(objects.Portgroup.fields)
# NOTE: node_uuid is not part of objects.Portgroup.fields
# because it's an API-only attribute
fields.append('node_uuid')
for field in fields:
# Skip fields we do not expose.
if not hasattr(self, field):
continue
self.fields.append(field)
setattr(self, field, kwargs.get(field, atypes.Unset))
# NOTE: node_id is an attribute created on-the-fly
# by _set_node_uuid(), it needs to be present in the fields so
# that as_dict() will contain node_id field when converting it
# before saving it in the database.
self.fields.append('node_id')
setattr(self, 'node_uuid', kwargs.get('node_id', atypes.Unset))
@staticmethod
def _convert_with_links(portgroup, url, fields=None):
"""Add links to the portgroup."""
if fields is None:
portgroup.ports = [
link.make_link('self', url, 'portgroups',
portgroup.uuid + "/ports"),
link.make_link('bookmark', url, 'portgroups',
portgroup.uuid + "/ports", bookmark=True)
]
# never expose the node_id attribute
portgroup.node_id = atypes.Unset
portgroup.links = [link.make_link('self', url,
'portgroups', portgroup.uuid),
link.make_link('bookmark', url,
'portgroups', portgroup.uuid,
bookmark=True)
]
if not sanitize:
return portgroup
@classmethod
def convert_with_links(cls, rpc_portgroup, fields=None, sanitize=True):
"""Add links to the portgroup."""
portgroup = Portgroup(**rpc_portgroup.as_dict())
api_utils.sanitize_dict(portgroup, fields)
if fields is not None:
api_utils.check_for_invalid_fields(fields, portgroup.as_dict())
portgroup = cls._convert_with_links(portgroup, api.request.host_url,
fields=fields)
if not sanitize:
return portgroup
portgroup.sanitize(fields)
return portgroup
def sanitize(self, fields=None):
"""Removes sensitive and unrequested data.
Will only keep the fields specified in the ``fields`` parameter.
:param fields:
list of fields to preserve, or ``None`` to preserve them all
:type fields: list of str
"""
if fields is not None:
self.unset_fields_except(fields)
# never expose the node_id attribute
self.node_id = atypes.Unset
@classmethod
def sample(cls, expand=True):
"""Return a sample of the portgroup."""
sample = cls(uuid='a594544a-2daf-420c-8775-17a8c3e0852f',
address='fe:54:00:77:07:d9',
name='node1-portgroup-01',
extra={'foo': 'bar'},
internal_info={'baz': 'boo'},
standalone_ports_supported=True,
mode='active-backup',
properties={},
created_at=datetime.datetime(2000, 1, 1, 12, 0, 0),
updated_at=datetime.datetime(2000, 1, 1, 12, 0, 0))
# NOTE(lucasagomes): node_uuid getter() method look at the
# _node_uuid variable
sample._node_uuid = '7ae81bb3-dec3-4289-8d6c-da80bd8001ae'
fields = None if expand else _DEFAULT_RETURN_FIELDS
return cls._convert_with_links(sample, 'http://localhost:6385',
fields=fields)
return portgroup
class PortgroupPatchType(types.JsonPatchType):
_api_base = Portgroup
_extra_non_removable_attrs = {'/mode'}
@staticmethod
def internal_attrs():
defaults = types.JsonPatchType.internal_attrs()
return defaults + ['/internal_info']
class PortgroupCollection(collection.Collection):
"""API representation of a collection of portgroups."""
portgroups = [Portgroup]
"""A list containing portgroup objects"""
def __init__(self, **kwargs):
self._type = 'portgroups'
@staticmethod
def convert_with_links(rpc_portgroups, limit, url=None, fields=None,
**kwargs):
collection = PortgroupCollection()
collection.portgroups = [Portgroup.convert_with_links(p, fields=fields,
sanitize=False)
for p in rpc_portgroups]
collection.next = collection.get_next(limit, url=url, fields=fields,
**kwargs)
for item in collection.portgroups:
item.sanitize(fields=fields)
return collection
@classmethod
def sample(cls):
"""Return a sample of the portgroup."""
sample = cls()
sample.portgroups = [Portgroup.sample(expand=False)]
return sample
def list_convert_with_links(rpc_portgroups, limit, url=None, fields=None,
**kwargs):
return collection.list_convert_with_links(
items=[convert_with_links(p, fields=fields, sanitize=False)
for p in rpc_portgroups],
item_name='portgroups',
limit=limit,
url=url,
fields=fields,
sanitize_func=api_utils.sanitize_dict,
**kwargs
)
class PortgroupsController(pecan.rest.RestController):
@ -266,8 +150,8 @@ class PortgroupsController(pecan.rest.RestController):
if not api_utils.allow_portgroups():
pecan.abort(http_client.NOT_FOUND)
try:
ident = types.uuid_or_name.validate(ident)
except exception.InvalidUuidOrName as e:
ident = args.uuid_or_name('portgroup', ident)
except exception.InvalidParameterValue as e:
pecan.abort(http_client.BAD_REQUEST, e.args[0])
if not remainder:
return
@ -333,12 +217,12 @@ class PortgroupsController(pecan.rest.RestController):
if detail is not None:
parameters['detail'] = detail
return PortgroupCollection.convert_with_links(portgroups, limit,
url=resource_url,
fields=fields,
sort_key=sort_key,
sort_dir=sort_dir,
**parameters)
return list_convert_with_links(portgroups, limit,
url=resource_url,
fields=fields,
sort_key=sort_key,
sort_dir=sort_dir,
**parameters)
def _get_portgroups_by_address(self, address):
"""Retrieve a portgroup by its address.
@ -357,9 +241,11 @@ class PortgroupsController(pecan.rest.RestController):
return []
@METRICS.timer('PortgroupsController.get_all')
@expose.expose(PortgroupCollection, types.uuid_or_name, types.macaddress,
types.uuid, int, str, str, types.listtype,
types.boolean)
@method.expose()
@args.validate(node=args.uuid_or_name, address=args.mac_address,
marker=args.uuid, limit=args.integer, sort_key=args.string,
sort_dir=args.string, fields=args.string_list,
detail=args.boolean)
def get_all(self, node=None, address=None, marker=None,
limit=None, sort_key='id', sort_dir='asc', fields=None,
detail=None):
@ -398,8 +284,10 @@ class PortgroupsController(pecan.rest.RestController):
detail=detail)
@METRICS.timer('PortgroupsController.detail')
@expose.expose(PortgroupCollection, types.uuid_or_name, types.macaddress,
types.uuid, int, str, str)
@method.expose()
@args.validate(node=args.uuid_or_name, address=args.mac_address,
marker=args.uuid, limit=args.integer, sort_key=args.string,
sort_dir=args.string)
def detail(self, node=None, address=None, marker=None,
limit=None, sort_key='id', sort_dir='asc'):
"""Retrieve a list of portgroups with detail.
@ -434,7 +322,8 @@ class PortgroupsController(pecan.rest.RestController):
resource_url=resource_url)
@METRICS.timer('PortgroupsController.get_one')
@expose.expose(Portgroup, types.uuid_or_name, types.listtype)
@method.expose()
@args.validate(portgroup_ident=args.uuid_or_name, fields=args.string_list)
def get_one(self, portgroup_ident, fields=None):
"""Retrieve information about the given portgroup.
@ -455,10 +344,12 @@ class PortgroupsController(pecan.rest.RestController):
rpc_portgroup = api_utils.get_rpc_portgroup_with_suffix(
portgroup_ident)
return Portgroup.convert_with_links(rpc_portgroup, fields=fields)
return convert_with_links(rpc_portgroup, fields=fields)
@METRICS.timer('PortgroupsController.post')
@expose.expose(Portgroup, body=Portgroup, status_code=http_client.CREATED)
@method.expose(status_code=http_client.CREATED)
@method.body('portgroup')
@args.validate(portgroup=PORTGROUP_VALIDATOR)
def post(self, portgroup):
"""Create a new portgroup.
@ -475,43 +366,43 @@ class PortgroupsController(pecan.rest.RestController):
raise exception.OperationNotPermitted()
if (not api_utils.allow_portgroup_mode_properties()
and (portgroup.mode is not atypes.Unset
or portgroup.properties is not atypes.Unset)):
and (portgroup.get('mode') or portgroup.get('properties'))):
raise exception.NotAcceptable()
if (portgroup.name
and not api_utils.is_valid_logical_name(portgroup.name)):
if (portgroup.get('name')
and not api_utils.is_valid_logical_name(portgroup['name'])):
error_msg = _("Cannot create portgroup with invalid name "
"'%(name)s'") % {'name': portgroup.name}
"'%(name)s'") % {'name': portgroup['name']}
raise exception.ClientSideError(
error_msg, status_code=http_client.BAD_REQUEST)
pg_dict = portgroup.as_dict()
api_utils.handle_post_port_like_extra_vif(pg_dict)
api_utils.handle_post_port_like_extra_vif(portgroup)
# NOTE(yuriyz): UUID is mandatory for notifications payload
if not pg_dict.get('uuid'):
pg_dict['uuid'] = uuidutils.generate_uuid()
if not portgroup.get('uuid'):
portgroup['uuid'] = uuidutils.generate_uuid()
new_portgroup = objects.Portgroup(context, **pg_dict)
node = api_utils.replace_node_uuid_with_id(portgroup)
new_portgroup = objects.Portgroup(context, **portgroup)
notify.emit_start_notification(context, new_portgroup, 'create',
node_uuid=portgroup.node_uuid)
node_uuid=node.uuid)
with notify.handle_error_notification(context, new_portgroup, 'create',
node_uuid=portgroup.node_uuid):
node_uuid=node.uuid):
new_portgroup.create()
notify.emit_end_notification(context, new_portgroup, 'create',
node_uuid=portgroup.node_uuid)
node_uuid=node.uuid)
# Set the HTTP Location Header
api.response.location = link.build_url('portgroups',
new_portgroup.uuid)
return Portgroup.convert_with_links(new_portgroup)
return convert_with_links(new_portgroup)
@METRICS.timer('PortgroupsController.patch')
@expose.validate(types.uuid_or_name, [PortgroupPatchType])
@expose.expose(Portgroup, types.uuid_or_name, body=[PortgroupPatchType])
@method.expose()
@method.body('patch')
@args.validate(portgroup_ident=args.uuid_or_name, patch=args.patch)
def patch(self, portgroup_ident, patch):
"""Update an existing portgroup.
@ -533,6 +424,8 @@ class PortgroupsController(pecan.rest.RestController):
or api_utils.is_path_updated(patch, '/properties'))):
raise exception.NotAcceptable()
api_utils.patch_validate_allowed_fields(patch, PATCH_ALLOWED_FIELDS)
rpc_portgroup = api_utils.get_rpc_portgroup_with_suffix(
portgroup_ident)
@ -547,30 +440,41 @@ class PortgroupsController(pecan.rest.RestController):
error_msg, status_code=http_client.BAD_REQUEST)
portgroup_dict = rpc_portgroup.as_dict()
# NOTE:
# 1) Remove node_id because it's an internal value and
# not present in the API object
# 2) Add node_uuid
portgroup_dict['node_uuid'] = portgroup_dict.pop('node_id', None)
portgroup = Portgroup(**api_utils.apply_jsonpatch(portgroup_dict,
patch))
rpc_node = api_utils.replace_node_id_with_uuid(portgroup_dict)
portgroup_dict = api_utils.apply_jsonpatch(portgroup_dict, patch)
if 'mode' not in portgroup_dict:
msg = _("'mode' is a mandatory attribute and can not be removed")
raise exception.ClientSideError(msg)
try:
if portgroup_dict['node_uuid'] != rpc_node.uuid:
rpc_node = objects.Node.get(api.request.context,
portgroup_dict['node_uuid'])
except exception.NodeNotFound as e:
# Change error code because 404 (NotFound) is inappropriate
# response for a POST request to patch a Portgroup
e.code = http_client.BAD_REQUEST # BadRequest
raise
api_utils.handle_patch_port_like_extra_vif(
rpc_portgroup, portgroup.internal_info, patch)
rpc_portgroup, portgroup_dict.get('internal_info'), patch)
# Update only the fields that have changed
for field in objects.Portgroup.fields:
try:
patch_val = getattr(portgroup, field)
except AttributeError:
# Ignore fields that aren't exposed in the API
continue
if patch_val == atypes.Unset:
patch_val = None
if rpc_portgroup[field] != patch_val:
rpc_portgroup[field] = patch_val
api_utils.patched_validate_with_schema(
portgroup_dict, PORTGROUP_PATCH_SCHEMA, PORTGROUP_PATCH_VALIDATOR)
api_utils.patch_update_changed_fields(
portgroup_dict, rpc_portgroup, fields=objects.Portgroup.fields,
schema=PORTGROUP_PATCH_SCHEMA, id_map={'node_id': rpc_node.id}
)
rpc_node = objects.Node.get_by_id(context, rpc_portgroup.node_id)
if (rpc_node.provision_state == ir_states.INSPECTING
and api_utils.allow_inspect_wait_state()):
msg = _('Cannot update portgroup "%(portgroup)s" on node '
@ -588,15 +492,15 @@ class PortgroupsController(pecan.rest.RestController):
new_portgroup = api.request.rpcapi.update_portgroup(
context, rpc_portgroup, topic)
api_portgroup = Portgroup.convert_with_links(new_portgroup)
api_portgroup = convert_with_links(new_portgroup)
notify.emit_end_notification(context, new_portgroup, 'update',
node_uuid=api_portgroup.node_uuid)
node_uuid=rpc_node.uuid)
return api_portgroup
@METRICS.timer('PortgroupsController.delete')
@expose.expose(None, types.uuid_or_name,
status_code=http_client.NO_CONTENT)
@method.expose(status_code=http_client.NO_CONTENT)
@args.validate(portgroup_ident=args.uuid_or_name)
def delete(self, portgroup_ident):
"""Delete a portgroup.

2
ironic/tests/unit/api/controllers/v1/test_port.py
View File

@ -1112,7 +1112,7 @@ class TestListPorts(test_api_base.BaseApiTest):
headers={api_base.Version.string: '1.24'},
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 portgroup',
response.json['error_message'])

14
ironic/tests/unit/api/controllers/v1/test_portgroup.py
View File

@ -26,16 +26,13 @@ from testtools.matchers import HasLength
from ironic.api.controllers import base as api_base
from ironic.api.controllers import v1 as api_v1
from ironic.api.controllers.v1 import notification_utils
from ironic.api.controllers.v1 import portgroup as api_portgroup
from ironic.api.controllers.v1 import utils as api_utils
from ironic.api import types as atypes
from ironic.common import exception
from ironic.common import states
from ironic.common import utils as common_utils
from ironic.conductor import rpcapi
from ironic import objects
from ironic.objects import fields as obj_fields
from ironic.tests import base
from ironic.tests.unit.api import base as test_api_base
from ironic.tests.unit.api import utils as apiutils
from ironic.tests.unit.objects import utils as obj_utils
@ -51,15 +48,6 @@ def _rpcapi_update_portgroup(self, context, portgroup, topic):
return portgroup
class TestPortgroupObject(base.TestCase):
def test_portgroup_init(self):
portgroup_dict = apiutils.portgroup_post_data(node_id=None)
del portgroup_dict['extra']
portgroup = api_portgroup.Portgroup(**portgroup_dict)
self.assertEqual(atypes.Unset, portgroup.extra)
class TestListPortgroups(test_api_base.BaseApiTest):
headers = {api_base.Version.string: str(api_v1.max_version())}
@ -1007,6 +995,7 @@ class TestPatch(test_api_base.BaseApiTest):
self.assertFalse(mock_upd.called)
def test_update_portgroup_internal_info_not_allowed(self, mock_upd):
mock_upd.return_value = self.portgroup
response = self.patch_json('/portgroups/%s' % self.portgroup.uuid,
[{'path': '/internal_info',
'value': False,
@ -1052,6 +1041,7 @@ class TestPatch(test_api_base.BaseApiTest):
[{'path': '/properties/abc', 'op': 'add', 'value': 123}], mock_upd)
def test_remove_mode_not_allowed(self, mock_upd):
mock_upd.return_value = self.portgroup
response = self.patch_json('/portgroups/%s' % self.portgroup.uuid,
[{'path': '/mode',
'op': 'remove'}],

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

@ -167,8 +167,8 @@ def portgroup_post_data(**kw):
if arg not in kw:
portgroup.pop(arg)
internal = portgroup_controller.PortgroupPatchType.internal_attrs()
return remove_internal(portgroup, internal)
return remove_other_fields(
portgroup, portgroup_controller.PORTGROUP_SCHEMA['properties'])
def post_get_test_portgroup(**kw):