-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Add failures_before_warning to checks #10969
Conversation
Signed-off-by: Jakub Sokołowski <jakub@status.im>
The new setting allows users to specify the number of check failures that have to happen before a service status us updated to be `warning`. This allows for more visibility for detected issues without creating alerts and pinging administrators. Unlike the previous behavior, which caused the service status to not update until it reached the configured `failures_before_critical` setting, now Consul updates the Web UI view with the `warning` state and the output of the service check when `failures_before_warning` is breached. The default value of `FailuresBeforeWarning` is the same as the value of `FailuresBeforeCritical`, which allows for retaining the previous default behavior of not triggering a warning. When `FailuresBeforeWarning` is set to a value higher than that of `FailuresBeforeCritical it has no effect as `FailuresBeforeCritical` takes precedence. Resolves: #10680 Signed-off-by: Jakub Sokołowski <jakub@status.im>
🤔 This PR has changes in the |
Our previous test values were not valid. We just added a CheckType.Validate() call to config building, which now causes these invalid checks to fail config testing. This commit corrects the values to be valid checks. Also move the Validate call for service checks into ServiceDefinition.Validate.
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/444028. |
Thanks for fixing the test and CI issues @kisunji ! |
It was all @dnephin! I just pressed the merge button 😆 |
Thanks @dnephin then! Appreciate it. Now our alerts will be a bit less noisy, which is great. Well, I guess I'll have to wait for a release to roll it out, but it's gonna be great. |
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/444828. |
☝️ Will manually cherry pick to make sure this goes into the next 1.10 release |
Ah I may have misled everyone! Judging by Daniel's comments in the original PR (and the docs) this was meant for the next minor version Expect to see this in our Consul 1.11 release! Sorry for the misunderstanding 😬 |
I've deployed v1.11.0-alpha to 12 hosts and set up few services with different values for I'm seeing no issues so far. |
Closes #10701
I don't have permission to make changes on that remote, and CI was not running on that PR, so opening another one to get CI running.