Add notifications for leases #134

DanNiESh · 2023-05-02T14:47:12Z

In progress

tzumainn · 2023-05-09T14:46:49Z

One question: is it possible for the Lease payload to include the Ironic Node payload? At the minimum, we probably want to have the node name and uuid and maybe the power state and provisioning state.

DanNiESh · 2023-05-09T14:54:13Z

he Ironic Node payload?

One question: is it possible for the Lease payload to include the Ironic Node payload? At the minimum, we probably want to have the node name and uuid and maybe the power state and provisioning state.

Yes, I can add them into lease payload.

tzumainn

Looks good! A few comments.

esi_leap/common/notification_utils.py

esi_leap/objects/lease.py

esi_leap/common/rpc.py

DanNiESh · 2023-05-30T20:08:21Z

I made changes to the code according to the comments. I will test it on demo server sometime tomorrow.

esi_leap/common/rpc.py

esi_leap/resource_objects/test_node.py

tzumainn

Hi! Some rather scattered comments, plus a bigger concern about the setting of resource information.

tzumainn · 2023-05-31T17:19:07Z

esi_leap/resource_objects/ironic_node.py

+        self._node_name = None
+        self._provision_state = None
+        self._power_state = None
+        self._node_properties = None


Ah - notifications won't work on dummy/fake nodes unless they have these values as well, correct? I'm not sure we want to set these values however; adding a comment above.

Yes. Notification only works on ironic nodes currently.

esi_leap/objects/lease.py

DanNiESh · 2023-06-03T00:17:01Z

I created a new class LeaseResourceEvent to pass in notification emit method. The new changes has been tested in demo server. A message json body looks like this:

{'message_id': 'f17a25ff-466e-45f7-9e59-5549ab3aa925', 'publisher_id': 'esi-leap-manager.esi-demo-ironic-controller.localdomain', 'event_type': 'esi_leap.LeaseResourceEvent.fulfill.end', 'priority': 'INFO', 'payload': {'esi_leap_object.name': 'LeaseCRUDPayload', 'esi_leap_object.namespace': 'esi_leap', 'esi_leap_object.version': '1.0', 'esi_leap_object.data': {'id': 113, 'name': None, 'uuid': '56a63bcd-43ac-440c-812d-e0f11166fc61', 'project_id': '218b3996b35b4e378f7701c78708d4d7', 'owner_id': '218b3996b35b4e378f7701c78708d4d7', 'resource_type': 'ironic_node', 'resource_uuid': 'f8b86846-86f2-4b63-95bc-fd3dfea93751', 'start_time': '2023-06-03T00:12:53Z', 'end_time': '9999-12-31T23:59:59Z', 'fulfill_time': None, 'expire_time': None, 'status': 'created', 'properties': {}, 'offer_uuid': None, 'parent_lease_uuid': None, 'node_name': 'dell-14', 'node_uuid': 'f8b86846-86f2-4b63-95bc-fd3dfea93751', 'node_provision_state': 'available', 'node_power_state': 'power off', 'node_properties': {'vendor': 'dell inc', 'local_gb': '557', 'cpus': '32', 'cpu_arch': 'x86_64', 'memory_mb': '393216', 'capabilities': 'cpu_vt:true,cpu_aes:true,cpu_hugepages:true,cpu_hugepages_1g:true,cpu_txt:true'}}}, 'timestamp': '2023-06-03 00:13:38.598030', '_unique_id': '1bdb824e578e4a1eb6c9e39bd6e424d7', '_context_user': None, '_context_project_id': None, '_context_system_scope': None, '_context_project': None, '_context_domain': None, '_context_user_domain': None, '_context_project_domain': None, '_context_is_admin': False, '_context_read_only': False, '_context_show_deleted': False, '_context_auth_token': None, '_context_request_id': 'req-f9524840-9c06-42cd-bb43-f434d2ba067d', '_context_global_request_id': None, '_context_resource_uuid': None, '_context_roles': [], '_context_user_identity': '- - - - -', '_context_is_admin_project': True}

tzumainn · 2023-06-05T13:42:01Z

esi_leap/objects/lease_resource_event.py

+
+
+@versioned_objects_base.VersionedObjectRegistry.register
+class LeaseResourceEvent(base.ESILEAPObject):


I took a look at the Ironic code, and I'm pretty sure that what's done here can simply be done in LeaseCRUDPayload (which makes sense to me because otherwise that class isn't used all that much). Take a look at the Payloads in ironic/objects/node.py; they set values similar to what you do here. If you agree, then you can just move the logic in this class into LeaseCRUDPayload, and instantiate Payloads instead.

New changes updated.

add unit tests Move emit notification to objects layer add lease_resource_event object

DanNiESh · 2023-06-08T16:48:43Z

Update: tested on demo server. The notification message is

{'message_id': '74f335a7-221b-46e1-b8f5-08b65dd548fa', 'publisher_id': 'esi-leap-manager.esi-demo-ironic-controller.localdomain', 'event_type': 'esi_leap.lease.fulfill.end', 'priority': 'INFO', 'payload': {'esi_leap_object.name': 'LeaseCRUDPayload', 'esi_leap_object.namespace': 'esi_leap', 'esi_leap_object.version': '1.0', 'esi_leap_object.data': {'id': 114, 'name': None, 'uuid': '412b071a-b37b-4a00-a0b4-ed90643b2d51', 'project_id': '218b3996b35b4e378f7701c78708d4d7', 'owner_id': '218b3996b35b4e378f7701c78708d4d7', 'resource_type': 'ironic_node', 'resource_uuid': 'f8b86846-86f2-4b63-95bc-fd3dfea93751', 'start_time': '2023-06-08T16:42:05Z', 'end_time': '9999-12-31T23:59:59Z', 'fulfill_time': '2023-06-08T16:43:02Z', 'expire_time': None, 'status': 'active', 'properties': {}, 'offer_uuid': None, 'parent_lease_uuid': None, 'node_name': 'dell-14', 'node_uuid': 'f8b86846-86f2-4b63-95bc-fd3dfea93751', 'node_provision_state': 'available', 'node_power_state': 'power on', 'node_properties': {'vendor': 'dell inc', 'local_gb': '557', 'cpus': '32', 'cpu_arch': 'x86_64', 'memory_mb': '393216', 'capabilities': 'cpu_vt:true,cpu_aes:true,cpu_hugepages:true,cpu_hugepages_1g:true,cpu_txt:true'}}}, 'timestamp': '2023-06-08 16:43:02.094990', '_unique_id': '3c3b9b5b725a4b99b7b27147d472cd10', '_context_user': None, '_context_project_id': None, '_context_system_scope': None, '_context_project': None, '_context_domain': None, '_context_user_domain': None, '_context_project_domain': None, '_context_is_admin': False, '_context_read_only': False, '_context_show_deleted': False, '_context_auth_token': None, '_context_request_id': 'req-f90b9c92-5955-47a0-a365-87fe33532457', '_context_global_request_id': None, '_context_resource_uuid': None, '_context_roles': [], '_context_user_identity': '- - - - -', '_context_is_admin_project': True}

tzumainn · 2023-06-08T16:55:05Z

Looks good - thanks!

tzumainn changed the title ~~Add notifications for leases~~ [WIP] Add notifications for leases May 2, 2023

DanNiESh force-pushed the events branch 5 times, most recently from ae7a636 to 360ffdc Compare May 23, 2023 02:01

DanNiESh force-pushed the events branch 5 times, most recently from 3e7a470 to e8246e0 Compare May 24, 2023 19:17

DanNiESh changed the title ~~[WIP] Add notifications for leases~~ Add notifications for leases May 24, 2023

DanNiESh requested a review from tzumainn May 24, 2023 19:24

tzumainn requested changes May 25, 2023

View reviewed changes

DanNiESh mentioned this pull request May 26, 2023

Add events to esi-leap CCI-MOC/esi#399

Closed

DanNiESh force-pushed the events branch from e8246e0 to 8feeac1 Compare May 30, 2023 19:47

tzumainn reviewed May 31, 2023

View reviewed changes

esi_leap/common/rpc.py Outdated Show resolved Hide resolved

tzumainn reviewed May 31, 2023

View reviewed changes

esi_leap/common/rpc.py Outdated Show resolved Hide resolved

tzumainn reviewed May 31, 2023

View reviewed changes

esi_leap/resource_objects/test_node.py Outdated Show resolved Hide resolved

tzumainn reviewed May 31, 2023

View reviewed changes

esi_leap/resource_objects/test_node.py Show resolved Hide resolved

tzumainn requested changes May 31, 2023

View reviewed changes

DanNiESh force-pushed the events branch 3 times, most recently from c391c15 to 8753b62 Compare June 3, 2023 00:06

tzumainn requested changes Jun 5, 2023

View reviewed changes

Add notifications for leases

f555930

add unit tests Move emit notification to objects layer add lease_resource_event object

DanNiESh force-pushed the events branch from 8753b62 to f555930 Compare June 5, 2023 18:20

tzumainn approved these changes Jun 8, 2023

View reviewed changes

tzumainn merged commit fda372a into CCI-MOC:master Jun 8, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add notifications for leases #134

Add notifications for leases #134

DanNiESh commented May 2, 2023

tzumainn commented May 9, 2023

DanNiESh commented May 9, 2023

tzumainn left a comment

DanNiESh commented May 30, 2023

tzumainn left a comment

tzumainn May 31, 2023

DanNiESh May 31, 2023

DanNiESh commented Jun 3, 2023

tzumainn Jun 5, 2023

DanNiESh Jun 5, 2023

DanNiESh commented Jun 8, 2023

tzumainn commented Jun 8, 2023



		@versioned_objects_base.VersionedObjectRegistry.register
		class LeaseResourceEvent(base.ESILEAPObject):

Add notifications for leases #134

Add notifications for leases #134

Conversation

DanNiESh commented May 2, 2023

tzumainn commented May 9, 2023

DanNiESh commented May 9, 2023

tzumainn left a comment

Choose a reason for hiding this comment

DanNiESh commented May 30, 2023

tzumainn left a comment

Choose a reason for hiding this comment

tzumainn May 31, 2023

Choose a reason for hiding this comment

DanNiESh May 31, 2023

Choose a reason for hiding this comment

DanNiESh commented Jun 3, 2023

tzumainn Jun 5, 2023

Choose a reason for hiding this comment

DanNiESh Jun 5, 2023

Choose a reason for hiding this comment

DanNiESh commented Jun 8, 2023

tzumainn commented Jun 8, 2023