-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add timeout when reading dashboard host:port from kubectl #3322
Add timeout when reading dashboard host:port from kubectl #3322
Conversation
kubectl releases older than August 2017 don't include a newline, which means there is effectively no hint that the output has completed. I believe this is why tests have been failing on the macOS. Should resolve test timeout panics such as kubernetes#3203
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: RA489, tstromberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@RA489: changing LGTM is restricted to assignees, and only kubernetes/minikube repo collaborators may be assigned issues. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@minikube-bot OK to test |
LGTM, but should we instead/also update kubectl on the slaves? |
Until we have a pre-flight check for bad kubectl versions (#3329), I actually rather like having a node around configured this way to find these issues. |
SGTM, but there may be functional issues with it in the future given that k8s only guarantees support for a few old versions (there's no guarantee kubectl 1.6 will work at all with k8s >1.10). |
kubectl releases older than August 2017 don't include a newline, which effectively results in there being no signal that output has completed. For these cases, we timeout reading after 5 seconds and pass whatever we received into the appropriate regexp.
This resolves the test timeout panics we have seen on the Mac Mini (kubectl v1.6.1) such as #3203.
Tested on kubectl v1.6.1 (no newline), v1.10.1 (newline), and a custom shell script which just sleeps forever.