Skip to content
This repository was archived by the owner on Oct 13, 2023. It is now read-only.

[17.06] cherry-pick Fix prefix-matching for service ps#77

Closed
andrewhsu wants to merge 6 commits into
docker-archive:17.06from
andrewhsu:ps
Closed

[17.06] cherry-pick Fix prefix-matching for service ps#77
andrewhsu wants to merge 6 commits into
docker-archive:17.06from
andrewhsu:ps

Conversation

@andrewhsu
Copy link
Copy Markdown
Contributor

Cherry-pick of:

thaJeztah and others added 3 commits June 14, 2017 00:26
The docker CLI matches objects either by ID _prefix_
or a full name match, but not partial name matches.

The correct order of resolution is;

- Full ID match (a name should not be able to mask an ID)
- Full name
- ID-prefix

This patch changes the way services are matched.

Also change to use the first matching service, if there's a
full match (by ID or Name) instead of continue looking for
other possible matches.

Error handling changed;

- Do not error early if multiple services were requested
  and one or more services were not found. Print the
  services that were not found after printing those that
  _were_ found instead
- Print an error if ID-prefix matching is ambiguous

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit 62796124432c7e56e7dda226c3c53c8c2356a30c)
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit b5baffde44cf16bc2eb030c09da76d39abb62252)
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
(cherry picked from commit 3718833f2cecde35ae1ed007a9b2d5bf507c7c66)
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
@andrewhsu andrewhsu mentioned this pull request Jun 14, 2017
2 tasks
@andrewhsu
Copy link
Copy Markdown
Contributor Author

Seeing failure in test result:

01:00:52.064 [DockerSwarmSuite] FAIL: docker_cli_swarm_test.go:1690: DockerSwarmSuite.TestSwarmServicePsMultipleServiceIDs
01:00:52.064 [DockerSwarmSuite] 
01:00:52.064 [DockerSwarmSuite] [d586526176fbf] waiting for daemon to start
01:00:52.064 [DockerSwarmSuite] [d586526176fbf] daemon started
01:00:52.064 [DockerSwarmSuite] 
01:00:52.064 [DockerSwarmSuite] docker_cli_swarm_test.go:1728:
01:00:52.064 [DockerSwarmSuite]     c.Assert(err, checker.IsNil)
01:00:52.064 [DockerSwarmSuite] ... value *exec.ExitError = &exec.ExitError{ProcessState:(*os.ProcessState)(0xc420218000), Stderr:[]uint8(nil)} ("exit status 1")
01:00:52.064 [DockerSwarmSuite] 
01:00:52.064 [DockerSwarmSuite] [d586526176fbf] exiting daemon

@dnephin
Copy link
Copy Markdown
Contributor

dnephin commented Jun 14, 2017

I fixed the test to use the proper exec helpers and I can't reproduce the failure, so I guess it's a flake. I'll push the test changes to this branch just in case it isn't.

Signed-off-by: Daniel Nephin <dnephin@gmail.com>
(cherry picked from commit 7e135e0)
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
@thaJeztah
Copy link
Copy Markdown
Member

@dnephin should those fixes be backported to moby/moby?

@dnephin
Copy link
Copy Markdown
Contributor

dnephin commented Jun 14, 2017

Yes, PR already open: moby/moby#33684 😄

@thaJeztah
Copy link
Copy Markdown
Member

Ugh; CI needs a kick;

22:34:47 [init] curl: (22) The requested URL returned error: 400 Bad Request
22:34:47 [init] The command '/bin/sh -c ./contrib/download-frozen-image-v2.sh /docker-frozen-images 	buildpack-

@andrewhsu
Copy link
Copy Markdown
Contributor Author

@dnephin with the test debug commit, failed at the same test:

00:57:59.454 FAIL: docker_cli_swarm_test.go:1690: DockerSwarmSuite.TestSwarmServicePsMultipleServiceIDs
00:57:59.454 
00:57:59.454 [d62ec432a3a1c] waiting for daemon to start
00:57:59.454 [d62ec432a3a1c] daemon started
00:57:59.454 
00:57:59.454 docker_cli_swarm_test.go:1730:
00:57:59.454     result.Assert(c, icmd.Success)
00:57:59.454 /go/src/github.com/docker/docker/pkg/testutil/cmd/command.go:64:
00:57:59.454     t.Fatalf("at %s:%d - %s", filepath.Base(file), line, err.Error())
00:57:59.454 ... Error: at docker_cli_swarm_test.go:1730 - 
00:57:59.454 Command:  /usr/local/cli/docker --host unix:///tmp/docker-integration/d62ec432a3a1c.sock service ps to
00:57:59.454 ExitCode: 1
00:57:59.454 Error:    exit status 1
00:57:59.454 Stdout:   
00:57:59.454 Stderr:   no such service: to
00:57:59.454 
00:57:59.454 
00:57:59.454 Failures:
00:57:59.454 ExitCode was 1 expected 0
00:57:59.454 Expected no error
00:57:59.454 
00:57:59.454 
00:57:59.454 [d62ec432a3a1c] exiting daemon

@thaJeztah
Copy link
Copy Markdown
Member

I suspect that we need the integration test updates from moby/moby#32800, because current tests test for the faulty behaviour 😞

@dnephin
Copy link
Copy Markdown
Contributor

dnephin commented Jun 15, 2017

Test fixed in dnephin@09b3718 I think.

@GordonTheTurtle
Copy link
Copy Markdown
Contributor

Please sign your commits following these rules:
https://github.com/moby/moby/blob/master/CONTRIBUTING.md#sign-your-work
The easiest way to do this is to amend the last commit:

$ git clone -b "ps" git@github.com:andrewhsu/docker-ce.git somewhere
$ cd somewhere
$ git rebase -i HEAD~842354081488
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f

Amending updates the existing PR. You DO NOT need to open a new one.

andrewhsu and others added 2 commits June 15, 2017 01:10
…xec helper."

This reverts commit 7beb1f7.

Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
Signed-off-by: Daniel Nephin <dnephin@gmail.com>
(cherry picked from commit 09b3718)
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
@andrewhsu
Copy link
Copy Markdown
Contributor Author

@thaJeztah punting this to 17.06.1

@dnephin i think we talked about this one earlier about splitting out the test that failed into an api test instead of an integration-cli test. would also desire a cherry-pick of something that is already in master branch of moby/moby.

@dnephin
Copy link
Copy Markdown
Contributor

dnephin commented Jun 20, 2017

I tried... moby/moby#33684

First I applied this patch to moby/moby, but @thaJeztah pointed out that it doesn't make sense to merge this fix into master, because it shouldn't be testing with the CLI anymore. Now it's just an API test, but it's being met with some resistance.

@vieux vieux changed the title cherry-pick Fix prefix-matching for service ps [17.06] cherry-pick Fix prefix-matching for service ps Jul 27, 2017
@vieux vieux added this to the 17.06.1 milestone Jul 27, 2017
@andrerom
Copy link
Copy Markdown

andrerom commented Aug 17, 2017

Somewhat related given the resistance/test-failures: Any chance of 17.06.1 being releases soon? kind of solves a few blockers, so if this is not one, maybe it can be moved to an 17.06.2 milestone given .1 is already in very late rc stage?

@andrewhsu andrewhsu modified the milestones: 17.06.1, 17.06.2 Aug 18, 2017
@andrewhsu andrewhsu removed this from the 17.06.2 milestone Nov 7, 2017
@andrewhsu
Copy link
Copy Markdown
Contributor Author

Closing this PR because the docker-ce 17.06 code-line is EoL.

@andrewhsu andrewhsu closed this Nov 7, 2017
@andrewhsu andrewhsu deleted the ps branch November 7, 2017 22:48
docker-jenkins pushed a commit that referenced this pull request Jan 23, 2018
Add pigz to recommended packages
Upstream-commit: a6543ce
Component: packaging
andrewhsu added a commit that referenced this pull request Apr 26, 2018
[18.03] update changelog for 18.03.1-ce
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Jan 30, 2020
Add pigz to recommended packages
Upstream-commit: a6543ce
Component: packaging
silvin-lubecki pushed a commit to silvin-lubecki/docker-ce that referenced this pull request Jan 30, 2020
Add pigz to recommended packages
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants