From bdb0fb1e75db19747c4e9fd84f5fa4bec6d9faa7 Mon Sep 17 00:00:00 2001 From: James Liu Date: Thu, 14 Dec 2023 11:51:56 +1100 Subject: backup: Delete RemoveAllRepositories from Strategy This is now deprecated in favour of removing individual repos. --- internal/backup/backup.go | 19 ------------------ internal/backup/backup_test.go | 37 ----------------------------------- internal/backup/pipeline.go | 8 -------- internal/backup/pipeline_test.go | 16 ++++----------- internal/backup/server_side.go | 19 ------------------ internal/backup/server_side_test.go | 39 ------------------------------------- 6 files changed, 4 insertions(+), 134 deletions(-) diff --git a/internal/backup/backup.go b/internal/backup/backup.go index afdf63d48..a15f47abe 100644 --- a/internal/backup/backup.go +++ b/internal/backup/backup.go @@ -211,25 +211,6 @@ func NewManagerLocal( } } -// RemoveAllRepositories removes all repositories in the specified storage name. -func (mgr *Manager) RemoveAllRepositories(ctx context.Context, req *RemoveAllRepositoriesRequest) error { - if err := setContextServerInfo(ctx, &req.Server, req.StorageName); err != nil { - return fmt.Errorf("manager: %w", err) - } - - repoClient, err := mgr.newRepoClient(ctx, req.Server) - if err != nil { - return fmt.Errorf("manager: %w", err) - } - - _, err = repoClient.RemoveAll(ctx, &gitalypb.RemoveAllRequest{StorageName: req.StorageName}) - if err != nil { - return fmt.Errorf("manager: %w", err) - } - - return nil -} - // RemoveRepository removes the specified repository from its storage. func (mgr *Manager) RemoveRepository(ctx context.Context, req *RemoveRepositoryRequest) error { if err := setContextServerInfo(ctx, &req.Server, req.Repo.StorageName); err != nil { diff --git a/internal/backup/backup_test.go b/internal/backup/backup_test.go index fa764cc52..81e87a33a 100644 --- a/internal/backup/backup_test.go +++ b/internal/backup/backup_test.go @@ -28,43 +28,6 @@ import ( "google.golang.org/protobuf/proto" ) -func TestManager_RemoveAllRepositories(t *testing.T) { - testhelper.SkipWithWAL(t, ` -RemoveAll is removing the entire content of the storage. This would also remove the database's and -the transaction manager's disk state. The RPC needs to be updated to shut down all partitions and -the database and only then perform the removal. - -Issue: https://gitlab.com/gitlab-org/gitaly/-/issues/5269`) - - t.Parallel() - - cfg := testcfg.Build(t) - cfg.SocketPath = testserver.RunGitalyServer(t, cfg, setup.RegisterAll) - - ctx := testhelper.Context(t) - - repo, repoPath := gittest.CreateRepository(t, ctx, cfg) - commitID := gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main")) - gittest.WriteTag(t, cfg, repoPath, "v1.0.0", commitID.Revision()) - - pool := client.NewPool() - defer testhelper.MustClose(t, pool) - - backupRoot := testhelper.TempDir(t) - sink := backup.NewFilesystemSink(backupRoot) - defer testhelper.MustClose(t, sink) - - locator, err := backup.ResolveLocator("pointer", sink) - require.NoError(t, err) - - fsBackup := backup.NewManager(sink, locator, pool) - err = fsBackup.RemoveAllRepositories(ctx, &backup.RemoveAllRepositoriesRequest{ - Server: storage.ServerInfo{Address: cfg.SocketPath, Token: cfg.Auth.Token}, - StorageName: repo.StorageName, - }) - require.NoError(t, err) -} - func TestManager_RemoveRepository(t *testing.T) { if testhelper.IsPraefectEnabled() { t.Skip("local backup manager expects to operate on the local filesystem so cannot operate through praefect") diff --git a/internal/backup/pipeline.go b/internal/backup/pipeline.go index bfb298350..feb0eaf96 100644 --- a/internal/backup/pipeline.go +++ b/internal/backup/pipeline.go @@ -22,7 +22,6 @@ type Strategy interface { Restore(context.Context, *RestoreRequest) error ListRepositories(context.Context, *ListRepositoriesRequest) ([]*gitalypb.Repository, error) RemoveRepository(context.Context, *RemoveRepositoryRequest) error - RemoveAllRepositories(context.Context, *RemoveAllRepositoriesRequest) error } // CreateRequest is the request to create a backup @@ -64,13 +63,6 @@ type RemoveRepositoryRequest struct { Repo *gitalypb.Repository } -// RemoveAllRepositoriesRequest is the request to remove all repositories in the specified -// storage name. -type RemoveAllRepositoriesRequest struct { - Server storage.ServerInfo - StorageName string -} - // ListRepositoriesRequest is the request to list repositories in a given storage. type ListRepositoriesRequest struct { Server storage.ServerInfo diff --git a/internal/backup/pipeline_test.go b/internal/backup/pipeline_test.go index 06d071f4b..9d05eae74 100644 --- a/internal/backup/pipeline_test.go +++ b/internal/backup/pipeline_test.go @@ -122,11 +122,10 @@ func TestPipeline(t *testing.T) { } type MockStrategy struct { - CreateFunc func(context.Context, *CreateRequest) error - RestoreFunc func(context.Context, *RestoreRequest) error - RemoveAllRepositoriesFunc func(context.Context, *RemoveAllRepositoriesRequest) error - RemoveRepositoryFunc func(context.Context, *RemoveRepositoryRequest) error - ListRepositoriesFunc func(context.Context, *ListRepositoriesRequest) ([]*gitalypb.Repository, error) + CreateFunc func(context.Context, *CreateRequest) error + RestoreFunc func(context.Context, *RestoreRequest) error + RemoveRepositoryFunc func(context.Context, *RemoveRepositoryRequest) error + ListRepositoriesFunc func(context.Context, *ListRepositoriesRequest) ([]*gitalypb.Repository, error) } func (s MockStrategy) Create(ctx context.Context, req *CreateRequest) error { @@ -143,13 +142,6 @@ func (s MockStrategy) Restore(ctx context.Context, req *RestoreRequest) error { return nil } -func (s MockStrategy) RemoveAllRepositories(ctx context.Context, req *RemoveAllRepositoriesRequest) error { - if s.RemoveAllRepositoriesFunc != nil { - return s.RemoveAllRepositoriesFunc(ctx, req) - } - return nil -} - func (s MockStrategy) RemoveRepository(ctx context.Context, req *RemoveRepositoryRequest) error { if s.RemoveRepositoryFunc != nil { return s.RemoveRepositoryFunc(ctx, req) diff --git a/internal/backup/server_side.go b/internal/backup/server_side.go index 1b2319d73..a1a9a37eb 100644 --- a/internal/backup/server_side.go +++ b/internal/backup/server_side.go @@ -95,25 +95,6 @@ func (ss ServerSideAdapter) Restore(ctx context.Context, req *RestoreRequest) er return nil } -// RemoveAllRepositories removes all repositories in the specified storage name. -func (ss ServerSideAdapter) RemoveAllRepositories(ctx context.Context, req *RemoveAllRepositoriesRequest) error { - if err := setContextServerInfo(ctx, &req.Server, req.StorageName); err != nil { - return fmt.Errorf("server-side remove all: %w", err) - } - - repoClient, err := ss.newRepoClient(ctx, req.Server) - if err != nil { - return fmt.Errorf("server-side remove all: %w", err) - } - - _, err = repoClient.RemoveAll(ctx, &gitalypb.RemoveAllRequest{StorageName: req.StorageName}) - if err != nil { - return fmt.Errorf("server-side remove all: %w", err) - } - - return nil -} - // RemoveRepository removes the specified repository from its storage. func (ss ServerSideAdapter) RemoveRepository(ctx context.Context, req *RemoveRepositoryRequest) error { if err := setContextServerInfo(ctx, &req.Server, req.Repo.StorageName); err != nil { diff --git a/internal/backup/server_side_test.go b/internal/backup/server_side_test.go index e4f1383c3..69669df60 100644 --- a/internal/backup/server_side_test.go +++ b/internal/backup/server_side_test.go @@ -259,45 +259,6 @@ func TestServerSideAdapter_Restore(t *testing.T) { } } -func TestServerSideAdapter_RemoveAllRepositories(t *testing.T) { - testhelper.SkipWithWAL(t, ` -RemoveAll is removing the entire content of the storage. This would also remove the database's and -the transaction manager's disk state. The RPC needs to be updated to shut down all partitions and -the database and only then perform the removal. - -Issue: https://gitlab.com/gitlab-org/gitaly/-/issues/5269`) - - t.Parallel() - - backupRoot := testhelper.TempDir(t) - sink := backup.NewFilesystemSink(backupRoot) - defer testhelper.MustClose(t, sink) - - locator, err := backup.ResolveLocator("pointer", sink) - require.NoError(t, err) - - cfg := testcfg.Build(t) - cfg.SocketPath = testserver.RunGitalyServer(t, cfg, setup.RegisterAll, - testserver.WithBackupSink(sink), - testserver.WithBackupLocator(locator), - ) - - ctx := testhelper.Context(t) - - repo, repoPath := gittest.CreateRepository(t, ctx, cfg) - gittest.WriteCommit(t, cfg, repoPath, gittest.WithBranch("main")) - - pool := client.NewPool() - defer testhelper.MustClose(t, pool) - - adapter := backup.NewServerSideAdapter(pool) - err = adapter.RemoveAllRepositories(ctx, &backup.RemoveAllRepositoriesRequest{ - Server: storage.ServerInfo{Address: cfg.SocketPath, Token: cfg.Auth.Token}, - StorageName: repo.StorageName, - }) - require.NoError(t, err) -} - func TestServerSideAdapter_RemoveRepository(t *testing.T) { t.Parallel() -- cgit v1.2.3