Skip to content

pkg/mount unit tests: skip some test under non-root#36505

Merged
AkihiroSuda merged 1 commit intomoby:masterfrom
kolyshkin:pkg-mount-tests
Mar 8, 2018
Merged

pkg/mount unit tests: skip some test under non-root#36505
AkihiroSuda merged 1 commit intomoby:masterfrom
kolyshkin:pkg-mount-tests

Conversation

@kolyshkin
Copy link
Copy Markdown
Contributor

@kolyshkin kolyshkin commented Mar 6, 2018

This makes go test . to pass if run as non-root user, skipping those tests that require superuser privileges (for mount).

(this used to be part of #36091; separated for the sake of cleaner review)

@kolyshkin
pkg/mount unit tests: skip some test under non-root
4aae776 
This makes `go test .` to pass if run as non-root user, skipping
those tests that require superuser privileges (for `mount`).

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
thaJeztah
thaJeztah approved these changes Mar 6, 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

@kolyshkin kolyshkin mentioned this pull request Mar 6, 2018
Merged
vdemeester
vdemeester approved these changes Mar 7, 2018
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

thaJeztah commented Mar 7, 2018

Janky failing on:

09:33:05 ----------------------------------------------------------------------
09:33:05 FAIL: docker_api_swarm_service_test.go:32: DockerSwarmSuite.TestAPIServiceUpdatePort
09:33:05 
09:33:05 [d8f875eac4559] waiting for daemon to start
09:33:05 [d8f875eac4559] daemon started
09:33:05 
09:33:05 docker_api_swarm_service_test.go:38:
09:33:05     waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1)
09:33:05 docker_utils_test.go:465:
09:33:05     c.Assert(v, checker, args...)
09:33:05 ... obtained int = 0
09:33:05 ... expected int = 1
09:33:05 
09:33:05 [d8f875eac4559] exiting daemon
09:33:17 
09:33:17 ----------------------------------------------------------------------
09:33:17 PASS: docker_api_swarm_test.go:179: DockerSwarmSuite.TestAPISwarmCAHash	12.030s
09:33:17 PASS: docker_api_swarm_test.go:868: DockerSwarmSuite.TestAPISwarmErrorHandling	0.740s
09:33:42 PASS: docker_api_swarm_test.go:533: DockerSwarmSuite.TestAPISwarmForceNewCluster	13.497s
09:33:42 SKIP: docker_api_swarm_test.go:912: DockerSwarmSuite.TestAPISwarmHealthcheckNone (Root cause of Issue #36386 is needed)
09:34:00 PASS: docker_api_swarm_test.go:35: DockerSwarmSuite.TestAPISwarmInit	13.398s
09:34:01 PASS: docker_api_swarm_test.go:515: DockerSwarmSuite.TestAPISwarmInvalidAddress	0.727s
09:34:49 PASS: docker_api_swarm_test.go:83: DockerSwarmSuite.TestAPISwarmJoinToken	47.576s
09:35:25 PASS: docker_api_swarm_test.go:287: DockerSwarmSuite.TestAPISwarmLeaderElection	10.933s
09:35:51 
09:35:51 ----------------------------------------------------------------------
09:35:51 FAIL: check_test.go:366: DockerSwarmSuite.TearDownTest
09:35:51 
09:35:51 unmount of /tmp/docker-execroot/dc6c9253fbcee/netns failed: invalid argument
09:35:51 check_test.go:371:
09:35:51     d.Stop(c)
09:35:51 daemon/daemon.go:389:
09:35:51     t.Fatalf("Error while stopping the daemon %s : %v", d.id, err)
09:35:51 ... Error: Error while stopping the daemon d1594f6dcded7 : exit status 130
09:35:51 
09:35:51 
09:35:51 ----------------------------------------------------------------------
09:35:51 PANIC: docker_api_swarm_test.go:262: DockerSwarmSuite.TestAPISwarmLeaderProxy
09:35:51 
09:35:51 [dc6c9253fbcee] waiting for daemon to start
09:35:51 [dc6c9253fbcee] daemon started
09:35:51 
09:35:51 [d1594f6dcded7] waiting for daemon to start
09:35:51 [d1594f6dcded7] daemon started
09:35:51 
09:35:51 [d48336733a460] waiting for daemon to start
09:35:51 [d48336733a460] daemon started
09:35:51 
09:35:51 [dc6c9253fbcee] exiting daemon
09:35:51 [d1594f6dcded7] daemon started
09:35:51 Attempt #2: daemon is still running with pid 9926
09:35:51 Attempt #3: daemon is still running with pid 9926
09:35:51 Attempt #4: daemon is still running with pid 9926
09:35:51 [d1594f6dcded7] exiting daemon
09:35:51 ... Panic: Fixture has panicked (see related PANIC)
09:35:51

Which is a flaky test, and is being tracked through #36501

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Mar 7, 2018

Also looking at;

unmount of /tmp/docker-execroot/dc6c9253fbcee/netns failed: invalid argument

which comes from

moby/integration-cli/daemon/daemon_unix.go

Lines 13 to 27 in 3a633a7

func cleanupExecRoot(c *check.C, execRoot string) {
// Cleanup network namespaces in the exec root of this
// daemon because this exec root is specific to this
// daemon instance and has no chance of getting
// cleaned up when a new daemon is instantiated with a
// new exec root.
netnsPath := filepath.Join(execRoot, "netns")
filepath.Walk(netnsPath, func(path string, info os.FileInfo, err error) error {
if err := unix.Unmount(path, unix.MNT_FORCE); err != nil {
c.Logf("unmount of %s failed: %v", path, err)
}
os.Remove(path)
return nil
})
}

Reading the UNMOUNT(2) manpage;

 EINVAL target is not a mount point.

 EINVAL umount2() was called with MNT_EXPIRE and either MNT_DETACH or
        MNT_FORCE.

 EINVAL (since Linux 2.6.34)
        umount2() was called with an invalid flag value in flags.

Also reading:

 MNT_FORCE (since Linux 2.1.116)
        Ask the filesystem to abort pending requests before attempting
        the unmount.  This may allow the unmount to complete without
        waiting for an inaccessible server, but could cause data loss.
        If, after aborting requests, some processes still have active
        references to the filesystem, the unmount will still fail.  As
        at Linux 4.12, MNT_FORCE is supported only on the following
        filesystems: 9p (since Linux 2.6.16), ceph (since Linux
        2.6.34), cifs (since Linux 2.6.12), fuse (since Linux 2.6.16),
        lustre (since Linux 3.11), and NFS (since Linux 2.1.116).

.. MNT_FORCE is supported only on 9p, ceph, cifs, fuse, lustre, and NFS

Should we just ignore EINVAL there?

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Mar 7, 2018

Or should we remove the MNT_FORCE?

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Mar 7, 2018

Opened #36511 for discussion

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 8, 2018 •
edited
Loading

Codecov Report

Merging #36505 into master will increase coverage by 0.42%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #36505      +/-   ##
==========================================
+ Coverage   34.64%   35.06%   +0.42%     
==========================================
  Files         613      613              
  Lines       45404    45574     +170     
==========================================
+ Hits        15730    15982     +252     
+ Misses      27614    27483     -131     
- Partials     2060     2109      +49

@AkihiroSuda AkihiroSuda merged commit 4a1d35c into moby:master Mar 8, 2018
@kolyshkin kolyshkin deleted the pkg-mount-tests branch March 13, 2018 01:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

+1 more reviewer

@vdemeester vdemeester vdemeester approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

status/4-merge

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@kolyshkin @thaJeztah @vdemeester @AkihiroSuda @yongtang @GordonTheTurtle