Skip to content

Increase dial timeout to allow for DNS failover#11

Merged
brandond merged 1 commit intorancher:mainfrom
brandond:fix_dns_timeout
Dec 23, 2022
Merged

Increase dial timeout to allow for DNS failover#11
brandond merged 1 commit intorancher:mainfrom
brandond:fix_dns_timeout

Conversation

@brandond
Copy link
Copy Markdown
Member

@brandond brandond commented Dec 22, 2022

The Transport config cribbed from https://github.com/google/go-containerregistry/blob/v0.12.1/pkg/v1/remote/options.go#L85-L101 sets a 5 second dial timeout. Unfortunately, the spec for DNS clients requires a 5 second delay before trying secondary nameservers. This meant that if the primary nameserver is unavailable, wharfie would timeout the dial before consulting secondary nameservers.

Raising this to 16 seconds will increase delays when failing over from unavailable upstream registries, but should properly handle DNS server failover.

Signed-off-by: Brad Davidson [email protected]

The Transport config cribbed from https://github.com/google/go-containerregistry/blob/47f093330862ba6982f644003621eddc9e7dc36e/pkg/v1/remote/options.go#L101-L117 sets a 5 second dial timeout. Unfortunately, the spec for DNS clients requires a 5 second delay before trying secondary nameservers. This meant that if the primary nameserver is unavailable, wharfie would timeout the dial before consulting secondary nameservers.

Raising this to 16 seconds will increase delays when failing over from unavailable upstream registries, but should properly handle DNS server failover.

Signed-off-by: Brad Davidson <[email protected]>
@brandond
Copy link
Copy Markdown
Member Author

brandond commented Dec 23, 2022

Ref: SURE-5646 @Martin-Weiss

@brandond brandond merged commit b8a78d5 into rancher:main Dec 23, 2022
@Martin-Weiss
Copy link
Copy Markdown

@brandond - thanks so much for your great help in debugging and unblocking us! You are the best!

Do you have a build that includes this potential fix so we can give it a try and validate it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants