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

Increase clusterclient timeout and retry intervals #356

Merged
merged 1 commit into from
Jun 21, 2018

Conversation

karan
Copy link
Contributor

@karan karan commented Jun 18, 2018

What this PR does / why we need it: Title says it all. Some vsphere images can take a while to spin up. Client side timeout should be much higher to protect against those cases.

Release note:

Increase clusterclient timeout and retry intervals

/kind clusterctl

@kubernetes/kube-deploy-reviewers

@k8s-ci-robot k8s-ci-robot added kind/clusterctl cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 18, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: karan

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jun 18, 2018
@karan
Copy link
Contributor Author

karan commented Jun 18, 2018

/assign @krousey

@ingvagabund
Copy link
Contributor

ingvagabund commented Jun 18, 2018

Some vsphere images can take a while to spin up.

Is the need for the increased timeout cloud provider specific? Is there a cloud provider for which we can with high confidence say if a resource is not ready in 5 minutes, it will never be ready?

Maybe instead of increasing the timeout (two times or three times) what about to introduce retries? E.g. if I retry TimeoutResourceReady 2 times and still nothing happens -> fail. For me as a user it is better to see "Timeouted waiting for a resources to be ready after %v, trying again..." every 5 minutes instead of waiting 15 minutes to get a "Timeouted ..." error message.

EDIT: ok, I checked the behaviour of the util.PollImmediate. With the PR merged one will get "Waiting for Cluster v1alpha resources to become available..." printed every 10s instead of 5s. Please, ignore my comment :).

@karan
Copy link
Contributor Author

karan commented Jun 18, 2018

The timeout increase is mainly to handle the tail-end of cases. For example one test I'm running would fail without increasing this timeout. The retries we have already make so you don't have to wait for 15 minutes if the machine came up sooner.

@karan
Copy link
Contributor Author

karan commented Jun 18, 2018

/uncc kris-nova k4leung4

@dims
Copy link
Member

dims commented Jun 21, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jun 21, 2018
@k8s-ci-robot k8s-ci-robot merged commit 96b45e6 into kubernetes-sigs:master Jun 21, 2018
@karan karan deleted the timeout branch June 26, 2018 21:33
jayunit100 pushed a commit to jayunit100/cluster-api that referenced this pull request Jan 31, 2020
…ice-matching-by-order

Net devices by order, remove invalid IP addrs
@killianmuldoon killianmuldoon added area/clusterctl Issues or PRs related to clusterctl and removed kind/clusterctl labels May 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/clusterctl Issues or PRs related to clusterctl cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. 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.

6 participants