From 0b0373d436c8ec8d45fbf5fd5b882d20a9a83ae6 Mon Sep 17 00:00:00 2001 From: Anil Belur Date: Mon, 7 Dec 2015 17:29:22 +0530 Subject: [PATCH] Fixes issue #2310 - machine ls --filter with engine label * Added a new members `Labels` to `FilterOptions struct`, and `EngineOptions` to `HostListItem struct`. `HostListItems` is already being read from the file store `config.json` which TestFilterHostsReturnSetLabel engine labels. * Modified `parseFilters()` and added new `func matchesLabel(`) which compares the one or more label values provided as input. * Also the changes include added tests for UT and integration. Note: Have kept `--filter label==` syntax which is similar to `docker images --filter` to keep the usability consistent. review comments from @dgageot Signed-off-by: Anil Belur --- commands/ls.go | 37 ++++++++++++++++++++++++++++++++++-- commands/ls_test.go | 31 ++++++++++++++++++++++++++++-- test/integration/cli/ls.bats | 37 ++++++++++++++++++++++++++++++------ 3 files changed, 95 insertions(+), 10 deletions(-) diff --git a/commands/ls.go b/commands/ls.go index 1a30b68f51..7e73d594f9 100644 --- a/commands/ls.go +++ b/commands/ls.go @@ -12,6 +12,7 @@ import ( "github.com/docker/machine/libmachine" "github.com/docker/machine/libmachine/drivers" + "github.com/docker/machine/libmachine/engine" "github.com/docker/machine/libmachine/host" "github.com/docker/machine/libmachine/log" "github.com/docker/machine/libmachine/persist" @@ -30,6 +31,7 @@ type FilterOptions struct { DriverName []string State []string Name []string + Labels []string } type HostListItem struct { @@ -39,6 +41,7 @@ type HostListItem struct { State state.State URL string SwarmOptions *swarm.Options + EngineOptions *engine.Options Error string DockerVersion string } @@ -125,6 +128,8 @@ func parseFilters(filters []string) (FilterOptions, error) { options.State = append(options.State, value) case "name": options.Name = append(options.Name, value) + case "label": + options.Labels = append(options.Labels, value) default: return options, fmt.Errorf("Unsupported filter key '%s'", key) } @@ -136,7 +141,8 @@ func filterHosts(hosts []*host.Host, filters FilterOptions) []*host.Host { if len(filters.SwarmName) == 0 && len(filters.DriverName) == 0 && len(filters.State) == 0 && - len(filters.Name) == 0 { + len(filters.Name) == 0 && + len(filters.Labels) == 0 { return hosts } @@ -167,8 +173,9 @@ func filterHost(host *host.Host, filters FilterOptions, swarmMasters map[string] driverMatches := matchesDriverName(host, filters.DriverName) stateMatches := matchesState(host, filters.State) nameMatches := matchesName(host, filters.Name) + labelMatches := matchesLabel(host, filters.Labels) - return swarmMatches && driverMatches && stateMatches && nameMatches + return swarmMatches && driverMatches && stateMatches && nameMatches && labelMatches } func matchesSwarmName(host *host.Host, swarmNames []string, swarmMasters map[string]string) bool { @@ -230,6 +237,29 @@ func matchesName(host *host.Host, names []string) bool { return false } +func matchesLabel(host *host.Host, labels []string) bool { + if len(labels) == 0 { + return true + } + + var englabels = make(map[string]string, len(host.HostOptions.EngineOptions.Labels)) + + if host.HostOptions != nil && host.HostOptions.EngineOptions.Labels != nil { + for _, s := range host.HostOptions.EngineOptions.Labels { + kv := strings.SplitN(s, "=", 2) + englabels[kv[0]] = kv[1] + } + } + + for _, l := range labels { + kv := strings.SplitN(l, "=", 2) + if val, exists := englabels[kv[0]]; exists && strings.EqualFold(val, kv[1]) { + return true + } + } + return false +} + func attemptGetHostState(h *host.Host, stateQueryChan chan<- HostListItem) { url := "" hostError := "" @@ -256,8 +286,10 @@ func attemptGetHostState(h *host.Host, stateQueryChan chan<- HostListItem) { } var swarmOptions *swarm.Options + var engineOptions *engine.Options if h.HostOptions != nil { swarmOptions = h.HostOptions.SwarmOptions + engineOptions = h.HostOptions.EngineOptions } stateQueryChan <- HostListItem{ @@ -267,6 +299,7 @@ func attemptGetHostState(h *host.Host, stateQueryChan chan<- HostListItem) { State: currentState, URL: url, SwarmOptions: swarmOptions, + EngineOptions: engineOptions, DockerVersion: dockerVersion, Error: hostError, } diff --git a/commands/ls_test.go b/commands/ls_test.go index 2b3ffdd5fa..39d4d5a040 100644 --- a/commands/ls_test.go +++ b/commands/ls_test.go @@ -9,6 +9,7 @@ import ( "errors" "github.com/docker/machine/drivers/fakedriver" + "github.com/docker/machine/libmachine/engine" "github.com/docker/machine/libmachine/host" "github.com/docker/machine/libmachine/mcndockerclient" "github.com/docker/machine/libmachine/state" @@ -41,14 +42,20 @@ func TestParseFiltersName(t *testing.T) { assert.Equal(t, actual, FilterOptions{Name: []string{"dev"}}) } +func TestParseFiltersLabel(t *testing.T) { + actual, err := parseFilters([]string{"label=com.example.foo=bar"}) + assert.EqualValues(t, actual, FilterOptions{Labels: []string{"com.example.foo=bar"}}) + assert.Nil(t, err, "returned err value must be Nil") +} + func TestParseFiltersAll(t *testing.T) { actual, _ := parseFilters([]string{"swarm=foo", "driver=bar", "state=Stopped", "name=dev"}) assert.Equal(t, actual, FilterOptions{SwarmName: []string{"foo"}, DriverName: []string{"bar"}, State: []string{"Stopped"}, Name: []string{"dev"}}) } func TestParseFiltersAllCase(t *testing.T) { - actual, err := parseFilters([]string{"sWarM=foo", "DrIver=bar", "StaTe=Stopped", "NAMe=dev"}) - assert.Equal(t, actual, FilterOptions{SwarmName: []string{"foo"}, DriverName: []string{"bar"}, State: []string{"Stopped"}, Name: []string{"dev"}}) + actual, err := parseFilters([]string{"sWarM=foo", "DrIver=bar", "StaTe=Stopped", "NAMe=dev", "LABEL=com=foo"}) + assert.Equal(t, actual, FilterOptions{SwarmName: []string{"foo"}, DriverName: []string{"bar"}, State: []string{"Stopped"}, Name: []string{"dev"}, Labels: []string{"com=foo"}}) assert.Nil(t, err, "err should be nil") } @@ -67,6 +74,7 @@ func TestFilterHostsReturnsFiltersValuesCaseInsensitive(t *testing.T) { SwarmName: []string{"fOo"}, DriverName: []string{"ViRtUaLboX"}, State: []string{"StOPpeD"}, + Labels: []string{"com.EXAMPLE.app=FOO"}, } hosts := []*host.Host{} actual := filterHosts(hosts, opts) @@ -85,6 +93,25 @@ func TestFilterHostsReturnsSameGivenNoFilters(t *testing.T) { assert.EqualValues(t, actual, hosts) } +func TestFilterHostsReturnSetLabel(t *testing.T) { + opts := FilterOptions{ + Labels: []string{"com.class.foo=bar"}, + } + hosts := []*host.Host{ + { + Name: "testhost", + DriverName: "fakedriver", + HostOptions: &host.Options{ + EngineOptions: &engine.Options{ + Labels: []string{"com.class.foo=bar"}, + }, + }, + }, + } + actual := filterHosts(hosts, opts) + assert.EqualValues(t, actual, hosts) +} + func TestFilterHostsReturnsEmptyGivenEmptyHosts(t *testing.T) { opts := FilterOptions{ SwarmName: []string{"foo"}, diff --git a/test/integration/cli/ls.bats b/test/integration/cli/ls.bats index e1e2eb81c6..a57c952078 100644 --- a/test/integration/cli/ls.bats +++ b/test/integration/cli/ls.bats @@ -3,6 +3,8 @@ load ${BASE_TEST_DIR}/helpers.bash setup () { + machine create -d none --url none --engine-label app=1 testmachine5 + machine create -d none --url none --engine-label foo=bar --engine-label app=1 testmachine4 machine create -d none --url none testmachine3 machine create -d none --url none testmachine2 machine create -d none --url none testmachine @@ -19,19 +21,42 @@ bootstrap_swarm () { machine create -d none --url tcp://127.0.0.1:2375 --swarm --swarm-discovery token://deadbeef testswarm3 } +@test "ls: filter on label 'machine ls --filter label=foo=bar'" { + run machine ls --filter label=foo=bar + [ "$status" -eq 0 ] + [[ ${#lines[@]} == 2 ]] + [[ ${lines[1]} =~ "testmachine4" ]] +} + +@test "ls: mutiple filters on label 'machine ls --filter label=foo=bar --filter label=app=1'" { + run machine ls --filter label=foo=bar --filter label=app=1 + [ "$status" -eq 0 ] + [[ ${#lines[@]} == 3 ]] + [[ ${lines[1]} =~ "testmachine4" ]] + [[ ${lines[2]} =~ "testmachine5" ]] +} + +@test "ls: non-existing filter on label 'machine ls --filter label=invalid=filter'" { + run machine ls --filter label=invalid=filter + [ "$status" -eq 0 ] + [[ ${#lines[@]} == 1 ]] +} + @test "ls: filter on driver 'machine ls --filter driver=none'" { run machine ls --filter driver=none [ "$status" -eq 0 ] - [[ ${#lines[@]} == 4 ]] + [[ ${#lines[@]} == 6 ]] [[ ${lines[1]} =~ "testmachine" ]] [[ ${lines[2]} =~ "testmachine2" ]] [[ ${lines[3]} =~ "testmachine3" ]] + [[ ${lines[4]} =~ "testmachine4" ]] + [[ ${lines[5]} =~ "testmachine5" ]] } @test "ls: filter on driver 'machine ls -q --filter driver=none'" { run machine ls -q --filter driver=none [ "$status" -eq 0 ] - [[ ${#lines[@]} == 3 ]] + [[ ${#lines[@]} == 5 ]] [[ ${lines[0]} == "testmachine" ]] [[ ${lines[1]} == "testmachine2" ]] [[ ${lines[2]} == "testmachine3" ]] @@ -41,7 +66,7 @@ bootstrap_swarm () { # Default state for 'none' driver is "Running" run machine ls --filter state="Running" [ "$status" -eq 0 ] - [[ ${#lines[@]} == 4 ]] + [[ ${#lines[@]} == 6 ]] [[ ${lines[1]} =~ "testmachine" ]] [[ ${lines[2]} =~ "testmachine2" ]] [[ ${lines[3]} =~ "testmachine3" ]] @@ -73,7 +98,7 @@ bootstrap_swarm () { @test "ls: filter on state 'machine ls -q --filter state=\"Running\"'" { run machine ls -q --filter state="Running" [ "$status" -eq 0 ] - [[ ${#lines[@]} == 3 ]] + [[ ${#lines[@]} == 5 ]] [[ ${lines[0]} == "testmachine" ]] [[ ${lines[1]} == "testmachine2" ]] [[ ${lines[2]} == "testmachine3" ]] @@ -96,7 +121,7 @@ bootstrap_swarm () { @test "ls: filter on name with regex 'machine ls --filter name=\"^t.*e\"'" { run machine ls --filter name="^t.*e" [ "$status" -eq 0 ] - [[ ${#lines[@]} == 4 ]] + [[ ${#lines[@]} == 6 ]] [[ ${lines[1]} =~ "testmachine" ]] [[ ${lines[2]} =~ "testmachine2" ]] [[ ${lines[3]} =~ "testmachine3" ]] @@ -105,7 +130,7 @@ bootstrap_swarm () { @test "ls: filter on name with regex 'machine ls -q --filter name=\"^t.*e\"'" { run machine ls -q --filter name="^t.*e" [ "$status" -eq 0 ] - [[ ${#lines[@]} == 3 ]] + [[ ${#lines[@]} == 5 ]] [[ ${lines[0]} == "testmachine" ]] [[ ${lines[1]} == "testmachine2" ]] [[ ${lines[2]} == "testmachine3" ]]