From a077f0e3dedcb305a88b03ba3136a8427d37266f Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Mon, 3 Oct 2022 10:31:15 -0700 Subject: [PATCH 1/4] Fix 4888 Signed-off-by: Alan Protasio --- pkg/alertmanager/alertstore/local/store.go | 30 ++++++++- .../alertstore/local/store_test.go | 17 ++++- pkg/alertmanager/multitenant_test.go | 66 +++++++++++++++++++ 3 files changed, 110 insertions(+), 3 deletions(-) diff --git a/pkg/alertmanager/alertstore/local/store.go b/pkg/alertmanager/alertstore/local/store.go index 0c1a4520eb2..775ad4ccbc0 100644 --- a/pkg/alertmanager/alertstore/local/store.go +++ b/pkg/alertmanager/alertstore/local/store.go @@ -14,7 +14,8 @@ import ( ) const ( - Name = "local" + Name = "local" + templatesDir = "templates" ) var ( @@ -148,9 +149,36 @@ func (f *Store) reloadConfigs() (map[string]alertspb.AlertConfigDesc, error) { // The file name must correspond to the user tenant ID user := strings.TrimSuffix(info.Name(), ext) + // Load template files + userTemplateDir := filepath.Join(f.cfg.Path, user, templatesDir) + var templates []*alertspb.TemplateDesc + + if _, e := os.Stat(userTemplateDir); e == nil { + err = filepath.Walk(filepath.Join(f.cfg.Path, user, templatesDir), func(path string, info os.FileInfo, err error) error { + if err != nil { + return errors.Wrapf(err, "unable to walk file path at %s", path) + } + // Ignore files that are directories + if info.IsDir() { + return nil + } + content, err := os.ReadFile(path) + if err != nil { + return errors.Wrapf(err, "unable to read alertmanager templates %s", path) + } + + templates = append(templates, &alertspb.TemplateDesc{ + Body: string(content), + Filename: info.Name(), + }) + return nil + }) + } + configs[user] = alertspb.AlertConfigDesc{ User: user, RawConfig: string(content), + Templates: templates, } return nil }) diff --git a/pkg/alertmanager/alertstore/local/store_test.go b/pkg/alertmanager/alertstore/local/store_test.go index 177727173bb..35b460ad505 100644 --- a/pkg/alertmanager/alertstore/local/store_test.go +++ b/pkg/alertmanager/alertstore/local/store_test.go @@ -81,17 +81,22 @@ func TestStore_GetAlertConfigs(t *testing.T) { // The storage contains some configs. { user1Cfg := prepareAlertmanagerConfig("user-1") - require.NoError(t, os.WriteFile(filepath.Join(storeDir, "user-1.yaml"), []byte(user1Cfg), os.ModePerm)) + user1Dir, user1TemplateDir := prepareUserDir(storeDir, "user-1") + require.NoError(t, os.WriteFile(filepath.Join(user1Dir, "user-1.yaml"), []byte(user1Cfg), os.ModePerm)) + + require.NoError(t, os.WriteFile(filepath.Join(user1TemplateDir, "template.tpl"), []byte("testTemplate"), os.ModePerm)) configs, err := store.GetAlertConfigs(ctx, []string{"user-1", "user-2"}) require.NoError(t, err) assert.Contains(t, configs, "user-1") assert.NotContains(t, configs, "user-2") assert.Equal(t, user1Cfg, configs["user-1"].RawConfig) + assert.Equal(t, "testTemplate", configs["user-1"].Templates[0].Body) // Add another user config. user2Cfg := prepareAlertmanagerConfig("user-2") - require.NoError(t, os.WriteFile(filepath.Join(storeDir, "user-2.yaml"), []byte(user2Cfg), os.ModePerm)) + user2Dir, _ := prepareUserDir(storeDir, "user-2") + require.NoError(t, os.WriteFile(filepath.Join(user2Dir, "user-2.yaml"), []byte(user2Cfg), os.ModePerm)) configs, err = store.GetAlertConfigs(ctx, []string{"user-1", "user-2"}) require.NoError(t, err) @@ -102,6 +107,14 @@ func TestStore_GetAlertConfigs(t *testing.T) { } } +func prepareUserDir(storeDir string, user string) (userDir string, templateDir string) { + userDir = filepath.Join(storeDir, user) + templateDir = filepath.Join(userDir, templatesDir) + os.MkdirAll(userDir, os.ModePerm) + os.MkdirAll(templateDir, os.ModePerm) + return +} + func prepareLocalStore(t *testing.T) (store *Store, storeDir string) { var err error diff --git a/pkg/alertmanager/multitenant_test.go b/pkg/alertmanager/multitenant_test.go index 85329b509fe..a03f896bb0f 100644 --- a/pkg/alertmanager/multitenant_test.go +++ b/pkg/alertmanager/multitenant_test.go @@ -14,11 +14,13 @@ import ( "os" "path/filepath" "regexp" + "sort" "strings" "sync" "testing" "time" + "github.com/cortexproject/cortex/pkg/alertmanager/alertstore/local" "github.com/go-kit/log" "github.com/prometheus/alertmanager/cluster/clusterpb" "github.com/prometheus/alertmanager/notify" @@ -159,6 +161,49 @@ func TestMultitenantAlertmanagerConfig_Validate(t *testing.T) { } } +func TestMultitenantAlertmanager_loadAndSyncConfigsLocalStorage(t *testing.T) { + storeDir := t.TempDir() + store, _ := local.NewStore(local.StoreConfig{Path: storeDir}) + config := `global: + resolve_timeout: 1m + smtp_require_tls: false + +route: + receiver: 'email' + +receivers: +- name: 'email' + email_configs: + - to: test@example.com + from: test@example.com + smarthost: smtp:2525 +` + user1Dir, user1TemplateDir := prepareUserDir(storeDir, "user-1") + user2Dir, _ := prepareUserDir(storeDir, "user-2") + require.NoError(t, os.WriteFile(filepath.Join(user1Dir, "user-1.yaml"), []byte(config), os.ModePerm)) + require.NoError(t, os.WriteFile(filepath.Join(user2Dir, "user-2.yaml"), []byte(config), os.ModePerm)) + require.NoError(t, os.WriteFile(filepath.Join(user1TemplateDir, "template.tpl"), []byte("testTemplate"), os.ModePerm)) + + originalFiles, err := listFiles(storeDir) + require.NoError(t, err) + require.Equal(t, 3, len(originalFiles)) + + cfg := mockAlertmanagerConfig(t) + cfg.DataDir = storeDir + reg := prometheus.NewPedanticRegistry() + am, err := createMultitenantAlertmanager(cfg, nil, nil, store, nil, nil, log.NewNopLogger(), reg) + require.NoError(t, err) + for i := 0; i < 5; i++ { + err = am.loadAndSyncConfigs(context.Background(), reasonPeriodic) + require.NoError(t, err) + require.Len(t, am.alertmanagers, 2) + files, err := listFiles(storeDir) + require.NoError(t, err) + // Verify if the files were not deleted + require.Equal(t, originalFiles, files) + } +} + func TestMultitenantAlertmanager_loadAndSyncConfigs(t *testing.T) { ctx := context.Background() @@ -1885,6 +1930,27 @@ func prepareInMemoryAlertStore() alertstore.AlertStore { return bucketclient.NewBucketAlertStore(objstore.NewInMemBucket(), nil, log.NewNopLogger()) } +func prepareUserDir(storeDir string, user string) (userDir string, templateDir string) { + userDir = filepath.Join(storeDir, user) + templateDir = filepath.Join(userDir, templatesDir) + os.MkdirAll(userDir, os.ModePerm) + os.MkdirAll(templateDir, os.ModePerm) + return +} + +func listFiles(dir string) ([]string, error) { + var r []string + err := filepath.Walk(dir, func(path string, info os.FileInfo, err error) error { + if !info.IsDir() { + r = append(r, path) + } + return nil + }) + sort.Strings(r) + + return r, err +} + func TestSafeTemplateFilepath(t *testing.T) { tests := map[string]struct { dir string From ee8c2f6d21f907221f7fa44efd45c26bf5f19cc7 Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Mon, 3 Oct 2022 10:36:35 -0700 Subject: [PATCH 2/4] Changelog Signed-off-by: Alan Protasio --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 543a3d6ee51..dba69562d29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,7 @@ * [BUGFIX] Ruler: Fix /ruler/rule_groups returns YAML with extra fields. #4767 * [BUGFIX] Respecting `-tracing.otel.sample-ratio` configuration when enabling OpenTelemetry tracing with X-ray. #4862 * [BUGFIX] QueryFrontend: fixed query_range requests when query has `start` equals to `end`. #4877 +* [BUGFIX] AlertManager: fixed issue introduced by #4495 where templates files were being deleted when using alertmanager local store. #4890 ## 1.13.0 2022-07-14 From dba6bc417adafabd2dc0c5c248812ad641b2a35c Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Mon, 3 Oct 2022 10:44:43 -0700 Subject: [PATCH 3/4] Make lint happy Signed-off-by: Alan Protasio --- pkg/alertmanager/alertstore/local/store.go | 4 ++++ pkg/alertmanager/alertstore/local/store_test.go | 10 +++++----- pkg/alertmanager/multitenant_test.go | 12 ++++++------ 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/pkg/alertmanager/alertstore/local/store.go b/pkg/alertmanager/alertstore/local/store.go index 775ad4ccbc0..15fdd548d69 100644 --- a/pkg/alertmanager/alertstore/local/store.go +++ b/pkg/alertmanager/alertstore/local/store.go @@ -173,6 +173,10 @@ func (f *Store) reloadConfigs() (map[string]alertspb.AlertConfigDesc, error) { }) return nil }) + + if err != nil { + return errors.Wrapf(err, "unable to load alertmanager config %s", path) + } } configs[user] = alertspb.AlertConfigDesc{ diff --git a/pkg/alertmanager/alertstore/local/store_test.go b/pkg/alertmanager/alertstore/local/store_test.go index 35b460ad505..c853ff84351 100644 --- a/pkg/alertmanager/alertstore/local/store_test.go +++ b/pkg/alertmanager/alertstore/local/store_test.go @@ -81,7 +81,7 @@ func TestStore_GetAlertConfigs(t *testing.T) { // The storage contains some configs. { user1Cfg := prepareAlertmanagerConfig("user-1") - user1Dir, user1TemplateDir := prepareUserDir(storeDir, "user-1") + user1Dir, user1TemplateDir := prepareUserDir(t, storeDir, "user-1") require.NoError(t, os.WriteFile(filepath.Join(user1Dir, "user-1.yaml"), []byte(user1Cfg), os.ModePerm)) require.NoError(t, os.WriteFile(filepath.Join(user1TemplateDir, "template.tpl"), []byte("testTemplate"), os.ModePerm)) @@ -95,7 +95,7 @@ func TestStore_GetAlertConfigs(t *testing.T) { // Add another user config. user2Cfg := prepareAlertmanagerConfig("user-2") - user2Dir, _ := prepareUserDir(storeDir, "user-2") + user2Dir, _ := prepareUserDir(t, storeDir, "user-2") require.NoError(t, os.WriteFile(filepath.Join(user2Dir, "user-2.yaml"), []byte(user2Cfg), os.ModePerm)) configs, err = store.GetAlertConfigs(ctx, []string{"user-1", "user-2"}) @@ -107,11 +107,11 @@ func TestStore_GetAlertConfigs(t *testing.T) { } } -func prepareUserDir(storeDir string, user string) (userDir string, templateDir string) { +func prepareUserDir(t *testing.T, storeDir string, user string) (userDir string, templateDir string) { userDir = filepath.Join(storeDir, user) templateDir = filepath.Join(userDir, templatesDir) - os.MkdirAll(userDir, os.ModePerm) - os.MkdirAll(templateDir, os.ModePerm) + require.NoError(t, os.MkdirAll(userDir, os.ModePerm)) + require.NoError(t, os.MkdirAll(templateDir, os.ModePerm)) return } diff --git a/pkg/alertmanager/multitenant_test.go b/pkg/alertmanager/multitenant_test.go index a03f896bb0f..8c953d054f6 100644 --- a/pkg/alertmanager/multitenant_test.go +++ b/pkg/alertmanager/multitenant_test.go @@ -20,7 +20,6 @@ import ( "testing" "time" - "github.com/cortexproject/cortex/pkg/alertmanager/alertstore/local" "github.com/go-kit/log" "github.com/prometheus/alertmanager/cluster/clusterpb" "github.com/prometheus/alertmanager/notify" @@ -42,6 +41,7 @@ import ( "github.com/cortexproject/cortex/pkg/alertmanager/alertspb" "github.com/cortexproject/cortex/pkg/alertmanager/alertstore" "github.com/cortexproject/cortex/pkg/alertmanager/alertstore/bucketclient" + "github.com/cortexproject/cortex/pkg/alertmanager/alertstore/local" "github.com/cortexproject/cortex/pkg/ring" "github.com/cortexproject/cortex/pkg/ring/kv/consul" "github.com/cortexproject/cortex/pkg/storage/bucket" @@ -178,8 +178,8 @@ receivers: from: test@example.com smarthost: smtp:2525 ` - user1Dir, user1TemplateDir := prepareUserDir(storeDir, "user-1") - user2Dir, _ := prepareUserDir(storeDir, "user-2") + user1Dir, user1TemplateDir := prepareUserDir(t, storeDir, "user-1") + user2Dir, _ := prepareUserDir(t, storeDir, "user-2") require.NoError(t, os.WriteFile(filepath.Join(user1Dir, "user-1.yaml"), []byte(config), os.ModePerm)) require.NoError(t, os.WriteFile(filepath.Join(user2Dir, "user-2.yaml"), []byte(config), os.ModePerm)) require.NoError(t, os.WriteFile(filepath.Join(user1TemplateDir, "template.tpl"), []byte("testTemplate"), os.ModePerm)) @@ -1930,11 +1930,11 @@ func prepareInMemoryAlertStore() alertstore.AlertStore { return bucketclient.NewBucketAlertStore(objstore.NewInMemBucket(), nil, log.NewNopLogger()) } -func prepareUserDir(storeDir string, user string) (userDir string, templateDir string) { +func prepareUserDir(t *testing.T, storeDir string, user string) (userDir string, templateDir string) { userDir = filepath.Join(storeDir, user) templateDir = filepath.Join(userDir, templatesDir) - os.MkdirAll(userDir, os.ModePerm) - os.MkdirAll(templateDir, os.ModePerm) + require.NoError(t, os.MkdirAll(userDir, os.ModePerm)) + require.NoError(t, os.MkdirAll(templateDir, os.ModePerm)) return } From be25df2fd8d9468426d280b9f6d451e5eab61b05 Mon Sep 17 00:00:00 2001 From: Alan Protasio Date: Mon, 3 Oct 2022 15:50:10 -0700 Subject: [PATCH 4/4] Addressing comments Signed-off-by: Alan Protasio --- pkg/alertmanager/alertstore/local/store.go | 12 +++++++----- pkg/alertmanager/alertstore/local/store_test.go | 10 ++++++---- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/alertmanager/alertstore/local/store.go b/pkg/alertmanager/alertstore/local/store.go index 15fdd548d69..3d619dbad3a 100644 --- a/pkg/alertmanager/alertstore/local/store.go +++ b/pkg/alertmanager/alertstore/local/store.go @@ -154,17 +154,17 @@ func (f *Store) reloadConfigs() (map[string]alertspb.AlertConfigDesc, error) { var templates []*alertspb.TemplateDesc if _, e := os.Stat(userTemplateDir); e == nil { - err = filepath.Walk(filepath.Join(f.cfg.Path, user, templatesDir), func(path string, info os.FileInfo, err error) error { + err = filepath.Walk(userTemplateDir, func(templatePath string, info os.FileInfo, err error) error { if err != nil { - return errors.Wrapf(err, "unable to walk file path at %s", path) + return errors.Wrapf(err, "unable to walk file path at %s", templatePath) } // Ignore files that are directories if info.IsDir() { return nil } - content, err := os.ReadFile(path) + content, err := os.ReadFile(templatePath) if err != nil { - return errors.Wrapf(err, "unable to read alertmanager templates %s", path) + return errors.Wrapf(err, "unable to read alertmanager templates %s", templatePath) } templates = append(templates, &alertspb.TemplateDesc{ @@ -175,8 +175,10 @@ func (f *Store) reloadConfigs() (map[string]alertspb.AlertConfigDesc, error) { }) if err != nil { - return errors.Wrapf(err, "unable to load alertmanager config %s", path) + return errors.Wrapf(err, "unable to list alertmanager templates: %s", userTemplateDir) } + } else if !os.IsNotExist(e) { + return errors.Wrapf(e, "unable to read alertmanager templates %s", path) } configs[user] = alertspb.AlertConfigDesc{ diff --git a/pkg/alertmanager/alertstore/local/store_test.go b/pkg/alertmanager/alertstore/local/store_test.go index c853ff84351..8f6a03a6708 100644 --- a/pkg/alertmanager/alertstore/local/store_test.go +++ b/pkg/alertmanager/alertstore/local/store_test.go @@ -81,7 +81,7 @@ func TestStore_GetAlertConfigs(t *testing.T) { // The storage contains some configs. { user1Cfg := prepareAlertmanagerConfig("user-1") - user1Dir, user1TemplateDir := prepareUserDir(t, storeDir, "user-1") + user1Dir, user1TemplateDir := prepareUserDir(t, storeDir, true, "user-1") require.NoError(t, os.WriteFile(filepath.Join(user1Dir, "user-1.yaml"), []byte(user1Cfg), os.ModePerm)) require.NoError(t, os.WriteFile(filepath.Join(user1TemplateDir, "template.tpl"), []byte("testTemplate"), os.ModePerm)) @@ -95,7 +95,7 @@ func TestStore_GetAlertConfigs(t *testing.T) { // Add another user config. user2Cfg := prepareAlertmanagerConfig("user-2") - user2Dir, _ := prepareUserDir(t, storeDir, "user-2") + user2Dir, _ := prepareUserDir(t, storeDir, false, "user-2") require.NoError(t, os.WriteFile(filepath.Join(user2Dir, "user-2.yaml"), []byte(user2Cfg), os.ModePerm)) configs, err = store.GetAlertConfigs(ctx, []string{"user-1", "user-2"}) @@ -107,11 +107,13 @@ func TestStore_GetAlertConfigs(t *testing.T) { } } -func prepareUserDir(t *testing.T, storeDir string, user string) (userDir string, templateDir string) { +func prepareUserDir(t *testing.T, storeDir string, createTemplateDir bool, user string) (userDir string, templateDir string) { userDir = filepath.Join(storeDir, user) templateDir = filepath.Join(userDir, templatesDir) require.NoError(t, os.MkdirAll(userDir, os.ModePerm)) - require.NoError(t, os.MkdirAll(templateDir, os.ModePerm)) + if createTemplateDir { + require.NoError(t, os.MkdirAll(templateDir, os.ModePerm)) + } return }