Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stricter timeouts for nodePort curling #36208

Merged
merged 1 commit into from
Nov 4, 2016

Conversation

bprashanth
Copy link
Contributor

@bprashanth bprashanth commented Nov 4, 2016

If the timeouts are indeed because of #34665 (comment), stricter timeouts will probably surface as a more isolated failure


This change is Reviewable

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Nov 4, 2016
@@ -223,7 +223,7 @@ func (config *NetworkingTestConfig) DialFromNode(protocol, targetIP string, targ
// busybox timeout doesn't support non-integer values.
cmd = fmt.Sprintf("echo 'hostName' | timeout -t 2 nc -w 1 -u %s %d", targetIP, targetPort)
} else {
cmd = fmt.Sprintf("curl -q -s --connect-timeout 1 http://%s:%d/hostName", targetIP, targetPort)
cmd = fmt.Sprintf("timeout -t 15 curl -q -s --connect-timeout 1 --max-time 10 http://%s:%d/hostName", targetIP, targetPort)
Copy link
Contributor

Choose a reason for hiding this comment

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

timeout is not installed on OS X by default. I don't know how much we care. cc @roberthbailey who's the person that complains most about this kind of stuff:)

@@ -223,7 +223,7 @@ func (config *NetworkingTestConfig) DialFromNode(protocol, targetIP string, targ
// busybox timeout doesn't support non-integer values.
cmd = fmt.Sprintf("echo 'hostName' | timeout -t 2 nc -w 1 -u %s %d", targetIP, targetPort)
} else {
cmd = fmt.Sprintf("curl -q -s --connect-timeout 1 http://%s:%d/hostName", targetIP, targetPort)
cmd = fmt.Sprintf("timeout -t 15 curl -q -s --connect-timeout 1 --max-time 10 http://%s:%d/hostName", targetIP, targetPort)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is code that runs inside a pod so we should have control over exactly what base image it runs on.

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, I missed the fact that it's called remotely. Sorry for that.

@gmarek gmarek added release-note-none Denotes a PR that doesn't merit a release note. lgtm "Looks good to me", indicates that a PR is ready to be merged. and removed release-note-label-needed labels Nov 4, 2016
@bprashanth bprashanth added the priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. label Nov 4, 2016
@bprashanth
Copy link
Contributor Author

P1 to help with flake

@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 3adc580 into kubernetes:master Nov 4, 2016
@bprashanth bprashanth deleted the curl_timeout branch January 26, 2017 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note-none Denotes a PR that doesn't merit a release note. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants