Skip to content

Fix daemon.getSourceMount() for /#37031

Merged
thaJeztah merged 1 commit intomoby:masterfrom
kolyshkin:getmnt
May 11, 2018
Merged

Fix daemon.getSourceMount() for /#37031
thaJeztah merged 1 commit intomoby:masterfrom
kolyshkin:getmnt

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin commented May 10, 2018 •
edited
Loading

A recent optimization (#36091) in getSourceMount() made it return an error in case when the source mount is "/". This prevented bind-mounted volumes from working in such cases.

Fixes: 871c957 ("getSourceMount(): simplify")

- What I did
Fixed getSourceMount() which I broke earlier

- How I did it
By removing the unnecessary and wrong check from the code

- How to verify it
I have added a unit test

- Description for the changelog
fix bind mounts not working in some cases

- A picture of a cute animal (not mandatory but encouraged)
squirrel

@kolyshkin
daemon.getSourceMount(): fix for / mount point
d8fd613 
A recent optimization in getSourceMount() made it return an error
in case when the found mount point is "/". This prevented bind-mounted
volumes from working in such cases.

A (rather trivial but adeqate) unit test case is added.

Fixes: 871c957 ("getSourceMount(): simplify")
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin kolyshkin mentioned this pull request May 10, 2018
Closed
@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented May 10, 2018

CI failure on z (logs here) appears to be unrelated; restarted. @selansen can you PTAL?

13:21:01 === RUN   TestCreateWithDuplicateNetworkNames
13:21:06 --- FAIL: TestCreateWithDuplicateNetworkNames (5.16s)
13:21:06 	daemon.go:285: [d8a021f676705] waiting for daemon to start
13:21:06 	daemon.go:317: [d8a021f676705] daemon started
13:21:06 	create_test.go:125: assertion failed: error is not nil: Error response from daemon: rpc error: code = FailedPrecondition desc = network l7rbpp0hjmeb84jsd7h7yoae2 is in use by task i0cz2xewiyqvj2buhf0wqrz9c
13:21:06 	daemon.go:275: [d8a021f676705] exiting daemon

cpuguy83
cpuguy83 approved these changes May 10, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@kolyshkin
Copy link
Copy Markdown
Contributor Author

kolyshkin commented May 10, 2018

CI failure on powerpc (logs here) looks unrelated; restarted

14:29:13 
14:29:13 ----------------------------------------------------------------------
14:29:13 FAIL: docker_api_swarm_test.go:359: DockerSwarmSuite.TestAPISwarmRaftQuorum
14:29:13 
14:29:13 [d45324fa9f762] waiting for daemon to start
14:29:13 [d45324fa9f762] daemon started
14:29:13 
14:29:13 [db5b610748059] waiting for daemon to start
14:29:13 [db5b610748059] daemon started
14:29:13 
14:29:13 [d9a744d8f6846] waiting for daemon to start
14:29:13 [d9a744d8f6846] daemon started
14:29:13 
14:29:13 [db5b610748059] exiting daemon
14:29:13 [d9a744d8f6846] exiting daemon
14:29:13 docker_api_swarm_test.go:385:
14:29:13     // d1 will eventually step down from leader because there is no longer an active quorum, wait for that to happen
14:29:13     waitAndAssert(c, defaultReconciliationTimeout, func(c *check.C) (interface{}, check.CommentInterface) {
14:29:13         _, err = cli.ServiceCreate(context.Background(), service.Spec, types.ServiceCreateOptions{})
14:29:13         return err.Error(), nil
14:29:13     }, checker.Contains, "Make sure more than half of the managers are online.")
14:29:13 docker_utils_test.go:435:
14:29:13     c.Assert(v, checker, args...)
14:29:13 ... obtained string = "Error response from daemon: rpc error: code = DeadlineExceeded desc = context deadline exceeded"
14:29:13 ... substring string = "Make sure more than half of the managers are online."
14:29:13 
14:29:13 [d45324fa9f762] exiting daemon

@codecov
Copy link
Copy Markdown

codecov bot commented May 11, 2018

Codecov Report

Merging #37031 into master will increase coverage by 0.25%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #37031      +/-   ##
==========================================
+ Coverage    35.1%   35.36%   +0.25%     
==========================================
  Files         615      615              
  Lines       45793    45791       -2     
==========================================
+ Hits        16077    16193     +116     
+ Misses      27606    27450     -156     
- Partials     2110     2148      +38

thaJeztah
thaJeztah approved these changes May 11, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah thaJeztah merged commit 379845e into moby:master May 11, 2018
@JTarasovic JTarasovic mentioned this pull request May 24, 2018
Closed
@pohly pohly mentioned this pull request Jul 25, 2018
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cpuguy83 cpuguy83 cpuguy83 approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

Assignees

No one assigned

Labels

status/2-code-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kolyshkin @cpuguy83 @thaJeztah @GordonTheTurtle