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.

Sign up for GitHub

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

Jump to bottom

Alerting: Add configuration options to migrate to an external Alertmanager #71318

Merged
merged 6 commits into from
Sep 5, 2023
Merged

Alerting: Add configuration options to migrate to an external Alertmanager #71318

merged 6 commits into from
Sep 5, 2023

Conversation

santihernandezc
Copy link
Contributor

@santihernandezc santihernandezc commented Jul 11, 2023 •
edited
Loading

This PR adds configuration options for disabling the internal Alertmanager and connecting to an external one.

  • disable_internal_alertmanager will shut down the internal Alertmanager
  • main_alertmanager_tenant_id is used for multi-tenancy, so each Grafana instance sends its configuration mapped to a given tenant
  • Basic auth is optional

@santihernandezc santihernandezc added this to the 10.1.x milestone Jul 11, 2023
@santihernandezc santihernandezc requested review from a team, gotjosh, JohnnyQQQQ, alexweav, stevesg and dylanherbig and removed request for a team July 17, 2023 14:22
@santihernandezc santihernandezc marked this pull request as ready for review July 17, 2023 14:22
@santihernandezc santihernandezc requested review from torkelo and a team as code owners July 17, 2023 14:22
@santihernandezc santihernandezc requested review from papagian, zserge and mildwonkey and removed request for a team July 17, 2023 14:22
alexweav
alexweav reviewed Jul 17, 2023
View reviewed changes
conf/defaults.ini Outdated Show resolved Hide resolved
@santihernandezc santihernandezc mentioned this pull request Aug 15, 2023
Closed
gotjosh
gotjosh reviewed Aug 21, 2023
View reviewed changes
Copy link
Contributor

@gotjosh gotjosh left a comment

Choose a reason for hiding this comment

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

Look good - left some feedback on the naming for consistency with the rule evaluation side.

conf/defaults.ini Outdated Show resolved Hide resolved
conf/defaults.ini Outdated Show resolved Hide resolved
conf/defaults.ini Outdated Show resolved Hide resolved
conf/defaults.ini Outdated Show resolved Hide resolved
conf/defaults.ini Outdated Show resolved Hide resolved
conf/defaults.ini Outdated Show resolved Hide resolved
alexweav
alexweav approved these changes Sep 1, 2023
View reviewed changes
Copy link
Contributor

@alexweav alexweav left a comment

Choose a reason for hiding this comment

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

LGTM, but please double check my comments

conf/defaults.ini Show resolved Hide resolved
pkg/setting/setting_unified_alerting.go Show resolved Hide resolved
gotjosh
gotjosh approved these changes Sep 5, 2023
View reviewed changes
Copy link
Contributor

@gotjosh gotjosh left a comment

Choose a reason for hiding this comment

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

LGTM

@santihernandezc santihernandezc merged commit 7a34cdb into main Sep 5, 2023
13 checks passed
@santihernandezc santihernandezc deleted the santihernandezc/external_alertmanager_migration_config_options branch September 5, 2023 14:24
@grafana-delivery-bot grafana-delivery-bot bot modified the milestones: 10.1.x, 10.2.x Sep 5, 2023
chauchausoup pushed a commit to chauchausoup/grafana that referenced this pull request Sep 15, 2023
@santihernandezc @chauchausoup
Alerting: Add configuration options to migrate to an external Alertma…
9b3424d 
…nager (grafana#71318)

* add configuration options to .ini file and parse them

* updates on config options, add external AM config to the main config struct

* separate external AM configs from general alerting configs, naming

* comments about usage of tenantID in basic auth & not using config options yet
rwwiv pushed a commit that referenced this pull request Oct 2, 2023
@santihernandezc @rwwiv
Alerting: Add configuration options to migrate to an external Alertma…
bb4d42d 
…nager (#71318)

* add configuration options to .ini file and parse them

* updates on config options, add external AM config to the main config struct

* separate external AM configs from general alerting configs, naming

* comments about usage of tenantID in basic auth & not using config options yet
@zerok zerok modified the milestones: 10.2.x, 10.2.0 Oct 23, 2023
@dnhn dnhn mentioned this pull request Oct 24, 2023
Closed
@BrewTestBot BrewTestBot mentioned this pull request Oct 25, 2023
Closed
@dragoangel dragoangel mentioned this pull request Feb 14, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dragoangel dragoangel dragoangel left review comments

@alexweav alexweav alexweav approved these changes

@gotjosh gotjosh gotjosh approved these changes

@JohnnyQQQQ JohnnyQQQQ Awaiting requested review from JohnnyQQQQ

@stevesg stevesg Awaiting requested review from stevesg

@dylanherbig dylanherbig Awaiting requested review from dylanherbig

@torkelo torkelo Awaiting requested review from torkelo torkelo is a code owner

@papagian papagian Awaiting requested review from papagian papagian was automatically assigned from grafana/backend-platform

@zserge zserge Awaiting requested review from zserge zserge was automatically assigned from grafana/backend-platform

@mildwonkey mildwonkey Awaiting requested review from mildwonkey mildwonkey was automatically assigned from grafana/backend-platform

Assignees
No one assigned
Labels
add to changelog area/alerting Grafana Alerting area/backend kata:am_unify no-backport Skip backport of PR
Projects
Alerting
Archived in project
Milestone
10.2.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@santihernandezc @gotjosh @dragoangel @alexweav @zerok @grafanabot