Merge branch 'jc-change-praefect-boostrap-env-vars' into 'master'

Move bootstrap env vars into bootstrap constructor

See merge request gitlab-org/gitaly!1670

8 files changed, 27 insertions, 29 deletions

diff --git a/changelogs/unreleased/jc-change-praefect-boostrap-env-vars.yml b/changelogs/unreleased/jc-change-praefect-boostrap-env-vars.yml
new file mode 100644
index 000000000..67050d4f7
--- /dev/null
+++ b/changelogs/unreleased/jc-change-praefect-boostrap-env-vars.yml
@@ -0,0 +1,5 @@
+---
+title: Move bootstrap env vars into bootstrap constructor
+merge_request: 1670
+author:
+type: other
diff --git a/cmd/gitaly-wrapper/main.go b/cmd/gitaly-wrapper/main.go
index e1c1e367f..ec0d2c522 100644
--- a/cmd/gitaly-wrapper/main.go
+++ b/cmd/gitaly-wrapper/main.go
@@ -12,7 +12,7 @@ import (
 	"time"
 
 	"github.com/sirupsen/logrus"
-	"gitlab.com/gitlab-org/gitaly/internal/config"
+	"gitlab.com/gitlab-org/gitaly/internal/bootstrap"
 	"gitlab.com/gitlab-org/gitaly/internal/ps"
 )
 
@@ -35,7 +35,7 @@ func main() {
 	log.Info("Wrapper started")
 
 	if pidFile() == "" {
-		log.Fatalf("missing pid file ENV variable %q", config.EnvPidFile)
+		log.Fatalf("missing pid file ENV variable %q", bootstrap.EnvPidFile)
 	}
 
 	log.WithField("pid_file", pidFile()).Info("finding gitaly")
@@ -91,7 +91,7 @@ func findGitaly() (*os.Process, error) {
 
 func spawnGitaly(bin string, args []string) (*os.Process, error) {
 	cmd := exec.Command(bin, args...)
-	cmd.Env = append(os.Environ(), fmt.Sprintf("%s=true", config.EnvUpgradesEnabled))
+	cmd.Env = append(os.Environ(), fmt.Sprintf("%s=true", bootstrap.EnvUpgradesEnabled))
 
 	cmd.Stdin = os.Stdin
 	cmd.Stdout = os.Stdout
@@ -153,7 +153,7 @@ func isGitaly(p *os.Process, gitalyBin string) bool {
 }
 
 func pidFile() string {
-	return os.Getenv(config.EnvPidFile)
+	return os.Getenv(bootstrap.EnvPidFile)
 }
 
 func jsonLogging() bool {
diff --git a/cmd/gitaly-wrapper/main_test.go b/cmd/gitaly-wrapper/main_test.go
index f4d98a374..2b5778142 100644
--- a/cmd/gitaly-wrapper/main_test.go
+++ b/cmd/gitaly-wrapper/main_test.go
@@ -8,20 +8,20 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/internal/config"
+	"gitlab.com/gitlab-org/gitaly/internal/bootstrap"
 )
 
 // TestStolenPid tests for regressions in https://gitlab.com/gitlab-org/gitaly/issues/1661
 func TestStolenPid(t *testing.T) {
 	defer func(oldValue string) {
-		os.Setenv(config.EnvPidFile, oldValue)
-	}(os.Getenv(config.EnvPidFile))
+		os.Setenv(bootstrap.EnvPidFile, oldValue)
+	}(os.Getenv(bootstrap.EnvPidFile))
 
 	pidFile, err := ioutil.TempFile("", "pidfile")
 	require.NoError(t, err)
 	defer os.Remove(pidFile.Name())
 
-	os.Setenv(config.EnvPidFile, pidFile.Name())
+	os.Setenv(bootstrap.EnvPidFile, pidFile.Name())
 
 	cmd := exec.Command("tail", "-f")
 	require.NoError(t, cmd.Start())
diff --git a/cmd/gitaly/main.go b/cmd/gitaly/main.go
index f8860df2c..82a60fc40 100644
--- a/cmd/gitaly/main.go
+++ b/cmd/gitaly/main.go
@@ -46,9 +46,7 @@ func main() {
 	flag.Usage = flagUsage
 	flag.Parse()
 
-	// gitaly-wrapper is supposed to set config.EnvUpgradesEnabled in order to enable graceful upgrades
-	_, isWrapped := os.LookupEnv(config.EnvUpgradesEnabled)
-	b, err := bootstrap.New(os.Getenv(config.EnvPidFile), isWrapped)
+	b, err := bootstrap.New()
 	if err != nil {
 		log.WithError(err).Fatal("init bootstrap")
 	}
diff --git a/cmd/praefect/main.go b/cmd/praefect/main.go
index 8fb6c0c9c..af7245a7e 100644
--- a/cmd/praefect/main.go
+++ b/cmd/praefect/main.go
@@ -120,9 +120,7 @@ func run(cfgs []starter.Config, conf config.Config) error {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	_, isWrapped := os.LookupEnv(config.EnvUpgradesEnabled)
-
-	b, err := bootstrap.New(os.Getenv(config.EnvPidFile), isWrapped)
+	b, err := bootstrap.New()
 	if err != nil {
 		return fmt.Errorf("unable to create a bootstrap: %v", err)
 	}
diff --git a/internal/bootstrap/bootstrap.go b/internal/bootstrap/bootstrap.go
index 41c1c7d89..214487e63 100644
--- a/internal/bootstrap/bootstrap.go
+++ b/internal/bootstrap/bootstrap.go
@@ -13,6 +13,13 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/config"
 )
 
+const (
+	// EnvPidFile is the name of the environment variable containing the pid file path
+	EnvPidFile = "GITALY_PID_FILE"
+	// EnvUpgradesEnabled is an environment variable that when defined gitaly must enable graceful upgrades on SIGHUP
+	EnvUpgradesEnabled = "GITALY_UPGRADES_ENABLED"
+)
+
 // Bootstrap handles graceful upgrades
 type Bootstrap struct {
 	// StopAction will be invoked during a graceful stop. It must wait until the shutdown is completed
@@ -54,7 +61,11 @@ type upgrader interface {
 // * upg.Exit() channel in p1 will be closed now and p1 can gracefully terminate already accepted connections
 // * upgrades cannot starts again if p1 and p2 are both running, an hard termination should be scheduled to overcome
 //   freezes during a graceful shutdown
-func New(pidFile string, upgradesEnabled bool) (*Bootstrap, error) {
+// gitaly-wrapper is supposed to set EnvUpgradesEnabled in order to enable graceful upgrades
+func New() (*Bootstrap, error) {
+	pidFile := os.Getenv(EnvPidFile)
+	_, upgradesEnabled := os.LookupEnv(EnvUpgradesEnabled)
+
 	// PIDFile is optional, if provided tableflip will keep it updated
 	upg, err := tableflip.New(tableflip.Options{PIDFile: pidFile})
 	if err != nil {
diff --git a/internal/config/config.go b/internal/config/config.go
index a814e315c..945e5b048 100644
--- a/internal/config/config.go
+++ b/internal/config/config.go
@@ -21,13 +21,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/helper/text"
 )
 
-const (
-	// EnvPidFile is the name of the environment variable containing the pid file path
-	EnvPidFile = "GITALY_PID_FILE"
-	// EnvUpgradesEnabled is an environment variable that when defined gitaly must enable graceful upgrades on SIGHUP
-	EnvUpgradesEnabled = "GITALY_UPGRADES_ENABLED"
-)
-
 var (
 	// Config stores the global configuration
 	Config Cfg
diff --git a/internal/praefect/config/config.go b/internal/praefect/config/config.go
index cdd4b3e81..b074c1f01 100644
--- a/internal/praefect/config/config.go
+++ b/internal/praefect/config/config.go
@@ -13,13 +13,6 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/praefect/models"
 )
 
-const (
-	// EnvPidFile is the name of the environment variable containing the pid file path
-	EnvPidFile = "PRAEFECT_PID_FILE"
-	// EnvUpgradesEnabled is an environment variable that when defined gitaly must enable graceful upgrades on SIGHUP
-	EnvUpgradesEnabled = "PRAEFECT_UPGRADES_ENABLED"
-)
-
 // Config is a container for everything found in the TOML config file
 type Config struct {
 	ListenAddr      string            `toml:"listen_addr"`