feat: sync Ironic portgroups as LAG interfaces to Nautobot via Oslo event handlers #1422
Conversation
haseebsyed12
force-pushed
the
ironic-portgroups-events
branch
from
a2ecff9 to
0cac23c
Compare
haseebsyed12
force-pushed
the
ironic-portgroups-events
branch
6 times, most recently
from
5fb1597 to
cc64582
Compare
cardoe
left a comment
cardoe
left a comment
There was a problem hiding this comment.
@stevekeay This will work with the rest of the code?
There was a problem hiding this comment.
---EDIT: after I wrote the below I see that Doug Specified exactly this behavior in PUC-917, so obviously disregard this:
While I definitely agree that we should have consistent names and other data in openstack, I am not feeling at all good about the functionality that is being added here.
These workflows/event handlers have a simple job to do: fix up data in Nautobot to look like a mirror of what is in OpenStack.
If there is "bad" data in OpenStack then it should be fixed in OpenStack, for sure, but not here!
At most, bad data in OpenStack should elicit a warning in the log. If the object can't be created in Nautobot due to validation rules or uniqueness or whatever then we must mangle the data or drop the record as required.
The behaviour proposed here is like "autocorrect" -- if an object is created in OpenStack with a bad name it will be automatically and silently changed. I can only think that this is going to confuse the heck out of our future selves.
If we absolutely need this behaviour (and I don't think we do) then it should not be a hidden side-effect of the mirroring workflow, it should be a separate thing.
| logger.info("Existing LAG interface found in Nautobot, updating") | ||
| lag_intf.name = event.lag_name # type: ignore | ||
| lag_intf.status = "Active" # type: ignore | ||
| lag_intf.type = "lag" # type: ignore | ||
|
|
||
| if event.address: | ||
| lag_intf.mac_address = event.address # type: ignore | ||
|
|
||
| if event.mode: | ||
| lag_intf.description = f"Bond mode: {event.mode}" # type: ignore |
There was a problem hiding this comment.
Could DRY this out, maybe extract a function and call it with the dict that you defined above (**attrs).
Also this has several race conditions, although I don't know if the nautobot API allows us to do very much about them. Will the workflow retry on failure?
There was a problem hiding this comment.
Adding idempotent create with exception handling. Shall I also add retry strategy to openstack-oslo-event.yaml workflow template ?
There was a problem hiding this comment.
I honestly don't know - I am not sure if our stack preserves the order of "events" at all, so there is a possibility that (for example) we will attempt to delete something that doesn't exist yet. I suspect that this scheme can result in drift between the two systems, and retries might help with that. Retries would also help with transient failures, e.g. a nautobot outage.
haseebsyed12
force-pushed
the
ironic-portgroups-events
branch
3 times, most recently
from
901eb13 to
f4a2a08
Compare
haseebsyed12
force-pushed
the
ironic-portgroups-events
branch
from
f4a2a08 to
85e1fef
Compare
haseebsyed12
changed the title
Implement automatic synchronization of Ironic portgroups to Nautobot
as LAG (Link Aggregation Group) interfaces via Oslo event handlers.