Skip to content

Conversation

mfencik
Copy link
Contributor

@mfencik mfencik commented Feb 12, 2025

This PR will enable us to set "tenant_vlan_id" on UCVNI in Nautobot when first subport is added to neutron trunk and user specifies the "segmentation_id" when adding the subport. The idea is that we will use the "tenant_vlan_id" as mapped VLAN that is local to the switchport during server provisioning.
This PR covers only the trunk driver part and there will be another one where we will adjust the understack mechanism driver, so when server will go through provisioning, we will detect the trunk_details on the port attached and do the needful to configure our network.

@mfencik mfencik force-pushed the understack_trunks branch 29 times, most recently from 13d8f25 to 67be9ec Compare February 18, 2025 19:34
@mfencik mfencik marked this pull request as ready for review February 19, 2025 17:19
Copy link
Collaborator

@skrobul skrobul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good, but have a few minor suggestions for potential improvements (comments inline).
Also the UnderStackTrunkDriver class could really use some docstrings, at least on the public methods with a high level explanation of why we react to certain events and what we intend to do.

Another potential refactoring opportunity is in how the configure_tenant_vlan_id and _subports_added are split, consider something similar to this:

    def _configure_subports(self, subports: list[SubPort]) -> None:
        for subport in subports:
            network_id = utils.fetch_subport_network_id(subport_id=subport.port_id)
            tenant_vid = self.nb.fetch_tenant_vid(
                network_id=network_id
            )

            if tenant_vid:
                self._update_vni(tvid, subport)
            elif tenant_vid != subport.segmentation_id:
                self._handle_mismatch(tenant_vid, subport, network_id)


    def _update_vni(tvid, subport):
        ucvni_uuid = utils.fetch_subport_network_id(subport_id=subport.port_id)
        self.nb.add_tenant_vlan_tag_to_ucvni(
            network_uuid=ucvni_uuid, vlan_tag=subport.segmentation_id
        )
        LOG.info(
            "Segmentation ID: %(seg_id)s is now set on Nautobot's UCVNI "
                "UUID: %(ucvni_uuid)s in the tenant_vlan_id custom field",
            {"seg_id": subport.segmentation_id, "ucvni_uuid": ucvni_uuid},
        )

    def _handle_mismatch(tenant_vlan_id, subport, subport_network_id):
        if tenant_vlan_id != subport.segmentation_id:
            err = SubportSegmentationIDError(
                seg_id=subport.segmentation_id,
                net_id=subport_network_id,
                nb_seg_id=tenant_vlan_id,
                subport_id=subport.port_id,
            )

            subport.delete()
            raise err

^ untested

@mfencik mfencik added this pull request to the merge queue Feb 20, 2025
Merged via the queue into main with commit d53f790 Feb 20, 2025
26 checks passed
@mfencik mfencik deleted the understack_trunks branch February 20, 2025 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants