Skip to content
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

Update prometheus/alertmanager to version v0.25.1-0.20230505130626-263ca5c9438e #5276

Merged

Conversation

krishnateja325
Copy link
Contributor

@krishnateja325 krishnateja325 commented Apr 19, 2023

What this PR does:
Update Prometheus to prometheus/alertmanager@263ca5c

This includes

Latest released version only contains changes till 2022-12-22. Hence updating the prometheus/alertmanger to include this change.
Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • [NA] Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch 2 times, most recently from 60aa916 to fe55d4e Compare April 19, 2023 20:47
@pull-request-size pull-request-size bot added size/M and removed size/S labels Apr 19, 2023
@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch 4 times, most recently from 416d73f to fce4af6 Compare April 21, 2023 02:06
@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch 3 times, most recently from 3640598 to 11b7ed0 Compare May 4, 2023 18:07
@alvinlin123
Copy link
Contributor

I think it would be useful if we can pull in the prometheus/alertmanager commit that includes prometheus/alertmanager#3352 as part of this PR too.

@krishnateja325
Copy link
Contributor Author

Sure, I can try including that as well @alvinlin123

@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch 3 times, most recently from 20e48ad to 9349480 Compare May 25, 2023 00:50
@pull-request-size pull-request-size bot added size/L and removed size/M labels May 25, 2023
@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch from 9349480 to f7e033e Compare May 25, 2023 01:04
@pull-request-size pull-request-size bot added size/M and removed size/L labels May 25, 2023
@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch from f7e033e to a485436 Compare May 30, 2023 18:50
@krishnateja325 krishnateja325 changed the title Update prometheus/alertmanager to version v0.25.1-0.20230203120921-7923bc5f8ec6 Update prometheus/alertmanager to version v0.25.1-0.20230505130626-263ca5c9438e May 30, 2023
@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch from a485436 to d90e1ac Compare May 30, 2023 22:21
@krishnateja325 krishnateja325 marked this pull request as ready for review May 30, 2023 22:42
Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I think it is really a great job! I have only small nit about the changelog.

CHANGELOG.md Outdated
@@ -1,6 +1,7 @@
# Changelog

## master / unreleased
* [CHANGE] Alertmanager: Validating new fields on the Webhook AM config. #5276
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this update includes multiple changes from AM and the validation change, shall we mention more update/changes we get?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure will include more changes in the change log.

@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch from d90e1ac to 5dcc5a7 Compare May 31, 2023 04:23
@@ -1,6 +1,11 @@
# Changelog

## master / unreleased
* [CHANGE] Updating prometheus/alertmanager from v0.25.0 to v0.25.1-0.20230505130626-263ca5c9438e. This includes the below changes. #5276
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Included user facing changes in the changelog.

@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch from 5dcc5a7 to 9165bbe Compare May 31, 2023 04:59
@@ -686,11 +696,17 @@ func (c *PushoverConfig) UnmarshalYAML(unmarshal func(interface{}) error) error
if err := unmarshal((*plain)(c)); err != nil {
return err
}
if c.UserKey == "" {
return fmt.Errorf("missing user key in Pushover config")
if c.UserKey == "" && c.UserKeyFile == "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to disallow UserKeyFile for PushOver config too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added validation to disallow UserKeyFile in PushOver Config.

if c.UserKey != "" && c.UserKeyFile != "" {
return fmt.Errorf("at most one of user_key & user_key_file must be configured")
}
if c.Token == "" && c.TokenFile == "" {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to disallow TokenFIle for PushOver also.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added validation to disallow TokenFile in PushOver Config.

…23bc5f8ec6

Signed-off-by: Krishna Teja Puttagunta <krishtez@amazon.com>
Signed-off-by: Krishna Teja Puttagunta <krishnateja325@gmail.com>
@krishnateja325 krishnateja325 force-pushed the alertmangerErrorFiltering branch from 9165bbe to 3e36cca Compare June 1, 2023 16:04
@pull-request-size pull-request-size bot added size/L and removed size/M labels Jun 1, 2023
@alvinlin123 alvinlin123 merged commit fba65a1 into cortexproject:master Jun 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants