Codecov Report

Merging #2662 into master will increase coverage by 0.18%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master   #2662      +/-   ##
=========================================
+ Coverage   62.02%   62.2%   +0.18%     
=========================================
  Files         134     134              
  Lines       21739   21743       +4     
=========================================
+ Hits        13483   13526      +43     
+ Misses       6810    6773      -37     
+ Partials     1446    1444       -2

So what would cli look like? docker node ls -f "node-label=*" ?

Would it make sense to change this to EngineLabels and NodeLabels to distinguish?

@dperny @cyli If possible, I would be in favour of that; I agree it's more readable and less likely to confuse their purpose (I think the original implementation in #2062 also did a rename?)

So what would cli look like? docker node ls -f "node-label=*" ?

See moby/moby#28209 (comment). To be consistent with --constraint (on docker service), it will likely be;

To be discussed (either keep the old behavior and ignore node-labels, or filter by both);

docker node ls --filter label=foo

Filter by engine labels:

docker node ls --filter engine.label=foo

Filter by node label:

docker node ls --filter node.label=foo

I don't think it's a great idea to rename the GRPC field, because it'll require refactoring the whole codebase to accommodate the new names. If we were to do that, I'd suggest doing it in a separate PR.

Otherwise, we can kick the CLI bikeshed down the road a bit.

I don't think it's a great idea to rename the GRPC field, because it'll require refactoring the whole codebase to accommodate the new names. If we were to do that, I'd suggest doing it in a separate PR.

SGTM.