adding config option for ALB & NLB healthy threshold count #453
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@szuecs please review, thanks |
|
@universam1 nice addition - thanks for adding 👍 |
|
Good idea, I like that addition, too. 👍 |
universam1
force-pushed
the
healthyThreshold
branch
3 times, most recently
from
2be8ffb to
c0f1020
Compare
|
@szuecs Not sure what to do about the coverage decrease. Tested this change in our clusters, works as expected |
|
@universam1 it's no an issue if coverage drops <1%. It's more a hint we should make it better in the future. |
universam1
force-pushed
the
healthyThreshold
branch
from
c0f1020 to
540b232
Compare
universam1
changed the title
universam1
force-pushed
the
healthyThreshold
branch
2 times, most recently
from
4808a5d to
7cb98cf
Compare
controller.go
Outdated
Comment on lines
227
to
232
| kingpin.Flag("alb-healthy-threshold-count", "The number of consecutive successful health checks required before considering an unhealthy target healthy. The range is 2–10. The default is 5. (ALB only)"). | ||
| Default(strconv.FormatUint(aws.DefaultAlbHealthyThresholdCount, 10)).UintVar(&albHealthyThresholdCount) | ||
| kingpin.Flag("alb-unhealthy-threshold-count", "The number of consecutive failed health checks required before considering a target unhealthy. The range is 2–10. The default is 2. (ALB only)"). | ||
| Default(strconv.FormatUint(aws.DefaultAlbUnhealthyThresholdCount, 10)).UintVar(&albUnhealthyThresholdCount) | ||
| kingpin.Flag("nlb-healthy-threshold-count", "The number of consecutive successful or failed health checks required before considering an target healthy or unhealthy. The range is 2 to 10. The default is 3. (NLB only)"). | ||
| Default(strconv.FormatUint(aws.DefaultNlbHealthyThresholdCount, 10)).UintVar(&nlbHealthyThresholdCount) |
There was a problem hiding this comment.
I think the default value is rendered in the help message:
--alb-healthy-threshold-count=5
The number of consecutive successful health
checks required before considering an unhealthy
target healthy. The range is 2–10. The default
is 5. (ALB only)
--alb-unhealthy-threshold-count=2
The number of consecutive failed health checks
required before considering a target unhealthy.
The range is 2–10. The default is 2. (ALB only)
--nlb-healthy-threshold-count=3
The number of consecutive successful or failed
health checks required before considering an
target healthy or unhealthy. The range is 2 to
10. The default is 3. (NLB only)
so lets remove The default is ....
For --nlb-healthy-threshold-count it should be considering _a_ target and I think we should use the same format for the range as for ALB: The range is 2–10 for consistency (I get that it is different in the AWS docs).
There was a problem hiding this comment.
Done, thanks for the review
There was a problem hiding this comment.
Thank you. _an_ target is still there though.
universam1
force-pushed
the
healthyThreshold
branch
from
7cb98cf to
a00ec35
Compare
Healthy threshold: The number of consecutive health checks successes required before considering an unhealthy target healthy This configuration is required since the default of "5" time the interval which is minimum of sum of 30s that is too long delay in our configurations Signed-off-by: Samuel Lang <[email protected]>
universam1
force-pushed
the
healthyThreshold
branch
from
a00ec35 to
bc74868
Compare
|
👍 |
1 similar comment
|
👍 |
universam1
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Healthy threshold: The number of consecutive health checks successes required before considering an unhealthy target healthy
This configuration is required since the default of "5" time the interval, which is minimum of 6s, sums up to 30s that is too long delay in our configurations.
Signed-off-by: Samuel Lang [email protected]