TestConntrackFlowsLeak: use busybox "nc" #43031

thaJeztah · 2021-11-18T17:01:42Z

extracted from #42300
this is another attempt at #34832
replaces / closes #34832

to address:

The appropriate/nc image was last built over 6 years ago, and uses the deprecated v2 schema 1 format; https://github.com/appropriate/docker-nc/tree/master/latest

The image is just a plain "apk install" of netbsd-netcat, but was added in 1c4286b (#32505), because at the time the busybox nc had some bugs.

These appear to be resolved, so we can use the busybox nc, from the frozen images.

thaJeztah · 2021-11-18T19:24:05Z

docker pull appropriate/nc
Using default tag: latest
latest: Pulling from appropriate/nc
Image docker.io/appropriate/nc:latest uses outdated schema1 manifest format. Please upgrade to a schema2 image for better future compatibility. More information at https://docs.docker.com/registry/spec/deprecated-schema-v1/
12b41071e6ce: Pull complete
d41ebf7a19c0: Pull complete
0b849cb5c85c: Pull complete
a3ed95caeb02: Pull complete
Digest: sha256:ff0f2b54fb1e1bfe277047fd660415310232d301e27bec9423aa741de961e6d4
Status: Downloaded newer image for appropriate/nc:latest
docker.io/appropriate/nc:latest

cpuguy83

This is definitely better than the previous image, however I'd be happier if we had a cached image for this.

fredericdalleau · 2021-11-18T20:14:00Z

alpine3.14 already contains, busybox's nc, wouldn't it do the trick?

thaJeztah · 2021-11-18T20:36:27Z

however I'd be happier if we had a cached image for this.

You mean, as a "frozen image", or pushing it somewhere to docker hub?
tbh, I was playing with the thought at some point to replace busybox with alpine in our frozen images (as it's a more flexible image), but then again, it's loaded in many tests, so even though it's "small", it's still a lot larger than the busybox image, so perhaps would make tests slower.

Installing the package is also very fast;

time apk add --no-cache -qq netcat-openbsd
real	0m 0.36s
user	0m 0.27s
sys	0m 0.05s

That said, we could run into outages of the alpine package repo (happens occasionally)

alpine3.14 already contains, busybox's nc, wouldn't it do the trick?

According to the PR that added it (discussion on that), that didn't work; #32505 (comment)

The nc version in alpine does not support for example the option -q. There is a bug in nc that let the command never return also when specify the -w option.
Feel free to try to change it to the alpine removing the -q option to let the command work, but most likely the test will fail timing out because the nc command will hang forever.

tianon · 2021-11-18T23:30:11Z

integration-cli/docker_cli_network_unix_test.go

+	}).Assert(c, icmd.Success)
+
 	// Launch the server, this will remain listening on an exposed port and reply to any request in a ping/pong fashion
 	cmd := "while true; do echo hello | nc -w 1 -lu 8080; done"


If we switch to the (already in our "frozen images") busybox image, I've had success with the following returning data correctly:

Suggested change

cmd := "while true; do echo hello | nc -w 1 -lu 8080; done"

cmd := "while true; do echo hello | nc -w 1 -l -u -p 8080; done"

tianon · 2021-11-18T23:30:30Z

integration-cli/docker_cli_network_unix_test.go

+	cli.DockerCmd(c, "run", "-d", "--name", "server", "--net", "testbind", "-p", "8080:8080/udp", imgName, "sh", "-c", cmd)

 	// Launch a container client, here the objective is to create a flow that is natted in order to expose the bug
 	cmd = "echo world | nc -q 1 -u 192.168.10.1 8080"


Suggested change

cmd = "echo world | nc -q 1 -u 192.168.10.1 8080"

cmd = "echo world | nc -w 1 -u 192.168.10.1 8080"

(I ran these exact commands in two containers and both worked "correctly" although the server container having a constant repeated 1 second timeout is kind of cute)

Edit: I guess worst case we add timeout 2 in front of this nc too? (if we see issues with it and need it to be killed more actively)

tianon · 2021-11-18T23:31:19Z

integration-cli/docker_cli_network_unix_test.go

+	// Replaces the image built from https://github.com/appropriate/docker-nc/tree/master/latest
+	imgName := "moby/nc"
+	icmd.RunCmd(icmd.Cmd{
+		Command: []string{dockerBinary, "build", "-t", imgName, "-"},
+		Stdin:   strings.NewReader("FROM alpine:3.14\nRUN apk add --no-cache netcat-openbsd\n"),
+	}).Assert(c, icmd.Success)


Suggested change

// Replaces the image built from https://github.com/appropriate/docker-nc/tree/master/latest

imgName := "moby/nc"

icmd.RunCmd(icmd.Cmd{

Command: []string{dockerBinary, "build", "-t", imgName, "-"},

Stdin: strings.NewReader("FROM alpine:3.14\nRUN apk add --no-cache netcat-openbsd\n"),

}).Assert(c, icmd.Success)

imgName := "busybox:latest"

The appropriate/nc image was last built over 6 years ago, and uses the deprecated v2 schema 1 format. https://github.com/appropriate/docker-nc/tree/master/latest The image is just a plain "apk install" of netbsd-netcat, but was added in 1c4286b, because at the time the busybox nc had some bugs. These appear to be resolved, so we can use the busybox nc, from the frozen images. Signed-off-by: Sebastiaan van Stijn <[email protected]>

thaJeztah · 2021-11-19T08:32:22Z

Thanks @tianon ❤️. I applied your changes; let's see if it passes 🤞

thaJeztah · 2021-11-19T09:38:44Z

TestWaitNodeAttachment failure on Windows 2022 / containerd looks unrelated (posting in case it becomes more flaky);

=== RUN   TestWaitNodeAttachment
    adapter_test.go:137: waitNodeAttachments did not exit yet, but should have
--- FAIL: TestWaitNodeAttachment (0.40s)

thaJeztah · 2021-11-19T11:14:02Z

And.. another flaky (already tracked in #39352)

=== RUN   TestDockerSuite/TestRunInteractiveWithRestartPolicy
    docker_cli_run_test.go:1796: assertion failed: 
        Command:  d:\CI\PR-43031\3\binary\docker.exe run -i --name test-inter-restart --restart=always busybox sh
        ExitCode: 0
        Stdout:   
        Stderr:   
        
        Failures:
        ExitCode was 0 expected 11
    --- FAIL: TestDockerSuite/TestRunInteractiveWithRestartPolicy (7.32s)

thaJeztah added status/2-code-review area/networking Networking area/testing kind/refactor PR's that refactor, or clean-up code labels Nov 18, 2021

thaJeztah added this to the 21.xx milestone Nov 18, 2021

cpuguy83 approved these changes Nov 18, 2021

View reviewed changes

tianon reviewed Nov 18, 2021

View reviewed changes

thaJeztah force-pushed the fix_TestConntrackFlowsLeak_v1 branch from 6cb1cf0 to 6d92d2c Compare November 19, 2021 08:27

thaJeztah changed the title ~~TestConntrackFlowsLeak: build image instead of pulling~~ TestConntrackFlowsLeak: use busybox "nc" Nov 19, 2021

AkihiroSuda approved these changes Nov 19, 2021

View reviewed changes

thaJeztah merged commit ea0f3dc into moby:master Nov 19, 2021

thaJeztah deleted the fix_TestConntrackFlowsLeak_v1 branch November 19, 2021 12:39

thaJeztah mentioned this pull request Nov 19, 2021

Remove support for pulling v2 schema1 #42300

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

TestConntrackFlowsLeak: use busybox "nc" #43031

TestConntrackFlowsLeak: use busybox "nc" #43031

Uh oh!

thaJeztah commented Nov 18, 2021 •

edited

Loading

Uh oh!

thaJeztah commented Nov 18, 2021

Uh oh!

cpuguy83 left a comment

Uh oh!

fredericdalleau commented Nov 18, 2021

Uh oh!

thaJeztah commented Nov 18, 2021

Uh oh!

tianon Nov 18, 2021

Uh oh!

tianon Nov 18, 2021

Uh oh!

tianon Nov 18, 2021 •

edited

Loading

Uh oh!

tianon Nov 18, 2021

Uh oh!

thaJeztah commented Nov 19, 2021

Uh oh!

thaJeztah commented Nov 19, 2021

Uh oh!

thaJeztah commented Nov 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	cmd := "while true; do echo hello \| nc -w 1 -lu 8080; done"
	cmd := "while true; do echo hello \| nc -w 1 -l -u -p 8080; done"

	cmd = "echo world \| nc -q 1 -u 192.168.10.1 8080"
	cmd = "echo world \| nc -w 1 -u 192.168.10.1 8080"

TestConntrackFlowsLeak: use busybox "nc" #43031

TestConntrackFlowsLeak: use busybox "nc" #43031

Uh oh!

Conversation

thaJeztah commented Nov 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thaJeztah commented Nov 18, 2021

Uh oh!

cpuguy83 left a comment

Choose a reason for hiding this comment

Uh oh!

fredericdalleau commented Nov 18, 2021

Uh oh!

thaJeztah commented Nov 18, 2021

Uh oh!

tianon Nov 18, 2021

Choose a reason for hiding this comment

Uh oh!

tianon Nov 18, 2021

Choose a reason for hiding this comment

Uh oh!

tianon Nov 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tianon Nov 18, 2021

Choose a reason for hiding this comment

Uh oh!

thaJeztah commented Nov 19, 2021

Uh oh!

thaJeztah commented Nov 19, 2021

Uh oh!

thaJeztah commented Nov 19, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

thaJeztah commented Nov 18, 2021 •

edited

Loading

tianon Nov 18, 2021 •

edited

Loading