IPAM Namespace Data Migrations #3759
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jathanism
force-pushed
the
u/jathanism-namespace-data-migrations
branch
from
507af41
to
e92f4cb
Compare
nautobot/ipam/utils/migrations.py
Outdated
| vrf.prefixes.update(namespace=vrf.namespace) | ||
| print(f" VRF {vrf.name!r} migrated to Namespace {vrf.namespace.name!r}") | ||
|
|
||
| # Case 00: Unique fields vs. Namespaces (name/rd) move to a Cleanup Namespace. |
There was a problem hiding this comment.
Was mirroring the cases from the wiki page. Case 1 requires no changes. See: https://github.com/nautobot/nautobot/wiki/2.0:-IPAM-Overhaul-Data-Migration-Plan#need-to-determine-which-vrfs-become-namespaces
IPAM Namespace data migrations progress report. Need to move VRF from IP Address to Interface: Done. Move duplicate Prefixes from Global Namespace to Cleanup Namespaces. VMInterface migrations to go along with Interface. - Had to augment `VRFDeviceAssignment` to take either `device` or `virtual_machine` and relax the constraints to allow the fields to be nullable, asserting their correctnext in the `clean()` method - Made black and flake8 happy. Implemented migrations for parenting IPAddresses to Prefixes in the correct Namespace. Make migrations a little prettier, and remove dead code. Re-remove the `Prefix.save()` uniqueness checks for `VRF.enforce_unique` Make the IPAddress migration code a little easier to read. Revised parent-finding algorithm to have affinity for Tenants - Additionally the Namepaces lookup will now always start with "Global" NS to try to keep as many objects in there as reasonably possible. - If an IP has a tenant, it'll try to filter possible ancestors by it.
jathanism
force-pushed
the
u/jathanism-namespace-data-migrations
branch
from
3f3e23a
to
4804e90
Compare
* Convert VRF-Prefix relationship to M2M * cleanup * black --------- Co-authored-by: Gary Snider <gary.snider@networktocode.com>
Comment on lines
+364
to
+367
| # TODO(jathan): This might be flawed if an IPAddress has a prefix_length that excludes it from a | ||
| # parent that should otherwise contain it. If we encounter issues in the future for | ||
| # identifying closest parent prefixes, this might be a starting point. | ||
| "prefix_length__lte": cidr.prefixlen, |
There was a problem hiding this comment.
Is this comment still accurate/relevant? This is a method on PrefixQuerySet, not IPAddressQuerySet so it seems wrong to me?
There was a problem hiding this comment.
It's correct so long as we also continue to include broadcast, which we currently are. @gsnider2195 please keep me honest.
There was a problem hiding this comment.
IPAddress should probably just be:
Prefix.objects.filter(network__lte=cidr.ip, broadcast__gte=cidr.ip)There was a problem hiding this comment.
Also, like we discussed for the data migrations, prefix_length needs to be lt for Prefixes, not lte
There was a problem hiding this comment.
Actually, we decided that with the uniqueness constraints we should never have a matching prefix with the same prefix length?
* Broadcast fixes * PR Feedback
* Cleanup namespaces detail, related views * Black * Apply suggestions from code review Co-authored-by: Gary Snider <75227981+gsnider2195@users.noreply.github.com> --------- Co-authored-by: Gary Snider <75227981+gsnider2195@users.noreply.github.com>
…utobot/nautobot into u/jathanism-namespace-data-migrations
- Added/clarified docstrings to ALL functions - Renamed functions for clarity - Moved things around - Removed lingering TODO comments - Had an existential crisis
It is now: "Created by Nautobot 2.0 IPAM data migrations."
jathanism
changed the title
…utobot/nautobot into u/jathanism-namespace-data-migrations
gsnider2195
merged commit 35c59e8
into
prototype/jathanism-3337-ipam-namespace
19 checks passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: DNE
What's Changed
Note: Current model changes are there to be able to debug and work with the data in its v1.x form. As data migrations are completed, model changes will continue to progress. Same with the data migrations where certain operations have been commented out. This is absolutely not fit for consumption...yet.
Done
enforce_unique=Truemoved to their own "VRF" namespacesenforce_unique=Falsemoved to a Cleanup namespaceTo Do
trace_paths, a mgmt command that will iterate each Prefix and call save to trigger reparenting. This should be a post-upgrade step as well.TODO