Merge branch 'zj-default-sql-failover' into 'master'

failover: Default to enabling SQL strategy

Closes #2682

See merge request gitlab-org/gitaly!2218

5 files changed, 42 insertions, 4 deletions

diff --git a/changelogs/unreleased/zj-default-sql-failover.yml b/changelogs/unreleased/zj-default-sql-failover.yml
new file mode 100644
index 000000000..7e41e9b86
--- /dev/null
+++ b/changelogs/unreleased/zj-default-sql-failover.yml
@@ -0,0 +1,5 @@
+---
+title: 'failover: Default to enabling SQL strategy'
+merge_request: 2218
+author:
+type: added
diff --git a/config.praefect.toml.example b/config.praefect.toml.example
index 8b768a93d..9552c868b 100644
--- a/config.praefect.toml.example
+++ b/config.praefect.toml.example
@@ -31,8 +31,8 @@ listen_addr = "127.0.0.1:2305"
 
 [failover]
 enabled = true
-election_strategy = "local" # Options: local, sql
-read_only_after_failover = false # Switch the virtual storage to read-only mode after after a failover.
+election_strategy = "sql" # Options: local, sql. Defaults to 'sql'.
+read_only_after_failover = true # Switch the virtual storage to read-only mode after after a failover.
 
 [[virtual_storage]]
 name = 'praefect'
diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go
index c69104cd9..e25f909ff 100644
--- a/internal/praefect/config/config.go
+++ b/internal/praefect/config/config.go
@@ -20,6 +20,8 @@ type Failover struct {
 	ReadOnlyAfterFailover bool   `toml:"read_only_after_failover"`
 }
 
+const sqlFailoverValue = "sql"
+
 // Config is a container for everything found in the TOML config file
 type Config struct {
 	ListenAddr           string            `toml:"listen_addr"`
@@ -46,7 +48,10 @@ type VirtualStorage struct {
 
 // FromFile loads the config for the passed file path
 func FromFile(filePath string) (Config, error) {
-	conf := &Config{}
+	conf := &Config{
+		// Sets the default Failover, to be overwritten when deserializing the TOML
+		Failover: Failover{Enabled: true, ElectionStrategy: sqlFailoverValue, ReadOnlyAfterFailover: true},
+	}
 	if _, err := toml.DecodeFile(filePath, conf); err != nil {
 		return Config{}, err
 	}
@@ -143,7 +148,8 @@ 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 == "sql")
+	return !c.MemoryQueueEnabled ||
+		(c.Failover.Enabled && c.Failover.ElectionStrategy == sqlFailoverValue)
 }
 
 func (c *Config) setDefaults() {
diff --git a/internal/praefect/config/config_test.go b/internal/praefect/config/config_test.go
index aa78dae87..982565229 100644
--- a/internal/praefect/config/config_test.go
+++ b/internal/praefect/config/config_test.go
@@ -265,6 +265,23 @@ func TestConfigParsing(t *testing.T) {
 				},
 				MemoryQueueEnabled:  true,
 				GracefulStopTimeout: config.Duration(30 * time.Second),
+				Failover: Failover{
+					Enabled:               true,
+					ElectionStrategy:      sqlFailoverValue,
+					ReadOnlyAfterFailover: true,
+				},
+			},
+		},
+		{
+			desc:     "overwriting default values in the config",
+			filePath: "testdata/config.overwritedefaults.toml",
+			expected: Config{
+				GracefulStopTimeout: config.Duration(time.Minute),
+				Failover: Failover{
+					Enabled:               false,
+					ElectionStrategy:      "local",
+					ReadOnlyAfterFailover: false,
+				},
 			},
 		},
 		{
@@ -272,6 +289,11 @@ func TestConfigParsing(t *testing.T) {
 			filePath: "testdata/config.empty.toml",
 			expected: Config{
 				GracefulStopTimeout: config.Duration(time.Minute),
+				Failover: Failover{
+					Enabled:               true,
+					ElectionStrategy:      sqlFailoverValue,
+					ReadOnlyAfterFailover: true,
+				},
 			},
 		},
 		{
diff --git a/internal/praefect/config/testdata/config.overwritedefaults.toml b/internal/praefect/config/testdata/config.overwritedefaults.toml
new file mode 100644
index 000000000..39771fcd6
--- /dev/null
+++ b/internal/praefect/config/testdata/config.overwritedefaults.toml
@@ -0,0 +1,5 @@
+[failover]
+enabled = false
+election_strategy = "local"
+read_only_after_failover = false
+