Merge pull request #5181 from blackpiglet/restic-volume-path-change

Add annotation "pv.kubernetes.io/migrated-to" for CSI checking.

changelogs/unreleased/5181-jxun

@@ -0,0 +1 @@
+Add annotation "pv.kubernetes.io/migrated-to" for CSI checking.

pkg/controller/pod_volume_backup_controller.go

@@ -209,19 +209,6 @@ func (r *PodVolumeBackupReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		Complete(r)
 }
 
-func (r *PodVolumeBackupReconciler) singlePathMatch(path string) (string, error) {
-	matches, err := r.FileSystem.Glob(path)
-	if err != nil {
-		return "", errors.WithStack(err)
-	}
-
-	if len(matches) != 1 {
-		return "", errors.Errorf("expected one matching path: %s, got %d", path, len(matches))
-	}
-
-	return matches[0], nil
-}
-
 // getParentSnapshot finds the most recent completed PodVolumeBackup for the
 // specified PVC and returns its Restic snapshot ID. Any errors encountered are
 // logged but not returned since they do not prevent a backup from proceeding.
@@ -318,7 +305,7 @@ func (r *PodVolumeBackupReconciler) buildResticCommand(ctx context.Context, log
 	pathGlob := fmt.Sprintf("/host_pods/%s/volumes/*/%s", string(pvb.Spec.Pod.UID), volDir)
 	log.WithField("pathGlob", pathGlob).Debug("Looking for path matching glob")
 
-	path, err := r.singlePathMatch(pathGlob)
+	path, err := kube.SinglePathMatch(pathGlob, r.FileSystem, log)
 	if err != nil {
 		return nil, errors.Wrap(err, "identifying unique volume path on host")
 	}

pkg/controller/pod_volume_restore_controller.go

@@ -216,19 +216,6 @@ func getResticInitContainerIndex(pod *corev1api.Pod) int {
 	return -1
 }
 
-func singlePathMatch(path string) (string, error) {
-	matches, err := filepath.Glob(path)
-	if err != nil {
-		return "", errors.WithStack(err)
-	}
-
-	if len(matches) != 1 {
-		return "", errors.Errorf("expected one matching path: %s, got %d", path, len(matches))
-	}
-
-	return matches[0], nil
-}
-
 func (c *PodVolumeRestoreReconciler) processRestore(ctx context.Context, req *velerov1api.PodVolumeRestore, pod *corev1api.Pod, log logrus.FieldLogger) error {
 	volumeDir, err := kube.GetVolumeDirectory(ctx, log, pod, req.Spec.Volume, c.Client)
 	if err != nil {
@@ -237,7 +224,9 @@ func (c *PodVolumeRestoreReconciler) processRestore(ctx context.Context, req *ve
 
 	// Get the full path of the new volume's directory as mounted in the daemonset pod, which
 	// will look like: /host_pods/<new-pod-uid>/volumes/<volume-plugin-name>/<volume-dir>
-	volumePath, err := singlePathMatch(fmt.Sprintf("/host_pods/%s/volumes/*/%s", string(req.Spec.Pod.UID), volumeDir))
+	volumePath, err := kube.SinglePathMatch(
+		fmt.Sprintf("/host_pods/%s/volumes/*/%s", string(req.Spec.Pod.UID), volumeDir),
+		c.fileSystem, log)
 	if err != nil {
 		return errors.Wrap(err, "error identifying path of volume")
 	}

pkg/util/kube/utils.go

@@ -34,15 +34,20 @@ import (
 	"k8s.io/apimachinery/pkg/util/wait"
 	corev1client "k8s.io/client-go/kubernetes/typed/core/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/vmware-tanzu/velero/pkg/util/filesystem"
 )
 
 // These annotations are taken from the Kubernetes persistent volume/persistent volume claim controller.
 // They cannot be directly importing because they are part of the kubernetes/kubernetes package, and importing that package is unsupported.
 // Their values are well-known and slow changing. They're duplicated here as constants to provide compile-time checking.
 // Originals can be found in kubernetes/kubernetes/pkg/controller/volume/persistentvolume/util/util.go.
-const KubeAnnBindCompleted = "pv.kubernetes.io/bind-completed"
-const KubeAnnBoundByController = "pv.kubernetes.io/bound-by-controller"
-const KubeAnnDynamicallyProvisioned = "pv.kubernetes.io/provisioned-by"
+const (
+	KubeAnnBindCompleted          = "pv.kubernetes.io/bind-completed"
+	KubeAnnBoundByController      = "pv.kubernetes.io/bound-by-controller"
+	KubeAnnDynamicallyProvisioned = "pv.kubernetes.io/provisioned-by"
+	KubeAnnMigratedTo             = "pv.kubernetes.io/migrated-to"
+)
 
 // NamespaceAndName returns a string in the format <namespace>/<name>
 func NamespaceAndName(objMeta metav1.Object) string {
@@ -163,6 +168,9 @@ func GetVolumeDirectory(ctx context.Context, log logrus.FieldLogger, pod *corev1
 	return pvc.Spec.VolumeName, nil
 }
 
+// isProvisionedByCSI function checks whether this is a CSI PV by annotation.
+// Either "pv.kubernetes.io/provisioned-by" or "pv.kubernetes.io/migrated-to" indicates
+// PV is provisioned by CSI.
 func isProvisionedByCSI(log logrus.FieldLogger, pv *corev1api.PersistentVolume, kbClient client.Client) (bool, error) {
 	if pv.Spec.CSI != nil {
 		return true, nil
@@ -171,14 +179,15 @@ func isProvisionedByCSI(log logrus.FieldLogger, pv *corev1api.PersistentVolume,
 	// Refer to https://github.com/vmware-tanzu/velero/issues/4496 for more details
 	if pv.Annotations != nil {
 		driverName := pv.Annotations[KubeAnnDynamicallyProvisioned]
-		if len(driverName) > 0 {
+		migratedDriver := pv.Annotations[KubeAnnMigratedTo]
+		if len(driverName) > 0 || len(migratedDriver) > 0 {
 			list := &storagev1api.CSIDriverList{}
 			if err := kbClient.List(context.TODO(), list); err != nil {
 				return false, err
 			}
 			for _, driver := range list.Items {
-				if driverName == driver.Name {
-					log.Debugf("the annotation %s=%s indicates the volume is provisioned by a CSI driver", KubeAnnDynamicallyProvisioned, driverName)
+				if driverName == driver.Name || migratedDriver == driver.Name {
+					log.Debugf("the annotation %s or %s equals to %s indicates the volume is provisioned by a CSI driver", KubeAnnDynamicallyProvisioned, KubeAnnMigratedTo, driverName)
 					return true, nil
 				}
 			}
@@ -187,6 +196,21 @@ func isProvisionedByCSI(log logrus.FieldLogger, pv *corev1api.PersistentVolume,
 	return false, nil
 }
 
+// SinglePathMatch function will be called by PVB and PVR controller to check whether pass-in volume path is valid.
+// Check whether there is only one match by the path's pattern (/host_pods/%s/volumes/*/volume_name/[mount|]).
+func SinglePathMatch(path string, fs filesystem.Interface, log logrus.FieldLogger) (string, error) {
+	matches, err := fs.Glob(path)
+	if err != nil {
+		return "", errors.WithStack(err)
+	}
+	if len(matches) != 1 {
+		return "", errors.Errorf("expected one matching path: %s, got %d", path, len(matches))
+	}
+
+	log.Debugf("This is a valid volume path: %s.", matches[0])
+	return matches[0], nil
+}
+
 // IsV1CRDReady checks a v1 CRD to see if it's ready, with both the Established and NamesAccepted conditions.
 func IsV1CRDReady(crd *apiextv1.CustomResourceDefinition) bool {
 	var isEstablished, namesAccepted bool

pkg/util/kube/utils_test.go

@@ -197,6 +197,13 @@ func TestGetVolumeDirectorySuccess(t *testing.T) {
 			pv:   builder.ForPersistentVolume("a-pv").ObjectMeta(builder.WithAnnotations(KubeAnnDynamicallyProvisioned, "csi.test.com")).Result(),
 			want: "a-pv/mount",
 		},
+		{
+			name: "Volume with CSI annotation 'pv.kubernetes.io/migrated-to' appends '/mount' to the volume name",
+			pod:  builder.ForPod("ns-1", "my-pod").Volumes(builder.ForVolume("my-vol").PersistentVolumeClaimSource("my-pvc").Result()).Result(),
+			pvc:  builder.ForPersistentVolumeClaim("ns-1", "my-pvc").VolumeName("a-pv").Result(),
+			pv:   builder.ForPersistentVolume("a-pv").ObjectMeta(builder.WithAnnotations(KubeAnnMigratedTo, "csi.test.com")).Result(),
+			want: "a-pv/mount",
+		},
 	}
 
 	csiDriver := storagev1api.CSIDriver{
@@ -425,3 +432,13 @@ func TestIsCRDReady(t *testing.T) {
 	_, err = IsCRDReady(obj)
 	assert.NotNil(t, err)
 }
+
+func TestSinglePathMatch(t *testing.T) {
+	fakeFS := velerotest.NewFakeFileSystem()
+	fakeFS.MkdirAll("testDir1/subpath", 0755)
+	fakeFS.MkdirAll("testDir2/subpath", 0755)
+
+	_, err := SinglePathMatch("./*/subpath", fakeFS, logrus.StandardLogger())
+	assert.NotNil(t, err)
+	assert.Contains(t, err.Error(), "expected one matching path")
+}