Add infra_ipv6 attribute to devices
#297
Conversation
Valid IPv6 interface addresses must be accepted by the mgmtdomain API endpoints. Additionally, an mgmtdomain can be either IPv4, IPv6 or both, meaning that there are now multiple sets of minimally required attributes to post to the endpoint. This should ensure backwards compatibility with API clients that only support ipv4_gw
The new version of this function can take the optional version argument to select whether to get an address from the IPv4 or the IPv6 network of the mgmtdomain.
This config setting will be used to choose which IP address version is preferred as the primary management address of a device in a dual-stack management domain.
This allows (but does not require) devices to be configured with dual-stack management (even though CNaaS-NMS will continue to use the primary management address).
This updates init_access_device_step1 to fetch, reserv and assign secondary management IPs from any dual-stack Mgmtdomain.
Prefix length and mgmt_gw should be available variables also for the secondary management address.
This repeats a lot of what `init_access_device_step1` does. Not sure why there is an overlap, but I'm not going to change it without being sure.
This also refactors redundant IP field value validation code. The logic for all IP addr fields is the same, so no need for duplication.
This field is required for installations that want to run IPv6 directly on the underlay.
Since this field is explicitly IPv6-only, it needs a separate check from the other IPv6 fields.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## develop #297 +/- ##
===========================================
- Coverage 65.19% 65.06% -0.13%
===========================================
Files 65 65
Lines 6950 7011 +61
===========================================
+ Hits 4531 4562 +31
- Misses 2419 2449 +30
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
Kudos, SonarCloud Quality Gate passed!
|
|
Blocking PR #266 has been merged, so maybe time to look at this again |
|
Main usecase for this is for SIKT if they want to "merge" in existing installations that is not running cnaas-nms into cnaas-nms in the future. Currently a single loopback interface in those two (?) installations are used both for underlay vxlan tunnels and management traffic. Vidar will look into the history and reasoning for this design, and if future designs will use the same setup or separate loopbacks for underlay and management. This might affect how we want to implement this with dualstack and pools etc. In meantime this PR will remain in kind of frozen state because SIKT has no current need or time to work on it? |
Sounds about right for the time being. |
This PR fixes #276.
I could not find any existing tests that explicitly test the existing
infra_ipattribute, so I'm not sure what the best strategy for testing this would be.The existing code assigns values to
Device.infra_ipwhen init-ing fabric devices, using thecnaas_nms.devicehandler.underlay.find_free_infra_ip()function:cnaas-nms/src/cnaas_nms/devicehandler/init_device.py
Line 724 in fada077
Question: Should this PR concern itself with also assigning an IPv6 address to
infra_ipv6? It seems the assignment is based on theinfra_lo_netsetting, so this may actually also need a new setting forinfra_ipv6_lo_net. Setting the value could be made dependent oninfra_ipv6_lo_netactually being present (i.e. "do nothing if IPv6 isn't used/configured")This PR is dependent on #266 being merged first (actual changes start from ce1d4c3)