Use os.ReadDir instead of filepath.Walk in netns cleanup #36511

thaJeztah · 2018-03-07T13:09:26Z

Use ioutil.ReadDir() instead of filepath.Walk(), because
filepath.Walk() does a lot of extra things we don't need
Log errors on directory removal (omitting os.IsNotExist)
Touch-up error messages for consistency

thaJeztah · 2018-03-07T13:10:10Z

ping @kolyshkin @cpuguy83 @fcrisciani PTAL; not entirely sure if this is correct, but opened this to discuss 👍

integration-cli/daemon/daemon_unix.go

thaJeztah · 2018-03-07T14:08:25Z

hm, this one is flaky /cc @selansen

13:44:44 --- FAIL: TestServiceWithPredefinedNetwork (11.75s)
13:44:44 	daemon.go:283: [dce2b71fc535a] waiting for daemon to start
13:44:44 	daemon.go:315: [dce2b71fc535a] daemon started
13:44:44 	service_test.go:52: timeout hit after 10s: task count at 1 waiting for 0
13:44:44 	daemon.go:273: [dce2b71fc535a] exiting daemon

selansen · 2018-03-07T14:43:34Z

It didnt fail for me and number of instance can never be 0. will check it again.

…

On Wed, Mar 7, 2018 at 9:20 AM, Sebastiaan van Stijn < ***@***.***> wrote: hm, this one is flaky /cc @selansen <https://github.com/selansen> 13:44:44 --- FAIL: TestServiceWithPredefinedNetwork (11.75s) 13:44:44 daemon.go:283: [dce2b71fc535a] waiting for daemon to start 13:44:44 daemon.go:315: [dce2b71fc535a] daemon started 13:44:44 service_test.go:52: timeout hit after 10s: task count at 1 waiting for 0 13:44:44 daemon.go:273: [dce2b71fc535a] exiting daemon — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#36511 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKEKnxkS-TkdkLLhoQavJtqF7ovTSQ1qks5tb-omgaJpZM4Sgaqu> .

thaJeztah · 2018-03-07T15:05:45Z

@selansen @vdemeester pointed me that that message probably comes from

moby/integration/network/service_test.go

Line 52 in 0b0af85

poll.WaitOn(t, noTasks(client), pollSettings)

(so the "teardown" of the test)

selansen · 2018-03-07T17:20:26Z

Interesting . I didnt see this issue while I test and when CI ran against my PR. Let me see if I can reproduce it locally. Else will check for service instead of task and see if it helps.

…

On Wed, Mar 7, 2018 at 10:07 AM, Sebastiaan van Stijn < ***@***.***> wrote: @selansen <https://github.com/selansen> @vdemeester <https://github.com/vdemeester> pointed me that that message probably comes from https://github.com/moby/moby/blob/ 0b0af85/integration/ network/service_test.go#L52 (so the "teardown" of the test) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#36511 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKEKnwKsPehBF03viOHzRVHEF3UTTiW2ks5tb_e2gaJpZM4Sgaqu> .

kolyshkin · 2018-03-07T20:33:43Z

The MNT_FORCE flag comes from commit 99a98cc (PR #25962). The only other use of MNT_FORCE is in libnetwork (moby/libnetwork@8a6b507#diff-50ec864109e0293aa03f973d35485e1dR396), both are by @mrjana (Jana, maybe you can shed some light on why this option is used?)

FS type is nsfs in both cases. My limited testing shows the flag is at least accepted (i.e. no EINVAL or any other error is returned):

root@kd:~/git/linux# docker run -d busybox top
f24191dc7156767505b674041477f10208847b477dc9ef9f4b1395ed46a62aee
root@kd:~/git/linux# grep nsfs /proc/self/mountinfo /proc/self/mounts
/proc/self/mountinfo:576 26 0:3 net:[4026532644] /run/docker/netns/bf03800bf58e rw shared:271 - nsfs nsfs rw
/proc/self/mounts:nsfs /run/docker/netns/bf03800bf58e nsfs rw 0 0
root@kd:~/git/linux# strace -eumount2 umount -f /run/docker/netns/bf03800bf58e 
umount2("/run/docker/netns/bf03800bf58e", MNT_FORCE) = 0
+++ exited with 0 +++
root@kd:~/git/linux# uname -a
Linux kd 4.14.23+ #46 SMP Fri Mar 2 10:31:35 PST 2018 x86_64 x86_64 x86_64 GNU/Linux

From limited reading of linux kernel source code it seems MNT_FORCE is used by any fs which implements umount_begin and is ignored otherwise, and it's working that way since Apr 2008 (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=42faad99658ee). The umount_begin is defined by 9p, ceph, cifs, fuse, nfs, and lustre (as of linux 4.14).

To sum it up:

MNT_FORCE is useless (it does nothing for nsfs) but harmless (it does not cause an error) for this case. I am slightly in favor of removing it (as it gives a bad example).
Yes, both ENOENT and EINVAL should be ignored.

codecov · 2018-03-08T13:27:22Z

Codecov Report

❗ No coverage uploaded for pull request base (master@5a685dc). Click here to learn what that means.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master   #36511   +/-   ##
=========================================
  Coverage          ?   37.33%           
=========================================
  Files             ?      609           
  Lines             ?    45210           
  Branches          ?        0           
=========================================
  Hits              ?    16878           
  Misses            ?    26044           
  Partials          ?     2288

thaJeztah · 2018-03-08T13:30:07Z

@kolyshkin thanks for doing the research! I updated the PR, and captioned your information in the commit message 👍

thaJeztah · 2018-03-09T09:01:43Z

Opened #36547 for the flaky TestServiceWithPredefinedNetwork

selansen · 2018-03-09T17:05:14Z

trying to reproduce it. on top this issue now.

…

On Fri, Mar 9, 2018 at 4:02 AM, Sebastiaan van Stijn < ***@***.***> wrote: Opened #36547 <#36547> for the flaky TestServiceWithPredefinedNetwork — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#36511 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AKEKnzj9GGMwMx9WUIuHwjNdhTeMX7b9ks5tckU2gaJpZM4Sgaqu> .

thaJeztah · 2018-03-13T14:19:27Z

All of experimental, janky, powerpc and z are failing on DockerSwarmSuite.TestAPIServiceUpdatePort and DockerSwarmSuite.TestSwarmPublishDuplicatePorts 😞 #36501

thaJeztah · 2018-03-14T11:01:24Z

Looks like this one has the same issue with tasks not getting to zero;

14:50:59 --- FAIL: TestCreateServiceSecretFileMode (11.61s)
14:50:59 	daemon.go:283: [dc83bb2ad8a0f] waiting for daemon to start
14:50:59 	daemon.go:315: [dc83bb2ad8a0f] daemon started
14:50:59 	create_test.go:199: timeout hit after 10s: task count at 3 waiting for 1
14:50:59 	daemon.go:273: [dc83bb2ad8a0f] exiting daemon

internal/test/daemon/daemon_unix.go

thaJeztah · 2019-01-20T12:06:54Z

Test is still failing; need to find out what's causing that;

15:10:09 FAIL: docker_cli_swarm_test.go:1534: DockerSwarmSuite.TestSwarmPublishDuplicatePorts
15:10:09 
15:10:09 [dc64601ed788f] waiting for daemon to start
15:10:09 [dc64601ed788f] daemon started
15:10:09 
15:10:09 docker_cli_swarm_test.go:1542:
15:10:09     // make sure task has been deployed.
15:10:09     waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1)
15:10:09 docker_utils_test.go:441:
15:10:09     c.Assert(v, checker, args...)
15:10:09 ... obtained int = 0
15:10:09 ... expected int = 1
15:10:09 
15:10:09 waited for 30.010791614s (out of 30s)
15:10:09 [dc64601ed788f] exiting daemon

Logs attached of that daemon

docker.log

time="2019-01-17T15:09:39.058206372Z" level=error msg="error reading the kernel parameter net.ipv4.neigh.default.gc_thresh1" error="open /proc/sys/net/ipv4/neigh/default/gc_thresh1: no such file or directory"
time="2019-01-17T15:09:39.058241863Z" level=error msg="error reading the kernel parameter net.ipv4.neigh.default.gc_thresh2" error="open /proc/sys/net/ipv4/neigh/default/gc_thresh2: no such file or directory"
time="2019-01-17T15:09:39.058269339Z" level=error msg="error reading the kernel parameter net.ipv4.neigh.default.gc_thresh3" error="open /proc/sys/net/ipv4/neigh/default/gc_thresh3: no such file or directory"
time="2019-01-17T15:09:39.306699373Z" level=error msg="Failed creating ingress network: network sandbox join failed: subnet sandbox join failed for \"10.255.0.0/16\": error creating vxlan interface: file exists"
time="2019-01-17T15:09:39.316535617Z" level=error msg="1d2702eeab7cb24fc43c6aa5d6182c8b07387027349fdf065b9ae140299113cf cleanup: failed to delete container from containerd: no such container"
time="2019-01-17T15:09:39.650731369Z" level=error msg="fatal task error" error="network sandbox join failed: subnet sandbox join failed for \"10.255.0.0/16\": error creating vxlan interface: file exists" module=node/agent/taskmanager node.id=ybrsk120feonm5ykxmzgoxg4m service.id=kfebrrvqpzlzwylxvai14hxak task.id=p2iv89z8vbo22yh4dm7khccdx
time="2019-01-17T15:09:40.026581150Z" level=error msg="fatal task error" error="network sandbox join failed: subnet sandbox join failed for \"10.255.0.0/16\": error creating vxlan interface: file exists" module=node/agent/taskmanager node.id=ybrsk120feonm5ykxmzgoxg4m service.id=kfebrrvqpzlzwylxvai14hxak task.id=kewrl8i9dew6lwl6f9ii4cu3x
time="2019-01-17T15:09:45.026694799Z" level=error msg="fatal task error" error="network sandbox join failed: subnet sandbox join failed for \"10.255.0.0/16\": error creating vxlan interface: file exists" module=node/agent/taskmanager node.id=ybrsk120feonm5ykxmzgoxg4m service.id=kfebrrvqpzlzwylxvai14hxak task.id=38ccvgtwsqxjvypjxeomm8txq
time="2019-01-17T15:09:50.054566586Z" level=error msg="fatal task error" error="network sandbox join failed: subnet sandbox join failed for \"10.255.0.0/16\": error creating vxlan interface: file exists" module=node/agent/taskmanager node.id=ybrsk120feonm5ykxmzgoxg4m service.id=kfebrrvqpzlzwylxvai14hxak task.id=zs96129xfdfw618pc8rh3578y
time="2019-01-17T15:09:55.046855590Z" level=error msg="fatal task error" error="network sandbox join failed: subnet sandbox join failed for \"10.255.0.0/16\": error creating vxlan interface: file exists" module=node/agent/taskmanager node.id=ybrsk120feonm5ykxmzgoxg4m service.id=kfebrrvqpzlzwylxvai14hxak task.id=r34fs115501b5wsxppzwc3ie9
time="2019-01-17T15:10:00.042987178Z" level=error msg="fatal task error" error="network sandbox join failed: subnet sandbox join failed for \"10.255.0.0/16\": error creating vxlan interface: file exists" module=node/agent/taskmanager node.id=ybrsk120feonm5ykxmzgoxg4m service.id=kfebrrvqpzlzwylxvai14hxak task.id=vwze8zbau6pwm8n11mlrmaqm7
time="2019-01-17T15:10:05.054697503Z" level=error msg="fatal task error" error="network sandbox join failed: subnet sandbox join failed for \"10.255.0.0/16\": error creating vxlan interface: file exists" module=node/agent/taskmanager node.id=ybrsk120feonm5ykxmzgoxg4m service.id=kfebrrvqpzlzwylxvai14hxak task.id=3zn00udup55ekv4t3jbsd9nf2
time="2019-01-17T15:10:09.114187625Z" level=error msg="failed to remove node" error="rpc error: code = Aborted desc = dispatcher is stopped" method="(*Dispatcher).Session" node.id=ybrsk120feonm5ykxmzgoxg4m node.session=4068tn9bq7a8tlh53i3a3cz10
time="2019-01-17T15:10:09.114705809Z" level=error msg="failed to restart task after waiting for previous restart" error="raft: failed to process the request: node is stopped" module=node node.id=ybrsk120feonm5ykxmzgoxg4m
time="2019-01-17T15:10:09.114979485Z" level=error msg="failed to receive changes from store watch API" error="rpc error: code = Unknown desc = context canceled"
time="2019-01-17T15:10:09.115310137Z" level=error msg="failed to retrieve ingress network iapela2qdh1mnu8a1shgsm04j: network iapela2qdh1mnu8a1shgsm04j not found"

This looks to be the error:

error creating vxlan interface: file exists

Test itself looks straightforward at a glance

moby/integration-cli/docker_cli_swarm_test.go

Lines 1534 to 1551 in ef91b40

    
           func (s *DockerSwarmSuite) TestSwarmPublishDuplicatePorts(c *check.C) { 
        
           	d := s.AddDaemon(c, true, true) 
        
           	out, err := d.Cmd("service", "create", "--no-resolve-image", "--detach=true", "--publish", "5005:80", "--publish", "5006:80", "--publish", "80", "--publish", "80", "busybox", "top") 
        
           	c.Assert(err, check.IsNil, check.Commentf("%s", out)) 
        
           	id := strings.TrimSpace(out) 
        
           	// make sure task has been deployed. 
        
           	waitAndAssert(c, defaultReconciliationTimeout, d.CheckActiveContainerCount, checker.Equals, 1) 
        
           	// Total len = 4, with 2 dynamic ports and 2 non-dynamic ports 
        
           	// Dynamic ports are likely to be 30000 and 30001 but doesn't matter 
        
           	out, err = d.Cmd("service", "inspect", "--format", "{{.Endpoint.Ports}} len={{len .Endpoint.Ports}}", id) 
        
           	c.Assert(err, check.IsNil, check.Commentf("%s", out)) 
        
           	c.Assert(out, checker.Contains, "len=4") 
        
           	c.Assert(out, checker.Contains, "{ tcp 80 5005 ingress}") 
        
           	c.Assert(out, checker.Contains, "{ tcp 80 5006 ingress}") 
        
           }

thaJeztah · 2019-07-16T12:25:50Z

The error creating vxlan interface: file exists problem was once reported in moby/libnetwork#945 (and moby/libnetwork#562, moby/libnetwork#751 before that)

That case was fixed through moby/libnetwork#1065

salzig · 2019-07-16T19:23:53Z

@thaJeztah just as a note, got this today with docker 18.09.7 on Ubuntu 16.04.

"network sandbox join failed: subnet sandbox join failed for "10.0.3.0/24": error creating vxlan interface: file exists"

kolyshkin

LGTM

kolyshkin · 2020-08-26T16:05:31Z

This needs to be rebased/retested/merged. @thaJeztah can you please revive it?

thaJeztah · 2020-08-27T11:30:19Z

Rebased to trigger CI again

thaJeztah · 2020-08-27T17:01:13Z

Hmm still failing on this one

Stacktrace
=== RUN   TestDockerSwarmSuite/TestSwarmPublishDuplicatePorts
    --- FAIL: TestDockerSwarmSuite/TestSwarmPublishDuplicatePorts (31.39s)
        docker_cli_swarm_test.go:1548: timeout hit after 30s: first check never completed
        poll.go:121: assertion failed: error is not nil: exit status 1

thaJeztah · 2021-01-26T15:29:53Z

Rebased again, to see if things changed since last year

- Use `os.ReadDir()` instead of `filepath.WalkDir()`, because `filepath.WalkDir()` does a lot of extra things we don't need - Log errors on directory removal (omitting `os.IsNotExist`) - Touch-up error messages for consistency Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

guoard · 2025-06-01T04:48:48Z

@thaJeztah PTAL.

thaJeztah requested review from dnephin and vdemeester as code owners March 7, 2018 13:09

GordonTheTurtle added the status/0-triage label Mar 7, 2018

thaJeztah added status/2-code-review area/testing and removed status/0-triage labels Mar 7, 2018

thaJeztah mentioned this pull request Mar 7, 2018

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

Merged

thaJeztah commented Mar 7, 2018

View reviewed changes

integration-cli/daemon/daemon_unix.go Outdated Show resolved Hide resolved

thaJeztah force-pushed the ignore-einval branch from 12e3d02 to 6decb7c Compare March 8, 2018 13:27

thaJeztah added the rebuild/* label Mar 10, 2018

GordonTheTurtle removed the rebuild/* label Mar 10, 2018

thaJeztah added the rebuild/* label Mar 10, 2018

GordonTheTurtle removed the rebuild/* label Mar 10, 2018

thaJeztah added the rebuild/* label Mar 13, 2018

GordonTheTurtle removed the rebuild/* label Mar 13, 2018

thaJeztah added the rebuild/* label Mar 14, 2018

GordonTheTurtle removed the rebuild/* label Mar 14, 2018

GordonTheTurtle assigned dnephin Mar 22, 2018

derek bot added the status/failing-ci Indicates that the PR in its current state fails the test suite label Dec 22, 2018

thaJeztah commented Jan 17, 2019

View reviewed changes

internal/test/daemon/daemon_unix.go Outdated Show resolved Hide resolved

thaJeztah force-pushed the ignore-einval branch from 073bbdf to d970f0f Compare January 17, 2019 13:22

thaJeztah force-pushed the ignore-einval branch from d970f0f to 8687d68 Compare February 1, 2019 23:33

thaJeztah force-pushed the ignore-einval branch from 8687d68 to 0fb4ca6 Compare March 16, 2019 15:11

thaJeztah force-pushed the ignore-einval branch from 0fb4ca6 to 7df60d8 Compare July 8, 2019 18:30

thaJeztah force-pushed the ignore-einval branch from 7df60d8 to 3a7be3d Compare July 16, 2019 11:41

kolyshkin approved these changes Jul 17, 2019

View reviewed changes

thaJeztah mentioned this pull request Oct 9, 2019

Various testutil improvements #40062

Merged

thaJeztah changed the title ~~Ignore EINVAL when cleaning up network namespaces~~ Use io.ReadDir instead of filepath.Walk in netns cleanup Apr 6, 2020

thaJeztah force-pushed the ignore-einval branch from 3a7be3d to 0aace81 Compare April 6, 2020 12:37

thaJeztah removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Apr 6, 2020

thaJeztah force-pushed the ignore-einval branch from 0aace81 to 6383213 Compare August 27, 2020 11:27

thaJeztah force-pushed the ignore-einval branch from 6383213 to ae4691f Compare January 26, 2021 15:29

thaJeztah force-pushed the ignore-einval branch from ae4691f to 0735c98 Compare July 27, 2021 14:19

thaJeztah force-pushed the ignore-einval branch 2 times, most recently from 7b009d5 to 7f183ec Compare August 26, 2022 18:06

thaJeztah changed the title ~~Use io.ReadDir instead of filepath.Walk in netns cleanup~~ Use os.ReadDir instead of filepath.Walk in netns cleanup Aug 26, 2022

thaJeztah force-pushed the ignore-einval branch from 7f183ec to 7be92f2 Compare October 2, 2022 00:41

thaJeztah force-pushed the ignore-einval branch from 7be92f2 to 408089c Compare December 29, 2022 23:50

thaJeztah force-pushed the ignore-einval branch from 408089c to 5d8286b Compare January 20, 2024 14:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Use os.ReadDir instead of filepath.Walk in netns cleanup #36511

Use os.ReadDir instead of filepath.Walk in netns cleanup #36511

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Use os.ReadDir instead of filepath.Walk in netns cleanup #36511

Are you sure you want to change the base?

Use os.ReadDir instead of filepath.Walk in netns cleanup #36511

Conversation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!