Add support for filtering on node labels #37650

anshulpundir · 2018-08-15T21:38:19Z

Full diff: moby/swarmkit@8852e88...496d19b

Relevant changes:

add filtering on node labels swarmkit#2662 add filtering on node labels
- A rewrite of make node ls filtering support both node label and engine label swarmkit#2062.
- Relates to Filtering nodes doesn't return a result #27231, Swarm mode node ls label filter should be on both engine and swarm node #28209

ping @andrewhsu @thaJeztah @dperny

yongtang

LGTM once janky is 💚

thaJeztah

Thanks! Looks like we need some changes to make the new filter work;

update newListNodesFilters() to accept the new filter

moby/daemon/cluster/filters.go

Lines 12 to 46 in 3a633a7

    
           func newListNodesFilters(filter filters.Args) (*swarmapi.ListNodesRequest_Filters, error) { 
        
           	accepted := map[string]bool{ 
        
           		"name":       true, 
        
           		"id":         true, 
        
           		"label":      true, 
        
           		"role":       true, 
        
           		"membership": true, 
        
           	} 
        
           	if err := filter.Validate(accepted); err != nil { 
        
           		return nil, err 
        
           	} 
        
           	f := &swarmapi.ListNodesRequest_Filters{ 
        
           		NamePrefixes: filter.Get("name"), 
        
           		IDPrefixes:   filter.Get("id"), 
        
           		Labels:       runconfigopts.ConvertKVStringsToMap(filter.Get("label")), 
        
           	} 
        
           	for _, r := range filter.Get("role") { 
        
           		if role, ok := swarmapi.NodeRole_value[strings.ToUpper(r)]; ok { 
        
           			f.Roles = append(f.Roles, swarmapi.NodeRole(role)) 
        
           		} else if r != "" { 
        
           			return nil, fmt.Errorf("Invalid role filter: '%s'", r) 
        
           		} 
        
           	} 
        
           	for _, a := range filter.Get("membership") { 
        
           		if membership, ok := swarmapi.NodeSpec_Membership_value[strings.ToUpper(a)]; ok { 
        
           			f.Memberships = append(f.Memberships, swarmapi.NodeSpec_Membership(membership)) 
        
           		} else if a != "" { 
        
           			return nil, fmt.Errorf("Invalid membership filter: '%s'", a) 
        
           		} 
        
           	} 
        
           	return f, nil 
        
           }

update the Swagger file to document the newly accepted filter; https://github.com/moby/moby/blob/bd06a5ea4df919ff323510fba10801b5673a3af6/api/swagger.yaml?utf8=✓#L8577-L8589
add a note to the API changelog to mention the new filter; https://github.com/moby/moby/blob/master/docs/api/version-history.md. This change will be in API 1.39, so either a new entry should be added for that version, or that would come with #37472 (once merged)

codecov · 2018-08-16T14:43:10Z

Codecov Report

❗ No coverage uploaded for pull request base (master@2629fe9). Click here to learn what that means.
The diff coverage is 0%.

@@            Coverage Diff            @@
##             master   #37650   +/-   ##
=========================================
  Coverage          ?   36.13%           
=========================================
  Files             ?      610           
  Lines             ?    45056           
  Branches          ?        0           
=========================================
  Hits              ?    16282           
  Misses            ?    26531           
  Partials          ?     2243

anshulpundir · 2018-08-16T23:43:40Z

addressed comments from @thaJeztah

thaJeztah · 2018-08-17T21:45:20Z

daemon/cluster/filters.go

looks like this should be singular

thaJeztah · 2018-08-17T21:45:50Z

docs/api/version-history.md

typo s/ilter/filter/

also s/label/node-label/

thaJeztah · 2018-08-17T21:48:59Z

api/swagger.yaml

also thinking if this should be node.label (full stop, instead of hyphen), as was proposed in #28209

ping @vdemeester @dperny?

andrewhsu · 2018-08-20T15:29:59Z

I'd suggest ignoring the z errors because we've had intermittent networking issues with our z infrastructure.

Downloading 'library/busybox:glibc@sha256:0b55a30394294ab23b9afd58fab94e61a923f5834fba7ddbae7f8e0c11ba85e6' (1 layers)...
curl: (28) Operation timed out after 300521 milliseconds with 0 out of 0 bytes received

thaJeztah · 2018-08-20T16:24:35Z

api/swagger.yaml

hm, looks like you changed the example value, instead of the "key"; we could discuss quickly, but I think the idea in #28209 was to have --filter node.label=foobar (so node.label instead of node-label)

@vdemeester ^^

Yeah, to have node.label, engine.label and label. The dot . was maintly to be consistent with constraint if I rembember correctly 👼

Ah; yes; that was the reason

thaJeztah · 2018-08-20T16:24:50Z

daemon/cluster/filters.go

same here (and other places below)

thaJeztah · 2018-08-20T16:25:53Z

docs/api/version-history.md

The examples here still don't match; e.g. label=<key>=<value> should be either node-label=<key>=<value> or node.label=<key>=<value>

thaJeztah

LGTM, thanks!

andrewhsu

LGTM

ignore the z error:

curl: (56) SSL read: error:00000000:lib(0):func(0):reason(0), errno 104

because z infrastructure issues

thaJeztah · 2018-08-21T00:00:52Z

Failure on experimental (think that's a flaky)

19:17:48 FAIL: docker_api_swarm_test.go:296: DockerSwarmSuite.TestAPISwarmLeaderElection
19:17:48 
19:17:48 [dfbd0fadafb65] waiting for daemon to start
19:17:48 [dfbd0fadafb65] daemon started
19:17:48 
19:17:48 [d96abff1621cb] waiting for daemon to start
19:17:48 [d96abff1621cb] daemon started
19:17:48 
19:17:48 [df10c3972bb8f] waiting for daemon to start
19:17:48 [df10c3972bb8f] daemon started
19:17:48 
19:17:48 [dfbd0fadafb65] exiting daemon
19:17:48 assertion failed: error is not nil: Error response from daemon: rpc error: code = DeadlineExceeded desc = context deadline exceeded
19:17:48 [d96abff1621cb] exiting daemon
19:17:48 [df10c3972bb8f] exiting daemon

…ring on node labels. Signed-off-by: Anshul Pundir <anshul.pundir@docker.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah · 2018-08-21T20:19:29Z

Dropped the swarmkit bump commit, because master is now ahead of that commit, and fixed the merge-conflict in docs/api/version-history.md

thaJeztah · 2018-08-21T22:28:50Z

Argh. Janky failing again on that swagger-check; #36714. I'll restart, but we can likely look at the "experimental" build as well (which should run the same tests)

anshulpundir · 2018-08-21T23:55:05Z

thx adjusting this PR! @thaJeztah

GordonTheTurtle added the status/0-triage label Aug 15, 2018

yongtang added status/2-code-review and removed status/0-triage labels Aug 15, 2018

yongtang approved these changes Aug 15, 2018

View reviewed changes

thaJeztah added impact/changelog area/swarm rebuild/z labels Aug 16, 2018

GordonTheTurtle removed the rebuild/z label Aug 16, 2018

thaJeztah requested changes Aug 16, 2018

View reviewed changes

thaJeztah added the impact/api label Aug 16, 2018

anshulpundir force-pushed the vndr branch from 67cddb5 to 6b2f444 Compare August 16, 2018 23:42

thaJeztah requested changes Aug 17, 2018

View reviewed changes

thaJeztah mentioned this pull request Aug 18, 2018

Global Default Address Pool feature support #37558

Merged

anshulpundir force-pushed the vndr branch from 6b2f444 to b3dacbe Compare August 20, 2018 00:57

thaJeztah requested changes Aug 20, 2018

View reviewed changes

anshulpundir force-pushed the vndr branch from b3dacbe to c42a514 Compare August 20, 2018 17:59

thaJeztah approved these changes Aug 20, 2018

View reviewed changes

thaJeztah added status/4-merge and removed status/2-code-review labels Aug 20, 2018

andrewhsu approved these changes Aug 20, 2018

View reviewed changes

thaJeztah mentioned this pull request Aug 21, 2018

Use warnings provided by daemon docker/cli#1225

Merged

thaJeztah changed the title ~~Bump SwarmKit to 496d19be63751d60a55887604683e5cb1a5541aa~~ Add support for filtering on node labels Aug 21, 2018

Changes to cluster/filter, swagger.yaml, version-history.md for filte…

514ce73

…ring on node labels. Signed-off-by: Anshul Pundir <anshul.pundir@docker.com> Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

thaJeztah force-pushed the vndr branch from c42a514 to 514ce73 Compare August 21, 2018 20:18

thaJeztah added the rebuild/janky label Aug 21, 2018

GordonTheTurtle removed the rebuild/janky label Aug 21, 2018

vdemeester added the rebuild/powerpc label Aug 22, 2018

GordonTheTurtle removed the rebuild/powerpc label Aug 22, 2018

thaJeztah merged commit a0385f7 into moby:master Aug 22, 2018

thaJeztah mentioned this pull request Oct 26, 2018

Update API version to v1.40 #38089

Merged

Drewster727 mentioned this pull request Aug 5, 2019

add npipe support to docker_swarm_service ansible/ansible#60073

Merged

thaJeztah mentioned this pull request Jan 26, 2021

Filtering nodes doesn't return a result #27231

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for filtering on node labels #37650

Add support for filtering on node labels #37650

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

	func newListNodesFilters(filter filters.Args) (*swarmapi.ListNodesRequest_Filters, error) {
	accepted := map[string]bool{
	"name": true,
	"id": true,
	"label": true,
	"role": true,
	"membership": true,
	}
	if err := filter.Validate(accepted); err != nil {
	return nil, err
	}
	f := &swarmapi.ListNodesRequest_Filters{
	NamePrefixes: filter.Get("name"),
	IDPrefixes: filter.Get("id"),
	Labels: runconfigopts.ConvertKVStringsToMap(filter.Get("label")),
	}

	for _, r := range filter.Get("role") {
	if role, ok := swarmapi.NodeRole_value[strings.ToUpper(r)]; ok {
	f.Roles = append(f.Roles, swarmapi.NodeRole(role))
	} else if r != "" {
	return nil, fmt.Errorf("Invalid role filter: '%s'", r)
	}
	}

	for _, a := range filter.Get("membership") {
	if membership, ok := swarmapi.NodeSpec_Membership_value[strings.ToUpper(a)]; ok {
	f.Memberships = append(f.Memberships, swarmapi.NodeSpec_Membership(membership))
	} else if a != "" {
	return nil, fmt.Errorf("Invalid membership filter: '%s'", a)
	}
	}

	return f, nil
	}

Add support for filtering on node labels #37650

Add support for filtering on node labels #37650

Uh oh!

Conversation

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Choose a reason for hiding th 8000 is comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!