feat: workflow updates Nautobot Interface.status instead of device.custom_field.conected_to_network #249

stevekeay · 2024-08-30T23:11:52Z

We used to flip a bit on the nautobot Device object for “connected_to_network”
when changing a device from/to “provisioning” and “tenant” settings.

We need this to be interface specific. Because of Ironic's knowlege of the
interface that is being used for PXE boot, we are called with a server interface
UUID.

We update the nautobot status field of connected switch interface.

This tells undersync which mode the port should be in - provisioning or
tenant (normal).

This would be released in tandem with https://github.com/RSS-Engineering/undersync/pull/14 fixes PUC-425

python/understack-workflows/understack_workflows/nautobot.py

skrobul · 2024-09-02T08:47:16Z

python/neutron-understack/neutron_understack/neutron_understack_mech.py

@@ -263,14 +263,16 @@ def __network_name(self, network_id: str):
            return "tenant"

    def _move_to_network(self, context):
+        interface_uuid = context.current["id"]


I am afraid this won't be as simple, but I am not 100% sure on that.

The way I understand it:

the ml2 driver calls update_port_postcommit with a relevant context that is of PortContext type

this gets passed to _move_to_network without modifications

Now, the problem I see here is that id fields corresponds to the ID of a port as given by Neutron. Unless I'm wrong, these are completely different to what Nautobot expects to identify an Interface.

These IDs are automatically assigned when Ironic creates Neutron ports for each of its baremetal Ports.

At the moment, both baremetal ports and neutron port IDs will not match as they are both randomly allocated - on baremetal side during the node inspection, and on the neutron side when the code linked above is executed.

I think the easiest way around that situation may be to match using the MAC address. Alternatively, we will have to pre-create the neutron ports with specific IDs which may be tricky.

One clarification on this - even though Ironic initially copies the baremetal port attributes presumably including the baremetal port id, it won't be preserved because Python Openstack SDK (which is used to interface with Neutron), does not pass on that attribute.

Even if it did, Neutron API wouldn't accept it

root@openstack-admin-client:/tmp# cat create_port_with_id.json { "port": { "id": "bc1ab041-13af-464c-81c6-87db1e15aaaa", "admin_state_up": true, "name": "private-port", "network_id": "a0c267bf-33af-48b9-b540-daa6d41bff3f", "port_security_enabled": false, "allowed_address_pairs": [] } } root@openstack-admin-client:/tmp# curl -X POST -H "Content-Type: application/json" -H "X-Auth-Token: $TOKEN" http://neutron-server.openstack.svc.cluster.local:9696/v2.0/ports -d @create_port_with_id.json {"NeutronError": {"type": "HTTPBadRequest", "message": "Attribute 'id' not allowed in POST", "detail": ""}}root@openstack-admin-client:/tmp# root@openstack-admin-client:/tmp#

I thought it was too good to be true when someone said "that is the ID you need". I reworked this in 35d1295 to find the interface by MAC address, we can revisit this in the future if we ever sync up the UUID values.

@skrobul We can go directly to python-neutronclient instead of using the OpenStack SDK. I've noticed the OpenStack SDK is a bit too high level in a number of places. It aims to be a higher level simple interface and I guess that makes sense.

My comments doesn't matter because like you said, neutron rejects that.

cardoe

So won't block this being merged but I think this would be an improvement for the test.

cardoe · 2024-09-04T13:39:54Z

python/neutron-understack/neutron_understack/tests/test_neutron_understack_mech.py

+def mock_context_data(file):
+    ref = pathlib.Path(__file__).joinpath(filename)
+    with ref.open("r") as f:
+        return ContextDouble(json.load(f))


I'd probably add in some more magic with fixtures.

Suggested change

return ContextDouble(json.load(f))

return ContextDouble(json.load(f))

@pytest.fixture

def neutron_update_port_postcommit() -> ContextDouble:

return mock_context_data("fixtures/neutron_update_port_postcommit.json")

cardoe · 2024-09-04T13:54:46Z

python/neutron-understack/neutron_understack/tests/test_neutron_understack_mech.py

+def test_update_port_postcommit(mock_argo_client):
+    context_data = mock_context_data(
+        "fixtures/neutron_update_port_postcommit.json"
+    )
+
+    UnderstackDriver.update_port_postcommit(context_data)


Suggested change

def test_update_port_postcommit(mock_argo_client):

context_data = mock_context_data(

"fixtures/neutron_update_port_postcommit.json"

)

UnderstackDriver.update_port_postcommit(context_data)

def test_update_port_postcommit(mock_argo_client, neutron_update_port_postcommit):

UnderstackDriver.update_port_postcommit(neutron_update_port_postcommit)

cardoe · 2024-09-04T13:55:19Z

python/neutron-understack/neutron_understack/tests/test_neutron_understack_mech.py

+            "interface_uuid": "e5d5cd73-ca9a-4b74-9d52-43188d0cdcaa",
+            "device_uuid": "41d18c6a-5548-4ee9-926f-4e3ebf43153f",


Suggested change

"interface_uuid": "e5d5cd73-ca9a-4b74-9d52-43188d0cdcaa",

"device_uuid": "41d18c6a-5548-4ee9-926f-4e3ebf43153f",

"interface_uuid": neutron_update_port_postcommit.current["id"],

"device_uuid": neutron_update_port_postcommit.current["device_id"],

We used to flip a bit on the nautobot Device object for “connected_to_network” when changing a device from/to “provisioning” and “tenant” settings. We need this to be interface specific. Because of Ironic's knowlege of the interface that is being used for PXE boot, we are called with a server interface UUID. We update the nautobot status field of connected switch interface. This tells undersync which mode the port should be in - provisioning or tenant (normal). We also make a change to the switch IDs that are passed to undersync. This used to represnt the set of all switches to which the server is connected. This is sub-optimal if there are multiple fabrics connected to the same server. We drop this logic and instead just pass a single switch ID, which is the switch where PXE is happening. Undersync will always update both switches in the VPC pair (VLAN GROUP) in any case - passing the extra switch IDs was redundant.

The UUID in Nautobot is not the same as the UUID in ironic/nova so use the MAC address to find the required switch port. In the earlier milestones we should not encounter LAGs but this is something to watch out for in future: - it is not clear exactly what the rules are with interface in Nautobot but when interfaces belong to a LAG. Nautobot seems to report that the LAG and its members all share a MAC address. This means the MAC address is no longer guaranteed to uniquely identify an interface. We have also been talking about how to sync/store the various UUID values in nautobot, so perhaps this will eventually be solved that way.

stevekeay force-pushed the PUC425 branch 2 times, most recently from ae193e9 to 26aa1aa Compare August 30, 2024 23:18

skrobul reviewed Sep 2, 2024

View reviewed changes

stevekeay force-pushed the PUC425 branch from 5847963 to 35d1295 Compare September 2, 2024 20:58

stevekeay mentioned this pull request Sep 4, 2024

chore: add testing and linting to neutron driver plugin #251

Merged

cardoe reviewed Sep 4, 2024

View reviewed changes

cardoe changed the title ~~feat: PUC-425 workflow updates Nautobot Interface.status instead of device.custom_field.conected_to_network~~ feat: workflow updates Nautobot Interface.status instead of device.custom_field.conected_to_network Sep 4, 2024

Steve Keay added 3 commits September 4, 2024 10:50

Have neutron ML2 driver pass port ID when invoking undersync workflow

6cd01da

cardoe force-pushed the PUC425 branch from 35d1295 to e729fc1 Compare September 4, 2024 15:50

cardoe approved these changes Sep 4, 2024

View reviewed changes

cardoe merged commit 5e36083 into rackerlabs:main Sep 4, 2024
12 checks passed

stevekeay deleted the PUC425 branch September 9, 2024 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: workflow updates Nautobot Interface.status instead of device.custom_field.conected_to_network #249

feat: workflow updates Nautobot Interface.status instead of device.custom_field.conected_to_network #249

stevekeay commented Aug 30, 2024 •

edited by cardoe

Loading

skrobul Sep 2, 2024

skrobul Sep 2, 2024

stevekeay Sep 2, 2024

cardoe Sep 4, 2024

cardoe Sep 4, 2024

cardoe left a comment

cardoe Sep 4, 2024

cardoe Sep 4, 2024

cardoe Sep 4, 2024

-        return ContextDouble(json.load(f))
+        return ContextDouble(json.load(f))
+@pytest.fixture
+def neutron_update_port_postcommit() -> ContextDouble:
+    return mock_context_data("fixtures/neutron_update_port_postcommit.json")

		"interface_uuid": "e5d5cd73-ca9a-4b74-9d52-43188d0cdcaa",
		"device_uuid": "41d18c6a-5548-4ee9-926f-4e3ebf43153f",

feat: workflow updates Nautobot Interface.status instead of device.custom_field.conected_to_network #249

feat: workflow updates Nautobot Interface.status instead of device.custom_field.conected_to_network #249

Conversation

stevekeay commented Aug 30, 2024 • edited by cardoe Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cardoe left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevekeay commented Aug 30, 2024 •

edited by cardoe

Loading