Merge pull request #8096 from Lyndon-Li/issue-fix-8072

Issue 8072: restic deprecation - warning messages

changelogs/unreleased/8096-Lyndon-Li

@@ -0,0 +1 @@
+Fix issue #8072, add the warning messages for restic deprecation

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.

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 {

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{

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

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 {

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 {

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)
 		})
 	}
 }

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:
+    <namespace>:   resource: /pods name: <pod 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:
+    <namespace>:   resource: /pods name: <pod 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