🐛 Set recommended leaderelection settings#1663
🐛 Set recommended leaderelection settings#1663dtfranz merged 1 commit intooperator-framework:mainfrom
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Cross-ref catalogd PR: |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1663 +/- ##
==========================================
- Coverage 67.42% 67.37% -0.05%
==========================================
Files 55 55
Lines 4632 4644 +12
==========================================
+ Hits 3123 3129 +6
- Misses 1284 1290 +6
Partials 225 225
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
| // https://github.com/openshift/enhancements/blob/61581dcd985130357d6e4b0e72b87ee35394bf6e/CONVENTIONS.md#handling-kube-apiserver-disruption | ||
| LeaseDuration: ptr.To(137 * time.Second), | ||
| RenewDeadline: ptr.To(107 * time.Second), | ||
| RetryPeriod: ptr.To(26 * time.Second), |
There was a problem hiding this comment.
🎉
I think we need to do the same for catalogd : https://github.com/operator-framework/operator-controller/blob/main/catalogd/cmd/catalogd/main.go
thetechnick
force-pushed
the
recommended-leaderelection-settings
branch
from
0618e88 to
63e2507
Compare
camilamacedo86
left a comment
camilamacedo86
left a comment
There was a problem hiding this comment.
I am OK with those changes 👍
/lgtm
thetechnick
added
the
acknowledge-critical-fixes-only
thetechnick
force-pushed
the
recommended-leaderelection-settings
branch
from
63e2507 to
3983002
Compare
|
@thetechnick Looks like more than 5000+ files changes for the PR, seems like a rebase gone wrong or something similar. |
|
Oh @thetechnick I think you just overview. |
| _ "k8s.io/client-go/plugin/pkg/client/auth" | ||
| "k8s.io/klog/v2" | ||
| "k8s.io/klog/v2/textlogger" | ||
| "k8s.io/utils/ptr" |
There was a problem hiding this comment.
would it make sense to try and avoid pulling in a new dependency just for a small helper that returns a pointer to value?
There was a problem hiding this comment.
that 5000k lines vendor tricked me :)
|
@thetechnick can you remove |
|
+1 on removing |
Extensive e2e tests revealed that operator-controller might run into leader election timeouts during cluster bootstrap, causing sporadic alerts being generated. This commit uses recommended settings for leaderelection LeaseDuration: 15s -> 137s RenewDeadline: 10s -> 107s RetryPeriod: 2s -> 26s Warning: This will increase potential down-time of catalogd to 163s in the worst case (up from 17s). (LeaseDuration + RetryPeriod)
tmshort
force-pushed
the
recommended-leaderelection-settings
branch
from
3983002 to
92cece5
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
7 participants
Description
Extensive e2e tests revealed that operator-controller and catalogd might run into leader election timeouts during cluster bootstrap, causing sporadic alerts being generated.
This commit uses recommended settings for leaderelection:
Warning: This will increase potential down-time of our components to 163s in the worst case (up from 17s). (LeaseDuration + RetryPeriod)
Reviewer Checklist