diff options
author | Sami Hiltunen <shiltunen@gitlab.com> | 2020-11-11 15:14:19 +0300 |
---|---|---|
committer | Sami Hiltunen <shiltunen@gitlab.com> | 2020-11-17 14:12:25 +0300 |
commit | 60a3583e1793fcd81965920ce149ad2345aca777 (patch) | |
tree | e2a41d6bed7c4e501f245ad92b126d5b0425fb1c /internal/praefect/config | |
parent | 5b408dd150d8b08cac58ccc3af4edce40d6042d4 (diff) |
validate election strategy configuration
Praefect currently does not validate election strategy configuration.
This commit adds validation for the config option.
Diffstat (limited to 'internal/praefect/config')
-rw-r--r-- | internal/praefect/config/config.go | 44 | ||||
-rw-r--r-- | internal/praefect/config/config_test.go | 73 |
2 files changed, 93 insertions, 24 deletions
diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go index ca03b7831..ebca60f43 100644 --- a/internal/praefect/config/config.go +++ b/internal/praefect/config/config.go @@ -16,12 +16,35 @@ import ( "gitlab.com/gitlab-org/gitaly/internal/gitaly/config/sentry" ) +// ElectionStrategy is a Praefect primary election strategy. +type ElectionStrategy string + +// validate validates the election strategy is a valid one. +func (es ElectionStrategy) validate() error { + switch es { + case ElectionStrategyLocal, ElectionStrategySQL, ElectionStrategyPerRepository: + return nil + default: + return fmt.Errorf("invalid election strategy: %q", es) + } +} + +const ( + // ElectionStrategyLocal configures a single node, in-memory election strategy. + ElectionStrategyLocal = "local" + // ElectionStrategySQL configures an SQL based strategy that elects a primary for a virtual storage. + ElectionStrategySQL = "sql" + // ElectionStrategyPerRepository configures an SQL based strategy that elects different primaries per repository. + ElectionStrategyPerRepository = "per_repository" +) + type Failover struct { - Enabled bool `toml:"enabled"` - ElectionStrategy string `toml:"election_strategy"` - ErrorThresholdWindow config.Duration `toml:"error_threshold_window"` - WriteErrorThresholdCount uint32 `toml:"write_error_threshold_count"` - ReadErrorThresholdCount uint32 `toml:"read_error_threshold_count"` + Enabled bool `toml:"enabled"` + // ElectionStrategy is the strategy to use for electing primaries nodes. + ElectionStrategy ElectionStrategy `toml:"election_strategy"` + ErrorThresholdWindow config.Duration `toml:"error_threshold_window"` + WriteErrorThresholdCount uint32 `toml:"write_error_threshold_count"` + ReadErrorThresholdCount uint32 `toml:"read_error_threshold_count"` // BootstrapInterval allows set a time duration that would be used on startup to make initial health check. // The default value is 1s. BootstrapInterval config.Duration `toml:"bootstrap_interval"` @@ -52,8 +75,6 @@ func (f Failover) ErrorThresholdsConfigured() (bool, error) { return true, nil } -const sqlFailoverValue = "sql" - // Reconciliation contains reconciliation specific configuration options. type Reconciliation struct { // SchedulingInterval the interval between each automatic reconciliation run. If set to 0, @@ -122,7 +143,7 @@ func FromFile(filePath string) (Config, error) { Reconciliation: DefaultReconciliationConfig(), Replication: DefaultReplicationConfig(), // Sets the default Failover, to be overwritten when deserializing the TOML - Failover: Failover{Enabled: true, ElectionStrategy: sqlFailoverValue}, + Failover: Failover{Enabled: true, ElectionStrategy: ElectionStrategySQL}, } if err := toml.Unmarshal(b, conf); err != nil { return Config{}, err @@ -153,6 +174,10 @@ var ( // Validate establishes if the config is valid func (c *Config) Validate() error { + if err := c.Failover.ElectionStrategy.validate(); err != nil { + return err + } + if c.ListenAddr == "" && c.SocketPath == "" && c.TLSListenAddr == "" { return errNoListener } @@ -209,8 +234,7 @@ func (c *Config) Validate() error { // NeedsSQL returns true if the driver for SQL needs to be initialized func (c *Config) NeedsSQL() bool { - return !c.MemoryQueueEnabled || - (c.Failover.Enabled && c.Failover.ElectionStrategy == sqlFailoverValue) + return !c.MemoryQueueEnabled || (c.Failover.Enabled && c.Failover.ElectionStrategy != ElectionStrategyLocal) } func (c *Config) setDefaults() { diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go index be87c9f89..fb65e80f7 100644 --- a/internal/praefect/config/config_test.go +++ b/internal/praefect/config/config_test.go @@ -27,21 +27,49 @@ func TestConfigValidation(t *testing.T) { {Storage: "internal-3.1", Address: "localhost:33458", Token: "secret-token-2"}, } + fromValidConfig := func(changeFunc func(*Config)) Config { + cfg := Config{ + ListenAddr: "localhost:1234", + Replication: DefaultReplicationConfig(), + VirtualStorages: []*VirtualStorage{ + {Name: "default", Nodes: vs1Nodes}, + {Name: "secondary", Nodes: vs2Nodes}, + }, + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, + } + + changeFunc(&cfg) + + return cfg + } + testCases := []struct { desc string config Config errMsg string }{ { - desc: "Valid config with ListenAddr", - config: Config{ - ListenAddr: "localhost:1234", - Replication: DefaultReplicationConfig(), - VirtualStorages: []*VirtualStorage{ - {Name: "default", Nodes: vs1Nodes}, - {Name: "secondary", Nodes: vs2Nodes}, - }, - }, + desc: "Valid config with ListenAddr", + config: fromValidConfig(func(*Config) {}), + }, + { + desc: "Valid config with local elector", + config: fromValidConfig(func(cfg *Config) { + cfg.Failover.ElectionStrategy = ElectionStrategyLocal + }), + }, + { + desc: "Valid config with per repository elector", + config: fromValidConfig(func(cfg *Config) { + cfg.Failover.ElectionStrategy = ElectionStrategyPerRepository + }), + }, + { + desc: "Invalid election strategy", + config: fromValidConfig(func(cfg *Config) { + cfg.Failover.ElectionStrategy = "invalid-strategy" + }), + errMsg: `invalid election strategy: "invalid-strategy"`, }, { desc: "Valid config with TLSListenAddr", @@ -51,6 +79,7 @@ func TestConfigValidation(t *testing.T) { VirtualStorages: []*VirtualStorage{ {Name: "default", Nodes: vs1Nodes}, }, + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, }, }, { @@ -61,6 +90,7 @@ func TestConfigValidation(t *testing.T) { VirtualStorages: []*VirtualStorage{ {Name: "default", Nodes: vs1Nodes}, }, + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, }, }, { @@ -71,6 +101,7 @@ func TestConfigValidation(t *testing.T) { VirtualStorages: []*VirtualStorage{ {Name: "default", Nodes: vs1Nodes}, }, + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, }, errMsg: "replication batch size was 0 but must be >=1", }, @@ -84,6 +115,7 @@ func TestConfigValidation(t *testing.T) { {Name: "default", Nodes: vs1Nodes}, }, Replication: DefaultReplicationConfig(), + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, }, errMsg: "no listen address or socket path configured", }, @@ -92,6 +124,7 @@ func TestConfigValidation(t *testing.T) { config: Config{ ListenAddr: "localhost:1234", Replication: DefaultReplicationConfig(), + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, }, errMsg: "no virtual storages configured", }, @@ -109,6 +142,7 @@ func TestConfigValidation(t *testing.T) { }), }, }, + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, }, errMsg: `virtual storage "default": internal gitaly storages are not unique`, }, @@ -129,6 +163,7 @@ func TestConfigValidation(t *testing.T) { }, }, }, + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, }, errMsg: `virtual storage "default": all gitaly nodes must have a storage`, }, @@ -149,6 +184,7 @@ func TestConfigValidation(t *testing.T) { }, }, }, + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, }, errMsg: `virtual storage "default": all gitaly nodes must have an address`, }, @@ -160,6 +196,7 @@ func TestConfigValidation(t *testing.T) { VirtualStorages: []*VirtualStorage{ {Name: "", Nodes: vs1Nodes}, }, + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, }, errMsg: `virtual storages must have a name`, }, @@ -172,6 +209,7 @@ func TestConfigValidation(t *testing.T) { {Name: "default", Nodes: vs1Nodes}, {Name: "default", Nodes: vs2Nodes}, }, + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, }, errMsg: `virtual storage "default": virtual storages must have unique names`, }, @@ -184,6 +222,7 @@ func TestConfigValidation(t *testing.T) { {Name: "default", Nodes: vs1Nodes}, {Name: "secondary", Nodes: nil}, }, + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, }, errMsg: `virtual storage "secondary": no primary gitaly backends configured`, }, @@ -196,6 +235,7 @@ func TestConfigValidation(t *testing.T) { {Name: "default", Nodes: vs1Nodes}, {Name: "secondary", Nodes: append(vs2Nodes, vs1Nodes[1])}, }, + Failover: Failover{ElectionStrategy: ElectionStrategySQL}, }, errMsg: `multiple storages have the same address`, }, @@ -283,7 +323,7 @@ func TestConfigParsing(t *testing.T) { Replication: Replication{BatchSize: 1}, Failover: Failover{ Enabled: true, - ElectionStrategy: sqlFailoverValue, + ElectionStrategy: ElectionStrategySQL, ErrorThresholdWindow: config.Duration(20 * time.Second), WriteErrorThresholdCount: 1500, ReadErrorThresholdCount: 100, @@ -319,7 +359,7 @@ func TestConfigParsing(t *testing.T) { Replication: DefaultReplicationConfig(), Failover: Failover{ Enabled: true, - ElectionStrategy: sqlFailoverValue, + ElectionStrategy: ElectionStrategySQL, BootstrapInterval: config.Duration(time.Second), MonitorInterval: config.Duration(3 * time.Second), }, @@ -437,17 +477,22 @@ func TestNeedsSQL(t *testing.T) { }, { desc: "Failover enabled with SQL election strategy", - config: Config{Failover: Failover{Enabled: true, ElectionStrategy: "sql"}}, + config: Config{Failover: Failover{Enabled: true, ElectionStrategy: ElectionStrategyPerRepository}}, expected: true, }, { desc: "Both PostgresQL and SQL election strategy enabled", - config: Config{Failover: Failover{Enabled: true, ElectionStrategy: "sql"}}, + config: Config{Failover: Failover{Enabled: true, ElectionStrategy: ElectionStrategyPerRepository}}, expected: true, }, { desc: "Both PostgresQL and SQL election strategy enabled but failover disabled", - config: Config{Failover: Failover{Enabled: false, ElectionStrategy: "sql"}}, + config: Config{Failover: Failover{Enabled: false, ElectionStrategy: ElectionStrategyPerRepository}}, + expected: true, + }, + { + desc: "Both PostgresQL and per_repository election strategy enabled but failover disabled", + config: Config{Failover: Failover{Enabled: false, ElectionStrategy: ElectionStrategyPerRepository}}, expected: true, }, } |