Merge branch 'ps-simplify-duration-in-config' into 'master'

Simplification of parameters with Duration type

See merge request gitlab-org/gitaly!2232

6 files changed, 38 insertions, 44 deletions

diff --git a/cmd/gitaly/main.go b/cmd/gitaly/main.go
index 0ead8eefd..a03e31ebd 100644
--- a/cmd/gitaly/main.go
+++ b/cmd/gitaly/main.go
@@ -154,5 +154,5 @@ func run(b *bootstrap.Bootstrap) error {
 		return fmt.Errorf("initialize gitaly-ruby: %v", err)
 	}
 
-	return b.Wait(config.Config.GracefulRestartTimeout)
+	return b.Wait(config.Config.GracefulRestartTimeout.Duration())
 }
diff --git a/internal/config/config.go b/internal/config/config.go
index 2b9a55b75..c7aef2f08 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -31,25 +31,24 @@ var (
 
 // Cfg is a container for all config derived from config.toml.
 type Cfg struct {
-	SocketPath                 string            `toml:"socket_path" split_words:"true"`
-	ListenAddr                 string            `toml:"listen_addr" split_words:"true"`
-	TLSListenAddr              string            `toml:"tls_listen_addr" split_words:"true"`
-	PrometheusListenAddr       string            `toml:"prometheus_listen_addr" split_words:"true"`
-	BinDir                     string            `toml:"bin_dir"`
-	Git                        Git               `toml:"git" envconfig:"git"`
-	Storages                   []Storage         `toml:"storage" envconfig:"storage"`
-	Logging                    Logging           `toml:"logging" envconfig:"logging"`
-	Prometheus                 prometheus.Config `toml:"prometheus"`
-	Auth                       auth.Config       `toml:"auth"`
-	TLS                        TLS               `toml:"tls"`
-	Ruby                       Ruby              `toml:"gitaly-ruby"`
-	Gitlab                     Gitlab            `toml:"gitlab"`
-	GitlabShell                GitlabShell       `toml:"gitlab-shell"`
-	Hooks                      Hooks             `toml:"hooks"`
-	Concurrency                []Concurrency     `toml:"concurrency"`
-	GracefulRestartTimeout     time.Duration
-	GracefulRestartTimeoutToml Duration `toml:"graceful_restart_timeout"`
-	InternalSocketDir          string   `toml:"internal_socket_dir"`
+	SocketPath             string            `toml:"socket_path" split_words:"true"`
+	ListenAddr             string            `toml:"listen_addr" split_words:"true"`
+	TLSListenAddr          string            `toml:"tls_listen_addr" split_words:"true"`
+	PrometheusListenAddr   string            `toml:"prometheus_listen_addr" split_words:"true"`
+	BinDir                 string            `toml:"bin_dir"`
+	Git                    Git               `toml:"git" envconfig:"git"`
+	Storages               []Storage         `toml:"storage" envconfig:"storage"`
+	Logging                Logging           `toml:"logging" envconfig:"logging"`
+	Prometheus             prometheus.Config `toml:"prometheus"`
+	Auth                   auth.Config       `toml:"auth"`
+	TLS                    TLS               `toml:"tls"`
+	Ruby                   Ruby              `toml:"gitaly-ruby"`
+	Gitlab                 Gitlab            `toml:"gitlab"`
+	GitlabShell            GitlabShell       `toml:"gitlab-shell"`
+	Hooks                  Hooks             `toml:"hooks"`
+	Concurrency            []Concurrency     `toml:"concurrency"`
+	GracefulRestartTimeout Duration          `toml:"graceful_restart_timeout"`
+	InternalSocketDir      string            `toml:"internal_socket_dir"`
 }
 
 // TLS configuration
@@ -173,9 +172,8 @@ func Validate() error {
 }
 
 func (c *Cfg) setDefaults() {
-	c.GracefulRestartTimeout = c.GracefulRestartTimeoutToml.Duration()
-	if c.GracefulRestartTimeout == 0 {
-		c.GracefulRestartTimeout = 1 * time.Minute
+	if c.GracefulRestartTimeout.Duration() == 0 {
+		c.GracefulRestartTimeout = Duration(time.Minute)
 	}
 
 	if c.Gitlab.SecretFile == "" {
diff --git a/internal/config/config_test.go b/internal/config/config_test.go
index 8e31520bc..b747e20a4 100644
--- a/internal/config/config_test.go
+++ b/internal/config/config_test.go
@@ -658,7 +658,7 @@ func TestLoadGracefulRestartTimeout(t *testing.T) {
 			err := Load(tmpFile)
 			assert.NoError(t, err)
 
-			assert.Equal(t, test.expected, Config.GracefulRestartTimeout)
+			assert.Equal(t, test.expected, Config.GracefulRestartTimeout.Duration())
 		})
 	}
 }
diff --git a/internal/config/ruby.go b/internal/config/ruby.go
index e5e43c170..bd9722d8b 100644
--- a/internal/config/ruby.go
+++ b/internal/config/ruby.go
@@ -8,15 +8,13 @@ import (
 
 // Ruby contains setting for Ruby worker processes
 type Ruby struct {
-	Dir                        string `toml:"dir"`
-	MaxRSS                     int    `toml:"max_rss"`
-	GracefulRestartTimeout     time.Duration
-	GracefulRestartTimeoutToml Duration `toml:"graceful_restart_timeout"`
-	RestartDelay               time.Duration
-	RestartDelayToml           Duration `toml:"restart_delay"`
-	NumWorkers                 int      `toml:"num_workers"`
-	LinguistLanguagesPath      string   `toml:"linguist_languages_path"`
-	RuggedGitConfigSearchPath  string   `toml:"rugged_git_config_search_path"`
+	Dir                       string   `toml:"dir"`
+	MaxRSS                    int      `toml:"max_rss"`
+	GracefulRestartTimeout    Duration `toml:"graceful_restart_timeout"`
+	RestartDelay              Duration `toml:"restart_delay"`
+	NumWorkers                int      `toml:"num_workers"`
+	LinguistLanguagesPath     string   `toml:"linguist_languages_path"`
+	RuggedGitConfigSearchPath string   `toml:"rugged_git_config_search_path"`
 }
 
 // Duration is a trick to let our TOML library parse durations from strings.
@@ -43,18 +41,16 @@ func (d Duration) MarshalText() ([]byte, error) {
 
 // ConfigureRuby validates the gitaly-ruby configuration and sets default values.
 func ConfigureRuby() error {
-	Config.Ruby.GracefulRestartTimeout = Config.Ruby.GracefulRestartTimeoutToml.Duration()
-	if Config.Ruby.GracefulRestartTimeout == 0 {
-		Config.Ruby.GracefulRestartTimeout = 10 * time.Minute
+	if Config.Ruby.GracefulRestartTimeout.Duration() == 0 {
+		Config.Ruby.GracefulRestartTimeout = Duration(10 * time.Minute)
 	}
 
 	if Config.Ruby.MaxRSS == 0 {
 		Config.Ruby.MaxRSS = 200 * 1024 * 1024
 	}
 
-	Config.Ruby.RestartDelay = Config.Ruby.RestartDelayToml.Duration()
-	if Config.Ruby.RestartDelay == 0 {
-		Config.Ruby.RestartDelay = 5 * time.Minute
+	if Config.Ruby.RestartDelay.Duration() == 0 {
+		Config.Ruby.RestartDelay = Duration(5 * time.Minute)
 	}
 
 	if len(Config.Ruby.Dir) == 0 {
diff --git a/internal/rubyserver/worker.go b/internal/rubyserver/worker.go
index 160e83c48..1e9c71b3d 100644
--- a/internal/rubyserver/worker.go
+++ b/internal/rubyserver/worker.go
@@ -123,7 +123,7 @@ func (w *worker) monitor() {
 				}
 
 				swMem.mark()
-				if swMem.elapsed() <= config.Config.Ruby.RestartDelay {
+				if swMem.elapsed() <= config.Config.Ruby.RestartDelay.Duration() {
 					break nextEvent
 				}
 
@@ -219,7 +219,7 @@ func (w *worker) waitTerminate(pid int) {
 	w.logPid(pid).Info("sending SIGTERM")
 	syscall.Kill(pid, syscall.SIGTERM)
 
-	time.Sleep(config.Config.Ruby.GracefulRestartTimeout)
+	time.Sleep(config.Config.Ruby.GracefulRestartTimeout.Duration())
 
 	w.logPid(pid).Info("sending SIGKILL")
 	syscall.Kill(pid, syscall.SIGKILL)
diff --git a/internal/rubyserver/worker_test.go b/internal/rubyserver/worker_test.go
index 5b0599b74..5156956fb 100644
--- a/internal/rubyserver/worker_test.go
+++ b/internal/rubyserver/worker_test.go
@@ -14,9 +14,9 @@ func TestWorker(t *testing.T) {
 	restartDelay := 10 * time.Millisecond
 
 	defer func(old time.Duration) {
-		config.Config.Ruby.RestartDelay = old
-	}(config.Config.Ruby.RestartDelay)
-	config.Config.Ruby.RestartDelay = restartDelay
+		config.Config.Ruby.RestartDelay = config.Duration(old)
+	}(config.Config.Ruby.RestartDelay.Duration())
+	config.Config.Ruby.RestartDelay = config.Duration(restartDelay)
 
 	events := make(chan supervisor.Event)
 	addr := "the address"