-
Notifications
You must be signed in to change notification settings - Fork 141
feat: add environment variables for leader election timing configuration #1365
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
feat: add environment variables for leader election timing configuration #1365
Conversation
|
Hey! Thank you for working on this. The PR looks good, but maybe we could also add some additional validation for these values? For instance, we should ensure that |
88595fc to
a227029
Compare
thanks for the review @Fedosin, added the validation checks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for configuring leader election timing parameters through environment variables to allow operators to adjust leader election behavior for different cluster configurations without requiring code changes.
- Add environment variable parsing for lease duration, renew deadline, and retry period
- Implement validation to ensure leader election timing constraints are met
- Add comprehensive test coverage for parsing and validation
- Document the new configuration options
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/util/env_resolver.go | Adds ValidateLeaderElectionConfig function to validate timing parameter relationships |
| pkg/util/env_resolver_test.go | Adds tests for leader election configuration validation with various valid and invalid scenarios |
| operator/main.go | Integrates environment variable parsing and validation for leader election timing parameters |
| operator/main_test.go | Adds integration tests for environment variable parsing |
| docs/operate.md | Documents the new leader election timing configuration options |
| CHANGELOG.md | Updates changelog with the new feature |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@wozniakjan addressed review comments, but I don't think I have access to start tests, would you mind kicking off tests again? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
492e532 to
2874ae2
Compare
Add support for configuring leader election timing parameters through environment variables. Changes: - Add environment variable parsing in operator/main.go for: - KEDA_HTTP_OPERATOR_LEADER_ELECTION_LEASE_DURATION - KEDA_HTTP_OPERATOR_LEADER_ELECTION_RENEW_DEADLINE - KEDA_HTTP_OPERATOR_LEADER_ELECTION_RETRY_PERIOD - Pass parsed durations to ctrl.NewManager Options - Add test in operator/main_test.go covering parsing, defaults and error handling - Add configuration docs All three environment variables are optional. When not set, controller-runtime uses Kubernetes defaults (LeaseDuration: 15s, RenewDeadline: 10s, RetryPeriod: 2s). This allows operators to adjust leader election timing for different cluster configurations and network conditions without requiring code changes. Fixes: kedacore#1331 Signed-off-by: Nader Ziada <[email protected]>
Signed-off-by: Nader Ziada <[email protected]>
Signed-off-by: Nader Ziada <[email protected]>
- take into account partial setting of params and default values - fix md file format - fix linter issues Signed-off-by: Nader Ziada <[email protected]>
Signed-off-by: Nader Ziada <[email protected]>
2874ae2 to
81939a7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
wozniakjan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thank you!
Add support for configuring leader election timing parameters through environment variables.
Changes:
All three environment variables are optional. When not set, controller-runtime uses Kubernetes defaults (LeaseDuration: 15s, RenewDeadline: 10s, RetryPeriod: 2s). This allows operators to adjust leader election timing for different cluster configurations and network conditions without requiring code changes.
Fixes: #1331
Checklist
README.mddocs/directory