fix(network): 3.8 relax constraints for resizing dynamic range reservations - #560
bryanfraschetti wants to merge 1 commit into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
[maas-code-reviewer review]
LLM-generated review from https://github.com/canonical/maas-code-reviewer.
Intended to assist a human reviewer, not replace one — suggestions may be
incorrect, please verify before acting.
The logic for calculating and verifying newly added segments properly accounts for all resizing and shifting scenarios. This nicely resolves the issue of dynamic range modifications failing when the range already contains active IPs.
I've spotted a minor typing issue in one of the tests that currently works by coincidence due to an early return, but could crash if the function's internal flow is changed. Otherwise, great work!
| unused = other_subnet.get_ipranges_available_for_dynamic_range( | ||
| exclude_ip_range_id=iprange.id | ||
| ) | ||
|
|
There was a problem hiding this comment.
The _is_existing_dynamic_range_resize_allowed method expects start_ip and end_ip to be of type IPAddress (which has a .version attribute), but iprange.netaddr_iprange.first and .last return integers. Although this test passes because the method currently returns False early (since subnet_id changed), it would crash with an AttributeError on start_ip.version if the early returns were ever bypassed or refactored. Consider passing IPAddress(iprange.start_ip) and IPAddress(iprange.end_ip) instead.
| if loaded_values.get("subnet_id") != self.subnet_id: | ||
| return False | ||
|
|
||
| existing_start_ip = loaded_values.get("start_ip") |
There was a problem hiding this comment.
Nit: existing_start_ip and existing_end_ip are reused here for the IPAddress objects, changing their type from string. It might be slightly cleaner and less prone to confusion to name the string variables differently (e.g. loaded_start_ip) or parse them directly.
bryanfraschetti
changed the title
bryanfraschetti
changed the title
bryanfraschetti
changed the title
…ns (canonical#280) When an IP is allocated inside a dynamic range reservation, the validation performed during a resize sees the range as discontinuous, since an allocated IP splits the available space into separate contiguous blocks. As a result, both shrinking and expanding the reservation are rejected with "Requested dynamic range conflicts with an existing IP address or range", even when the requested change is otherwise valid. This commit addresses the issue by comparing the requested range against the previously persisted range and requiring only the newly added segments (i.e., the expanded portions) to be contained within an unused range. Pure shrinks add no segments and are always allowed. Resolves LP:2143090
e2e74be to
e20a3ee
Compare
2 participants
When an IP is allocated inside a dynamic range reservation, the validation performed during a resize sees the range as discontinuous, since an allocated IP splits the available space into separate contiguous blocks. As a result, both shrinking and expanding the reservation are rejected with "Requested dynamic range conflicts with an existing IP address or range", even when the requested change is otherwise valid.
This commit addresses the issue by comparing the requested range against the previously persisted range and requiring only the newly added segments (i.e., the expanded portions) to be contained within an unused range. Pure shrinks add no segments and are always allowed.
Resolves LP:2143090
(cherry picked from commit 8e8c3f6)