praefect: Move of the service checks

The set of the checks that is used to make sure praefect is ready
to serve the requests is moved to another location. That is done
because we should be able to call them from the RPC handler as
they will be used by the ReadinessCheck RPC.

Part of: https://gitlab.com/gitlab-org/gitlab/-/issues/348174

6 files changed, 69 insertions, 54 deletions

diff --git a/cmd/praefect/subcmd.go b/cmd/praefect/subcmd.go
index 292da9a30..523c41af5 100644
--- a/cmd/praefect/subcmd.go
+++ b/cmd/praefect/subcmd.go
@@ -12,10 +12,9 @@ import (
 
 	gitalyauth "gitlab.com/gitlab-org/gitaly/v15/auth"
 	"gitlab.com/gitlab-org/gitaly/v15/client"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore/glsql"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect/service"
 	"google.golang.org/grpc"
 )
 
@@ -43,17 +42,10 @@ var subcommands = map[string]subcmd{
 	removeRepositoryCmdName:       newRemoveRepository(logger, os.Stdout),
 	trackRepositoryCmdName:        newTrackRepository(logger, os.Stdout),
 	listUntrackedRepositoriesName: newListUntrackedRepositories(logger, os.Stdout),
-	checkCmdName: newCheckSubcommand(
-		os.Stdout,
-		praefect.NewPraefectMigrationCheck,
-		praefect.NewGitalyNodeConnectivityCheck,
-		praefect.NewPostgresReadWriteCheck,
-		praefect.NewUnavailableReposCheck,
-		praefect.NewClockSyncCheck(helper.CheckClockSync),
-	),
-	metadataCmdName:     newMetadataSubcommand(os.Stdout),
-	verifyCmdName:       newVerifySubcommand(os.Stdout),
-	listStoragesCmdName: newListStorages(os.Stdout),
+	checkCmdName:                  newCheckSubcommand(os.Stdout, service.AllChecks()...),
+	metadataCmdName:               newMetadataSubcommand(os.Stdout),
+	verifyCmdName:                 newVerifySubcommand(os.Stdout),
+	listStoragesCmdName:           newListStorages(os.Stdout),
 }
 
 // subCommand returns an exit code, to be fed into os.Exit.
diff --git a/cmd/praefect/subcmd_check.go b/cmd/praefect/subcmd_check.go
index d50d475d3..7e69140d9 100644
--- a/cmd/praefect/subcmd_check.go
+++ b/cmd/praefect/subcmd_check.go
@@ -8,8 +8,8 @@ import (
 	"io"
 	"time"
 
-	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect/config"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect/service"
 )
 
 const (
@@ -19,10 +19,10 @@ const (
 type checkSubcommand struct {
 	w          io.Writer
 	quiet      bool
-	checkFuncs []praefect.CheckFunc
+	checkFuncs []service.CheckFunc
 }
 
-func newCheckSubcommand(writer io.Writer, checkFuncs ...praefect.CheckFunc) *checkSubcommand {
+func newCheckSubcommand(writer io.Writer, checkFuncs ...service.CheckFunc) *checkSubcommand {
 	return &checkSubcommand{
 		w:          writer,
 		checkFuncs: checkFuncs,
@@ -43,8 +43,8 @@ func (cmd *checkSubcommand) FlagSet() *flag.FlagSet {
 
 var errFatalChecksFailed = errors.New("checks failed")
 
-func (cmd *checkSubcommand) Exec(flags *flag.FlagSet, cfg config.Config) error {
-	var allChecks []*praefect.Check
+func (cmd *checkSubcommand) Exec(_ *flag.FlagSet, cfg config.Config) error {
+	var allChecks []*service.Check
 	for _, checkFunc := range cmd.checkFuncs {
 		allChecks = append(allChecks, checkFunc(cfg, cmd.w, cmd.quiet))
 	}
@@ -60,7 +60,7 @@ func (cmd *checkSubcommand) Exec(flags *flag.FlagSet, cfg config.Config) error {
 
 		if err := check.Run(ctx); err != nil {
 			failedChecks++
-			if check.Severity == praefect.Fatal {
+			if check.Severity == service.Fatal {
 				passed = false
 			}
 			fmt.Fprintf(cmd.w, "Failed (%s) error: %s\n", check.Severity, err.Error())
@@ -85,7 +85,7 @@ func (cmd *checkSubcommand) Exec(flags *flag.FlagSet, cfg config.Config) error {
 	return nil
 }
 
-func (cmd *checkSubcommand) printCheckDetails(check *praefect.Check) {
+func (cmd *checkSubcommand) printCheckDetails(check *service.Check) {
 	if cmd.quiet {
 		fmt.Fprintf(cmd.w, "Checking %s...", check.Name)
 		return
diff --git a/cmd/praefect/subcmd_check_test.go b/cmd/praefect/subcmd_check_test.go
index d076510f9..3df258a47 100644
--- a/cmd/praefect/subcmd_check_test.go
+++ b/cmd/praefect/subcmd_check_test.go
@@ -11,8 +11,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect/config"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect/service"
 )
 
 func TestCheckSubcommand_Exec(t *testing.T) {
@@ -20,36 +20,36 @@ func TestCheckSubcommand_Exec(t *testing.T) {
 
 	testCases := []struct {
 		desc                string
-		checks              []praefect.CheckFunc
+		checks              []service.CheckFunc
 		expectedQuietOutput string
 		expectedOutput      string
 		expectedError       error
 	}{
 		{
 			desc: "all checks pass",
-			checks: []praefect.CheckFunc{
-				func(cfg config.Config, w io.Writer, quiet bool) *praefect.Check {
-					return &praefect.Check{
+			checks: []service.CheckFunc{
+				func(cfg config.Config, w io.Writer, quiet bool) *service.Check {
+					return &service.Check{
 						Name:        "check 1",
 						Description: "checks a",
 						Run:         func(ctx context.Context) error { return nil },
-						Severity:    praefect.Fatal,
+						Severity:    service.Fatal,
 					}
 				},
-				func(cfg config.Config, w io.Writer, quiet bool) *praefect.Check {
-					return &praefect.Check{
+				func(cfg config.Config, w io.Writer, quiet bool) *service.Check {
+					return &service.Check{
 						Name:        "check 2",
 						Description: "checks b",
 						Run:         func(ctx context.Context) error { return nil },
-						Severity:    praefect.Fatal,
+						Severity:    service.Fatal,
 					}
 				},
-				func(cfg config.Config, w io.Writer, quiet bool) *praefect.Check {
-					return &praefect.Check{
+				func(cfg config.Config, w io.Writer, quiet bool) *service.Check {
+					return &service.Check{
 						Name:        "check 3",
 						Description: "checks c",
 						Run:         func(ctx context.Context) error { return nil },
-						Severity:    praefect.Fatal,
+						Severity:    service.Fatal,
 					}
 				},
 			},
@@ -72,29 +72,29 @@ All checks passed.
 		},
 		{
 			desc: "a fatal check fails",
-			checks: []praefect.CheckFunc{
-				func(cfg config.Config, w io.Writer, quiet bool) *praefect.Check {
-					return &praefect.Check{
+			checks: []service.CheckFunc{
+				func(cfg config.Config, w io.Writer, quiet bool) *service.Check {
+					return &service.Check{
 						Name:        "check 1",
 						Description: "checks a",
 						Run:         func(ctx context.Context) error { return nil },
-						Severity:    praefect.Fatal,
+						Severity:    service.Fatal,
 					}
 				},
-				func(cfg config.Config, w io.Writer, quiet bool) *praefect.Check {
-					return &praefect.Check{
+				func(cfg config.Config, w io.Writer, quiet bool) *service.Check {
+					return &service.Check{
 						Name:        "check 2",
 						Description: "checks b",
 						Run:         func(ctx context.Context) error { return errors.New("i failed") },
-						Severity:    praefect.Fatal,
+						Severity:    service.Fatal,
 					}
 				},
-				func(cfg config.Config, w io.Writer, quiet bool) *praefect.Check {
-					return &praefect.Check{
+				func(cfg config.Config, w io.Writer, quiet bool) *service.Check {
+					return &service.Check{
 						Name:        "check 3",
 						Description: "checks c",
 						Run:         func(ctx context.Context) error { return nil },
-						Severity:    praefect.Fatal,
+						Severity:    service.Fatal,
 					}
 				},
 			},
@@ -117,29 +117,29 @@ Checking check 3...Passed
 		},
 		{
 			desc: "only warning checks fail",
-			checks: []praefect.CheckFunc{
-				func(cfg config.Config, w io.Writer, quiet bool) *praefect.Check {
-					return &praefect.Check{
+			checks: []service.CheckFunc{
+				func(cfg config.Config, w io.Writer, quiet bool) *service.Check {
+					return &service.Check{
 						Name:        "check 1",
 						Description: "checks a",
 						Run:         func(ctx context.Context) error { return nil },
-						Severity:    praefect.Fatal,
+						Severity:    service.Fatal,
 					}
 				},
-				func(cfg config.Config, w io.Writer, quiet bool) *praefect.Check {
-					return &praefect.Check{
+				func(cfg config.Config, w io.Writer, quiet bool) *service.Check {
+					return &service.Check{
 						Name:        "check 2",
 						Description: "checks b",
 						Run:         func(ctx context.Context) error { return errors.New("i failed but not too badly") },
-						Severity:    praefect.Warning,
+						Severity:    service.Warning,
 					}
 				},
-				func(cfg config.Config, w io.Writer, quiet bool) *praefect.Check {
-					return &praefect.Check{
+				func(cfg config.Config, w io.Writer, quiet bool) *service.Check {
+					return &service.Check{
 						Name:        "check 3",
 						Description: "checks c",
 						Run:         func(ctx context.Context) error { return errors.New("i failed but not too badly") },
-						Severity:    praefect.Warning,
+						Severity:    service.Warning,
 					}
 				},
 			},
diff --git a/internal/praefect/checks.go b/internal/praefect/service/checks.go
index 32d026d45..06f82c01d 100644
--- a/internal/praefect/checks.go
+++ b/internal/praefect/service/checks.go
@@ -1,4 +1,4 @@
-package praefect
+package service
 
 import (
 	"context"
@@ -12,6 +12,7 @@ import (
 	migrate "github.com/rubenv/sql-migrate"
 	gitalyauth "gitlab.com/gitlab-org/gitaly/v15/auth"
 	"gitlab.com/gitlab-org/gitaly/v15/client"
+	"gitlab.com/gitlab-org/gitaly/v15/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/helper/env"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect/config"
 	"gitlab.com/gitlab-org/gitaly/v15/internal/praefect/datastore"
@@ -48,6 +49,17 @@ type Check struct {
 // CheckFunc is a function type that takes a praefect config and returns a Check
 type CheckFunc func(conf config.Config, w io.Writer, quiet bool) *Check
 
+// AllChecks returns slice of all checks that can be executed for praefect.
+func AllChecks() []CheckFunc {
+	return []CheckFunc{
+		NewPraefectMigrationCheck,
+		NewGitalyNodeConnectivityCheck,
+		NewPostgresReadWriteCheck,
+		NewUnavailableReposCheck,
+		NewClockSyncCheck(helper.CheckClockSync),
+	}
+}
+
 // NewPraefectMigrationCheck returns a Check that checks if all praefect migrations have run
 func NewPraefectMigrationCheck(conf config.Config, w io.Writer, quiet bool) *Check {
 	return &Check{
diff --git a/internal/praefect/checks_test.go b/internal/praefect/service/checks_test.go
index b2e4e96e9..851d21692 100644
--- a/internal/praefect/checks_test.go
+++ b/internal/praefect/service/checks_test.go
@@ -1,6 +1,6 @@
 //go:build !gitaly_test_sha256
 
-package praefect
+package service
 
 import (
 	"bytes"
diff --git a/internal/praefect/service/helper_test.go b/internal/praefect/service/helper_test.go
new file mode 100644
index 000000000..4bf810464
--- /dev/null
+++ b/internal/praefect/service/helper_test.go
@@ -0,0 +1,11 @@
+package service
+
+import (
+	"testing"
+
+	"gitlab.com/gitlab-org/gitaly/v15/internal/testhelper"
+)
+
+func TestMain(m *testing.M) {
+	testhelper.Run(m)
+}