-
Notifications
You must be signed in to change notification settings - Fork 6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Dynamic network segments for each VLAN Group #771
base: main
Are you sure you want to change the base?
Conversation
9a809bc
to
db6c164
Compare
python/neutron-understack/neutron_understack/vlan_group_name_convention.py
Outdated
Show resolved
Hide resolved
python/neutron-understack/neutron_understack/neutron_understack_mech.py
Outdated
Show resolved
Hide resolved
python/neutron-understack/neutron_understack/neutron_understack_mech.py
Outdated
Show resolved
Hide resolved
"-3f": "storage-appliance", | ||
"-4f": "storage-appliance", | ||
"-1d": "bmc", | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading through now, we won't need this code because we just need to create the correct network segment ranges for the "network" vlan groups which will match up with Nautobot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like we would need to find the baremetal port, perhaps by MAC address, and then look up its physical_network. This complicates things, so leaving it as a task to clean up later once we get this all working.
d15a22a
to
2118f7c
Compare
f37859a
to
8894c60
Compare
Based on my limited understanding of what happens in Cisco's driver: https://github.com/ansao-aci/group-based-policy/blob/master/gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py As well as Arista's driver: https://github.com/stackhpc/networking-arista/blob/2e36773c2d6a6e9fc31c23cc9647fe978572250f/networking_arista/ml2/mechanism_arista.py
We refactor identifiers in the code to reflect those changes
The context only has the information we need in the later phases of the process. We will be called early in the process with incomplete information, this is normal and we should not raise an error in these conditions.
Neutron is now the authority for assigning those numbers
Our provisioning network uses this.
907bf94
to
91af998
Compare
We don't want to "bind" the VXLAN segments that are passed to bind_port(). We want to make a dynamic VLAN-type segment and bind that instead. We are no longer using any VLAN-type networks, so remove the code that deals with those (it was handling the provisioning network which was still confiured as VLAN type for legacy reasons, but that is now moving to VXLAN-type network, same as all the others.)
71a7222
to
6f7aecb
Compare
Will you include a detailed description in the PR? |
766acad
to
89fcaf0
Compare
This will record which sub-ports belong to which network segments.
89fcaf0
to
f42e5c5
Compare
We were leaving the old vlans in place when we should have been cleaning them up and replacing with the new set.
3a39404
to
d292b12
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
python/neutron-understack/neutron_understack/tests/test_trunk.py:1
- [nitpick] The removal of the entire trunk tests may lead to a gap in test coverage for trunk-related functionality. Please ensure that either this functionality is no longer needed or that its behavior is covered by alternative tests.
Entire file removed
python/neutron-understack/neutron_understack/vlan_group_name_convention.py
Outdated
Show resolved
Hide resolved
Committing co-pilot suggestion Co-authored-by: Copilot <[email protected]>
This was removed but potentially still useful so I am putting it back.
This suppports the OVN router and it may be re-instated in future but for now it is broken by our change to openstack vlan allocation so removing it for now.
This was opened to address PUC-787