Skip to content
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

IPAM: Child address list for IP prefix is garbled #11209

Closed
peteeckel opened this issue Dec 16, 2022 · 11 comments · Fixed by #12820
Closed

IPAM: Child address list for IP prefix is garbled #11209

peteeckel opened this issue Dec 16, 2022 · 11 comments · Fixed by #12820
Assignees
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application

Comments

@peteeckel
Copy link
Contributor

NetBox version

v3.4.0

Python version

3.8

Steps to Reproduce

  1. View a prefix with multiple non-sequentially assigned child addresses
  2. Click on "IP Addresses"

Expected Behavior

The list of IP address is displayed, with "xx IPs available" rows in rows between assigned IP address ranges

Observed Behavior

All "xx IPs available" placeholders are displayed at the top of the list.

Screenshot 2022-12-16 at 09 58 40

After clicking on the row header in order to sort the list, the "xx IPs available" placeholders are all gone.

@peteeckel peteeckel added the type: bug A confirmed report of unexpected behavior in the application label Dec 16, 2022
@peteeckel peteeckel changed the title Child address list for IP prefix is garbled IPAM: Child address list for IP prefix is garbled Dec 16, 2022
@kkthxbye-code
Copy link
Contributor

kkthxbye-code commented Dec 16, 2022

This is caused by an incomplete fix to #9328:

899b612

To be clear, the now intended functionality is remove the avaliable ip rows when the table is sorted. The bug here is that they are only removed when the list has just been sorted, and not when the list was allready sorted when loaded.

A workaround to the problem of the avaliable IP's not being shown is to press the small x in the column header to reset the sort.

@kkthxbye-code kkthxbye-code added the status: under review Further discussion is needed to determine this issue's scope and/or implementation label Dec 16, 2022
@peteeckel
Copy link
Contributor Author

peteeckel commented Dec 16, 2022

Thanks! It makes perfect sense to remove the available IP rows when the list is sorted by any column other than IP addresses. When sorted by IP addresses the would rows make sense, otherwise they don't - or am I missing something?

Removing the sorting by clicking on "x" indeed works around the problem. The question is, however, why the list initially came up sorted by "status" when I first opened it ... is the sorting stored somewhere? When I open the list of IPs for other prefixes there is no sorting enabled by default.

[Edit] The list came up sorted by IP address, not by status ... I was fooled (again) by the red "x" being displayed next to "status" when actually it is referring to the column left of it.

@kkthxbye-code
Copy link
Contributor

When sorted by IP addresses the would rows make sense, otherwise they don't - or am I missing something?

I can't remember exactly (so don't hold me to this), but I believe it's a question of it being hard to implement with the way ordering and the addition of the available IP rows work. The solution is a compromise as the default sort is already sorted by IP. The thing that would be missing is a reversed sort by IP address, but the amount of work required is probably not worth it.

The question is, however, why the list initially came up sorted by "status" when I first opened it ... is the sorting stored somewhere?

Yes all sorting (and chosen columns) is stored on a user-basis. You can see them and reset them in the bottom of the preference view: /user/preferences/

@peteeckel
Copy link
Contributor Author

That makes sense, thanks for the information. I agree, having the available IP rows as long as the sorting is by IP address would be nice, though it's certainly not worth a major effort.

@jeremystretch
Copy link
Member

I believe it's a question of it being hard to implement with the way ordering and the addition of the available IP rows work.

Pretty much. We just don't have a great mechanism for annotating available IPs in between IPAddress records.

Fixing the bug itself is a bit tricky because we need to infer how the table is ordered to prepare the data, but we can't initialize the table until we have the data prepared. We'll probably need to introduce a utility function for determining a table's ordering, that can be used by both prep_table_data() under the view and by a table's configure() method.

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label Feb 15, 2023
@jeremystretch jeremystretch added status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation and removed status: under review Further discussion is needed to determine this issue's scope and/or implementation pending closure Requires immediate attention to avoid being closed for inactivity labels Feb 23, 2023
@jeremystretch
Copy link
Member

This is also an issue when filtering the objects list, as non-matching objects may be incorrectly annotated as available space.

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. NetBox is governed by a small group of core maintainers which means not all opened issues may receive direct feedback. Do not attempt to circumvent this process by "bumping" the issue; doing so will result in its immediate closure and you may be barred from participating in any future discussions. Please see our contributing guide.

@github-actions github-actions bot added the pending closure Requires immediate attention to avoid being closed for inactivity label May 25, 2023
@DanSheps DanSheps added status: accepted This issue has been accepted for implementation and removed status: needs owner This issue is tentatively accepted pending a volunteer committed to its implementation pending closure Requires immediate attention to avoid being closed for inactivity labels Jun 6, 2023
@DanSheps DanSheps self-assigned this Jun 6, 2023
DanSheps added a commit that referenced this issue Jun 6, 2023
@jeremystretch jeremystretch added the severity: low Does not significantly disrupt application functionality, or a workaround is available label Jun 23, 2023
jeremystretch pushed a commit that referenced this issue Sep 13, 2023
@jeremystretch jeremystretch self-assigned this Sep 13, 2023
jeremystretch added a commit that referenced this issue Sep 13, 2023
…12820)

* Fixes #11209 - Do not add available ips when IPAddressTable sort preferences are saved

* Refine check to account scenario right after clearing ordering string

* Introduce get_table_ordering() utility to determine intended ordering given a request

* Apply fix to VLAN ranges as well

---------

Co-authored-by: Jeremy Stretch <jstretch@netboxlabs.com>
@7qbit
Copy link

7qbit commented Sep 22, 2023

please revise this problem again. we need sorting of “Available IP addresses” on TOP.

In addition, status=sorting does not meet the expectation of sorting by status; here, confusingly available IP addresses are removed.

Thank You.

@AM-2027
Copy link

AM-2027 commented Sep 25, 2023

Hello all,

we have recently updated our instance to 3.6.2.
Because most users have saved a sort, they could not find available IP addresses. Only after having removed the sorting.

I think you have a malfunction here, users don't expect the sorting function to remove the objects now, but a sorting by all possible "statuses".

Second problem is that it is no longer possible to sort all available IP addresses on top, which was very handy before.

I find this a malfunction because sorting behavior is not the usual one you think.

@peteeckel
Copy link
Contributor Author

peteeckel commented Sep 25, 2023

I agree with you that it would be useful to have a list of available IP ranges in one place.

However, having a couple of 'xxx IP Addresses Available' lines on top of a list sorted by criteria other than IP address is not really helpful, even more so as there is no other information about these ranges available. That was confusing, misleading and IMHO a bug (which was fixed subsequently).

One option to solve both issues could be a separate tab listing all free address ranges, sorted by IP address. That way they would all be visible in one place without messing up the IP address list in other views. That could even be implemented as a plugin rather easily if a feature request were unsuccessful.

Suggestion: Instead of insisting on a bugfix being reverted, please open a FR for the functionality described above.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
severity: low Does not significantly disrupt application functionality, or a workaround is available status: accepted This issue has been accepted for implementation type: bug A confirmed report of unexpected behavior in the application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants