diff --git a/pkg/controller/restic_repository_controller.go b/pkg/controller/restic_repository_controller.go index 9d6f36960..56067c2aa 100644 --- a/pkg/controller/restic_repository_controller.go +++ b/pkg/controller/restic_repository_controller.go @@ -152,9 +152,21 @@ func (c *resticRepositoryController) initializeRepo(req *v1.ResticRepository, lo return c.patchResticRepository(req, repoNotReady(err.Error())) } + repoIdentifier, err := restic.GetRepoIdentifier(loc, req.Spec.VolumeNamespace) + if err != nil { + return c.patchResticRepository(req, func(r *v1.ResticRepository) { + r.Status.Message = err.Error() + r.Status.Phase = v1.ResticRepositoryPhaseNotReady + + if r.Spec.MaintenanceFrequency.Duration <= 0 { + r.Spec.MaintenanceFrequency = metav1.Duration{Duration: restic.DefaultMaintenanceFrequency} + } + }) + } + // defaulting - if the patch fails, return an error so the item is returned to the queue if err := c.patchResticRepository(req, func(r *v1.ResticRepository) { - r.Spec.ResticIdentifier = restic.GetRepoIdentifier(loc, r.Spec.VolumeNamespace) + r.Spec.ResticIdentifier = repoIdentifier if r.Spec.MaintenanceFrequency.Duration <= 0 { r.Spec.MaintenanceFrequency = metav1.Duration{Duration: restic.DefaultMaintenanceFrequency} @@ -236,6 +248,11 @@ func dueForMaintenance(req *v1.ResticRepository, now time.Time) bool { } func (c *resticRepositoryController) checkNotReadyRepo(req *v1.ResticRepository, log logrus.FieldLogger) error { + // no identifier: can't possibly be ready, so just return + if req.Spec.ResticIdentifier == "" { + return nil + } + log.Info("Checking restic repository for readiness") // we need to ensure it (first check, if check fails, attempt to init) diff --git a/pkg/restic/config.go b/pkg/restic/config.go index 440c01512..eba29f6fa 100644 --- a/pkg/restic/config.go +++ b/pkg/restic/config.go @@ -21,6 +21,8 @@ import ( "path" "strings" + "github.com/pkg/errors" + velerov1api "github.com/heptio/velero/pkg/apis/velero/v1" "github.com/heptio/velero/pkg/cloudprovider/aws" "github.com/heptio/velero/pkg/persistence" @@ -40,8 +42,8 @@ var getAWSBucketRegion = aws.GetBucketRegion // 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 { - var provider, bucket, prefix, bucketAndPrefix string +func getRepoPrefix(location *velerov1api.BackupStorageLocation) (string, error) { + var bucket, prefix string if location.Spec.ObjectStorage != nil { layout := persistence.NewObjectStoreLayout(location.Spec.ObjectStorage.Prefix) @@ -49,14 +51,13 @@ func getRepoPrefix(location *velerov1api.BackupStorageLocation) string { bucket = location.Spec.ObjectStorage.Bucket prefix = layout.GetResticDir() } - bucketAndPrefix = path.Join(bucket, prefix) - var locationSpecProvider = location.Spec.Provider - if !strings.Contains(locationSpecProvider, "/") { - locationSpecProvider = "velero.io/" + locationSpecProvider + var provider = location.Spec.Provider + if !strings.Contains(provider, "/") { + provider = "velero.io/" + provider } - switch BackendType(locationSpecProvider) { + switch BackendType(provider) { case AWSBackend: var url string switch { @@ -73,20 +74,23 @@ func getRepoPrefix(location *velerov1api.BackupStorageLocation) string { url = fmt.Sprintf("s3-%s.amazonaws.com", region) } - return fmt.Sprintf("s3:%s/%s", strings.TrimSuffix(url, "/"), bucketAndPrefix) + return fmt.Sprintf("s3:%s/%s", strings.TrimSuffix(url, "/"), path.Join(bucket, prefix)), nil case AzureBackend: - provider = "azure" + return fmt.Sprintf("azure:%s:/%s", bucket, prefix), nil case GCPBackend: - provider = "gs" + return fmt.Sprintf("gs:%s:/%s", bucket, prefix), nil } - return fmt.Sprintf("%s:%s:/%s", provider, bucket, prefix) + return "", errors.New("restic repository prefix (resticRepoPrefix) not specified in backup storage location's config") } // 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 { - prefix := getRepoPrefix(location) +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) + return fmt.Sprintf("%s/%s", strings.TrimSuffix(prefix, "/"), name), nil } diff --git a/pkg/restic/config_test.go b/pkg/restic/config_test.go index d107433a7..6e4bc3b4b 100644 --- a/pkg/restic/config_test.go +++ b/pkg/restic/config_test.go @@ -42,7 +42,9 @@ func TestGetRepoIdentifier(t *testing.T) { }, }, } - assert.Equal(t, "s3:s3.amazonaws.com/bucket/prefix/restic/repo-1", GetRepoIdentifier(backupLocation, "repo-1")) + id, err := GetRepoIdentifier(backupLocation, "repo-1") + assert.NoError(t, err) + assert.Equal(t, "s3:s3.amazonaws.com/bucket/prefix/restic/repo-1", id) // stub implementation of getAWSBucketRegion getAWSBucketRegion = func(string) (string, error) { @@ -59,7 +61,9 @@ func TestGetRepoIdentifier(t *testing.T) { }, }, } - assert.Equal(t, "s3:s3-us-west-2.amazonaws.com/bucket/restic/repo-1", GetRepoIdentifier(backupLocation, "repo-1")) + id, err = GetRepoIdentifier(backupLocation, "repo-1") + assert.NoError(t, err) + assert.Equal(t, "s3:s3-us-west-2.amazonaws.com/bucket/restic/repo-1", id) backupLocation = &velerov1api.BackupStorageLocation{ Spec: velerov1api.BackupStorageLocationSpec{ @@ -72,7 +76,9 @@ func TestGetRepoIdentifier(t *testing.T) { }, }, } - assert.Equal(t, "s3:s3-us-west-2.amazonaws.com/bucket/prefix/restic/repo-1", GetRepoIdentifier(backupLocation, "repo-1")) + id, err = GetRepoIdentifier(backupLocation, "repo-1") + assert.NoError(t, err) + assert.Equal(t, "s3:s3-us-west-2.amazonaws.com/bucket/prefix/restic/repo-1", id) backupLocation = &velerov1api.BackupStorageLocation{ Spec: velerov1api.BackupStorageLocationSpec{ @@ -88,7 +94,9 @@ func TestGetRepoIdentifier(t *testing.T) { }, }, } - assert.Equal(t, "s3:alternate-url/bucket/prefix/restic/repo-1", GetRepoIdentifier(backupLocation, "repo-1")) + id, err = GetRepoIdentifier(backupLocation, "repo-1") + assert.NoError(t, err) + assert.Equal(t, "s3:alternate-url/bucket/prefix/restic/repo-1", id) backupLocation = &velerov1api.BackupStorageLocation{ Spec: velerov1api.BackupStorageLocationSpec{ @@ -104,7 +112,9 @@ func TestGetRepoIdentifier(t *testing.T) { }, }, } - assert.Equal(t, "s3:alternate-url-with-trailing-slash/bucket/prefix/restic/repo-1", GetRepoIdentifier(backupLocation, "repo-1")) + id, err = GetRepoIdentifier(backupLocation, "repo-1") + assert.NoError(t, err) + assert.Equal(t, "s3:alternate-url-with-trailing-slash/bucket/prefix/restic/repo-1", id) backupLocation = &velerov1api.BackupStorageLocation{ Spec: velerov1api.BackupStorageLocationSpec{ @@ -117,7 +127,9 @@ func TestGetRepoIdentifier(t *testing.T) { }, }, } - assert.Equal(t, "azure:bucket:/prefix/restic/repo-1", GetRepoIdentifier(backupLocation, "repo-1")) + id, err = GetRepoIdentifier(backupLocation, "repo-1") + assert.NoError(t, err) + assert.Equal(t, "azure:bucket:/prefix/restic/repo-1", id) backupLocation = &velerov1api.BackupStorageLocation{ Spec: velerov1api.BackupStorageLocationSpec{ @@ -130,5 +142,40 @@ func TestGetRepoIdentifier(t *testing.T) { }, }, } - assert.Equal(t, "gs:bucket-2:/prefix-2/restic/repo-2", GetRepoIdentifier(backupLocation, "repo-2")) + id, err = GetRepoIdentifier(backupLocation, "repo-2") + assert.NoError(t, err) + assert.Equal(t, "gs:bucket-2:/prefix-2/restic/repo-2", id) + + backupLocation = &velerov1api.BackupStorageLocation{ + Spec: velerov1api.BackupStorageLocationSpec{ + Provider: "unsupported-provider", + StorageType: velerov1api.StorageType{ + ObjectStorage: &velerov1api.ObjectStorageLocation{ + Bucket: "bucket-2", + Prefix: "prefix-2", + }, + }, + }, + } + id, err = GetRepoIdentifier(backupLocation, "repo-1") + assert.EqualError(t, err, "cannot determine restic repository prefix for backup storage location") + assert.Empty(t, id) + + backupLocation = &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", + }, + }, + }, + } + id, err = GetRepoIdentifier(backupLocation, "repo-1") + assert.NoError(t, err) + assert.Equal(t, "custom:prefix:/restic/repo-1", id) }