Merge branch 'main' into cache-dir-for-udmprepo

This commit is contained in:
Lyndon-Li
2025-10-21 16:42:28 +08:00
8 changed files with 286 additions and 91 deletions

View File

@@ -0,0 +1 @@
Fix issue #9193, don't connect repo in repo controller

View File

@@ -27,7 +27,6 @@ import (
"strings"
"time"
"github.com/kopia/kopia/repo"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
@@ -122,7 +121,7 @@ func (urp *unifiedRepoProvider) InitRepo(ctx context.Context, param RepoParam) e
return errors.Wrap(err, "error to get repo options")
}
err = urp.repoService.Init(ctx, *repoOption, true)
err = urp.repoService.Create(ctx, *repoOption)
if err != nil {
return errors.Wrap(err, "error to init backup repo")
}
@@ -158,7 +157,7 @@ func (urp *unifiedRepoProvider) ConnectToRepo(ctx context.Context, param RepoPar
return errors.Wrap(err, "error to get repo options")
}
err = urp.repoService.Init(ctx, *repoOption, false)
err = urp.repoService.Connect(ctx, *repoOption)
if err != nil {
return errors.Wrap(err, "error to connect backup repo")
}
@@ -194,20 +193,18 @@ func (urp *unifiedRepoProvider) PrepareRepo(ctx context.Context, param RepoParam
return errors.Wrap(err, "error to get repo options")
}
err = urp.repoService.Init(ctx, *repoOption, false)
if err == nil {
if created, err := urp.repoService.IsCreated(ctx, *repoOption); err != nil {
return errors.Wrap(err, "error to check backup repo")
} else if created {
log.Debug("Repo has already been initialized remotely")
return nil
}
if !errors.Is(err, repo.ErrRepositoryNotInitialized) {
return errors.Wrap(err, "error to connect to backup repo")
}
if param.BackupLocation.Spec.AccessMode == velerov1api.BackupStorageLocationAccessModeReadOnly {
return errors.Errorf("cannot create new backup repo for read-only backup storage location %s/%s", param.BackupLocation.Namespace, param.BackupLocation.Name)
}
err = urp.repoService.Init(ctx, *repoOption, true)
err = urp.repoService.Create(ctx, *repoOption)
if err != nil {
return errors.Wrap(err, "error to create backup repo")
}

View File

@@ -23,7 +23,6 @@ import (
"testing"
"github.com/aws/aws-sdk-go-v2/aws"
"github.com/kopia/kopia/repo"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
@@ -613,7 +612,8 @@ func TestPrepareRepo(t *testing.T) {
funcTable localFuncTable
getter *credmock.SecretStore
repoService *reposervicenmocks.BackupRepoService
retFuncInit func(context.Context, udmrepo.RepoOptions, bool) error
retFuncCreate func(context.Context, udmrepo.RepoOptions) error
retFuncCheck func(context.Context, udmrepo.RepoOptions) (bool, error)
credStoreReturn string
credStoreError error
readOnlyBSL bool
@@ -643,6 +643,24 @@ func TestPrepareRepo(t *testing.T) {
},
expectedErr: "error to get repo options: error to get storage variables: fake-store-option-error",
},
{
name: "check error",
getter: new(credmock.SecretStore),
credStoreReturn: "fake-password",
funcTable: localFuncTable{
getStorageVariables: func(*velerov1api.BackupStorageLocation, string, string, map[string]string) (map[string]string, error) {
return map[string]string{}, nil
},
getStorageCredentials: func(*velerov1api.BackupStorageLocation, velerocredentials.FileStore) (map[string]string, error) {
return map[string]string{}, nil
},
},
repoService: new(reposervicenmocks.BackupRepoService),
retFuncCheck: func(ctx context.Context, repoOption udmrepo.RepoOptions) (bool, error) {
return false, errors.New("fake-error")
},
expectedErr: "error to check backup repo: fake-error",
},
{
name: "already initialized",
getter: new(credmock.SecretStore),
@@ -656,10 +674,10 @@ func TestPrepareRepo(t *testing.T) {
},
},
repoService: new(reposervicenmocks.BackupRepoService),
retFuncInit: func(ctx context.Context, repoOption udmrepo.RepoOptions, createNew bool) error {
if !createNew {
return nil
}
retFuncCheck: func(ctx context.Context, repoOption udmrepo.RepoOptions) (bool, error) {
return true, nil
},
retFuncCreate: func(ctx context.Context, repoOption udmrepo.RepoOptions) error {
return errors.New("fake-error")
},
},
@@ -677,35 +695,14 @@ func TestPrepareRepo(t *testing.T) {
},
},
repoService: new(reposervicenmocks.BackupRepoService),
retFuncInit: func(ctx context.Context, repoOption udmrepo.RepoOptions, createNew bool) error {
if !createNew {
return repo.ErrRepositoryNotInitialized
}
retFuncCheck: func(ctx context.Context, repoOption udmrepo.RepoOptions) (bool, error) {
return false, nil
},
retFuncCreate: func(ctx context.Context, repoOption udmrepo.RepoOptions) error {
return errors.New("fake-error-2")
},
expectedErr: "cannot create new backup repo for read-only backup storage location velero/fake-bsl",
},
{
name: "connect fail",
getter: new(credmock.SecretStore),
credStoreReturn: "fake-password",
funcTable: localFuncTable{
getStorageVariables: func(*velerov1api.BackupStorageLocation, string, string, map[string]string) (map[string]string, error) {
return map[string]string{}, nil
},
getStorageCredentials: func(*velerov1api.BackupStorageLocation, velerocredentials.FileStore) (map[string]string, error) {
return map[string]string{}, nil
},
},
repoService: new(reposervicenmocks.BackupRepoService),
retFuncInit: func(ctx context.Context, repoOption udmrepo.RepoOptions, createNew bool) error {
if !createNew {
return errors.New("fake-error-1")
}
return errors.New("fake-error-2")
},
expectedErr: "error to connect to backup repo: fake-error-1",
},
{
name: "initialize error",
getter: new(credmock.SecretStore),
@@ -719,10 +716,10 @@ func TestPrepareRepo(t *testing.T) {
},
},
repoService: new(reposervicenmocks.BackupRepoService),
retFuncInit: func(ctx context.Context, repoOption udmrepo.RepoOptions, createNew bool) error {
if !createNew {
return repo.ErrRepositoryNotInitialized
}
retFuncCheck: func(ctx context.Context, repoOption udmrepo.RepoOptions) (bool, error) {
return false, nil
},
retFuncCreate: func(ctx context.Context, repoOption udmrepo.RepoOptions) error {
return errors.New("fake-error-2")
},
expectedErr: "error to create backup repo: fake-error-2",
@@ -740,10 +737,10 @@ func TestPrepareRepo(t *testing.T) {
},
},
repoService: new(reposervicenmocks.BackupRepoService),
retFuncInit: func(ctx context.Context, repoOption udmrepo.RepoOptions, createNew bool) error {
if !createNew {
return repo.ErrRepositoryNotInitialized
}
retFuncCheck: func(ctx context.Context, repoOption udmrepo.RepoOptions) (bool, error) {
return false, nil
},
retFuncCreate: func(ctx context.Context, repoOption udmrepo.RepoOptions) error {
return nil
},
},
@@ -767,7 +764,8 @@ func TestPrepareRepo(t *testing.T) {
log: velerotest.NewLogger(),
}
tc.repoService.On("Init", mock.Anything, mock.Anything, mock.Anything).Return(tc.retFuncInit)
tc.repoService.On("IsCreated", mock.Anything, mock.Anything).Return(tc.retFuncCheck)
tc.repoService.On("Create", mock.Anything, mock.Anything, mock.Anything).Return(tc.retFuncCreate)
if tc.readOnlyBSL {
bsl.Spec.AccessMode = velerov1api.BackupStorageLocationAccessModeReadOnly
@@ -1134,7 +1132,7 @@ func TestInitRepo(t *testing.T) {
},
},
repoService: new(reposervicenmocks.BackupRepoService),
retFuncInit: func(context.Context, udmrepo.RepoOptions, bool) error {
retFuncInit: func(context.Context, udmrepo.RepoOptions) error {
return errors.New("fake-error-1")
},
expectedErr: "error to init backup repo: fake-error-1",
@@ -1152,7 +1150,7 @@ func TestInitRepo(t *testing.T) {
},
},
repoService: new(reposervicenmocks.BackupRepoService),
retFuncInit: func(context.Context, udmrepo.RepoOptions, bool) error {
retFuncInit: func(context.Context, udmrepo.RepoOptions) error {
return nil
},
},
@@ -1177,7 +1175,7 @@ func TestInitRepo(t *testing.T) {
}
if tc.repoService != nil {
tc.repoService.On("Init", mock.Anything, mock.Anything, mock.Anything).Return(tc.retFuncInit)
tc.repoService.On("Create", mock.Anything, mock.Anything).Return(tc.retFuncInit)
}
if tc.readOnlyBSL {
@@ -1228,7 +1226,7 @@ func TestConnectToRepo(t *testing.T) {
},
},
repoService: new(reposervicenmocks.BackupRepoService),
retFuncInit: func(context.Context, udmrepo.RepoOptions, bool) error {
retFuncInit: func(context.Context, udmrepo.RepoOptions) error {
return errors.New("fake-error-1")
},
expectedErr: "error to connect backup repo: fake-error-1",
@@ -1246,7 +1244,7 @@ func TestConnectToRepo(t *testing.T) {
},
},
repoService: new(reposervicenmocks.BackupRepoService),
retFuncInit: func(context.Context, udmrepo.RepoOptions, bool) error {
retFuncInit: func(context.Context, udmrepo.RepoOptions) error {
return nil
},
},
@@ -1271,7 +1269,7 @@ func TestConnectToRepo(t *testing.T) {
}
if tc.repoService != nil {
tc.repoService.On("Init", mock.Anything, mock.Anything, mock.Anything).Return(tc.retFuncInit)
tc.repoService.On("Connect", mock.Anything, mock.Anything).Return(tc.retFuncInit)
}
err := urp.ConnectToRepo(t.Context(), RepoParam{
@@ -1329,7 +1327,7 @@ func TestBoostRepoConnect(t *testing.T) {
return errors.New("fake-error-1")
},
},
retFuncInit: func(context.Context, udmrepo.RepoOptions, bool) error {
retFuncInit: func(context.Context, udmrepo.RepoOptions) error {
return errors.New("fake-error-2")
},
expectedErr: "error to connect backup repo: fake-error-2",
@@ -1356,7 +1354,7 @@ func TestBoostRepoConnect(t *testing.T) {
return errors.New("fake-error-1")
},
},
retFuncInit: func(context.Context, udmrepo.RepoOptions, bool) error {
retFuncInit: func(context.Context, udmrepo.RepoOptions) error {
return nil
},
},
@@ -1383,7 +1381,7 @@ func TestBoostRepoConnect(t *testing.T) {
return nil
},
},
retFuncInit: func(context.Context, udmrepo.RepoOptions, bool) error {
retFuncInit: func(context.Context, udmrepo.RepoOptions) error {
return nil
},
},
@@ -1411,7 +1409,7 @@ func TestBoostRepoConnect(t *testing.T) {
if tc.repoService != nil {
tc.repoService.On("Open", mock.Anything, mock.Anything).Return(tc.retFuncOpen[0], tc.retFuncOpen[1])
tc.repoService.On("Init", mock.Anything, mock.Anything, mock.Anything).Return(tc.retFuncInit)
tc.repoService.On("Connect", mock.Anything, mock.Anything).Return(tc.retFuncInit)
}
if tc.backupRepo != nil {

View File

@@ -95,19 +95,28 @@ func NewKopiaRepoService(logger logrus.FieldLogger) udmrepo.BackupRepoService {
return ks
}
func (ks *kopiaRepoService) Init(ctx context.Context, repoOption udmrepo.RepoOptions, createNew bool) error {
func (ks *kopiaRepoService) Create(ctx context.Context, repoOption udmrepo.RepoOptions) error {
repoCtx := kopia.SetupKopiaLog(ctx, ks.logger)
if createNew {
if err := CreateBackupRepo(repoCtx, repoOption, ks.logger); err != nil {
return err
}
return writeInitParameters(repoCtx, repoOption, ks.logger)
}
}
func (ks *kopiaRepoService) Connect(ctx context.Context, repoOption udmrepo.RepoOptions) error {
repoCtx := kopia.SetupKopiaLog(ctx, ks.logger)
return ConnectBackupRepo(repoCtx, repoOption, ks.logger)
}
func (ks *kopiaRepoService) IsCreated(ctx context.Context, repoOption udmrepo.RepoOptions) (bool, error) {
repoCtx := kopia.SetupKopiaLog(ctx, ks.logger)
return IsBackupRepoCreated(repoCtx, repoOption, ks.logger)
}
func (ks *kopiaRepoService) Open(ctx context.Context, repoOption udmrepo.RepoOptions) (udmrepo.BackupRepo, error) {
repoConfig := repoOption.ConfigFilePath
if repoConfig == "" {

View File

@@ -24,6 +24,7 @@ import (
"github.com/kopia/kopia/repo"
"github.com/kopia/kopia/repo/blob"
"github.com/kopia/kopia/repo/format"
"github.com/pkg/errors"
"github.com/vmware-tanzu/velero/pkg/repository/udmrepo"
@@ -47,10 +48,6 @@ var backendStores = []kopiaBackendStore{
// CreateBackupRepo creates a Kopia repository and then connect to it.
// The storage must be empty, otherwise, it will fail
func CreateBackupRepo(ctx context.Context, repoOption udmrepo.RepoOptions, logger logrus.FieldLogger) error {
if repoOption.ConfigFilePath == "" {
return errors.New("invalid config file path")
}
backendStore, err := setupBackendStore(ctx, repoOption.StorageType, repoOption.StorageOptions, logger)
if err != nil {
return errors.Wrap(err, "error to setup backend storage")
@@ -66,11 +63,6 @@ func CreateBackupRepo(ctx context.Context, repoOption udmrepo.RepoOptions, logge
return errors.Wrap(err, "error to create repo with storage")
}
err = connectWithStorage(ctx, st, repoOption)
if err != nil {
return errors.Wrap(err, "error to connect repo with storage")
}
return nil
}
@@ -99,6 +91,34 @@ func ConnectBackupRepo(ctx context.Context, repoOption udmrepo.RepoOptions, logg
return nil
}
func IsBackupRepoCreated(ctx context.Context, repoOption udmrepo.RepoOptions, logger logrus.FieldLogger) (bool, error) {
backendStore, err := setupBackendStore(ctx, repoOption.StorageType, repoOption.StorageOptions, logger)
if err != nil {
return false, errors.Wrap(err, "error to setup backend storage")
}
st, err := backendStore.store.Connect(ctx, false, logger)
if err != nil {
return false, errors.Wrap(err, "error to connect to storage")
}
var formatBytes byteBuffer
if err := st.GetBlob(ctx, format.KopiaRepositoryBlobID, 0, -1, &formatBytes); err != nil {
if errors.Is(err, blob.ErrBlobNotFound) {
return false, nil
}
return false, errors.Wrap(err, "error to read format blob")
}
_, err = format.ParseKopiaRepositoryJSON(formatBytes.buffer)
if err != nil {
return false, err
}
return true, nil
}
func findBackendStore(storage string) *kopiaBackendStore {
for _, options := range backendStores {
if strings.EqualFold(options.name, storage) {
@@ -160,3 +180,20 @@ func ensureEmpty(ctx context.Context, s blob.Storage) error {
return errors.Wrap(err, "error to list blobs")
}
type byteBuffer struct {
buffer []byte
}
func (b *byteBuffer) Write(p []byte) (n int, err error) {
b.buffer = append(b.buffer, p...)
return len(p), nil
}
func (b *byteBuffer) Reset() {
b.buffer = nil
}
func (b *byteBuffer) Length() int {
return len(b.buffer)
}

View File

@@ -17,12 +17,16 @@ limitations under the License.
package kopialib
import (
"context"
"testing"
"github.com/kopia/kopia/repo/blob"
velerotest "github.com/vmware-tanzu/velero/pkg/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/vmware-tanzu/velero/pkg/repository/udmrepo"
storagemocks "github.com/vmware-tanzu/velero/pkg/repository/udmrepo/kopialib/backend/mocks"
@@ -55,10 +59,6 @@ func TestCreateBackupRepo(t *testing.T) {
listBlobErr error
expectedErr string
}{
{
name: "invalid config file",
expectedErr: "invalid config file path",
},
{
name: "storage setup fail, invalid type",
repoOptions: udmrepo.RepoOptions{
@@ -238,3 +238,103 @@ func TestConnectBackupRepo(t *testing.T) {
})
}
}
func TestIsBackupRepoCreated(t *testing.T) {
testCases := []struct {
name string
backendStore *storagemocks.Store
repoOptions udmrepo.RepoOptions
connectErr error
setupError error
returnStore *storagemocks.Storage
retFuncGetBlob func(context.Context, blob.ID, int64, int64, blob.OutputBuffer) error
expected bool
expectedErr string
}{
{
name: "storage setup fail, invalid type",
repoOptions: udmrepo.RepoOptions{
ConfigFilePath: "fake-file",
},
expectedErr: "error to setup backend storage: error to find storage type",
},
{
name: "storage setup fail, backend store steup fail",
repoOptions: udmrepo.RepoOptions{
ConfigFilePath: "fake-file",
StorageType: udmrepo.StorageTypeAzure,
},
backendStore: new(storagemocks.Store),
setupError: errors.New("fake-setup-error"),
expectedErr: "error to setup backend storage: error to setup storage: fake-setup-error",
},
{
name: "storage connect fail",
repoOptions: udmrepo.RepoOptions{
ConfigFilePath: "fake-file",
StorageType: udmrepo.StorageTypeAzure,
},
backendStore: new(storagemocks.Store),
connectErr: errors.New("fake-connect-error"),
expectedErr: "error to connect to storage: fake-connect-error",
},
{
name: "get blob error",
repoOptions: udmrepo.RepoOptions{
ConfigFilePath: "fake-file",
StorageType: udmrepo.StorageTypeAzure,
},
backendStore: new(storagemocks.Store),
returnStore: new(storagemocks.Storage),
retFuncGetBlob: func(context.Context, blob.ID, int64, int64, blob.OutputBuffer) error {
return errors.New("fake-get-blob-error")
},
expectedErr: "error to read format blob: fake-get-blob-error",
},
{
name: "wrong format",
repoOptions: udmrepo.RepoOptions{
ConfigFilePath: "fake-file",
StorageType: udmrepo.StorageTypeAzure,
},
backendStore: new(storagemocks.Store),
returnStore: new(storagemocks.Storage),
retFuncGetBlob: func(ctx context.Context, id blob.ID, offset int64, length int64, output blob.OutputBuffer) error {
output.Write([]byte("fake-buffer"))
return nil
},
expectedErr: "invalid format blob: invalid character 'k' in literal false (expecting 'l')",
},
}
logger := velerotest.NewLogger()
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
backendStores = []kopiaBackendStore{
{udmrepo.StorageTypeAzure, "fake store", tc.backendStore},
{udmrepo.StorageTypeFs, "fake store", tc.backendStore},
{udmrepo.StorageTypeGcs, "fake store", tc.backendStore},
{udmrepo.StorageTypeS3, "fake store", tc.backendStore},
}
if tc.backendStore != nil {
tc.backendStore.On("Connect", mock.Anything, mock.Anything, mock.Anything).Return(tc.returnStore, tc.connectErr)
tc.backendStore.On("Setup", mock.Anything, mock.Anything, mock.Anything).Return(tc.setupError)
}
if tc.returnStore != nil {
tc.returnStore.On("GetBlob", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(tc.retFuncGetBlob)
}
created, err := IsBackupRepoCreated(t.Context(), tc.repoOptions, logger)
if tc.expectedErr == "" {
require.NoError(t, err)
} else {
require.EqualError(t, err, tc.expectedErr)
}
assert.Equal(t, tc.expected, created)
})
}
}

View File

@@ -34,6 +34,42 @@ func (_m *BackupRepoService) ClientSideCacheLimit(repoOption map[string]string)
return r0
}
// Connect provides a mock function with given fields: ctx, repoOption
func (_m *BackupRepoService) Connect(ctx context.Context, repoOption udmrepo.RepoOptions) error {
ret := _m.Called(ctx, repoOption)
if len(ret) == 0 {
panic("no return value specified for Connect")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, udmrepo.RepoOptions) error); ok {
r0 = rf(ctx, repoOption)
} else {
r0 = ret.Error(0)
}
return r0
}
// Create provides a mock function with given fields: ctx, repoOption
func (_m *BackupRepoService) Create(ctx context.Context, repoOption udmrepo.RepoOptions) error {
ret := _m.Called(ctx, repoOption)
if len(ret) == 0 {
panic("no return value specified for Create")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, udmrepo.RepoOptions) error); ok {
r0 = rf(ctx, repoOption)
} else {
r0 = ret.Error(0)
}
return r0
}
// DefaultMaintenanceFrequency provides a mock function with no fields
func (_m *BackupRepoService) DefaultMaintenanceFrequency() time.Duration {
ret := _m.Called()
@@ -52,22 +88,32 @@ func (_m *BackupRepoService) DefaultMaintenanceFrequency() time.Duration {
return r0
}
// Init provides a mock function with given fields: ctx, repoOption, createNew
func (_m *BackupRepoService) Init(ctx context.Context, repoOption udmrepo.RepoOptions, createNew bool) error {
ret := _m.Called(ctx, repoOption, createNew)
// IsCreated provides a mock function with given fields: ctx, repoOption
func (_m *BackupRepoService) IsCreated(ctx context.Context, repoOption udmrepo.RepoOptions) (bool, error) {
ret := _m.Called(ctx, repoOption)
if len(ret) == 0 {
panic("no return value specified for Init")
panic("no return value specified for IsCreated")
}
var r0 error
if rf, ok := ret.Get(0).(func(context.Context, udmrepo.RepoOptions, bool) error); ok {
r0 = rf(ctx, repoOption, createNew)
var r0 bool
var r1 error
if rf, ok := ret.Get(0).(func(context.Context, udmrepo.RepoOptions) (bool, error)); ok {
return rf(ctx, repoOption)
}
if rf, ok := ret.Get(0).(func(context.Context, udmrepo.RepoOptions) bool); ok {
r0 = rf(ctx, repoOption)
} else {
r0 = ret.Error(0)
r0 = ret.Get(0).(bool)
}
return r0
if rf, ok := ret.Get(1).(func(context.Context, udmrepo.RepoOptions) error); ok {
r1 = rf(ctx, repoOption)
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// Maintain provides a mock function with given fields: ctx, repoOption

View File

@@ -77,10 +77,17 @@ type AdvancedFeatureInfo struct {
// BackupRepoService is used to initialize, open or maintain a backup repository
type BackupRepoService interface {
// Init creates a backup repository or connect to an existing backup repository.
// Create creates a new backup repository.
// repoOption: option to the backup repository and the underlying backup storage.
// createNew: indicates whether to create a new or connect to an existing backup repository.
Init(ctx context.Context, repoOption RepoOptions, createNew bool) error
Create(ctx context.Context, repoOption RepoOptions) error
// Connect connects to an existing backup repository.
// repoOption: option to the backup repository and the underlying backup storage.
Connect(ctx context.Context, repoOption RepoOptions) error
// IsCreated checks if the backup repository has been created in the underlying backup storage.
// repoOption: option to the underlying backup storage
IsCreated(ctx context.Context, repoOption RepoOptions) (bool, error)
// Open opens an backup repository that has been created/connected.
// repoOption: options to open the backup repository and the underlying storage.