d/network: filter networks individually#50831
Closed
corhere wants to merge 1 commit intomoby:masterfrom
Closed
Conversation
3671109 to
fe710e7
Compare
corhere
commented
Aug 28, 2025
Comment on lines
+69
to
+73
| if f.args.Contains("idOrName") && | ||
| !f.args.Match("name", nw.Name) && | ||
| !f.args.Match("id", nw.Name) { | ||
| return false | ||
| } |
Contributor
Author
There was a problem hiding this comment.
idOrName is not in the set of accepted filters for list-networks API requests. It is used internally, incorrectly:
The net result is that when
idOrName is in the filter, unless both name and id terms are also included it will always match due to the semantics of (filter.Args).Match returning true when the term is not present in the filter. And matching the id term against the container name is very suspicious. But that's what the old implementation also did, so this refactor maintains bug-for-bug compatibility.
Internally a network is represented by either a libnetwork.Network or a swarmapi.Network. The daemon functions backing the API server map these values to the Engine API network.Inspect type on demand. Since they have to convert, the functions to get a list of networks have to loop over the slice of Networks and append them to a slice of network.Inspect values. The function used to filter the list of networks by a user-supplied predicate takes a []network.Inspect and returns a shorter slice. Therefore the daemon functions backing the API server have to loop through the list twice; once to convert, and again inside the FilterNetworks function to prune networks from the slice. Each time an item is deleted from a slice, all items at higher indices need to be copied to lower indices in the backing array. Replace FilterNetworks with a function that accepts a single network summary and returns a boolean. The daemon network-list functions can filter in the conversion loop, which cuts down on a nontrivial amount of copying. Split the validation of the filter predicate from filter evaluation to both make it more ergonomic to use inside loops, and to make invalid states (a filter with an ill-formed predicate) unrepresentable. This change also future-proofs the network filter for when we change network.Inspect to a distinct type that embeds a network.Summary. Callers that need to build a filtered list of network.Inspect values will be able to evaluate the filter on the embedded Summary then append the full Inspect value of matching networks to the output slice. Signed-off-by: Cory Snider <csnider@mirantis.com>
fe710e7 to
47372bc
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
- What I did
I improved the code to filter a list of networks by a predicate specified in an Engine API request. It is simpler, more readable, more future-proof and more efficient.
- How I did it
Internally a network is represented by either a
libnetwork.Networkor aswarmapi.Network. The daemon functions backing the API server map these values to the Engine APInetwork.Inspecttype on demand. Since they have to convert, the functions to get a list of networks have to loop over the slice ofNetworksand append them to a slice ofnetwork.Inspectvalues.The function used to filter the list of networks by a user-supplied predicate takes a
[]network.Inspectand returns a shorter slice. Therefore the daemon functions backing the API server have to loop through the list twice; once to convert, and again inside the FilterNetworks function to prune networks from the slice. Each time an item is deleted from a slice, all items at higher indices need to be copied to lower indices in the backing array.Replace
FilterNetworkswith a function that accepts a single network summary and returns a boolean. The daemon network-list functions can filter in the conversion loop, which cuts down on a nontrivial amount of copying. Split the validation of the filter predicate from filter evaluation to both make it more ergonomic to use inside loops, and to make invalid states (a filter with an ill-formed predicate) unrepresentable.This change also future-proofs the network filter for when
network.Inspectbecomes a distinct type that embeds anetwork.Summary. Callers that need to build a filtered list ofnetwork.Inspectvalues will be able to evaluate the filter on the embeddedSummarythen append the fullInspectvalue of matching networks to the output slice, all without any further changes to the filter code.- How to verify it
Existing unit/integration tests.
- Human readable description for the release notes
- A picture of a cute animal (not mandatory but encouraged)