Skip to content

Commit 35ccd29

Browse files
stevesgstevesg
authored
Validate alertmanager storage configuration when sharding enabled. (#4162)
* Validate alertmanager storage configuration when sharding enabled. Storing the "fullstate" objects is not implemented for the "local" and "configdb" storage backend types, or for the legacy storage configuration. Check that these are not given when sharding is enabled. Signed-off-by: Steve Simpson <steve.simpson@grafana.com> * Review comments. Signed-off-by: Steve Simpson <steve.simpson@grafana.com>
1 parent 851b164 commit 35ccd29

File tree

6 files changed

+69
-21
lines changed

6 files changed

+69
-21
lines changed

‎integration/alertmanager_test.go

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -235,13 +235,10 @@ func TestAlertmanagerClustering(t *testing.T) {
235235

236236
func TestAlertmanagerSharding(t *testing.T) {
237237
tests := map[string]struct {
238-
legacyAlertStore bool
239238
replicationFactor int
240239
}{
241-
"legacy alertstore, RF = 2": {legacyAlertStore: true, replicationFactor: 2},
242-
"bucket alertstore, RF = 2": {legacyAlertStore: false, replicationFactor: 2},
243-
"legacy alertstore, RF = 3": {legacyAlertStore: true, replicationFactor: 3},
244-
"bucket alertstore, RF = 3": {legacyAlertStore: false, replicationFactor: 3},
240+
"RF = 2": {replicationFactor: 2},
241+
"RF = 3": {replicationFactor: 3},
245242
}
246243

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

253-
flags := mergeFlags(AlertmanagerFlags(), AlertmanagerS3Flags(testCfg.legacyAlertStore))
250+
flags := mergeFlags(AlertmanagerFlags(), AlertmanagerS3Flags(false))
254251

255252
// Start dependencies.
256253
consul := e2edb.NewConsul()

‎pkg/alertmanager/alertstore/config.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,3 +70,13 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet) {
7070
cfg.Local.RegisterFlagsWithPrefix(prefix, f)
7171
cfg.RegisterFlagsWithPrefix(prefix, f)
7272
}
73+
74+
// IsFullStateSupported returns if the given configuration supports access to FullState objects.
75+
func (cfg *Config) IsFullStateSupported() bool {
76+
for _, backend := range bucket.SupportedBackends {
77+
if cfg.Backend == backend {
78+
return true
79+
}
80+
}
81+
return false
82+
}

‎pkg/alertmanager/multitenant.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,9 @@ const (
8686
var (
8787
statusTemplate *template.Template
8888

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

9294
func init() {
@@ -175,7 +177,7 @@ func (cfg *ClusterConfig) RegisterFlags(f *flag.FlagSet) {
175177
}
176178

177179
// Validate config and returns error on failure
178-
func (cfg *MultitenantAlertmanagerConfig) Validate() error {
180+
func (cfg *MultitenantAlertmanagerConfig) Validate(storageCfg alertstore.Config) error {
179181
if cfg.ExternalURL.URL != nil && strings.HasSuffix(cfg.ExternalURL.Path, "/") {
180182
return errInvalidExternalURL
181183
}
@@ -188,6 +190,15 @@ func (cfg *MultitenantAlertmanagerConfig) Validate() error {
188190
return err
189191
}
190192

193+
if cfg.ShardingEnabled {
194+
if !cfg.Store.IsDefaults() {
195+
return errShardingLegacyStorage
196+
}
197+
if !storageCfg.IsFullStateSupported() {
198+
return errShardingUnsupportedStorage
199+
}
200+
}
201+
191202
return nil
192203
}
193204

‎pkg/alertmanager/multitenant_test.go

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -94,45 +94,75 @@ func mockAlertmanagerConfig(t *testing.T) *MultitenantAlertmanagerConfig {
9494

9595
func TestMultitenantAlertmanagerConfig_Validate(t *testing.T) {
9696
tests := map[string]struct {
97-
setup func(t *testing.T, cfg *MultitenantAlertmanagerConfig)
97+
setup func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config)
9898
expected error
9999
}{
100100
"should pass with default config": {
101-
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig) {},
101+
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {},
102102
expected: nil,
103103
},
104104
"should fail if persistent interval is 0": {
105-
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig) {
105+
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
106106
cfg.Persister.Interval = 0
107107
},
108108
expected: errInvalidPersistInterval,
109109
},
110110
"should fail if persistent interval is negative": {
111-
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig) {
111+
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
112112
cfg.Persister.Interval = -1
113113
},
114114
expected: errInvalidPersistInterval,
115115
},
116116
"should fail if external URL ends with /": {
117-
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig) {
117+
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
118118
require.NoError(t, cfg.ExternalURL.Set("http://localhost/prefix/"))
119119
},
120120
expected: errInvalidExternalURL,
121121
},
122122
"should succeed if external URL does not end with /": {
123-
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig) {
123+
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
124124
require.NoError(t, cfg.ExternalURL.Set("http://localhost/prefix"))
125125
},
126126
expected: nil,
127127
},
128+
"should succeed if sharding enabled and new storage configuration given with bucket client": {
129+
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
130+
cfg.ShardingEnabled = true
131+
storageCfg.Backend = "s3"
132+
},
133+
expected: nil,
134+
},
135+
"should fail if sharding enabled and new storage store configuration given with local type": {
136+
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
137+
cfg.ShardingEnabled = true
138+
storageCfg.Backend = "local"
139+
},
140+
expected: errShardingUnsupportedStorage,
141+
},
142+
"should fail if sharding enabled and new storage store configuration given with configdb type": {
143+
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
144+
cfg.ShardingEnabled = true
145+
storageCfg.Backend = "configdb"
146+
},
147+
expected: errShardingUnsupportedStorage,
148+
},
149+
"should fail if sharding enabled and legacy store configuration given": {
150+
setup: func(t *testing.T, cfg *MultitenantAlertmanagerConfig, storageCfg *alertstore.Config) {
151+
cfg.ShardingEnabled = true
152+
cfg.Store.Type = "s3"
153+
},
154+
expected: errShardingLegacyStorage,
155+
},
128156
}
129157

130158
for testName, testData := range tests {
131159
t.Run(testName, func(t *testing.T) {
132160
cfg := &MultitenantAlertmanagerConfig{}
161+
storageCfg := alertstore.Config{}
133162
flagext.DefaultValues(cfg)
134-
testData.setup(t, cfg)
135-
assert.Equal(t, testData.expected, cfg.Validate())
163+
flagext.DefaultValues(&storageCfg)
164+
testData.setup(t, cfg, &storageCfg)
165+
assert.Equal(t, testData.expected, cfg.Validate(storageCfg))
136166
})
137167
}
138168
}

‎pkg/cortex/cortex.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -236,12 +236,12 @@ func (c *Config) Validate(log log.Logger) error {
236236
if err := c.Compactor.Validate(); err != nil {
237237
return errors.Wrap(err, "invalid compactor config")
238238
}
239-
if err := c.Alertmanager.Validate(); err != nil {
240-
return errors.Wrap(err, "invalid alertmanager config")
241-
}
242239
if err := c.AlertmanagerStorage.Validate(); err != nil {
243240
return errors.Wrap(err, "invalid alertmanager storage config")
244241
}
242+
if err := c.Alertmanager.Validate(c.AlertmanagerStorage); err != nil {
243+
return errors.Wrap(err, "invalid alertmanager config")
244+
}
245245

246246
if c.Storage.Engine == storage.StorageEngineBlocks && c.Querier.SecondStoreEngine != storage.StorageEngineChunks && len(c.Schema.Configs) > 0 {
247247
level.Warn(log).Log("schema configuration is not used by the blocks storage engine, and will have no effect")

‎pkg/storage/bucket/client.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ const (
3737
)
3838

3939
var (
40-
supportedBackends = []string{S3, GCS, Azure, Swift, Filesystem}
40+
SupportedBackends = []string{S3, GCS, Azure, Swift, Filesystem}
4141

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

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

6969
// RegisterFlags registers the backend storage config.

0 commit comments

Comments
 (0)