Skip to content

Return "invalid parameter" when linking to non-existing container#39852

Merged
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:detect_invalid_linked_container
Sep 11, 2019
Merged

Return "invalid parameter" when linking to non-existing container#39852
thaJeztah merged 1 commit intomoby:masterfrom
thaJeztah:detect_invalid_linked_container

Conversation

@thaJeztah
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah commented Sep 2, 2019

fixes #39823
fixes docker/for-mac#3871

Trying to link to a non-existing container is not valid, and should return an
"invalid parameter" (400) error. Returning a "not found" error in this situation
would make the client report the container's image could not be found.

This was referenced Sep 2, 2019
Closed
Closed
@thaJeztah thaJeztah force-pushed the detect_invalid_linked_container branch from 12ddad2 to cf470e9 Compare September 2, 2019 22:14
@thaJeztah thaJeztah requested a review from vdemeester September 2, 2019 22:15
@thaJeztah thaJeztah force-pushed the detect_invalid_linked_container branch from cf470e9 to 2c3e001 Compare September 2, 2019 22:15
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Sep 3, 2019

Only failure is a flaky test (tracked through #38521)

[2019-09-02T22:52:41.080Z] FAIL: docker_cli_start_test.go:190: DockerSuite.TestStartReturnCorrectExitCode
[2019-09-02T22:52:41.080Z] 
[2019-09-02T22:52:41.080Z] assertion failed: expected an error, got nil

@thaJeztah
Return "invalid parameter" when linking to non-existing container
422067b 
Trying to link to a non-existing container is not valid, and should return an
"invalid parameter" (400) error. Returning a "not found" error in this situation
would make the client report the container's image could not be found.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah thaJeztah force-pushed the detect_invalid_linked_container branch from 2c3e001 to 422067b Compare September 10, 2019 21:07
@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Sep 10, 2019

ping @tiborvass @cpuguy83 PTAL

@thaJeztah thaJeztah mentioned this pull request Sep 10, 2019
Merged
tao12345666333
tao12345666333 approved these changes Sep 11, 2019
View reviewed changes
Copy link
Copy Markdown
Contributor

@tao12345666333 tao12345666333 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

vdemeester
vdemeester approved these changes Sep 11, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Copy Markdown
Member Author

thaJeztah commented Sep 11, 2019

Failure on RS1 is https://ci.docker.com/public/job/moby/job/PR-39852/5/execution/node/180/log/

which is known to be flaky (#36801), and for which I have a PR to skip it conditionally; #39856

23:48:42  --- FAIL: TestJSONFileLoggerWithOpts (0.01s)
23:48:42      jsonfilelog_test.go:187: open C:\Users\ContainerAdministrator\AppData\Local\Temp\docker-logger-996957197\container.log.1: The process cannot access the file because it is being used by another process.
23:48:42  FAIL

Everything else is green, so I'll go ahead and merge this one

@thaJeztah thaJeztah merged commit cd9e4ec into moby:master Sep 11, 2019
@thaJeztah thaJeztah deleted the detect_invalid_linked_container branch September 11, 2019 07:51
@thaJeztah thaJeztah added this to the 20.03.0 milestone Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tiborvass tiborvass tiborvass approved these changes

+2 more reviewers

@vdemeester vdemeester vdemeester approved these changes

@tao12345666333 tao12345666333 tao12345666333 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/api API area/daemon Core Engine process/cherry-picked status/2-code-review

Projects

None yet

Milestone

20.10.0

Development

Successfully merging this pull request may close these issues.

Docker run reports wrong image missing when using --link [BUG] --link to unexisting container yelds to pulling again image

5 participants

@thaJeztah @vdemeester @tiborvass @tao12345666333 @GordonTheTurtle