diff --git a/changelogs/unreleased/9676-Lyndon-Li‎‎ b/changelogs/unreleased/9676-Lyndon-Li‎‎ new file mode 100644 index 000000000..2fb765e29 --- /dev/null +++ b/changelogs/unreleased/9676-Lyndon-Li‎‎ @@ -0,0 +1 @@ +Fix issue #9470, remove restic from repository \ No newline at end of file diff --git a/pkg/controller/backup_deletion_controller_test.go b/pkg/controller/backup_deletion_controller_test.go index ab3687438..58d9b0420 100644 --- a/pkg/controller/backup_deletion_controller_test.go +++ b/pkg/controller/backup_deletion_controller_test.go @@ -821,12 +821,12 @@ func TestGetSnapshotsInBackup(t *testing.T) { { VolumeNamespace: "ns-1", SnapshotID: "snap-3", - RepositoryType: "restic", + RepositoryType: "kopia", }, { VolumeNamespace: "ns-1", SnapshotID: "snap-4", - RepositoryType: "restic", + RepositoryType: "kopia", }, }, }, @@ -876,7 +876,7 @@ func TestGetSnapshotsInBackup(t *testing.T) { { VolumeNamespace: "ns-1", SnapshotID: "snap-3", - RepositoryType: "restic", + RepositoryType: "kopia", }, }, }, diff --git a/pkg/controller/backup_repository_controller.go b/pkg/controller/backup_repository_controller.go index 3f33bc814..eb90660f4 100644 --- a/pkg/controller/backup_repository_controller.go +++ b/pkg/controller/backup_repository_controller.go @@ -43,7 +43,6 @@ import ( "github.com/vmware-tanzu/velero/pkg/constant" "github.com/vmware-tanzu/velero/pkg/label" "github.com/vmware-tanzu/velero/pkg/metrics" - repoconfig "github.com/vmware-tanzu/velero/pkg/repository/config" "github.com/vmware-tanzu/velero/pkg/repository/maintenance" repomanager "github.com/vmware-tanzu/velero/pkg/repository/manager" "github.com/vmware-tanzu/velero/pkg/util/kube" @@ -249,7 +248,7 @@ func (r *BackupRepoReconciler) Reconcile(ctx context.Context, req ctrl.Request) } if backupRepo.Status.Phase == "" || backupRepo.Status.Phase == velerov1api.BackupRepositoryPhaseNew { - if err := r.initializeRepo(ctx, backupRepo, bsl, log); err != nil { + if err := r.initializeRepo(ctx, backupRepo, log); err != nil { log.WithError(err).Error("error initialize repository") return ctrl.Result{}, errors.WithStack(err) } @@ -267,7 +266,7 @@ func (r *BackupRepoReconciler) Reconcile(ctx context.Context, req ctrl.Request) switch backupRepo.Status.Phase { case velerov1api.BackupRepositoryPhaseNotReady: - ready, err := r.checkNotReadyRepo(ctx, backupRepo, bsl, log) + ready, err := r.checkNotReadyRepo(ctx, backupRepo, log) if err != nil { return ctrl.Result{}, err } else if !ready { @@ -315,16 +314,7 @@ func (r *BackupRepoReconciler) getBSL(ctx context.Context, req *velerov1api.Back return loc, nil } -func (r *BackupRepoReconciler) getIdentifierByBSL(bsl *velerov1api.BackupStorageLocation, req *velerov1api.BackupRepository) (string, error) { - repoIdentifier, err := repoconfig.GetRepoIdentifier(bsl, req.Spec.VolumeNamespace) - if err != nil { - return "", errors.Wrapf(err, "error to get identifier for repo %s", req.Name) - } - - return repoIdentifier, nil -} - -func (r *BackupRepoReconciler) initializeRepo(ctx context.Context, req *velerov1api.BackupRepository, bsl *velerov1api.BackupStorageLocation, log logrus.FieldLogger) error { +func (r *BackupRepoReconciler) initializeRepo(ctx context.Context, req *velerov1api.BackupRepository, log logrus.FieldLogger) error { log.WithField("repoConfig", r.backupRepoConfig).Info("Initializing backup repository") config, err := getBackupRepositoryConfig(ctx, r, r.backupRepoConfig, r.namespace, req.Name, req.Spec.RepositoryType, log) @@ -558,7 +548,7 @@ func dueForMaintenance(req *velerov1api.BackupRepository, now time.Time) bool { return req.Status.LastMaintenanceTime == nil || req.Status.LastMaintenanceTime.Add(req.Spec.MaintenanceFrequency.Duration).Before(now) } -func (r *BackupRepoReconciler) checkNotReadyRepo(ctx context.Context, req *velerov1api.BackupRepository, bsl *velerov1api.BackupStorageLocation, log logrus.FieldLogger) (bool, error) { +func (r *BackupRepoReconciler) checkNotReadyRepo(ctx context.Context, req *velerov1api.BackupRepository, log logrus.FieldLogger) (bool, error) { log.Info("Checking backup repository for readiness") // we need to ensure it (first check, if check fails, attempt to init) diff --git a/pkg/controller/backup_repository_controller_test.go b/pkg/controller/backup_repository_controller_test.go index cecd4d7d8..8a458033f 100644 --- a/pkg/controller/backup_repository_controller_test.go +++ b/pkg/controller/backup_repository_controller_test.go @@ -107,17 +107,8 @@ func TestCheckNotReadyRepo(t *testing.T) { reconciler := mockBackupRepoReconciler(t, "PrepareRepo", rr, nil) err := reconciler.Client.Create(t.Context(), rr) require.NoError(t, err) - location := velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Config: map[string]string{"resticRepoPrefix": "s3:test.amazonaws.com/bucket/restic"}, - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: velerov1api.DefaultNamespace, - Name: rr.Spec.BackupStorageLocation, - }, - } - _, err = reconciler.checkNotReadyRepo(t.Context(), rr, &location, reconciler.logger) + _, err = reconciler.checkNotReadyRepo(t.Context(), rr, reconciler.logger) require.NoError(t, err) assert.Equal(t, velerov1api.BackupRepositoryPhaseReady, rr.Status.Phase) // ResticIdentifier should remain empty for kopia @@ -411,17 +402,8 @@ func TestInitializeRepo(t *testing.T) { reconciler := mockBackupRepoReconciler(t, "PrepareRepo", rr, nil) err := reconciler.Client.Create(t.Context(), rr) require.NoError(t, err) - location := velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Config: map[string]string{"resticRepoPrefix": "s3:test.amazonaws.com/bucket/restic"}, - }, - ObjectMeta: metav1.ObjectMeta{ - Namespace: velerov1api.DefaultNamespace, - Name: rr.Spec.BackupStorageLocation, - }, - } - err = reconciler.initializeRepo(t.Context(), rr, &location, reconciler.logger) + err = reconciler.initializeRepo(t.Context(), rr, reconciler.logger) require.NoError(t, err) assert.Equal(t, velerov1api.BackupRepositoryPhaseReady, rr.Status.Phase) } @@ -1442,7 +1424,7 @@ func TestDeleteOldMaintenanceJobWithConfigMap(t *testing.T) { MaintenanceFrequency: metav1.Duration{Duration: testMaintenanceFrequency}, BackupStorageLocation: "default", VolumeNamespace: "test-ns", - RepositoryType: "restic", + RepositoryType: "kopia", }, Status: velerov1api.BackupRepositoryStatus{ Phase: velerov1api.BackupRepositoryPhaseReady, @@ -1479,7 +1461,7 @@ func TestDeleteOldMaintenanceJobWithConfigMap(t *testing.T) { MaintenanceFrequency: metav1.Duration{Duration: testMaintenanceFrequency}, BackupStorageLocation: "default", VolumeNamespace: "test-ns", - RepositoryType: "restic", + RepositoryType: "kopia", }, Status: velerov1api.BackupRepositoryStatus{ Phase: velerov1api.BackupRepositoryPhaseReady, @@ -1498,8 +1480,8 @@ func TestDeleteOldMaintenanceJobWithConfigMap(t *testing.T) { Name: "repo-maintenance-job-config", }, Data: map[string]string{ - "global": `{"keepLatestMaintenanceJobs": 5}`, - "test-ns-default-restic": `{"keepLatestMaintenanceJobs": 2}`, + "global": `{"keepLatestMaintenanceJobs": 5}`, + "test-ns-default-kopia": `{"keepLatestMaintenanceJobs": 2}`, }, }, }, @@ -1596,7 +1578,7 @@ func TestInitializeRepoWithRepositoryTypes(t *testing.T) { nil, ) - err := reconciler.initializeRepo(t.Context(), rr, location, reconciler.logger) + err := reconciler.initializeRepo(t.Context(), rr, reconciler.logger) require.NoError(t, err) // Verify ResticIdentifier is NOT set for kopia diff --git a/pkg/podvolume/restorer_test.go b/pkg/podvolume/restorer_test.go index 36a1fc034..e10146578 100644 --- a/pkg/podvolume/restorer_test.go +++ b/pkg/podvolume/restorer_test.go @@ -204,24 +204,6 @@ func TestRestorePodVolumes(t *testing.T) { }, }, }, - { - name: "get repository type fail", - pvbs: []*velerov1api.PodVolumeBackup{ - createPVBObj(true, true, 1, "restic"), - createPVBObj(true, true, 2, "kopia"), - }, - kubeClientObj: []runtime.Object{ - createNodeAgentDaemonset(), - }, - restoredPod: createPodObj(false, false, false, 2), - sourceNamespace: "fake-ns", - errs: []expectError{ - { - err: "multiple repository type in one backup", - prefixOnly: true, - }, - }, - }, { name: "ensure repo fail", pvbs: []*velerov1api.PodVolumeBackup{ diff --git a/pkg/repository/config/config.go b/pkg/repository/config/config.go index 46a5478e6..04761e95b 100644 --- a/pkg/repository/config/config.go +++ b/pkg/repository/config/config.go @@ -17,14 +17,7 @@ limitations under the License. package config import ( - "fmt" - "path" "strings" - - "github.com/pkg/errors" - - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" - "github.com/vmware-tanzu/velero/pkg/persistence" ) type BackendType string @@ -40,56 +33,6 @@ const ( CredentialsFileKey = "credentialsFile" ) -// this func is assigned to a package-level variable so it can be -// replaced when unit-testing -var getAWSBucketRegion = GetAWSBucketRegion - -// getRepoPrefix returns the prefix of the value of the --repo flag for -// restic commands, i.e. everything except the "/". -func getRepoPrefix(location *velerov1api.BackupStorageLocation) (string, error) { - var bucket, prefix string - - if location.Spec.ObjectStorage != nil { - layout := persistence.NewObjectStoreLayout(location.Spec.ObjectStorage.Prefix) - - bucket = location.Spec.ObjectStorage.Bucket - prefix = layout.GetResticDir() - } - - backendType := GetBackendType(location.Spec.Provider, location.Spec.Config) - - if repoPrefix := location.Spec.Config["resticRepoPrefix"]; repoPrefix != "" { - return repoPrefix, nil - } - - switch backendType { - case AWSBackend: - var url string - // non-AWS, S3-compatible object store - if s3Url := location.Spec.Config["s3Url"]; s3Url != "" { - url = strings.TrimSuffix(s3Url, "/") - } else { - var err error - region := location.Spec.Config["region"] - if region == "" { - region, err = getAWSBucketRegion(bucket, location.Spec.Config) - } - if err != nil { - return "", errors.Wrapf(err, "failed to detect the region via bucket: %s", bucket) - } - url = fmt.Sprintf("s3-%s.amazonaws.com", region) - } - - return fmt.Sprintf("s3:%s/%s", url, path.Join(bucket, prefix)), nil - case AzureBackend: - return fmt.Sprintf("azure:%s:/%s", bucket, prefix), nil - case GCPBackend: - return fmt.Sprintf("gs:%s:/%s", bucket, prefix), nil - } - - return "", errors.Errorf("invalid backend type %s, provider %s", backendType, location.Spec.Provider) -} - // GetBackendType returns a backend type that is known by Velero. // If the provider doesn't indicate a known backend type, but the endpoint is // specified, Velero regards it as a S3 compatible object store and return AWSBackend as the type. @@ -111,14 +54,3 @@ func GetBackendType(provider string, config map[string]string) BackendType { func IsBackendTypeValid(backendType BackendType) bool { return (backendType == AWSBackend || backendType == AzureBackend || backendType == GCPBackend || backendType == FSBackend) } - -// GetRepoIdentifier returns the string to be used as the value of the --repo flag in -// restic commands for the given repository. -func GetRepoIdentifier(location *velerov1api.BackupStorageLocation, name string) (string, error) { - prefix, err := getRepoPrefix(location) - if err != nil { - return "", err - } - - return fmt.Sprintf("%s/%s", strings.TrimSuffix(prefix, "/"), name), nil -} diff --git a/pkg/repository/config/config_test.go b/pkg/repository/config/config_test.go deleted file mode 100644 index aac5fc9bc..000000000 --- a/pkg/repository/config/config_test.go +++ /dev/null @@ -1,261 +0,0 @@ -/* -Copyright 2018, 2019 the Velero contributors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package config - -import ( - "testing" - - "github.com/pkg/errors" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" -) - -func TestGetRepoIdentifier(t *testing.T) { - testCases := []struct { - name string - bsl *velerov1api.BackupStorageLocation - repoName string - getAWSBucketRegion func(s string, config map[string]string) (string, error) - expected string - expectedErr string - }{ - { - name: "error is returned if BSL uses unsupported provider and resticRepoPrefix is not set", - bsl: &velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Provider: "unsupported-provider", - StorageType: velerov1api.StorageType{ - ObjectStorage: &velerov1api.ObjectStorageLocation{ - Bucket: "bucket-2", - Prefix: "prefix-2", - }, - }, - }, - }, - repoName: "repo-1", - expectedErr: "invalid backend type velero.io/unsupported-provider, provider unsupported-provider", - }, - { - name: "resticRepoPrefix in BSL config is used if set", - bsl: &velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Provider: "custom-repo-identifier", - Config: map[string]string{ - "resticRepoPrefix": "custom:prefix:/restic", - }, - StorageType: velerov1api.StorageType{ - ObjectStorage: &velerov1api.ObjectStorageLocation{ - Bucket: "bucket", - Prefix: "prefix", - }, - }, - }, - }, - repoName: "repo-1", - expected: "custom:prefix:/restic/repo-1", - }, - { - name: "s3Url in BSL config is used", - bsl: &velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Provider: "custom-repo-identifier", - Config: map[string]string{ - "s3Url": "s3Url", - }, - StorageType: velerov1api.StorageType{ - ObjectStorage: &velerov1api.ObjectStorageLocation{ - Bucket: "bucket", - Prefix: "prefix", - }, - }, - }, - }, - repoName: "repo-1", - expected: "s3:s3Url/bucket/prefix/restic/repo-1", - }, - { - name: "s3.amazonaws.com URL format is used if region cannot be determined for AWS BSL", - bsl: &velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Provider: "aws", - StorageType: velerov1api.StorageType{ - ObjectStorage: &velerov1api.ObjectStorageLocation{ - Bucket: "bucket", - }, - }, - }, - }, - repoName: "repo-1", - getAWSBucketRegion: func(s string, config map[string]string) (string, error) { - return "", errors.New("no region found") - }, - expected: "", - expectedErr: "failed to detect the region via bucket: bucket: no region found", - }, - { - name: "s3.s3-.amazonaws.com URL format is used if region can be determined for AWS BSL", - bsl: &velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Provider: "aws", - StorageType: velerov1api.StorageType{ - ObjectStorage: &velerov1api.ObjectStorageLocation{ - Bucket: "bucket", - }, - }, - }, - }, - repoName: "repo-1", - getAWSBucketRegion: func(string, map[string]string) (string, error) { - return "eu-west-1", nil - }, - expected: "s3:s3-eu-west-1.amazonaws.com/bucket/restic/repo-1", - }, - { - name: "prefix is included in repo identifier if set for AWS BSL", - bsl: &velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Provider: "aws", - StorageType: velerov1api.StorageType{ - ObjectStorage: &velerov1api.ObjectStorageLocation{ - Bucket: "bucket", - Prefix: "prefix", - }, - }, - }, - }, - repoName: "repo-1", - getAWSBucketRegion: func(s string, config map[string]string) (string, error) { - return "eu-west-1", nil - }, - expected: "s3:s3-eu-west-1.amazonaws.com/bucket/prefix/restic/repo-1", - }, - { - name: "s3Url is used in repo identifier if set for AWS BSL", - bsl: &velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Provider: "aws", - Config: map[string]string{ - "s3Url": "alternate-url", - }, - StorageType: velerov1api.StorageType{ - ObjectStorage: &velerov1api.ObjectStorageLocation{ - Bucket: "bucket", - Prefix: "prefix", - }, - }, - }, - }, - repoName: "repo-1", - getAWSBucketRegion: func(s string, config map[string]string) (string, error) { - return "eu-west-1", nil - }, - expected: "s3:alternate-url/bucket/prefix/restic/repo-1", - }, - { - name: "region is used in repo identifier if set for AWS BSL", - bsl: &velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Provider: "aws", - Config: map[string]string{ - "region": "us-west-1", - }, - StorageType: velerov1api.StorageType{ - ObjectStorage: &velerov1api.ObjectStorageLocation{ - Bucket: "bucket", - Prefix: "prefix", - }, - }, - }, - }, - repoName: "aws-repo", - getAWSBucketRegion: func(s string, config map[string]string) (string, error) { - return "eu-west-1", nil - }, - expected: "s3:s3-us-west-1.amazonaws.com/bucket/prefix/restic/aws-repo", - }, - { - name: "trailing slash in s3Url is not included in repo identifier for AWS BSL", - bsl: &velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Provider: "aws", - Config: map[string]string{ - "s3Url": "alternate-url-with-trailing-slash/", - }, - StorageType: velerov1api.StorageType{ - ObjectStorage: &velerov1api.ObjectStorageLocation{ - Bucket: "bucket", - Prefix: "prefix", - }, - }, - }, - }, - repoName: "aws-repo", - getAWSBucketRegion: func(s string, config map[string]string) (string, error) { - return "eu-west-1", nil - }, - expected: "s3:alternate-url-with-trailing-slash/bucket/prefix/restic/aws-repo", - }, - { - name: "repo identifier includes bucket and prefix for Azure BSL", - bsl: &velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Provider: "azure", - StorageType: velerov1api.StorageType{ - ObjectStorage: &velerov1api.ObjectStorageLocation{ - Bucket: "azure-bucket", - Prefix: "azure-prefix", - }, - }, - }, - }, - repoName: "azure-repo", - expected: "azure:azure-bucket:/azure-prefix/restic/azure-repo", - }, - { - name: "repo identifier includes bucket and prefix for GCP BSL", - bsl: &velerov1api.BackupStorageLocation{ - Spec: velerov1api.BackupStorageLocationSpec{ - Provider: "gcp", - StorageType: velerov1api.StorageType{ - ObjectStorage: &velerov1api.ObjectStorageLocation{ - Bucket: "gcp-bucket", - Prefix: "gcp-prefix", - }, - }, - }, - }, - repoName: "gcp-repo", - expected: "gs:gcp-bucket:/gcp-prefix/restic/gcp-repo", - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - getAWSBucketRegion = tc.getAWSBucketRegion - id, err := GetRepoIdentifier(tc.bsl, tc.repoName) - assert.Equal(t, tc.expected, id) - if tc.expectedErr == "" { - assert.NoError(t, err) - } else { - require.EqualError(t, err, tc.expectedErr) - assert.Empty(t, id) - } - }) - } -}