pkg/labels: print all leaf CIDRs, not just the last one. #28224
Merged
Conversation
This file contains hidden or 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
33919a5 to
4829b40
Compare
|
/test |
4829b40 to
bb33022
Compare
|
/test |
|
Confirmed that this works as expected: |
nathanjsweet
approved these changes
Sep 26, 2023
joestringer
reviewed
Oct 2, 2023
bb33022 to
b72b0fb
Compare
|
Rebased, refactored to use a real data structure. This improves performance over the existing, incorrect method as well as my previous naive attempt: This reduces time from 167µs to 108µs. Fewer allocs, too. |
b72b0fb to
e49753d
Compare
|
Code golf: got it even faster :-) |
e49753d to
9945ca0
Compare
joestringer
approved these changes
Oct 20, 2023
|
We might consider adding |
This package implemented a single function and caused import loops. So, just move it up a level to get rid of the loop. Signed-off-by: Casey Callendrello <cdc@isovalent.com>
When printing Labels (e.g. by `cilium identity list`), CIDR labels are de-expanded to only show the relevant ones. For example, the CIDR label 1.1.1.0/24 is under-the-hood expanded to 0.0.0.0/0, 0.0.0.0/1, ..., 1.1.1.0/23, 1.1.1.0/24. Then, when the identity is printed for human consumption, the CIDR labels are de-expanded. However, this code did not anticipate a single identity having multiple "leaf" CIDR labels. So, change the printing logic to output all interesting CIDRs. This adds a small utility datatype to efficiently prune a tree of cidrs to the leaves. It winds up being about 30% faster than the original, incorrect version. Signed-off-by: Casey Callendrello <cdc@isovalent.com>
9945ca0 to
6967cf1
Compare
|
/test |
|
Is anyone still planning to tackle the backports for |
|
Downgrading the v1.13 label to |
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.
When printing Labels (e.g. by
cilium identity list), CIDR labels are de-expanded to only show the relevant ones. For example, the CIDR label1.1.1.0/24is under-the-hood expanded to0.0.0.0/0, 0.0.0.0/1, ..., 1.1.0.0/23, 1.1.1.0/24. Then, when the identity is printed for human consumption, the CIDR labels are de-expanded. However, this code did not anticipate a single identity having multiple "leaf" CIDR labels.So, change the printing logic to output all interesting CIDRs. This adds a O(N^2) check, but given that N is never over ~100, it shouldn't be an issue.
Note that this output is only for human consumption; it is not persisted or used as an API.