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

Validate alertmanager storage configuration when sharding enabled. #4162

Merged
merged 2 commits into from
May 11, 2021
Merged

Validate alertmanager storage configuration when sharding enabled. #4162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c488867
Validate alertmanager storage configuration when sharding enabled.
stevesg May 6, 2021
5fa97d9
Review comments.
stevesg May 11, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions integration/alertmanager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,10 @@ func TestAlertmanagerClustering(t *testing.T) {

func TestAlertmanagerSharding(t *testing.T) {
tests := map[string]struct {
legacyAlertStore bool
replicationFactor int
}{
"legacy alertstore, RF = 2": {legacyAlertStore: true, replicationFactor: 2},
"bucket alertstore, RF = 2": {legacyAlertStore: false, replicationFactor: 2},
"legacy alertstore, RF = 3": {legacyAlertStore: true, replicationFactor: 3},
"bucket alertstore, RF = 3": {legacyAlertStore: false, replicationFactor: 3},
"RF = 2": {replicationFactor: 2},
"RF = 3": {replicationFactor: 3},
}

for testName, testCfg := range tests {
Expand All @@ -250,7 +247,7 @@ func TestAlertmanagerSharding(t *testing.T) {
require.NoError(t, err)
defer s.Close()

flags := mergeFlags(AlertmanagerFlags(), AlertmanagerS3Flags(testCfg.legacyAlertStore))
flags := mergeFlags(AlertmanagerFlags(), AlertmanagerS3Flags(false))
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to other reviewers: this integration test was working anyway for the legacy alertstore because it doesn't test storing/loading state to/from storage.


// Start dependencies.
consul := e2edb.NewConsul()
Expand Down
10 changes: 10 additions & 0 deletions pkg/alertmanager/alertstore/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,13 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
cfg.Local.RegisterFlagsWithPrefix(prefix, f)
cfg.RegisterFlagsWithPrefix(prefix, f)
}

// IsFullStateSupported returns if the given configuration supports access to FullState objects.
func (cfg *Config) IsFullStateSupported() bool {
for _, backend := range bucket.SupportedBackends {
if cfg.Backend == backend {
return true
}
}
return false
}
15 changes: 13 additions & 2 deletions pkg/alertmanager/multitenant.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ const (
var (
statusTemplate *template.Template

errInvalidExternalURL = errors.New("the configured external URL is invalid: should not end with /")
errInvalidExternalURL = errors.New("the configured external URL is invalid: should not end with /")
errShardingLegacyStorage = errors.New("deprecated -alertmanager.storage.* not supported with -alertmanager.sharding-enabled, use -alertmanager-storage.*")
errShardingUnsupportedStorage = errors.New("the configured alertmanager storage backend is not supported when sharding is enabled")
)

func init() {
Expand Down Expand Up @@ -174,7 +176,7 @@ func (cfg *ClusterConfig) RegisterFlags(f *flag.FlagSet) {
}

// Validate config and returns error on failure
func (cfg *MultitenantAlertmanagerConfig) Validate() error {
func (cfg *MultitenantAlertmanagerConfig) Validate(storageCfg alertstore.Config) error {
if cfg.ExternalURL.URL != nil && strings.HasSuffix(cfg.ExternalURL.Path, "/") {
return errInvalidExternalURL
}
Expand All @@ -187,6 +189,15 @@ func (cfg *MultitenantAlertmanagerConfig) Validate() error {
return err
}

if cfg.ShardingEnabled {
if !cfg.Store.IsDefaults() {
return errShardingLegacyStorage
}
if !storageCfg.IsFullStateSupported() {
return errShardingUnsupportedStorage
}
}

return nil
}

Expand Down
46 changes: 38 additions & 8 deletions pkg/alertmanager/multitenant_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,45 +92,75 @@ func mockAlertmanagerConfig(t *testing.T) *MultitenantAlertmanagerConfig {

func TestMultitenantAlertmanagerConfig_Validate(t *testing.T) {
tests := map[string]struct {
setup func(t *testing.T, cfg *MultitenantAlertmanagerConfig)
setup func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config)
expected error
}{
"should pass with default config": {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig) {},
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {},
expected: nil,
},
"should fail if persistent interval is 0": {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig) {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
cfg.Persister.Interval = 0
},
expected: errInvalidPersistInterval,
},
"should fail if persistent interval is negative": {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig) {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
cfg.Persister.Interval = -1
},
expected: errInvalidPersistInterval,
},
"should fail if external URL ends with /": {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig) {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
require.NoError(t, cfg.ExternalURL.Set("http://localhost/prefix/"))
},
expected: errInvalidExternalURL,
},
"should succeed if external URL does not end with /": {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig) {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
require.NoError(t, cfg.ExternalURL.Set("http://localhost/prefix"))
},
expected: nil,
},
"should succeed if sharding enabled and new storage configuration given with bucket client": {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
cfg.ShardingEnabled = true
storageCfg.Backend = "s3"
},
expected: nil,
},
"should fail if sharding enabled and new storage store configuration given with local type": {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
cfg.ShardingEnabled = true
storageCfg.Backend = "local"
},
expected: errShardingUnsupportedStorage,
},
"should fail if sharding enabled and new storage store configuration given with configdb type": {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
cfg.ShardingEnabled = true
storageCfg.Backend = "configdb"
},
expected: errShardingUnsupportedStorage,
},
"should fail if sharding enabled and legacy store configuration given": {
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
cfg.ShardingEnabled = true
cfg.Store.Type = "s3"
},
expected: errShardingLegacyStorage,
},
}

for testName, testData := range tests {
t.Run(testName, func(t *testing.T) {
cfg := &MultitenantAlertmanagerConfig{}
storageCfg := alertstore.Config{}
flagext.DefaultValues(cfg)
testData.setup(t, cfg)
assert.Equal(t, testData.expected, cfg.Validate())
flagext.DefaultValues(&storageCfg)
testData.setup(t, cfg, &storageCfg)
assert.Equal(t, testData.expected, cfg.Validate(storageCfg))
})
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/cortex/cortex.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,12 @@ func (c *Config) Validate(log log.Logger) error {
if err := c.Compactor.Validate(); err != nil {
return errors.Wrap(err, "invalid compactor config")
}
if err := c.Alertmanager.Validate(); err != nil {
return errors.Wrap(err, "invalid alertmanager config")
}
if err := c.AlertmanagerStorage.Validate(); err != nil {
return errors.Wrap(err, "invalid alertmanager storage config")
}
if err := c.Alertmanager.Validate(c.AlertmanagerStorage); err != nil {
return errors.Wrap(err, "invalid alertmanager config")
}

if c.Storage.Engine == storage.StorageEngineBlocks && c.Querier.SecondStoreEngine != storage.StorageEngineChunks && len(c.Schema.Configs) > 0 {
level.Warn(log).Log("schema configuration is not used by the blocks storage engine, and will have no effect")
Expand Down
4 changes: 2 additions & 2 deletions pkg/storage/bucket/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const (
)

var (
supportedBackends = []string{S3, GCS, Azure, Swift, Filesystem}
SupportedBackends = []string{S3, GCS, Azure, Swift, Filesystem}

ErrUnsupportedStorageBackend = errors.New("unsupported storage backend")
)
Expand All @@ -63,7 +63,7 @@ type Config struct {

// Returns the supportedBackends for the package and any custom backends injected into the config.
func (cfg *Config) supportedBackends() []string {
return append(supportedBackends, cfg.ExtraBackends...)
return append(SupportedBackends, cfg.ExtraBackends...)
}

// RegisterFlags registers the backend storage config.
Expand Down