diff --git a/changelogs/unreleased/8096-Lyndon-Li b/changelogs/unreleased/8096-Lyndon-Li new file mode 100644 index 000000000..9c0e2dd0d --- /dev/null +++ b/changelogs/unreleased/8096-Lyndon-Li @@ -0,0 +1 @@ +Fix issue #8072, add the warning messages for restic deprecation \ No newline at end of file diff --git a/pkg/cmd/cli/install/install.go b/pkg/cmd/cli/install/install.go index c51cee658..b73438e78 100644 --- a/pkg/cmd/cli/install/install.go +++ b/pkg/cmd/cli/install/install.go @@ -364,8 +364,10 @@ func (o *Options) Validate(c *cobra.Command, args []string, f client.Factory) er return err } - if err := uploader.ValidateUploaderType(o.UploaderType); err != nil { + if msg, err := uploader.ValidateUploaderType(o.UploaderType); err != nil { return err + } else if msg != "" { + fmt.Printf("⚠️ %s\n", msg) } // If we're only installing CRDs, we can skip the rest of the validation. diff --git a/pkg/cmd/server/server.go b/pkg/cmd/server/server.go index d8938ca56..df965ed42 100644 --- a/pkg/cmd/server/server.go +++ b/pkg/cmd/server/server.go @@ -288,8 +288,10 @@ type server struct { } func newServer(f client.Factory, config serverConfig, logger *logrus.Logger) (*server, error) { - if err := uploader.ValidateUploaderType(config.uploaderType); err != nil { + if msg, err := uploader.ValidateUploaderType(config.uploaderType); err != nil { return nil, err + } else if msg != "" { + logger.Warn(msg) } if config.clientQPS < 0.0 { diff --git a/pkg/cmd/server/server_test.go b/pkg/cmd/server/server_test.go index 4d222b776..6e5995586 100644 --- a/pkg/cmd/server/server_test.go +++ b/pkg/cmd/server/server_test.go @@ -203,6 +203,13 @@ func Test_newServer(t *testing.T) { }, logger) assert.Error(t, err) + // invalid clientQPS Restic uploader + _, err = newServer(factory, serverConfig{ + uploaderType: uploader.ResticType, + clientQPS: -1, + }, logger) + assert.Error(t, err) + // invalid clientBurst factory.On("SetClientQPS", mock.Anything).Return() _, err = newServer(factory, serverConfig{ diff --git a/pkg/podvolume/backupper.go b/pkg/podvolume/backupper.go index 8d06cb222..5576f4e40 100644 --- a/pkg/podvolume/backupper.go +++ b/pkg/podvolume/backupper.go @@ -36,6 +36,7 @@ import ( "github.com/vmware-tanzu/velero/pkg/label" "github.com/vmware-tanzu/velero/pkg/nodeagent" "github.com/vmware-tanzu/velero/pkg/repository" + "github.com/vmware-tanzu/velero/pkg/uploader" uploaderutil "github.com/vmware-tanzu/velero/pkg/uploader/util" "github.com/vmware-tanzu/velero/pkg/util/boolptr" "github.com/vmware-tanzu/velero/pkg/util/kube" @@ -163,10 +164,13 @@ func (b *backupper) getMatchAction(resPolicies *resourcepolicies.Policies, pvc * return nil, errors.Errorf("failed to check resource policies for empty volume") } +var funcGetRepositoryType = getRepositoryType + func (b *backupper) BackupPodVolumes(backup *velerov1api.Backup, pod *corev1api.Pod, volumesToBackup []string, resPolicies *resourcepolicies.Policies, log logrus.FieldLogger) ([]*velerov1api.PodVolumeBackup, *PVCBackupSummary, []error) { if len(volumesToBackup) == 0 { return nil, nil, nil } + log.Infof("pod %s/%s has volumes to backup: %v", pod.Namespace, pod.Name, volumesToBackup) var ( @@ -189,6 +193,13 @@ func (b *backupper) BackupPodVolumes(backup *velerov1api.Backup, pod *corev1api. } } + if msg, err := uploader.ValidateUploaderType(b.uploaderType); err != nil { + skipAllPodVolumes(pod, volumesToBackup, err, pvcSummary, log) + return nil, pvcSummary, []error{err} + } else if msg != "" { + log.Warn(msg) + } + if err := kube.IsPodRunning(pod); err != nil { skipAllPodVolumes(pod, volumesToBackup, err, pvcSummary, log) return nil, pvcSummary, nil @@ -196,18 +207,21 @@ func (b *backupper) BackupPodVolumes(backup *velerov1api.Backup, pod *corev1api. err := nodeagent.IsRunningInNode(b.ctx, backup.Namespace, pod.Spec.NodeName, b.crClient) if err != nil { - return nil, nil, []error{err} + skipAllPodVolumes(pod, volumesToBackup, err, pvcSummary, log) + return nil, pvcSummary, []error{err} } - repositoryType := getRepositoryType(b.uploaderType) + repositoryType := funcGetRepositoryType(b.uploaderType) if repositoryType == "" { err := errors.Errorf("empty repository type, uploader %s", b.uploaderType) - return nil, nil, []error{err} + skipAllPodVolumes(pod, volumesToBackup, err, pvcSummary, log) + return nil, pvcSummary, []error{err} } repo, err := b.repoEnsurer.EnsureRepo(b.ctx, backup.Namespace, pod.Namespace, backup.Spec.StorageLocation, repositoryType) if err != nil { - return nil, nil, []error{err} + skipAllPodVolumes(pod, volumesToBackup, err, pvcSummary, log) + return nil, pvcSummary, []error{err} } // get a single non-exclusive lock since we'll wait for all individual diff --git a/pkg/podvolume/backupper_test.go b/pkg/podvolume/backupper_test.go index 06cec20de..16d4ce286 100644 --- a/pkg/podvolume/backupper_test.go +++ b/pkg/podvolume/backupper_test.go @@ -309,22 +309,38 @@ func TestBackupPodVolumes(t *testing.T) { corev1api.AddToScheme(scheme) tests := []struct { - name string - bsl string - uploaderType string - volumes []string - sourcePod *corev1api.Pod - kubeClientObj []runtime.Object - ctlClientObj []runtime.Object - veleroClientObj []runtime.Object - veleroReactors []reactor - runtimeScheme *runtime.Scheme - pvbs int - errs []string + name string + bsl string + uploaderType string + volumes []string + sourcePod *corev1api.Pod + kubeClientObj []runtime.Object + ctlClientObj []runtime.Object + veleroClientObj []runtime.Object + veleroReactors []reactor + runtimeScheme *runtime.Scheme + pvbs int + mockGetRepositoryType bool + errs []string }{ { name: "empty volume list", }, + { + name: "wrong uploader type", + volumes: []string{ + "fake-volume-1", + "fake-volume-2", + }, + sourcePod: createPodObj(true, false, false, 2), + kubeClientObj: []runtime.Object{ + createNodeAgentPodObj(true), + }, + uploaderType: "fake-uploader-type", + errs: []string{ + "invalid uploader type 'fake-uploader-type', valid upload types are: 'restic', 'kopia'", + }, + }, { name: "pod is not running", volumes: []string{ @@ -348,7 +364,8 @@ func TestBackupPodVolumes(t *testing.T) { "fake-volume-1", "fake-volume-2", }, - sourcePod: createPodObj(true, false, false, 2), + sourcePod: createPodObj(true, false, false, 2), + uploaderType: "kopia", errs: []string{ "daemonset pod not found in running state in node fake-node-name", }, @@ -363,9 +380,10 @@ func TestBackupPodVolumes(t *testing.T) { kubeClientObj: []runtime.Object{ createNodeAgentPodObj(true), }, - uploaderType: "fake-uploader-type", + uploaderType: "kopia", + mockGetRepositoryType: true, errs: []string{ - "empty repository type, uploader fake-uploader-type", + "empty repository type, uploader kopia", }, }, { @@ -542,6 +560,12 @@ func TestBackupPodVolumes(t *testing.T) { require.NoError(t, err) + if test.mockGetRepositoryType { + funcGetRepositoryType = func(string) string { return "" } + } else { + funcGetRepositoryType = getRepositoryType + } + pvbs, _, errs := bp.BackupPodVolumes(backupObj, test.sourcePod, test.volumes, nil, velerotest.NewLogger()) if errs == nil { diff --git a/pkg/uploader/types.go b/pkg/uploader/types.go index 02106e266..fb79f7c9f 100644 --- a/pkg/uploader/types.go +++ b/pkg/uploader/types.go @@ -39,12 +39,17 @@ const ( // ValidateUploaderType validates if the input param is a valid uploader type. // It will return an error if it's invalid. -func ValidateUploaderType(t string) error { +func ValidateUploaderType(t string) (string, error) { t = strings.TrimSpace(t) if t != ResticType && t != KopiaType { - return fmt.Errorf("invalid uploader type '%s', valid upload types are: '%s', '%s'", t, ResticType, KopiaType) + return "", fmt.Errorf("invalid uploader type '%s', valid upload types are: '%s', '%s'", t, ResticType, KopiaType) } - return nil + + if t == ResticType { + return fmt.Sprintf("Uploader '%s' is deprecated, don't use it for new backups, otherwise the backups won't be available for restore when this functionality is removed in a future version of Velero", t), nil + } + + return "", nil } type SnapshotInfo struct { diff --git a/pkg/uploader/types_test.go b/pkg/uploader/types_test.go index 492051bf2..e92f20c79 100644 --- a/pkg/uploader/types_test.go +++ b/pkg/uploader/types_test.go @@ -1,34 +1,47 @@ package uploader -import "testing" +import ( + "testing" + + "github.com/stretchr/testify/assert" +) func TestValidateUploaderType(t *testing.T) { tests := []struct { name string input string - wantErr bool + wantErr string + wantMsg string }{ { "'restic' is a valid type", "restic", - false, + "", + "Uploader 'restic' is deprecated, don't use it for new backups, otherwise the backups won't be available for restore when this functionality is removed in a future version of Velero", }, { "' kopia ' is a valid type (space will be trimmed)", " kopia ", - false, + "", + "", }, { "'anything_else' is invalid", "anything_else", - true, + "invalid uploader type 'anything_else', valid upload types are: 'restic', 'kopia'", + "", }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if err := ValidateUploaderType(tt.input); (err != nil) != tt.wantErr { - t.Errorf("ValidateUploaderType(), input = '%s' error = %v, wantErr %v", tt.input, err, tt.wantErr) + msg, err := ValidateUploaderType(tt.input) + if tt.wantErr != "" { + assert.EqualError(t, err, tt.wantErr) + } else { + assert.NoError(t, err) } + + assert.Equal(t, tt.wantMsg, msg) }) } } diff --git a/site/content/docs/main/file-system-backup.md b/site/content/docs/main/file-system-backup.md index 1e4917eb4..5dac5e3c5 100644 --- a/site/content/docs/main/file-system-backup.md +++ b/site/content/docs/main/file-system-backup.md @@ -28,6 +28,7 @@ Cons: - It access the file system from the mounted hostpath directory, so Velero Node Agent pods need to run as root user and even under privileged mode in some environments. **NOTE:** hostPath volumes are not supported, but the [local volume type][5] is supported. +**NOTE:** restic is under the deprecation process by following [Velero Deprecation Policy][17], for more details, see the Restic Deprecation section. ## Setup File System Backup @@ -643,6 +644,39 @@ If you want to constraint the CPU/memory usage, you need to [customize the resou During the restore, the repository may also cache data/metadata so as to reduce the network footprint and speed up the restore. The repository uses its own policy to store and clean up the cache. For Kopia repository, the cache is stored in the node-agent pod's root file system and the cleanup is triggered for the data/metadata that are older than 10 minutes (not configurable at present). So you should prepare enough disk space, otherwise, the node-agent pod may be evicted due to running out of the ephemeral storage. +## Restic Deprecation + +According to the [Velero Deprecation Policy][17], restic path is being deprecated starting from v1.15, specifically: +- For 1.15 and 1.16, if restic path is used by a backup, the backup still creates and succeeds but you will see warnings +- For 1.17 and 1.18, backups with restic path are disabled, but you are still allowed to restore from your previous restic backups +- From 1.19, both backups and restores with restic path will be disabled, you are not able to use 1.19 or higher to restore your restic backup data + +For 1.15 and 1.16, you will see below warnings if `--uploader-type=restic` is used in Velero installation: +In the output of installation: +``` +⚠️ Uploader 'restic' is deprecated, don't use it for new backups, otherwise the backups won't be available for restore when this functionality is removed in a future version of Velero +``` +In Velero server log: +``` +level=warning msg="Uploader 'restic' is deprecated, don't use it for new backups, otherwise the backups won't be available for restore when this functionality is removed in a future version of Velero +``` +In the output of `velero backup describe` command for a backup with fs-backup: +``` + Namespaces: + : resource: /pods name: message: /Uploader 'restic' is deprecated, don't use it for new backups, otherwise the backups won't be available for restore when this functionality is removed in a future version of Velero +``` + +And you will see below warnings you upgrade from v1.9 or lower to 1.15 or 1.16: +In Velero server log: +``` +level=warning msg="Uploader 'restic' is deprecated, don't use it for new backups, otherwise the backups won't be available for restore when this functionality is removed in a future version of Velero +``` +In the output of `velero backup describe` command for a backup with fs-backup: +``` + Namespaces: + : resource: /pods name: message: /Uploader 'restic' is deprecated, don't use it for new backups, otherwise the backups won't be available for restore when this functionality is removed in a future version of Velero +``` + [1]: https://github.com/restic/restic [2]: https://github.com/kopia/kopia @@ -660,3 +694,4 @@ For Kopia repository, the cache is stored in the node-agent pod's root file syst [14]: https://kubernetes.io/docs/concepts/workloads/pods/pod-qos/ [15]: customize-installation.md#customize-resource-requests-and-limits [16]: performance-guidance.md +[17]: https://github.com/vmware-tanzu/velero/blob/main/GOVERNANCE.md#deprecation-policy