diff --git a/changelogs/unreleased/5181-jxun b/changelogs/unreleased/5181-jxun new file mode 100644 index 000000000..4333691d2 --- /dev/null +++ b/changelogs/unreleased/5181-jxun @@ -0,0 +1 @@ +Add annotation "pv.kubernetes.io/migrated-to" for CSI checking. \ No newline at end of file diff --git a/pkg/controller/pod_volume_backup_controller.go b/pkg/controller/pod_volume_backup_controller.go index d7fab3548..abec6d601 100644 --- a/pkg/controller/pod_volume_backup_controller.go +++ b/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") } diff --git a/pkg/controller/pod_volume_restore_controller.go b/pkg/controller/pod_volume_restore_controller.go index e52de6fdb..2b81d363a 100644 --- a/pkg/controller/pod_volume_restore_controller.go +++ b/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//volumes// - 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") } diff --git a/pkg/util/kube/utils.go b/pkg/util/kube/utils.go index 24b2ef6c7..bf7ac0011 100644 --- a/pkg/util/kube/utils.go +++ b/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 / 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 diff --git a/pkg/util/kube/utils_test.go b/pkg/util/kube/utils_test.go index 4a6db6069..178fa425f 100644 --- a/pkg/util/kube/utils_test.go +++ b/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") +}