Skip to content

Fix dns fallback regression#41008

Merged
thaJeztah merged 3 commits intomoby:masterfrom
tiborvass:fix-dns-fallback-regression
May 28, 2020
Merged

Fix dns fallback regression#41008
thaJeztah merged 3 commits intomoby:masterfrom
tiborvass:fix-dns-fallback-regression

Conversation

@tiborvass
Copy link
Copy Markdown
Contributor

@tiborvass tiborvass commented May 21, 2020 •
edited by thaJeztah
Loading

fixes #41003
fixes #41041

Ref moby/libnetwork#2551

To run only the test and confirm it fails if vendor commit is reverted:

TEST_SKIP_INTEGRATION_CLI=1 TEST_INTEGRATION_DIR=./integration/network TESTFLAGS='-test.run TestDaemonDNSFallback' make test-integration

cpuguy83
cpuguy83 approved these changes May 21, 2020
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

arkodg
arkodg approved these changes May 21, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

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

thanks for adding the test case !

@tiborvass tiborvass mentioned this pull request May 21, 2020
Merged
@tiborvass tiborvass force-pushed the fix-dns-fallback-regression branch from e217da7 to ce6f004 Compare May 21, 2020 20:13
@tiborvass tiborvass marked this pull request as draft May 21, 2020 23:09
@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented May 25, 2020

@tiborvass I see you reverted the bump (probably to verify the integration-test); could you

  • remove the revert
  • swap the "bump" and "test-case", so that git bisect doesn't break?
  • move this PR out of draft

@thaJeztah thaJeztah added the area/networking Networking label May 25, 2020
@thaJeztah thaJeztah added this to the 20.03.0 milestone May 25, 2020
@tiborvass tiborvass force-pushed the fix-dns-fallback-regression branch 5 times, most recently from 98f0c77 to d2ff268 Compare May 27, 2020 16:37
@tiborvass tiborvass marked this pull request as ready for review May 27, 2020 16:46
@tiborvass tiborvass requested a review from tianon as a code owner May 27, 2020 16:46
@tiborvass tiborvass marked this pull request as draft May 27, 2020 20:04
@tiborvass tiborvass force-pushed the fix-dns-fallback-regression branch 2 times, most recently from d9e0648 to 5942ae5 Compare May 27, 2020 22:08
hack: add more debugging to understand exit codepath
b280ea1 
Signed-off-by: Tibor Vass <tibor@docker.com>
@tiborvass tiborvass force-pushed the fix-dns-fallback-regression branch from 5942ae5 to 9c15a65 Compare May 27, 2020 23:40
@tiborvass tiborvass marked this pull request as ready for review May 28, 2020 00:43
@tiborvass tiborvass force-pushed the fix-dns-fallback-regression branch 2 times, most recently from 17e3965 to acffb76 Compare May 28, 2020 02:05
tianon
tianon reviewed May 28, 2020
View reviewed changes
vendor.conf Outdated
Copy link
Copy Markdown
Member

@tianon tianon May 28, 2020

Choose a reason for hiding this comment

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

Is this comment still accurate / doesn't this PR need to include an update to LIBNETWORK_COMMIT in hack/dockerfile/install/proxy.installer? 😅

Copy link
Copy Markdown
Contributor Author

@tiborvass tiborvass May 28, 2020

Choose a reason for hiding this comment

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

Why am I seeing this when it's FINALLY green? :D

vendor libnetwork to 2e24aed516bd5c836e11378bb457dd612aa868ed
8319918 
Signed-off-by: Tibor Vass <tibor@docker.com>
@tiborvass tiborvass force-pushed the fix-dns-fallback-regression branch from acffb76 to e5ad8b1 Compare May 28, 2020 03:29
integration: Add TestDaemonDNSFallback
9a78e91 
Signed-off-by: Tibor Vass <tibor@docker.com>
@tiborvass tiborvass force-pushed the fix-dns-fallback-regression branch from e5ad8b1 to 9a78e91 Compare May 28, 2020 04:31
@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented May 28, 2020

Two unrelated tests failing; apparently they're (still) flaky;

on ppc64le

=== RUN   TestCheckCapacityAndRotate
--- FAIL: TestCheckCapacityAndRotate (30.01s)
    logfile_test.go:268: timeout hit after 30s: waiting for /tmp/TestCheckCapacityAndRotate293573117/log736876856.1 to exist

on windows RS5

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

thaJeztah
thaJeztah approved these changes May 28, 2020
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

@thaJeztah thaJeztah merged commit 9a1bef2 into moby:master May 28, 2020
@thaJeztah thaJeztah mentioned this pull request May 28, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tianon tianon tianon left review comments

@cpuguy83 cpuguy83 cpuguy83 approved these changes

@thaJeztah thaJeztah thaJeztah approved these changes

@AkihiroSuda AkihiroSuda AkihiroSuda approved these changes

+1 more reviewer

@arkodg arkodg arkodg approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/networking Networking process/cherry-picked status/4-merge

Projects

None yet

Milestone

20.10.0

Development

Successfully merging this pull request may close these issues.

Secondary DNS servers are ignored, 8.8.8.8 is not being added DNS resolution failure in compose stack with docker engine 19.03.9

6 participants

@tiborvass @thaJeztah @tianon @cpuguy83 @AkihiroSuda @arkodg