From f9d9b334dab309156eed75ce93693152018746bd Mon Sep 17 00:00:00 2001 From: Yuriy Zveryanskyy Date: Tue, 31 Jan 2017 18:05:04 +0200 Subject: [PATCH] Add ironic port group CRUD notifications This patch adds notifications for create, update or delete port groups. Event types are: baremetal.portgroup.{create, update, delete}.{start,end,error}. Developer documentation updated. "portgroup_uuid" field added to port payload. Closes-Bug: #1660292 Change-Id: I9a8ce6c34e9c704b1aeeb526babcb20a5b1261db --- doc/source/deploy/notifications.rst | 40 +++++++++- .../api/controllers/v1/notification_utils.py | 5 +- ironic/api/controllers/v1/port.py | 31 ++++--- ironic/api/controllers/v1/portgroup.py | 60 ++++++++++---- ironic/objects/port.py | 10 ++- ironic/objects/portgroup.py | 49 ++++++++++++ .../unit/api/v1/test_notification_utils.py | 36 ++++++++- ironic/tests/unit/api/v1/test_portgroups.py | 80 +++++++++++++++++-- ironic/tests/unit/api/v1/test_ports.py | 36 ++++++--- ironic/tests/unit/objects/test_chassis.py | 21 +---- ironic/tests/unit/objects/test_node.py | 21 +---- ironic/tests/unit/objects/test_objects.py | 6 +- ironic/tests/unit/objects/test_port.py | 21 +---- ironic/tests/unit/objects/test_portgroup.py | 7 +- ironic/tests/unit/objects/utils.py | 24 ++++++ ...p-crud-notifications-91204635528972b2.yaml | 10 +++ 16 files changed, 347 insertions(+), 110 deletions(-) create mode 100644 releasenotes/notes/portgroup-crud-notifications-91204635528972b2.yaml diff --git a/doc/source/deploy/notifications.rst b/doc/source/deploy/notifications.rst index 0fd705f172..b75705dc2c 100644 --- a/doc/source/deploy/notifications.rst +++ b/doc/source/deploy/notifications.rst @@ -197,13 +197,14 @@ Example of port CRUD notification:: "payload":{ "ironic_object.namespace":"ironic", "ironic_object.name":"PortCRUDPayload", - "ironic_object.version":"1.0", + "ironic_object.version":"1.1", "ironic_object.data":{ "address": "77:66:23:34:11:b7", "created_at": "2016-02-11T15:23:03+00:00", "node_uuid": "5b236cab-ad4e-4220-b57c-e827e858745a", "extra": {}, "local_link_connection": {}, + "portgroup_uuid": "bd2f385e-c51c-4752-82d1-7a9ec2c25f24", "pxe_enabled": True, "updated_at": "2016-03-27T20:41:03+00:00", "uuid": "1be26c0b-03f2-4d2e-ae87-c02d7f33c123" @@ -213,6 +214,43 @@ Example of port CRUD notification:: "publisher_id":"ironic-api.hostname02" } +List of CRUD notifications for port group: + +* ``baremetal.portgroup.create.start`` +* ``baremetal.portgroup.create.end`` +* ``baremetal.portgroup.create.error`` +* ``baremetal.portgroup.update.start`` +* ``baremetal.portgroup.update.end`` +* ``baremetal.portgroup.update.error`` +* ``baremetal.portgroup.delete.start`` +* ``baremetal.portgroup.delete.end`` +* ``baremetal.portgroup.delete.error`` + +Example of portgroup CRUD notification:: + + { + "priority": "info", + "payload":{ + "ironic_object.namespace":"ironic", + "ironic_object.name":"PortgroupCRUDPayload", + "ironic_object.version":"1.0", + "ironic_object.data":{ + "address": "11:44:32:87:61:e5", + "created_at": "2017-01-11T11:33:03+00:00", + "node_uuid": "5b236cab-ad4e-4220-b57c-e827e858745a", + "extra": {}, + "mode": "7", + "name": "portgroup-node-18", + "properties": {}, + "standalone_ports_supported": True, + "updated_at": "2017-01-31T11:41:07+00:00", + "uuid": "db033a40-bfed-4c84-815a-3db26bb268bb", + } + }, + "event_type":"baremetal.portgroup.update.end", + "publisher_id":"ironic-api.hostname02" + } + Node maintenance notifications ------------------------------ diff --git a/ironic/api/controllers/v1/notification_utils.py b/ironic/api/controllers/v1/notification_utils.py index b5db6545f7..8f058aa378 100644 --- a/ironic/api/controllers/v1/notification_utils.py +++ b/ironic/api/controllers/v1/notification_utils.py @@ -26,6 +26,7 @@ from ironic.objects import fields from ironic.objects import node as node_objects from ironic.objects import notification from ironic.objects import port as port_objects +from ironic.objects import portgroup as portgroup_objects LOG = log.getLogger(__name__) CONF = cfg.CONF @@ -37,7 +38,9 @@ CRUD_NOTIFY_OBJ = { 'node': (node_objects.NodeCRUDNotification, node_objects.NodeCRUDPayload), 'port': (port_objects.PortCRUDNotification, - port_objects.PortCRUDPayload) + port_objects.PortCRUDPayload), + 'portgroup': (portgroup_objects.PortgroupCRUDNotification, + portgroup_objects.PortgroupCRUDPayload) } diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index a7af9a8c0f..0a25fb2f5c 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -533,13 +533,15 @@ class PortsController(rest.RestController): new_port = objects.Port(context, **pdict) + notify_extra = {'node_uuid': port.node_uuid, + 'portgroup_uuid': port.portgroup_uuid} notify.emit_start_notification(context, new_port, 'create', - node_uuid=port.node_uuid) + **notify_extra) with notify.handle_error_notification(context, new_port, 'create', - node_uuid=port.node_uuid): + **notify_extra): new_port.create() notify.emit_end_notification(context, new_port, 'create', - node_uuid=port.node_uuid) + **notify_extra) # Set the HTTP Location Header pecan.response.location = link.build_url('ports', new_port.uuid) return Port.convert_with_links(new_port) @@ -607,17 +609,19 @@ class PortsController(rest.RestController): rpc_port[field] = patch_val rpc_node = objects.Node.get_by_id(context, rpc_port.node_id) + notify_extra = {'node_uuid': rpc_node.uuid, + 'portgroup_uuid': port.portgroup_uuid} notify.emit_start_notification(context, rpc_port, 'update', - node_uuid=rpc_node.uuid) + **notify_extra) with notify.handle_error_notification(context, rpc_port, 'update', - node_uuid=rpc_node.uuid): + **notify_extra): topic = pecan.request.rpcapi.get_topic_for(rpc_node) new_port = pecan.request.rpcapi.update_port(context, rpc_port, topic) api_port = Port.convert_with_links(new_port) notify.emit_end_notification(context, new_port, 'update', - node_uuid=api_port.node_uuid) + **notify_extra) return api_port @@ -638,11 +642,20 @@ class PortsController(rest.RestController): rpc_port = objects.Port.get_by_uuid(context, port_uuid) rpc_node = objects.Node.get_by_id(context, rpc_port.node_id) + + portgroup_uuid = None + if rpc_port.portgroup_id: + portgroup = objects.Portgroup.get_by_id(context, + rpc_port.portgroup_id) + portgroup_uuid = portgroup.uuid + + notify_extra = {'node_uuid': rpc_node.uuid, + 'portgroup_uuid': portgroup_uuid} notify.emit_start_notification(context, rpc_port, 'delete', - node_uuid=rpc_node.uuid) + **notify_extra) with notify.handle_error_notification(context, rpc_port, 'delete', - node_uuid=rpc_node.uuid): + **notify_extra): topic = pecan.request.rpcapi.get_topic_for(rpc_node) pecan.request.rpcapi.destroy_port(context, rpc_port, topic) notify.emit_end_notification(context, rpc_port, 'delete', - node_uuid=rpc_node.uuid) + **notify_extra) diff --git a/ironic/api/controllers/v1/portgroup.py b/ironic/api/controllers/v1/portgroup.py index 2ac6baabe0..fb72a91c17 100644 --- a/ironic/api/controllers/v1/portgroup.py +++ b/ironic/api/controllers/v1/portgroup.py @@ -13,6 +13,7 @@ import datetime from ironic_lib import metrics_utils +from oslo_utils import uuidutils import pecan from six.moves import http_client import wsme @@ -21,6 +22,7 @@ from wsme import types as wtypes 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 @@ -429,7 +431,8 @@ class PortgroupsController(pecan.rest.RestController): if not api_utils.allow_portgroups(): raise exception.NotFound() - cdict = pecan.request.context.to_policy_values() + context = pecan.request.context + cdict = context.to_policy_values() policy.authorize('baremetal:portgroup:create', cdict, cdict) if self.parent_node_ident: @@ -452,9 +455,20 @@ class PortgroupsController(pecan.rest.RestController): if vif: common_utils.warn_about_deprecated_extra_vif_port_id() - new_portgroup = objects.Portgroup(pecan.request.context, - **pg_dict) - new_portgroup.create() + # NOTE(yuriyz): UUID is mandatory for notifications payload + if not pg_dict.get('uuid'): + pg_dict['uuid'] = uuidutils.generate_uuid() + + new_portgroup = objects.Portgroup(context, **pg_dict) + + notify.emit_start_notification(context, new_portgroup, 'create', + node_uuid=portgroup.node_uuid) + with notify.handle_error_notification(context, new_portgroup, 'create', + node_uuid=portgroup.node_uuid): + new_portgroup.create() + notify.emit_end_notification(context, new_portgroup, 'create', + node_uuid=portgroup.node_uuid) + # Set the HTTP Location Header pecan.response.location = link.build_url('portgroups', new_portgroup.uuid) @@ -472,7 +486,8 @@ class PortgroupsController(pecan.rest.RestController): if not api_utils.allow_portgroups(): raise exception.NotFound() - cdict = pecan.request.context.to_policy_values() + context = pecan.request.context + cdict = context.to_policy_values() policy.authorize('baremetal:portgroup:update', cdict, cdict) if self.parent_node_ident: @@ -520,14 +535,21 @@ class PortgroupsController(pecan.rest.RestController): if rpc_portgroup[field] != patch_val: rpc_portgroup[field] = patch_val - rpc_node = objects.Node.get_by_id(pecan.request.context, - rpc_portgroup.node_id) - topic = pecan.request.rpcapi.get_topic_for(rpc_node) + rpc_node = objects.Node.get_by_id(context, rpc_portgroup.node_id) - new_portgroup = pecan.request.rpcapi.update_portgroup( - pecan.request.context, rpc_portgroup, topic) + notify.emit_start_notification(context, rpc_portgroup, 'update', + node_uuid=rpc_node.uuid) + with notify.handle_error_notification(context, rpc_portgroup, 'update', + node_uuid=rpc_node.uuid): + topic = pecan.request.rpcapi.get_topic_for(rpc_node) + new_portgroup = pecan.request.rpcapi.update_portgroup( + context, rpc_portgroup, topic) - return Portgroup.convert_with_links(new_portgroup) + api_portgroup = Portgroup.convert_with_links(new_portgroup) + notify.emit_end_notification(context, new_portgroup, 'update', + node_uuid=api_portgroup.node_uuid) + + return api_portgroup @METRICS.timer('PortgroupsController.delete') @expose.expose(None, types.uuid_or_name, @@ -540,7 +562,8 @@ class PortgroupsController(pecan.rest.RestController): if not api_utils.allow_portgroups(): raise exception.NotFound() - cdict = pecan.request.context.to_policy_values() + context = pecan.request.context + cdict = context.to_policy_values() policy.authorize('baremetal:portgroup:delete', cdict, cdict) if self.parent_node_ident: @@ -549,6 +572,13 @@ class PortgroupsController(pecan.rest.RestController): rpc_portgroup = api_utils.get_rpc_portgroup(portgroup_ident) rpc_node = objects.Node.get_by_id(pecan.request.context, rpc_portgroup.node_id) - topic = pecan.request.rpcapi.get_topic_for(rpc_node) - pecan.request.rpcapi.destroy_portgroup(pecan.request.context, - rpc_portgroup, topic) + + notify.emit_start_notification(context, rpc_portgroup, 'delete', + node_uuid=rpc_node.uuid) + with notify.handle_error_notification(context, rpc_portgroup, 'delete', + node_uuid=rpc_node.uuid): + topic = pecan.request.rpcapi.get_topic_for(rpc_node) + pecan.request.rpcapi.destroy_portgroup(context, rpc_portgroup, + topic) + notify.emit_end_notification(context, rpc_portgroup, 'delete', + node_uuid=rpc_node.uuid) diff --git a/ironic/objects/port.py b/ironic/objects/port.py index a1fb0b2599..b8ca40149d 100644 --- a/ironic/objects/port.py +++ b/ironic/objects/port.py @@ -307,7 +307,8 @@ class PortCRUDNotification(notification.NotificationBase): @base.IronicObjectRegistry.register class PortCRUDPayload(notification.NotificationPayloadBase): # Version 1.0: Initial version - VERSION = '1.0' + # Version 1.1: Add "portgroup_uuid" field + VERSION = '1.1' SCHEMA = { 'address': ('port', 'address'), @@ -326,12 +327,13 @@ class PortCRUDPayload(notification.NotificationPayloadBase): nullable=True), 'pxe_enabled': object_fields.BooleanField(nullable=True), 'node_uuid': object_fields.UUIDField(), + 'portgroup_uuid': object_fields.UUIDField(nullable=True), 'created_at': object_fields.DateTimeField(nullable=True), 'updated_at': object_fields.DateTimeField(nullable=True), 'uuid': object_fields.UUIDField() - # TODO(yuriyz): add "portgroup_uuid" field with portgroup notifications } - def __init__(self, port, node_uuid): - super(PortCRUDPayload, self).__init__(node_uuid=node_uuid) + def __init__(self, port, node_uuid, portgroup_uuid): + super(PortCRUDPayload, self).__init__(node_uuid=node_uuid, + portgroup_uuid=portgroup_uuid) self.populate_schema(port=port) diff --git a/ironic/objects/portgroup.py b/ironic/objects/portgroup.py index 34e6f4b60b..164e8ccab6 100644 --- a/ironic/objects/portgroup.py +++ b/ironic/objects/portgroup.py @@ -23,6 +23,7 @@ from ironic.common import utils from ironic.db import api as dbapi from ironic.objects import base from ironic.objects import fields as object_fields +from ironic.objects import notification @base.IronicObjectRegistry.register @@ -280,3 +281,51 @@ class Portgroup(base.IronicObject, object_base.VersionedObjectDictCompat): current = self.get_by_uuid(self._context, uuid=self.uuid) self.obj_refresh(current) self.obj_reset_changes() + + +@base.IronicObjectRegistry.register +class PortgroupCRUDNotification(notification.NotificationBase): + """Notification when ironic creates, updates or deletes a portgroup.""" + # Version 1.0: Initial version + VERSION = '1.0' + + fields = { + 'payload': object_fields.ObjectField('PortgroupCRUDPayload') + } + + +@base.IronicObjectRegistry.register +class PortgroupCRUDPayload(notification.NotificationPayloadBase): + # Version 1.0: Initial version + VERSION = '1.0' + + SCHEMA = { + 'address': ('portgroup', 'address'), + 'extra': ('portgroup', 'extra'), + 'mode': ('portgroup', 'mode'), + 'name': ('portgroup', 'name'), + 'properties': ('portgroup', 'properties'), + 'standalone_ports_supported': ('portgroup', + 'standalone_ports_supported'), + 'created_at': ('portgroup', 'created_at'), + 'updated_at': ('portgroup', 'updated_at'), + 'uuid': ('portgroup', 'uuid') + } + + fields = { + 'address': object_fields.MACAddressField(nullable=True), + 'extra': object_fields.FlexibleDictField(nullable=True), + 'mode': object_fields.StringField(nullable=True), + 'name': object_fields.StringField(nullable=True), + 'node_uuid': object_fields.UUIDField(), + 'properties': object_fields.FlexibleDictField(nullable=True), + 'standalone_ports_supported': object_fields.BooleanField( + nullable=True), + 'created_at': object_fields.DateTimeField(nullable=True), + 'updated_at': object_fields.DateTimeField(nullable=True), + 'uuid': object_fields.UUIDField() + } + + def __init__(self, portgroup, node_uuid): + super(PortgroupCRUDPayload, self).__init__(node_uuid=node_uuid) + self.populate_schema(portgroup=portgroup) diff --git a/ironic/tests/unit/api/v1/test_notification_utils.py b/ironic/tests/unit/api/v1/test_notification_utils.py index 0300e66389..a3172070b0 100644 --- a/ironic/tests/unit/api/v1/test_notification_utils.py +++ b/ironic/tests/unit/api/v1/test_notification_utils.py @@ -30,16 +30,20 @@ class APINotifyTestCase(tests_base.TestCase): self.node_notify_mock = mock.Mock() self.port_notify_mock = mock.Mock() self.chassis_notify_mock = mock.Mock() + self.portgroup_notify_mock = mock.Mock() self.node_notify_mock.__name__ = 'NodeCRUDNotification' self.port_notify_mock.__name__ = 'PortCRUDNotification' self.chassis_notify_mock.__name__ = 'ChassisCRUDNotification' + self.portgroup_notify_mock.__name__ = 'PortgroupCRUDNotification' _notification_mocks = { 'chassis': (self.chassis_notify_mock, notif_utils.CRUD_NOTIFY_OBJ['chassis'][1]), 'node': (self.node_notify_mock, notif_utils.CRUD_NOTIFY_OBJ['node'][1]), 'port': (self.port_notify_mock, - notif_utils.CRUD_NOTIFY_OBJ['port'][1]) + notif_utils.CRUD_NOTIFY_OBJ['port'][1]), + 'portgroup': (self.portgroup_notify_mock, + notif_utils.CRUD_NOTIFY_OBJ['portgroup'][1]) } self.addCleanup(self._restore, notif_utils.CRUD_NOTIFY_OBJ.copy()) notif_utils.CRUD_NOTIFY_OBJ = _notification_mocks @@ -121,6 +125,7 @@ class APINotifyTestCase(tests_base.TestCase): def test_port_notification(self): node_uuid = uuidutils.generate_uuid() + portgroup_uuid = uuidutils.generate_uuid() port = obj_utils.get_test_port(self.context, address='11:22:33:77:88:99', local_link_connection={'a': 25}, @@ -130,18 +135,45 @@ class APINotifyTestCase(tests_base.TestCase): test_status = fields.NotificationStatus.SUCCESS notif_utils._emit_api_notification(self.context, port, 'create', test_level, test_status, - node_uuid=node_uuid) + node_uuid=node_uuid, + portgroup_uuid=portgroup_uuid) init_kwargs = self.port_notify_mock.call_args[1] payload = init_kwargs['payload'] event_type = init_kwargs['event_type'] self.assertEqual('port', event_type.object) self.assertEqual(port.uuid, payload.uuid) self.assertEqual(node_uuid, payload.node_uuid) + self.assertEqual(portgroup_uuid, payload.portgroup_uuid) self.assertEqual('11:22:33:77:88:99', payload.address) self.assertEqual({'a': 25}, payload.local_link_connection) self.assertEqual({'as': 34}, payload.extra) self.assertEqual(False, payload.pxe_enabled) + def test_portgroup_notification(self): + node_uuid = uuidutils.generate_uuid() + portgroup = obj_utils.get_test_portgroup(self.context, + address='22:55:88:AA:BB:99', + name='new01', + mode='mode2', + extra={'bs': 11}) + test_level = fields.NotificationLevel.INFO + test_status = fields.NotificationStatus.SUCCESS + notif_utils._emit_api_notification(self.context, portgroup, 'create', + test_level, test_status, + node_uuid=node_uuid) + init_kwargs = self.portgroup_notify_mock.call_args[1] + payload = init_kwargs['payload'] + event_type = init_kwargs['event_type'] + self.assertEqual('portgroup', event_type.object) + self.assertEqual(portgroup.uuid, payload.uuid) + self.assertEqual(node_uuid, payload.node_uuid) + self.assertEqual(portgroup.address, payload.address) + self.assertEqual(portgroup.name, payload.name) + self.assertEqual(portgroup.mode, payload.mode) + self.assertEqual(portgroup.extra, payload.extra) + self.assertEqual(portgroup.standalone_ports_supported, + payload.standalone_ports_supported) + @mock.patch('ironic.objects.node.NodeMaintenanceNotification') def test_node_maintenance_notification(self, maintenance_mock): maintenance_mock.__name__ = 'NodeMaintenanceNotification' diff --git a/ironic/tests/unit/api/v1/test_portgroups.py b/ironic/tests/unit/api/v1/test_portgroups.py index 45920f10e9..d2901823bc 100644 --- a/ironic/tests/unit/api/v1/test_portgroups.py +++ b/ironic/tests/unit/api/v1/test_portgroups.py @@ -27,11 +27,14 @@ from wsme import types as wtypes 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.common import exception 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 @@ -443,7 +446,8 @@ class TestPatch(test_api_base.BaseApiTest): self.mock_gtf.return_value = 'test-topic' self.addCleanup(p.stop) - def test_update_byid(self, mock_upd): + @mock.patch.object(notification_utils, '_emit_api_notification') + def test_update_byid(self, mock_notify, mock_upd): extra = {'foo': 'bar'} mock_upd.return_value = self.portgroup mock_upd.return_value.extra = extra @@ -458,6 +462,14 @@ class TestPatch(test_api_base.BaseApiTest): kargs = mock_upd.call_args[0][1] self.assertEqual(extra, kargs.extra) + mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update', + obj_fields.NotificationLevel.INFO, + obj_fields.NotificationStatus.START, + node_uuid=self.node.uuid), + mock.call(mock.ANY, mock.ANY, 'update', + obj_fields.NotificationLevel.INFO, + obj_fields.NotificationStatus.END, + node_uuid=self.node.uuid)]) def test_update_byname(self, mock_upd): extra = {'foo': 'bar'} @@ -540,7 +552,8 @@ class TestPatch(test_api_base.BaseApiTest): kargs = mock_upd.call_args[0][1] self.assertEqual(address, kargs.address) - def test_replace_address_already_exist(self, mock_upd): + @mock.patch.object(notification_utils, '_emit_api_notification') + def test_replace_address_already_exist(self, mock_notify, mock_upd): address = 'aa:aa:aa:aa:aa:aa' mock_upd.side_effect = exception.MACAlreadyExists(mac=address) response = self.patch_json('/portgroups/%s' % self.portgroup.uuid, @@ -556,6 +569,14 @@ class TestPatch(test_api_base.BaseApiTest): kargs = mock_upd.call_args[0][1] self.assertEqual(address, kargs.address) + mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update', + obj_fields.NotificationLevel.INFO, + obj_fields.NotificationStatus.START, + node_uuid=self.node.uuid), + mock.call(mock.ANY, mock.ANY, 'update', + obj_fields.NotificationLevel.ERROR, + obj_fields.NotificationStatus.ERROR, + node_uuid=self.node.uuid)]) def test_replace_node_uuid(self, mock_upd): mock_upd.return_value = self.portgroup @@ -876,10 +897,11 @@ class TestPost(test_api_base.BaseApiTest): super(TestPost, self).setUp() self.node = obj_utils.create_test_node(self.context) + @mock.patch.object(notification_utils, '_emit_api_notification') @mock.patch.object(common_utils, 'warn_about_deprecated_extra_vif_port_id', autospec=True) @mock.patch.object(timeutils, 'utcnow', autospec=True) - def test_create_portgroup(self, mock_utcnow, mock_warn): + def test_create_portgroup(self, mock_utcnow, mock_warn, mock_notify): pdict = apiutils.post_get_test_portgroup() test_time = datetime.datetime(2000, 1, 1, 0, 0) mock_utcnow.return_value = test_time @@ -899,6 +921,14 @@ class TestPost(test_api_base.BaseApiTest): self.assertEqual(urlparse.urlparse(response.location).path, expected_location) self.assertEqual(0, mock_warn.call_count) + mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'create', + obj_fields.NotificationLevel.INFO, + obj_fields.NotificationStatus.START, + node_uuid=self.node.uuid), + mock.call(mock.ANY, mock.ANY, 'create', + obj_fields.NotificationLevel.INFO, + obj_fields.NotificationStatus.END, + node_uuid=self.node.uuid)]) @mock.patch.object(timeutils, 'utcnow', autospec=True) def test_create_portgroup_v123(self, mock_utcnow): @@ -942,7 +972,9 @@ class TestPost(test_api_base.BaseApiTest): # Check that 'id' is not in first arg of positional args self.assertNotIn('id', cp_mock.call_args[0][0]) - def test_create_portgroup_generate_uuid(self): + @mock.patch.object(notification_utils.LOG, 'exception', autospec=True) + @mock.patch.object(notification_utils.LOG, 'warning', autospec=True) + def test_create_portgroup_generate_uuid(self, mock_warn, mock_except): pdict = apiutils.post_get_test_portgroup() del pdict['uuid'] response = self.post_json('/portgroups', pdict, headers=self.headers) @@ -950,6 +982,24 @@ class TestPost(test_api_base.BaseApiTest): headers=self.headers) self.assertEqual(pdict['address'], result['address']) self.assertTrue(uuidutils.is_uuid_like(result['uuid'])) + self.assertFalse(mock_warn.called) + self.assertFalse(mock_except.called) + + @mock.patch.object(notification_utils, '_emit_api_notification') + @mock.patch.object(objects.Portgroup, 'create') + def test_create_portgroup_error(self, mock_create, mock_notify): + mock_create.side_effect = Exception() + pdict = apiutils.post_get_test_portgroup() + self.post_json('/portgroups', pdict, headers=self.headers, + expect_errors=True) + mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'create', + obj_fields.NotificationLevel.INFO, + obj_fields.NotificationStatus.START, + node_uuid=self.node.uuid), + mock.call(mock.ANY, mock.ANY, 'create', + obj_fields.NotificationLevel.ERROR, + obj_fields.NotificationStatus.ERROR, + node_uuid=self.node.uuid)]) def test_create_portgroup_valid_extra(self): pdict = apiutils.post_get_test_portgroup( @@ -1137,12 +1187,22 @@ class TestDelete(test_api_base.BaseApiTest): self.assertEqual('application/json', response.content_type) self.assertIn(self.portgroup.address, response.json['error_message']) - def test_delete_portgroup_byid(self, mock_dpt): + @mock.patch.object(notification_utils, '_emit_api_notification') + def test_delete_portgroup_byid(self, mock_notify, mock_dpt): self.delete('/portgroups/%s' % self.portgroup.uuid, headers=self.headers) self.assertTrue(mock_dpt.called) + mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'delete', + obj_fields.NotificationLevel.INFO, + obj_fields.NotificationStatus.START, + node_uuid=self.node.uuid), + mock.call(mock.ANY, mock.ANY, 'delete', + obj_fields.NotificationLevel.INFO, + obj_fields.NotificationStatus.END, + node_uuid=self.node.uuid)]) - def test_delete_portgroup_node_locked(self, mock_dpt): + @mock.patch.object(notification_utils, '_emit_api_notification') + def test_delete_portgroup_node_locked(self, mock_notify, mock_dpt): self.node.reserve(self.context, 'fake', self.node.uuid) mock_dpt.side_effect = exception.NodeLocked(node='fake-node', host='fake-host') @@ -1151,6 +1211,14 @@ class TestDelete(test_api_base.BaseApiTest): self.assertEqual(http_client.CONFLICT, ret.status_code) self.assertTrue(ret.json['error_message']) self.assertTrue(mock_dpt.called) + mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'delete', + obj_fields.NotificationLevel.INFO, + obj_fields.NotificationStatus.START, + node_uuid=self.node.uuid), + mock.call(mock.ANY, mock.ANY, 'delete', + obj_fields.NotificationLevel.ERROR, + obj_fields.NotificationStatus.ERROR, + node_uuid=self.node.uuid)]) def test_delete_portgroup_invalid_api_version(self, mock_dpt): response = self.delete('/portgroups/%s' % self.portgroup.uuid, diff --git a/ironic/tests/unit/api/v1/test_ports.py b/ironic/tests/unit/api/v1/test_ports.py index b6f1bc36e0..00a3c459e4 100644 --- a/ironic/tests/unit/api/v1/test_ports.py +++ b/ironic/tests/unit/api/v1/test_ports.py @@ -489,11 +489,13 @@ class TestPatch(test_api_base.BaseApiTest): mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.START, - node_uuid=self.node.uuid), + node_uuid=self.node.uuid, + portgroup_uuid=wtypes.Unset), mock.call(mock.ANY, mock.ANY, 'update', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.END, - node_uuid=self.node.uuid)]) + node_uuid=self.node.uuid, + portgroup_uuid=wtypes.Unset)]) def test_update_byaddress_not_allowed(self, mock_upd): extra = {'foo': 'bar'} @@ -556,11 +558,13 @@ class TestPatch(test_api_base.BaseApiTest): mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'update', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.START, - node_uuid=self.node.uuid), + node_uuid=self.node.uuid, + portgroup_uuid=wtypes.Unset), mock.call(mock.ANY, mock.ANY, 'update', obj_fields.NotificationLevel.ERROR, obj_fields.NotificationStatus.ERROR, - node_uuid=self.node.uuid)]) + node_uuid=self.node.uuid, + portgroup_uuid=wtypes.Unset)]) def test_replace_node_uuid(self, mock_upd): mock_upd.return_value = self.port @@ -982,11 +986,13 @@ class TestPost(test_api_base.BaseApiTest): mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'create', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.START, - node_uuid=self.node.uuid), + node_uuid=self.node.uuid, + portgroup_uuid=self.portgroup.uuid), mock.call(mock.ANY, mock.ANY, 'create', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.END, - node_uuid=self.node.uuid)]) + node_uuid=self.node.uuid, + portgroup_uuid=self.portgroup.uuid)]) self.assertEqual(0, mock_warn.call_count) def test_create_port_min_api_version(self): @@ -1036,11 +1042,13 @@ class TestPost(test_api_base.BaseApiTest): mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'create', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.START, - node_uuid=self.node.uuid), + node_uuid=self.node.uuid, + portgroup_uuid=self.portgroup.uuid), mock.call(mock.ANY, mock.ANY, 'create', obj_fields.NotificationLevel.ERROR, obj_fields.NotificationStatus.ERROR, - node_uuid=self.node.uuid)]) + node_uuid=self.node.uuid, + portgroup_uuid=self.portgroup.uuid)]) def test_create_port_valid_extra(self): pdict = post_get_test_port(extra={'str': 'foo', 'int': 123, @@ -1396,11 +1404,13 @@ class TestDelete(test_api_base.BaseApiTest): mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'delete', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.START, - node_uuid=self.node.uuid), + node_uuid=self.node.uuid, + portgroup_uuid=None), mock.call(mock.ANY, mock.ANY, 'delete', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.END, - node_uuid=self.node.uuid)]) + node_uuid=self.node.uuid, + portgroup_uuid=None)]) @mock.patch.object(notification_utils, '_emit_api_notification') def test_delete_port_node_locked(self, mock_notify, mock_dpt): @@ -1414,11 +1424,13 @@ class TestDelete(test_api_base.BaseApiTest): mock_notify.assert_has_calls([mock.call(mock.ANY, mock.ANY, 'delete', obj_fields.NotificationLevel.INFO, obj_fields.NotificationStatus.START, - node_uuid=self.node.uuid), + node_uuid=self.node.uuid, + portgroup_uuid=None), mock.call(mock.ANY, mock.ANY, 'delete', obj_fields.NotificationLevel.ERROR, obj_fields.NotificationStatus.ERROR, - node_uuid=self.node.uuid)]) + node_uuid=self.node.uuid, + portgroup_uuid=None)]) def test_portgroups_subresource_delete(self, mock_dpt): portgroup = obj_utils.create_test_portgroup(self.context, diff --git a/ironic/tests/unit/objects/test_chassis.py b/ironic/tests/unit/objects/test_chassis.py index 199f92a632..00a20bbe84 100644 --- a/ironic/tests/unit/objects/test_chassis.py +++ b/ironic/tests/unit/objects/test_chassis.py @@ -25,7 +25,7 @@ from ironic.tests.unit.db import utils from ironic.tests.unit.objects import utils as obj_utils -class TestChassisObject(base.DbTestCase): +class TestChassisObject(base.DbTestCase, obj_utils.SchemasTestMixIn): def setUp(self): super(TestChassisObject, self).setUp() @@ -121,21 +121,4 @@ class TestChassisObject(base.DbTestCase): self.assertEqual(self.context, chassis[0]._context) def test_payload_schemas(self): - """Assert that the chassis' Payload SCHEMAs have the expected properties. - - A payload's SCHEMA should: - - 1. Have each of its keys in the payload's fields - 2. Have each member of the schema match with a corresponding field - in the Chassis object - """ - payloads = obj_utils.get_payloads_with_schemas(objects.chassis) - for payload in payloads: - for schema_key in payload.SCHEMA: - self.assertIn(schema_key, payload.fields, - "for %s, schema key %s is not in fields" - % (payload, schema_key)) - chassis_key = payload.SCHEMA[schema_key][1] - self.assertIn(chassis_key, objects.Chassis.fields, - "for %s, schema key %s has invalid chassis " - "field %s" % (payload, schema_key, chassis_key)) + self._check_payload_schemas(objects.chassis, objects.Chassis.fields) diff --git a/ironic/tests/unit/objects/test_node.py b/ironic/tests/unit/objects/test_node.py index 0a0dc09242..1c08a9c189 100644 --- a/ironic/tests/unit/objects/test_node.py +++ b/ironic/tests/unit/objects/test_node.py @@ -25,7 +25,7 @@ from ironic.tests.unit.db import utils from ironic.tests.unit.objects import utils as obj_utils -class TestNodeObject(base.DbTestCase): +class TestNodeObject(base.DbTestCase, obj_utils.SchemasTestMixIn): def setUp(self): super(TestNodeObject, self).setUp() @@ -244,21 +244,4 @@ class TestNodeObject(base.DbTestCase): self.assertEqual(expect, values['properties']) def test_payload_schemas(self): - """Assert that the node's Payload SCHEMAs have the expected properties. - - A payload's SCHEMA should: - - 1. Have each of its keys in the payload's fields - 2. Have each member of the schema match with a corresponding field - in the Node object - """ - payloads = obj_utils.get_payloads_with_schemas(objects.node) - for payload in payloads: - for schema_key in payload.SCHEMA: - self.assertIn(schema_key, payload.fields, - "for %s, schema key %s is not in fields" - % (payload, schema_key)) - node_key = payload.SCHEMA[schema_key][1] - self.assertIn(node_key, objects.Node.fields, - "for %s, schema key %s has invalid node field %s" - % (payload, schema_key, node_key)) + self._check_payload_schemas(objects.node, objects.Node.fields) diff --git a/ironic/tests/unit/objects/test_objects.py b/ironic/tests/unit/objects/test_objects.py index af5edcde62..b16075156f 100644 --- a/ironic/tests/unit/objects/test_objects.py +++ b/ironic/tests/unit/objects/test_objects.py @@ -428,9 +428,11 @@ expected_object_fingerprints = { 'NodeCRUDNotification': '1.0-59acc533c11d306f149846f922739c15', 'NodeCRUDPayload': '1.1-35c16dd49d75812763e4e99bfebc3191', 'PortCRUDNotification': '1.0-59acc533c11d306f149846f922739c15', - 'PortCRUDPayload': '1.0-88acd98c9b08b4c8810e77793152057b', + 'PortCRUDPayload': '1.1-1ecf2d63b68014c52cb52d0227f8b5b8', 'NodeMaintenanceNotification': '1.0-59acc533c11d306f149846f922739c15', - 'NodeConsoleNotification': '1.0-59acc533c11d306f149846f922739c15' + 'NodeConsoleNotification': '1.0-59acc533c11d306f149846f922739c15', + 'PortgroupCRUDNotification': '1.0-59acc533c11d306f149846f922739c15', + 'PortgroupCRUDPayload': '1.0-b73c1fecf0cef3aa56bbe3c7e2275018', } diff --git a/ironic/tests/unit/objects/test_port.py b/ironic/tests/unit/objects/test_port.py index de5adb1627..a049a9a224 100644 --- a/ironic/tests/unit/objects/test_port.py +++ b/ironic/tests/unit/objects/test_port.py @@ -24,7 +24,7 @@ from ironic.tests.unit.db import utils from ironic.tests.unit.objects import utils as obj_utils -class TestPortObject(base.DbTestCase): +class TestPortObject(base.DbTestCase, obj_utils.SchemasTestMixIn): def setUp(self): super(TestPortObject, self).setUp() @@ -129,21 +129,4 @@ class TestPortObject(base.DbTestCase): self.assertEqual(self.context, ports[0]._context) def test_payload_schemas(self): - """Assert that the port's Payload SCHEMAs have the expected properties. - - A payload's SCHEMA should: - - 1. Have each of its keys in the payload's fields - 2. Have each member of the schema match with a corresponding field - in the Port object - """ - payloads = obj_utils.get_payloads_with_schemas(objects.port) - for payload in payloads: - for schema_key in payload.SCHEMA: - self.assertIn(schema_key, payload.fields, - "for %s, schema key %s is not in fields" - % (payload, schema_key)) - port_key = payload.SCHEMA[schema_key][1] - self.assertIn(port_key, objects.Port.fields, - "for %s, schema key %s has invalid port field %s" - % (payload, schema_key, port_key)) + self._check_payload_schemas(objects.port, objects.Port.fields) diff --git a/ironic/tests/unit/objects/test_portgroup.py b/ironic/tests/unit/objects/test_portgroup.py index b1b65e7aba..a2e747a868 100644 --- a/ironic/tests/unit/objects/test_portgroup.py +++ b/ironic/tests/unit/objects/test_portgroup.py @@ -18,9 +18,10 @@ from ironic.common import exception from ironic import objects from ironic.tests.unit.db import base from ironic.tests.unit.db import utils +from ironic.tests.unit.objects import utils as obj_utils -class TestPortgroupObject(base.DbTestCase): +class TestPortgroupObject(base.DbTestCase, obj_utils.SchemasTestMixIn): def setUp(self): super(TestPortgroupObject, self).setUp() @@ -147,3 +148,7 @@ class TestPortgroupObject(base.DbTestCase): self.assertThat(portgroups, matchers.HasLength(1)) self.assertIsInstance(portgroups[0], objects.Portgroup) self.assertEqual(self.context, portgroups[0]._context) + + def test_payload_schemas(self): + self._check_payload_schemas(objects.portgroup, + objects.Portgroup.fields) diff --git a/ironic/tests/unit/objects/utils.py b/ironic/tests/unit/objects/utils.py index 46c7db4b92..745b682261 100644 --- a/ironic/tests/unit/objects/utils.py +++ b/ironic/tests/unit/objects/utils.py @@ -245,3 +245,27 @@ def get_payloads_with_schemas(from_module): payloads.append(payload) return payloads + + +class SchemasTestMixIn(object): + def _check_payload_schemas(self, from_module, fields): + """Assert that the Payload SCHEMAs have the expected properties. + + A payload's SCHEMA should: + + 1. Have each of its keys in the payload's fields + 2. Have each member of the schema match with a corresponding field + in the object + """ + resource = from_module.__name__.split('.')[-1] + payloads = get_payloads_with_schemas(from_module) + for payload in payloads: + for schema_key in payload.SCHEMA: + self.assertIn(schema_key, payload.fields, + "for %s, schema key %s is not in fields" + % (payload, schema_key)) + key = payload.SCHEMA[schema_key][1] + self.assertIn(key, fields, + "for %s, schema key %s has invalid %s " + "field %s" % (payload, schema_key, resource, + key)) diff --git a/releasenotes/notes/portgroup-crud-notifications-91204635528972b2.yaml b/releasenotes/notes/portgroup-crud-notifications-91204635528972b2.yaml new file mode 100644 index 0000000000..9fb4a39c7e --- /dev/null +++ b/releasenotes/notes/portgroup-crud-notifications-91204635528972b2.yaml @@ -0,0 +1,10 @@ +--- +features: + - | + Adds notifications for creation, updates, or deletions of port groups. + Event types are formatted as follows: + + * baremetal.portgroup.{create, update, delete}.{start,end,error} + + Also adds portgroup_uuid field to port notifications, port payload version + bumped to 1.1.