Fix TestAlertmanagerSharding integration test. #4083
Merged
Conversation
This file contains hidden or 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
In rare cases the test can fail when creating the silence with the message: > `creating the silence failed with status 404 and error the Alertmanager is not configured` The checking of the metric later on also fails: > ```unable to find metrics [cortex_alertmanager_silences] ...``` This appears to be because the alertmanager has not fully configured the user yet. The `tenants_owned` metric is polled before beginning the test, but this metric gets updated _before_ the call to `setConfig()`, where the `Alertmanager` instance for the user is created. To try and make the test more reliable, we can intead check the `last_reload_successful` metric. This metric is per-user, so does not exist for until `setConfig()` has completed, when it is set to `1`. Additionally, the failure is very hard to reproduce due to the test trying multiple alertmanager instances to create the silence (because the user may not exist in the first instance). This "retrying" will hide the fact the alertmanager has not been configured yet, and often cover up the failure. Therefore, the change makes the issue easier to reproduce by attempting to create the silence in all three instances, and expecting exactly one of them to return an error. This has the side-effect that the silence is created twice. Signed-off-by: Steve Simpson <[email protected]>
3 tasks
pstibrany
approved these changes
Apr 16, 2021
pracucci
approved these changes
Apr 16, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
In rare cases the test can fail when creating the silence with the
message:
The checking of the metric later on also fails:
This appears to be because the alertmanager has not fully configured the
user yet. The
tenants_ownedmetric is polled before beginning thetest, but this metric gets updated before the call to
setConfig(),where the
Alertmanagerinstance for the user is created.To try and make the test more reliable, we can intead check the
last_reload_successfulmetric. This metric is per-user, so does notexist for until
setConfig()has completed, when it is set to1.Additionally, the failure is very hard to reproduce due to the test
trying multiple alertmanager instances to create the silence (because
the user may not exist in the first instance). This "retrying" will hide
the fact the alertmanager has not been configured yet, and often cover
up the failure. Therefore, the change makes the issue easier to
reproduce by attempting to create the silence in all three instances,
and expecting exactly one of them to return an error. This has the
side-effect that the silence is created twice.
Signed-off-by: Steve Simpson [email protected]