make node ls filtering support both node label and engine label#2062
make node ls filtering support both node label and engine label#2062allencloud wants to merge 1 commit intomoby:masterfrom
Conversation
b46ba7a to
37c40ee
Compare
Codecov Report
@@ Coverage Diff @@
## master #2062 +/- ##
=========================================
- Coverage 59.62% 55.9% -3.73%
=========================================
Files 118 55 -63
Lines 19544 8771 -10773
=========================================
- Hits 11654 4903 -6751
+ Misses 6541 3342 -3199
+ Partials 1349 526 -823Continue to review full report at Codecov.
|
37c40ee to
22e1b87
Compare
| if len(request.Filters.Labels) == 0 { | ||
| return true | ||
| } | ||
| if filterMatchLabels(e.Spec.Annotations.Labels, request.Filters.Labels) == true { |
There was a problem hiding this comment.
I think that for the SwarmKit side, this should be a separate filter, or at least a way to distinguish between "node" labels and "engine" labels.
We can still decide to keep the "docker" UX the same (filtering by labels using both "engine" and "node" labels), but at least it gives control over what is filtered on at the API level.
@aluzzardi @allencloud wdyt?
There was a problem hiding this comment.
I think the separated thing is reasonable. Updated that.
22e1b87 to
4384ff7
Compare
|
I think what @thaJeztah had in mind was two separate fields in the API to filter on node labels vs. engine labels. |
4384ff7 to
99c64e5
Compare
|
Yes; didn't look at the last changes in depth, but they should be "separate" options/filters in SwarmKit |
47139d7 to
b8c67ec
Compare
Signed-off-by: allencloud <allen.sun@daocloud.io>
b8c67ec to
0b97e37
Compare
| repeated NodeRole roles = 5; | ||
| map<string, string> node_labels = 3; | ||
| map<string, string> engine_labels = 4; | ||
| repeated NodeSpec.Membership memberships = 5; |
There was a problem hiding this comment.
It isn't okay to renumber existing values.
|
@allencloud are you still working on this PR? We might want to push this relatively soon, so if you're unable to get to it, I'd be happy to carry it for you. |
|
Hi, @nishanttotla |
This pr tries to fix moby/moby#27231 and moby/moby#28209
Currently in swarmkit
ListNodeonly supports engine label filtering, while not node label filtering. But this PR makes both supported.Signed-off-by: allencloud allen.sun@daocloud.io