From e19c6904261f72e1f73517dd98c06bc361049e6e Mon Sep 17 00:00:00 2001 From: Ganesh Vernekar <15064823+codesome@users.noreply.github.com> Date: Mon, 12 Jul 2021 11:15:16 +0530 Subject: [PATCH] Alerting: Fix potential panic in Alertmanager when starting up (#36562) * Alerting: Fix potential panic in Alertmanager when starting up Signed-off-by: Ganesh Vernekar * Fix reviews Signed-off-by: Ganesh Vernekar --- pkg/services/ngalert/api/api_alertmanager.go | 3 +++ pkg/services/ngalert/notifier/alertmanager.go | 12 +++++++++++- pkg/services/ngalert/notifier/alerts.go | 5 +++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/pkg/services/ngalert/api/api_alertmanager.go b/pkg/services/ngalert/api/api_alertmanager.go index db30e7df622..c0b3dabb10f 100644 --- a/pkg/services/ngalert/api/api_alertmanager.go +++ b/pkg/services/ngalert/api/api_alertmanager.go @@ -153,6 +153,9 @@ func (srv AlertmanagerSrv) RouteGetAMAlerts(c *models.ReqContext) response.Respo if errors.Is(err, notifier.ErrGetAlertsBadPayload) { return ErrResp(http.StatusBadRequest, err, "") } + if errors.Is(err, notifier.ErrGetAlertsUnavailable) { + return ErrResp(http.StatusServiceUnavailable, err, "") + } // any other error here should be an unexpected failure and thus an internal error return ErrResp(http.StatusInternalServerError, err, "") } diff --git a/pkg/services/ngalert/notifier/alertmanager.go b/pkg/services/ngalert/notifier/alertmanager.go index 85e1e3b4085..241e79a8c6b 100644 --- a/pkg/services/ngalert/notifier/alertmanager.go +++ b/pkg/services/ngalert/notifier/alertmanager.go @@ -104,6 +104,8 @@ type Alertmanager struct { reloadConfigMtx sync.RWMutex config []byte + + initialised bool } func New(cfg *setting.Cfg, store store.AlertingStore, m *metrics.Metrics) (*Alertmanager, error) { @@ -269,7 +271,15 @@ func (am *Alertmanager) SyncAndApplyConfigFromDatabase() error { // applyConfig applies a new configuration by re-initializing all components using the configuration provided. // It is not safe to call concurrently. -func (am *Alertmanager) applyConfig(cfg *apimodels.PostableUserConfig, rawConfig []byte) error { +func (am *Alertmanager) applyConfig(cfg *apimodels.PostableUserConfig, rawConfig []byte) (err error) { + defer func() { + if err == nil { + // We consider AM as initialised only when the config has been + // applied at least once successfully. Until then, some objects + // can still be nil. + am.initialised = true + } + }() // First, let's make sure this config is not already loaded var configChanged bool if rawConfig == nil { diff --git a/pkg/services/ngalert/notifier/alerts.go b/pkg/services/ngalert/notifier/alerts.go index 8f0d515d5e5..02078db01c3 100644 --- a/pkg/services/ngalert/notifier/alerts.go +++ b/pkg/services/ngalert/notifier/alerts.go @@ -16,6 +16,7 @@ import ( var ( ErrGetAlertsInternal = fmt.Errorf("unable to retrieve alerts(s) due to an internal error") + ErrGetAlertsUnavailable = fmt.Errorf("unable to retrieve alerts(s) as alertmanager is not initialised yet") ErrGetAlertsBadPayload = fmt.Errorf("unable to retrieve alerts") ErrGetAlertGroupsBadPayload = fmt.Errorf("unable to retrieve alerts groups") ) @@ -27,6 +28,10 @@ func (am *Alertmanager) GetAlerts(active, silenced, inhibited bool, filter []str res = apimodels.GettableAlerts{} ) + if !am.initialised { + return res, ErrGetAlertsUnavailable + } + matchers, err := parseFilter(filter) if err != nil { am.logger.Error("failed to parse matchers", "err", err)