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

Fix the ProbeEtcd timeout. #532

Merged
merged 2 commits into from
Sep 14, 2022
Merged

Conversation

ishan16696
Copy link
Member

@ishan16696 ishan16696 commented Sep 13, 2022

What this PR does / why we need it:
We have observed that currently ProbeEtcd is using the longer timeout which is set using this flag --etcd-connection-timeout and in our landscapes --etcd-connection-timeout is set to 5mins. (check here)
Due to this longer Timeout ProbeEtcd is keep waiting instead of failing the probe and meanwhile when ProbeEtcd is waiting if corresponding etcd comes-up then ProbeEtcd get succeeds this might leads to some race-conditions between the go-routines as ProbeEtcd is suppose to fail.
This PR fixes this issue by using the shorter Timeout of 5sec in ProbeEtcd.

Which issue(s) this PR fixes:
Fixes #

Special notes for your reviewer:
cc @timuthy

Release note:

To avoid potential race-condition between go-routines updated `probeEtcd func()` to use shorter timeout.

@ishan16696 ishan16696 requested a review from a team as a code owner September 13, 2022 11:02
@gardener-robot gardener-robot added needs/review Needs review size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) labels Sep 13, 2022
@gardener-robot-ci-2 gardener-robot-ci-2 added reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) and removed reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) labels Sep 13, 2022
@gardener-robot-ci-3 gardener-robot-ci-3 added the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 14, 2022
@gardener-robot-ci-2 gardener-robot-ci-2 removed the reviewed/ok-to-test Has approval for testing (check PR in detail before setting this label because PR is run on CI/CD) label Sep 14, 2022
@timuthy
Copy link
Member

timuthy commented Sep 14, 2022

/assign

Copy link
Member

@timuthy timuthy left a comment

Choose a reason for hiding this comment

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

/lgtm

@gardener-robot gardener-robot added reviewed/lgtm Has approval for merging and removed needs/review Needs review labels Sep 14, 2022
@ishan16696 ishan16696 merged commit 9f00fdb into gardener:master Sep 14, 2022
@gardener-robot gardener-robot added the status/closed Issue is closed (either delivered or triaged) label Sep 14, 2022
@ishan16696 ishan16696 deleted the updateTimeout branch September 15, 2022 04:22
aaronfern pushed a commit to aaronfern/etcd-backup-restore that referenced this pull request Sep 16, 2022
* Fix the ProbeEtcd timeout.

* Improved the logging while closing the snapshotter.
aaronfern added a commit that referenced this pull request Sep 16, 2022
* Fix the ProbeEtcd timeout. (#532)

* Fix the ProbeEtcd timeout.

* Improved the logging while closing the snapshotter.

* always update member peer url and changed the way to identify scaleup

* fixed unit tests

* addressed review comments

* addressed review comments

* used Errorf instead of Error

* made some modifications on how to initialize peerURLTLSEnabled

* removed commented code

* addressed review comment - removed commented code

* Update to golang 1.18.6 (#535)

* Update Dockerfile to use go 1.18.6

* Update pipeline definition to use go 1.18.6

Co-authored-by: Ishan Tyagi <[email protected]>
Co-authored-by: Madhav Bhargava <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs/ok-to-test Needs approval for testing (check PR in detail before setting this label because PR is run on CI/CD) reviewed/lgtm Has approval for merging size/xs Size of pull request is tiny (see gardener-robot robot/bots/size.py) status/closed Issue is closed (either delivered or triaged)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants