-
Notifications
You must be signed in to change notification settings - Fork 55
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
Restore ProbeForever behavior of probing first before sleeping #175
Restore ProbeForever behavior of probing first before sleeping #175
Conversation
The previous beheavior of ProbeForever was to try a probe and then sleep for the `probeInterval` duration. When it was modified to use a `time.Ticker` instead of a sleep, it changed the behavior to wait the `probeInterval` first and then do the probe. This commit restore the previous behavior by running the probe first. Based on https://stackoverflow.com/a/54752803.
Hi @Fricounet. Thanks for your PR. I'm waiting for a kubernetes-csi member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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-sigs/prow repository. |
/ok-to-test |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Fricounet, jsafrane 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 |
I've tagged v0.18.1, I (or dependabot) will update all sidecars. |
What type of PR is this?
/kind bug
What this PR does / why we need it:
The previous beheavior of ProbeForever was to try a probe and then sleep for the
probeInterval
duration. When it was modified to use atime.Ticker
instead of a sleep in #149, it changed the behavior to wait theprobeInterval
first and then do the probe. This PR restores the previous behavior by running the probe first because it is causing issues when trying to start some of the sidecars (kubernetes-csi/external-attacher#561). And in general, it slows down the startup of the sidecars by at least 1s, even when the plugin is already ready to accept connections.Based on https://stackoverflow.com/a/54752803.
Which issue(s) this PR fixes:
Fixes #177
Special notes for your reviewer:
Does this PR introduce a user-facing change?: