Merge branch 'ps-deprecate-get-storage-path' into 'master'

Deprecate helper.GetStorageByName

See merge request gitlab-org/gitaly!2889

6 files changed, 13 insertions, 12 deletions

diff --git a/internal/git/bitmap.go b/internal/git/bitmap.go
index 4d645b4ef..0ff93b619 100644
--- a/internal/git/bitmap.go
+++ b/internal/git/bitmap.go
@@ -10,7 +10,7 @@ import (
 	grpc_ctxtags "github.com/grpc-ecosystem/go-grpc-middleware/tags"
 	"github.com/prometheus/client_golang/prometheus"
 	"gitlab.com/gitlab-org/gitaly/internal/git/packfile"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/internal/storage"
 )
 
 var badBitmapRequestCount = prometheus.NewCounterVec(
@@ -26,10 +26,10 @@ func init() { prometheus.MustRegister(badBitmapRequestCount) }
 // WarnIfTooManyBitmaps checks for too many (more than one) bitmaps in
 // repoPath, and if it finds any, it logs a warning. This is to help us
 // investigate https://gitlab.com/gitlab-org/gitaly/issues/1728.
-func WarnIfTooManyBitmaps(ctx context.Context, storageName, repoPath string) {
+func WarnIfTooManyBitmaps(ctx context.Context, locator storage.Locator, storageName, repoPath string) {
 	logEntry := ctxlogrus.Extract(ctx)
 
-	storageRoot, err := helper.GetStorageByName(storageName)
+	storageRoot, err := locator.GetStorageByName(storageName)
 	if err != nil {
 		logEntry.WithError(err).Info("bitmap check failed")
 		return
diff --git a/internal/git/objectpool/pool.go b/internal/git/objectpool/pool.go
index 2f9fc4b09..bf73f2929 100644
--- a/internal/git/objectpool/pool.go
+++ b/internal/git/objectpool/pool.go
@@ -14,7 +14,6 @@ import (
 
 	"gitlab.com/gitlab-org/gitaly/internal/git"
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
 	"gitlab.com/gitlab-org/gitaly/internal/storage"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
@@ -45,7 +44,7 @@ type ObjectPool struct {
 // shard. Relative path is validated to match the expected naming and directory
 // structure. If the shard cannot be found, this function returns an error.
 func NewObjectPool(cfg config.Cfg, locator storage.Locator, storageName, relativePath string) (pool *ObjectPool, err error) {
-	storagePath, err := helper.GetStorageByName(storageName)
+	storagePath, err := locator.GetStorageByName(storageName)
 	if err != nil {
 		return nil, err
 	}
@@ -157,7 +156,7 @@ func FromRepo(cfg config.Cfg, locator storage.Locator, repo *gitalypb.Repository
 		return nil, err
 	}
 
-	altPathRelativeToStorage, err := objectPathRelativeToStorage(repo.GetStorageName(), dir, repoPath)
+	altPathRelativeToStorage, err := objectPathRelativeToStorage(locator, repo.GetStorageName(), dir, repoPath)
 	if err != nil {
 		return nil, err
 	}
@@ -213,8 +212,8 @@ func getAlternateObjectDir(locator storage.Locator, repo *gitalypb.Repository) (
 
 // objectPathRelativeToStorage takes an object path that's relative to a repository's object directory
 // and returns the path relative to the storage path of the repository.
-func objectPathRelativeToStorage(storageName, path, repoPath string) (string, error) {
-	storagePath, err := helper.GetStorageByName(storageName)
+func objectPathRelativeToStorage(locator storage.Locator, storageName, path, repoPath string) (string, error) {
+	storagePath, err := locator.GetStorageByName(storageName)
 	if err != nil {
 		return "", err
 	}
diff --git a/internal/git/stats/git_test.go b/internal/git/stats/git_test.go
index 0909b8400..3ff65f981 100644
--- a/internal/git/stats/git_test.go
+++ b/internal/git/stats/git_test.go
@@ -12,7 +12,7 @@ import (
 	"github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus/ctxlogrus"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/require"
-	"gitlab.com/gitlab-org/gitaly/internal/helper"
+	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 	"gitlab.com/gitlab-org/gitaly/internal/testhelper"
 	"gitlab.com/gitlab-org/gitaly/proto/go/gitalypb"
 )
@@ -48,7 +48,8 @@ func TestLogObjectInfo(t *testing.T) {
 	}
 
 	t.Run("shared repo with multiple alternates", func(t *testing.T) {
-		storagePath, err := helper.GetStorageByName(repo1.StorageName)
+		locator := config.NewLocator(config.Config)
+		storagePath, err := locator.GetStorageByName(repo1.GetStorageName())
 		require.NoError(t, err)
 
 		tmpDir, err := ioutil.TempDir(storagePath, "")
diff --git a/internal/gitaly/service/smarthttp/upload_pack.go b/internal/gitaly/service/smarthttp/upload_pack.go
index dd7b665f0..ceac032c0 100644
--- a/internal/gitaly/service/smarthttp/upload_pack.go
+++ b/internal/gitaly/service/smarthttp/upload_pack.go
@@ -70,7 +70,7 @@ func (s *server) PostUploadPack(stream gitalypb.SmartHTTPService_PostUploadPackS
 		return err
 	}
 
-	git.WarnIfTooManyBitmaps(ctx, req.GetRepository().GetStorageName(), repoPath)
+	git.WarnIfTooManyBitmaps(ctx, s.locator, req.GetRepository().GetStorageName(), repoPath)
 	globalOpts := git.UploadPackFilterConfig()
 
 	for _, o := range req.GitConfigOptions {
diff --git a/internal/gitaly/service/ssh/upload_pack.go b/internal/gitaly/service/ssh/upload_pack.go
index 946c8b69c..3eb8a451e 100644
--- a/internal/gitaly/service/ssh/upload_pack.go
+++ b/internal/gitaly/service/ssh/upload_pack.go
@@ -76,7 +76,7 @@ func (s *server) sshUploadPack(stream gitalypb.SSHService_SSHUploadPackServer, r
 		return err
 	}
 
-	git.WarnIfTooManyBitmaps(ctx, req.GetRepository().StorageName, repoPath)
+	git.WarnIfTooManyBitmaps(ctx, s.locator, req.GetRepository().StorageName, repoPath)
 
 	globalOpts := git.UploadPackFilterConfig()
 	for _, o := range req.GitConfigOptions {
diff --git a/internal/helper/repo.go b/internal/helper/repo.go
index 82d71eba7..2d61a73ec 100644
--- a/internal/helper/repo.go
+++ b/internal/helper/repo.go
@@ -68,6 +68,7 @@ func GetPath(repo repository.GitRepo) (string, error) {
 
 // GetStorageByName will return the path for the storage, which is fetched by
 // its key. An error is return if it cannot be found.
+// Deprecated: please use storage.Locator to define the storage path.
 func GetStorageByName(storageName string) (string, error) {
 	storagePath, ok := config.Config.StoragePath(storageName)
 	if !ok {