From 61a6c1ba2afaf44f6c17f35e64bae81a648d326e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wenkai=20Yin=28=E5=B0=B9=E6=96=87=E5=BC=80=29?= Date: Thu, 28 Sep 2023 10:33:09 +0800 Subject: [PATCH 01/10] Create the backup repository only when it doesn't exist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When preparing a backup repository, Velero tries to connect to it, if fails then create it. The repository status always records the error reported by creation but the real reason maybe caused by the connect operation. This is confuseing and hard to debug Signed-off-by: Wenkai Yin(尹文开) --- pkg/repository/provider/unified_repo.go | 7 ++++-- pkg/repository/provider/unified_repo_test.go | 24 +++++++++++++++++++- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/pkg/repository/provider/unified_repo.go b/pkg/repository/provider/unified_repo.go index 51b9ef28731..988174fa909 100644 --- a/pkg/repository/provider/unified_repo.go +++ b/pkg/repository/provider/unified_repo.go @@ -26,6 +26,7 @@ import ( "strings" "time" + "github.com/kopia/kopia/repo" "github.com/pkg/errors" "github.com/sirupsen/logrus" @@ -188,11 +189,13 @@ func (urp *unifiedRepoProvider) PrepareRepo(ctx context.Context, param RepoParam log.Debug("Repo has already been initialized remotely") return nil } - log.Infof("failed to connect to the repo: %v, will try to create it", err) + if !errors.Is(err, repo.ErrRepositoryNotInitialized) { + return errors.Wrap(err, "error to connect to backup repo") + } err = urp.repoService.Init(ctx, *repoOption, true) if err != nil { - return errors.Wrap(err, "error to init backup repo") + return errors.Wrap(err, "error to create backup repo") } log.Debug("Prepare repo complete") diff --git a/pkg/repository/provider/unified_repo_test.go b/pkg/repository/provider/unified_repo_test.go index ff6c31b4a7b..74cdc74b225 100644 --- a/pkg/repository/provider/unified_repo_test.go +++ b/pkg/repository/provider/unified_repo_test.go @@ -23,6 +23,7 @@ import ( "testing" awscredentials "github.com/aws/aws-sdk-go/aws/credentials" + "github.com/kopia/kopia/repo" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -648,7 +649,28 @@ func TestPrepareRepo(t *testing.T) { } return errors.New("fake-error-2") }, - expectedErr: "error to init backup repo: fake-error-2", + expectedErr: "error to connect to backup repo: fake-error-1", + }, + { + name: "not initialize", + getter: new(credmock.SecretStore), + credStoreReturn: "fake-password", + funcTable: localFuncTable{ + getStorageVariables: func(*velerov1api.BackupStorageLocation, string, string) (map[string]string, error) { + return map[string]string{}, nil + }, + getStorageCredentials: func(*velerov1api.BackupStorageLocation, velerocredentials.FileStore) (map[string]string, error) { + return map[string]string{}, nil + }, + }, + repoService: new(reposervicenmocks.BackupRepoService), + retFuncInit: func(ctx context.Context, repoOption udmrepo.RepoOptions, createNew bool) error { + if !createNew { + return repo.ErrRepositoryNotInitialized + } + return errors.New("fake-error-2") + }, + expectedErr: "error to create backup repo: fake-error-2", }, } From 24e37c51150beace7fb5310b20c2261a89ecbe11 Mon Sep 17 00:00:00 2001 From: Lyndon-Li Date: Thu, 28 Sep 2023 17:13:27 +0800 Subject: [PATCH 02/10] fix CI out of disk space problem Signed-off-by: Lyndon-Li --- .github/workflows/push.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index b1af90a3794..c4818c39b4d 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -73,7 +73,7 @@ jobs: run: | sudo swapoff -a sudo rm -f /mnt/swapfile - docker image prune -a --force + docker system prune -a --force # Build and push Velero image to docker registry docker login -u ${{ secrets.DOCKER_USER }} -p ${{ secrets.DOCKER_PASSWORD }} From 64595cc0f7e0c10d7545f4150facad464e346d11 Mon Sep 17 00:00:00 2001 From: Xun Jiang Date: Thu, 28 Sep 2023 20:30:05 +0800 Subject: [PATCH 03/10] Add go clean in Dockerfile and action. Signed-off-by: Xun Jiang --- .github/workflows/push.yml | 5 ++++- Dockerfile | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/push.yml b/.github/workflows/push.yml index c4818c39b4d..810dc2f7b90 100644 --- a/.github/workflows/push.yml +++ b/.github/workflows/push.yml @@ -48,7 +48,10 @@ jobs: version: latest - name: Build - run: make local + run: | + make local + # Clean go cache to ease the build environment storage pressure. + go clean -modcache -cache - name: Test run: make test diff --git a/Dockerfile b/Dockerfile index 587a0e09887..b853cb89d27 100644 --- a/Dockerfile +++ b/Dockerfile @@ -43,7 +43,8 @@ RUN mkdir -p /output/usr/bin && \ go build -o /output/${BIN} \ -ldflags "${LDFLAGS}" ${PKG}/cmd/${BIN} && \ go build -o /output/velero-helper \ - -ldflags "${LDFLAGS}" ${PKG}/cmd/velero-helper + -ldflags "${LDFLAGS}" ${PKG}/cmd/velero-helper && \ + go clean -modcache -cache # Restic binary build section FROM --platform=$BUILDPLATFORM golang:1.20-bullseye as restic-builder @@ -65,7 +66,8 @@ COPY . /go/src/github.com/vmware-tanzu/velero RUN mkdir -p /output/usr/bin && \ export GOARM=$(echo "${GOARM}" | cut -c2-) && \ - /go/src/github.com/vmware-tanzu/velero/hack/build-restic.sh + /go/src/github.com/vmware-tanzu/velero/hack/build-restic.sh && \ + go clean -modcache -cache # Velero image packing section FROM gcr.io/distroless/base-nossl-debian11:nonroot From 8e01d1b9be31d30f88e90003af6c4ce15c64ee65 Mon Sep 17 00:00:00 2001 From: David Zaninovic <74072514+dzaninovic@users.noreply.github.com> Date: Thu, 28 Sep 2023 09:44:46 -0400 Subject: [PATCH 04/10] Add support for block volumes (#6680) Signed-off-by: David Zaninovic --- changelogs/unreleased/6680-dzaninovic | 1 + .../CLI/PoC/overlays/plugins/node-agent.yaml | 6 ++ .../volume-snapshot-data-movement.md | 91 ++++++++--------- pkg/builder/persistent_volume_builder.go | 6 ++ pkg/cmd/cli/install/install.go | 3 + pkg/controller/data_upload_controller.go | 28 +++++- pkg/controller/data_upload_controller_test.go | 18 +++- pkg/datapath/file_system.go | 15 +-- pkg/datapath/types.go | 2 +- pkg/exposer/csi_snapshot.go | 14 +-- pkg/exposer/generic_restore.go | 12 +-- pkg/exposer/host_path.go | 17 +++- pkg/exposer/host_path_test.go | 37 +++++-- pkg/exposer/types.go | 1 + pkg/install/daemonset.go | 17 +++- pkg/install/daemonset_test.go | 2 +- pkg/install/deployment.go | 7 ++ pkg/install/resources.go | 4 + pkg/podvolume/backupper.go | 21 +++- pkg/uploader/kopia/block_backup.go | 55 +++++++++++ pkg/uploader/kopia/block_restore.go | 99 +++++++++++++++++++ pkg/uploader/kopia/snapshot.go | 56 +++++++---- pkg/uploader/kopia/snapshot_test.go | 52 +++++++++- pkg/uploader/provider/kopia.go | 11 +-- pkg/uploader/provider/kopia_test.go | 20 ++-- pkg/uploader/provider/restic_test.go | 3 + pkg/util/kube/pvc_pv.go | 20 ++++ pkg/util/kube/utils.go | 79 +++++++++++---- pkg/util/kube/utils_test.go | 56 +++++++++++ .../docs/main/csi-snapshot-data-movement.md | 20 +--- .../docs/main/customize-installation.md | 8 ++ site/content/docs/main/file-system-backup.md | 20 +--- tilt-resources/examples/node-agent.yaml | 6 ++ 33 files changed, 615 insertions(+), 192 deletions(-) create mode 100644 changelogs/unreleased/6680-dzaninovic create mode 100644 pkg/uploader/kopia/block_backup.go create mode 100644 pkg/uploader/kopia/block_restore.go diff --git a/changelogs/unreleased/6680-dzaninovic b/changelogs/unreleased/6680-dzaninovic new file mode 100644 index 00000000000..b4d735d1452 --- /dev/null +++ b/changelogs/unreleased/6680-dzaninovic @@ -0,0 +1 @@ +Add support for block volumes with Kopia \ No newline at end of file diff --git a/design/CLI/PoC/overlays/plugins/node-agent.yaml b/design/CLI/PoC/overlays/plugins/node-agent.yaml index dbb4ce18db6..0db26e2c54d 100644 --- a/design/CLI/PoC/overlays/plugins/node-agent.yaml +++ b/design/CLI/PoC/overlays/plugins/node-agent.yaml @@ -49,6 +49,9 @@ spec: - mountPath: /host_pods mountPropagation: HostToContainer name: host-pods + - mountPath: /var/lib/kubelet/plugins + mountPropagation: HostToContainer + name: host-plugins - mountPath: /scratch name: scratch - mountPath: /credentials @@ -60,6 +63,9 @@ spec: - hostPath: path: /var/lib/kubelet/pods name: host-pods + - hostPath: + path: /var/lib/kubelet/plugins + name: host-plugins - emptyDir: {} name: scratch - name: cloud-credentials diff --git a/design/volume-snapshot-data-movement/volume-snapshot-data-movement.md b/design/volume-snapshot-data-movement/volume-snapshot-data-movement.md index d006b29e21f..6a88b8b48e4 100644 --- a/design/volume-snapshot-data-movement/volume-snapshot-data-movement.md +++ b/design/volume-snapshot-data-movement/volume-snapshot-data-movement.md @@ -703,33 +703,38 @@ type Provider interface { In this case, we will extend the default kopia uploader to add the ability, when a given volume is for a block mode and is mapped as a device, we will use the [StreamingFile](https://pkg.go.dev/github.com/kopia/kopia@v0.13.0/fs#StreamingFile) to stream the device and backup to the kopia repository. ```go -func getLocalBlockEntry(kopiaEntry fs.Entry, log logrus.FieldLogger) (fs.Entry, error) { - path := kopiaEntry.LocalFilesystemPath() +func getLocalBlockEntry(sourcePath string) (fs.Entry, error) { + source, err := resolveSymlink(sourcePath) + if err != nil { + return nil, errors.Wrap(err, "resolveSymlink") + } - fileInfo, err := os.Lstat(path) + fileInfo, err := os.Lstat(source) if err != nil { - return nil, errors.Wrapf(err, "Unable to get the source device information %s", path) + return nil, errors.Wrapf(err, "unable to get the source device information %s", source) } if (fileInfo.Sys().(*syscall.Stat_t).Mode & syscall.S_IFMT) != syscall.S_IFBLK { - return nil, errors.Errorf("Source path %s is not a block device", path) + return nil, errors.Errorf("source path %s is not a block device", source) } - device, err := os.Open(path) + + device, err := os.Open(source) if err != nil { if os.IsPermission(err) || err.Error() == ErrNotPermitted { - return nil, errors.Wrapf(err, "No permission to open the source device %s, make sure that node agent is running in privileged mode", path) + return nil, errors.Wrapf(err, "no permission to open the source device %s, make sure that node agent is running in privileged mode", source) } - return nil, errors.Wrapf(err, "Unable to open the source device %s", path) + return nil, errors.Wrapf(err, "unable to open the source device %s", source) } - return virtualfs.StreamingFileFromReader(kopiaEntry.Name(), device), nil + sf := virtualfs.StreamingFileFromReader(source, device) + return virtualfs.NewStaticDirectory(source, []fs.Entry{sf}), nil } ``` In the `pkg/uploader/kopia/snapshot.go` this is used in the Backup call like ```go - if volMode == PersistentVolumeFilesystem { + if volMode == uploader.PersistentVolumeFilesystem { // to be consistent with restic when backup empty dir returns one error for upper logic handle dirs, err := os.ReadDir(source) if err != nil { @@ -742,15 +747,17 @@ In the `pkg/uploader/kopia/snapshot.go` this is used in the Backup call like source = filepath.Clean(source) ... - sourceEntry, err := getLocalFSEntry(source) - if err != nil { - return nil, false, errors.Wrap(err, "Unable to get local filesystem entry") - } + var sourceEntry fs.Entry - if volMode == PersistentVolumeBlock { - sourceEntry, err = getLocalBlockEntry(sourceEntry, log) + if volMode == uploader.PersistentVolumeBlock { + sourceEntry, err = getLocalBlockEntry(source) + if err != nil { + return nil, false, errors.Wrap(err, "unable to get local block device entry") + } + } else { + sourceEntry, err = getLocalFSEntry(source) if err != nil { - return nil, false, errors.Wrap(err, "Unable to get local block device entry") + return nil, false, errors.Wrap(err, "unable to get local filesystem entry") } } @@ -766,6 +773,8 @@ We only need to extend two functions the rest will be passed through. ```go type BlockOutput struct { *restore.FilesystemOutput + + targetFileName string } var _ restore.Output = &BlockOutput{} @@ -773,30 +782,15 @@ var _ restore.Output = &BlockOutput{} const bufferSize = 128 * 1024 func (o *BlockOutput) WriteFile(ctx context.Context, relativePath string, remoteFile fs.File) error { - - targetFileName, err := filepath.EvalSymlinks(o.TargetPath) - if err != nil { - return errors.Wrapf(err, "Unable to evaluate symlinks for %s", targetFileName) - } - - fileInfo, err := os.Lstat(targetFileName) - if err != nil { - return errors.Wrapf(err, "Unable to get the target device information for %s", targetFileName) - } - - if (fileInfo.Sys().(*syscall.Stat_t).Mode & syscall.S_IFMT) != syscall.S_IFBLK { - return errors.Errorf("Target file %s is not a block device", targetFileName) - } - remoteReader, err := remoteFile.Open(ctx) if err != nil { - return errors.Wrapf(err, "Failed to open remote file %s", remoteFile.Name()) + return errors.Wrapf(err, "failed to open remote file %s", remoteFile.Name()) } defer remoteReader.Close() - targetFile, err := os.Create(targetFileName) + targetFile, err := os.Create(o.targetFileName) if err != nil { - return errors.Wrapf(err, "Failed to open file %s", targetFileName) + return errors.Wrapf(err, "failed to open file %s", o.targetFileName) } defer targetFile.Close() @@ -807,7 +801,7 @@ func (o *BlockOutput) WriteFile(ctx context.Context, relativePath string, remote bytesToWrite, err := remoteReader.Read(buffer) if err != nil { if err != io.EOF { - return errors.Wrapf(err, "Failed to read data from remote file %s", targetFileName) + return errors.Wrapf(err, "failed to read data from remote file %s", o.targetFileName) } readData = false } @@ -819,7 +813,7 @@ func (o *BlockOutput) WriteFile(ctx context.Context, relativePath string, remote bytesToWrite -= bytesWritten offset += bytesWritten } else { - return errors.Wrapf(err, "Failed to write data to file %s", targetFileName) + return errors.Wrapf(err, "failed to write data to file %s", o.targetFileName) } } } @@ -829,42 +823,43 @@ func (o *BlockOutput) WriteFile(ctx context.Context, relativePath string, remote } func (o *BlockOutput) BeginDirectory(ctx context.Context, relativePath string, e fs.Directory) error { - targetFileName, err := filepath.EvalSymlinks(o.TargetPath) + var err error + o.targetFileName, err = filepath.EvalSymlinks(o.TargetPath) if err != nil { - return errors.Wrapf(err, "Unable to evaluate symlinks for %s", targetFileName) + return errors.Wrapf(err, "unable to evaluate symlinks for %s", o.targetFileName) } - fileInfo, err := os.Lstat(targetFileName) + fileInfo, err := os.Lstat(o.targetFileName) if err != nil { - return errors.Wrapf(err, "Unable to get the target device information for %s", o.TargetPath) + return errors.Wrapf(err, "unable to get the target device information for %s", o.TargetPath) } if (fileInfo.Sys().(*syscall.Stat_t).Mode & syscall.S_IFMT) != syscall.S_IFBLK { - return errors.Errorf("Target file %s is not a block device", o.TargetPath) + return errors.Errorf("target file %s is not a block device", o.TargetPath) } return nil } ``` -Of note, we do need to add root access to the daemon set node agent to access the new mount. +Additional mount is required in the node-agent specification to resolve symlinks to the block devices from /host_pods/POD_ID/volumeDevices/kubernetes.io~csi directory. ```yaml -... - mountPath: /var/lib/kubelet/plugins mountPropagation: HostToContainer name: host-plugins - .... - hostPath: path: /var/lib/kubelet/plugins name: host-plugins +``` + +Privileged mode is required to access the block devices in /var/lib/kubelet/plugins/kubernetes.io/csi/volumeDevices/publish directory as confirmed by testing on EKS and Minikube. -... +```yaml SecurityContext: &corev1.SecurityContext{ - Privileged: &c.privilegedAgent, + Privileged: &c.privilegedNodeAgent, }, - ``` ## Plugin Data Movers diff --git a/pkg/builder/persistent_volume_builder.go b/pkg/builder/persistent_volume_builder.go index 5fee88c1964..4cf2e47f208 100644 --- a/pkg/builder/persistent_volume_builder.go +++ b/pkg/builder/persistent_volume_builder.go @@ -95,6 +95,12 @@ func (b *PersistentVolumeBuilder) StorageClass(name string) *PersistentVolumeBui return b } +// VolumeMode sets the PersistentVolume's volume mode. +func (b *PersistentVolumeBuilder) VolumeMode(volMode corev1api.PersistentVolumeMode) *PersistentVolumeBuilder { + b.object.Spec.VolumeMode = &volMode + return b +} + // NodeAffinityRequired sets the PersistentVolume's NodeAffinity Requirement. func (b *PersistentVolumeBuilder) NodeAffinityRequired(req *corev1api.NodeSelector) *PersistentVolumeBuilder { b.object.Spec.NodeAffinity = &corev1api.VolumeNodeAffinity{ diff --git a/pkg/cmd/cli/install/install.go b/pkg/cmd/cli/install/install.go index 25b7d09992a..72fde9ef404 100644 --- a/pkg/cmd/cli/install/install.go +++ b/pkg/cmd/cli/install/install.go @@ -66,6 +66,7 @@ type Options struct { BackupStorageConfig flag.Map VolumeSnapshotConfig flag.Map UseNodeAgent bool + PrivilegedNodeAgent bool //TODO remove UseRestic when migration test out of using it UseRestic bool Wait bool @@ -110,6 +111,7 @@ func (o *Options) BindFlags(flags *pflag.FlagSet) { flags.BoolVar(&o.RestoreOnly, "restore-only", o.RestoreOnly, "Run the server in restore-only mode. Optional.") flags.BoolVar(&o.DryRun, "dry-run", o.DryRun, "Generate resources, but don't send them to the cluster. Use with -o. Optional.") flags.BoolVar(&o.UseNodeAgent, "use-node-agent", o.UseNodeAgent, "Create Velero node-agent daemonset. Optional. Velero node-agent hosts Velero modules that need to run in one or more nodes(i.e. Restic, Kopia).") + flags.BoolVar(&o.PrivilegedNodeAgent, "privileged-node-agent", o.PrivilegedNodeAgent, "Use privileged mode for the node agent. Optional. Required to backup block devices.") flags.BoolVar(&o.Wait, "wait", o.Wait, "Wait for Velero deployment to be ready. Optional.") flags.DurationVar(&o.DefaultRepoMaintenanceFrequency, "default-repo-maintain-frequency", o.DefaultRepoMaintenanceFrequency, "How often 'maintain' is run for backup repositories by default. Optional.") flags.DurationVar(&o.GarbageCollectionFrequency, "garbage-collection-frequency", o.GarbageCollectionFrequency, "How often the garbage collection runs for expired backups.(default 1h)") @@ -198,6 +200,7 @@ func (o *Options) AsVeleroOptions() (*install.VeleroOptions, error) { SecretData: secretData, RestoreOnly: o.RestoreOnly, UseNodeAgent: o.UseNodeAgent, + PrivilegedNodeAgent: o.PrivilegedNodeAgent, UseVolumeSnapshots: o.UseVolumeSnapshots, BSLConfig: o.BackupStorageConfig.Data(), VSLConfig: o.VolumeSnapshotConfig.Data(), diff --git a/pkg/controller/data_upload_controller.go b/pkg/controller/data_upload_controller.go index 18f7e09f957..8bc650f5f9f 100644 --- a/pkg/controller/data_upload_controller.go +++ b/pkg/controller/data_upload_controller.go @@ -177,7 +177,10 @@ func (r *DataUploadReconciler) Reconcile(ctx context.Context, req ctrl.Request) return ctrl.Result{}, nil } - exposeParam := r.setupExposeParam(du) + exposeParam, err := r.setupExposeParam(du) + if err != nil { + return r.errorOut(ctx, du, err, "failed to set exposer parameters", log) + } // Expose() will trigger to create one pod whose volume is restored by a given volume snapshot, // but the pod maybe is not in the same node of the current controller, so we need to return it here. @@ -735,18 +738,33 @@ func (r *DataUploadReconciler) closeDataPath(ctx context.Context, duName string) r.dataPathMgr.RemoveAsyncBR(duName) } -func (r *DataUploadReconciler) setupExposeParam(du *velerov2alpha1api.DataUpload) interface{} { +func (r *DataUploadReconciler) setupExposeParam(du *velerov2alpha1api.DataUpload) (interface{}, error) { if du.Spec.SnapshotType == velerov2alpha1api.SnapshotTypeCSI { + pvc := &corev1.PersistentVolumeClaim{} + err := r.client.Get(context.Background(), types.NamespacedName{ + Namespace: du.Spec.SourceNamespace, + Name: du.Spec.SourcePVC, + }, pvc) + + if err != nil { + return nil, errors.Wrapf(err, "failed to get PVC %s/%s", du.Spec.SourceNamespace, du.Spec.SourcePVC) + } + + accessMode := exposer.AccessModeFileSystem + if pvc.Spec.VolumeMode != nil && *pvc.Spec.VolumeMode == corev1.PersistentVolumeBlock { + accessMode = exposer.AccessModeBlock + } + return &exposer.CSISnapshotExposeParam{ SnapshotName: du.Spec.CSISnapshot.VolumeSnapshot, SourceNamespace: du.Spec.SourceNamespace, StorageClass: du.Spec.CSISnapshot.StorageClass, HostingPodLabels: map[string]string{velerov1api.DataUploadLabel: du.Name}, - AccessMode: exposer.AccessModeFileSystem, + AccessMode: accessMode, Timeout: du.Spec.OperationTimeout.Duration, - } + }, nil } - return nil + return nil, nil } func (r *DataUploadReconciler) setupWaitExposePara(du *velerov2alpha1api.DataUpload) interface{} { diff --git a/pkg/controller/data_upload_controller_test.go b/pkg/controller/data_upload_controller_test.go index 270a084ad9f..34bc4a6aaed 100644 --- a/pkg/controller/data_upload_controller_test.go +++ b/pkg/controller/data_upload_controller_test.go @@ -306,6 +306,7 @@ func TestReconcile(t *testing.T) { name string du *velerov2alpha1api.DataUpload pod *corev1.Pod + pvc *corev1.PersistentVolumeClaim snapshotExposerList map[velerov2alpha1api.SnapshotType]exposer.SnapshotExposer dataMgr *datapath.Manager expectedProcessed bool @@ -345,11 +346,21 @@ func TestReconcile(t *testing.T) { }, { name: "Dataupload should be accepted", du: dataUploadBuilder().Result(), - pod: builder.ForPod(velerov1api.DefaultNamespace, dataUploadName).Volumes(&corev1.Volume{Name: "dataupload-1"}).Result(), + pod: builder.ForPod("fake-ns", dataUploadName).Volumes(&corev1.Volume{Name: "test-pvc"}).Result(), + pvc: builder.ForPersistentVolumeClaim("fake-ns", "test-pvc").Result(), expectedProcessed: false, expected: dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseAccepted).Result(), expectedRequeue: ctrl.Result{}, }, + { + name: "Dataupload should fail to get PVC information", + du: dataUploadBuilder().Result(), + pod: builder.ForPod("fake-ns", dataUploadName).Volumes(&corev1.Volume{Name: "wrong-pvc"}).Result(), + expectedProcessed: true, + expected: dataUploadBuilder().Phase(velerov2alpha1api.DataUploadPhaseFailed).Result(), + expectedRequeue: ctrl.Result{}, + expectedErrMsg: "failed to get PVC", + }, { name: "Dataupload should be prepared", du: dataUploadBuilder().SnapshotType(fakeSnapshotType).Result(), @@ -448,6 +459,11 @@ func TestReconcile(t *testing.T) { require.NoError(t, err) } + if test.pvc != nil { + err = r.client.Create(ctx, test.pvc) + require.NoError(t, err) + } + if test.dataMgr != nil { r.dataPathMgr = test.dataMgr } else { diff --git a/pkg/datapath/file_system.go b/pkg/datapath/file_system.go index 741f6ae086d..fba9eac7b1c 100644 --- a/pkg/datapath/file_system.go +++ b/pkg/datapath/file_system.go @@ -133,10 +133,10 @@ func (fs *fileSystemBR) StartBackup(source AccessPoint, realSource string, paren if !fs.initialized { return errors.New("file system data path is not initialized") } - volMode := getPersistentVolumeMode(source) go func() { - snapshotID, emptySnapshot, err := fs.uploaderProv.RunBackup(fs.ctx, source.ByPath, realSource, tags, forceFull, parentSnapshot, volMode, fs) + snapshotID, emptySnapshot, err := fs.uploaderProv.RunBackup(fs.ctx, source.ByPath, realSource, tags, forceFull, + parentSnapshot, source.VolMode, fs) if err == provider.ErrorCanceled { fs.callbacks.OnCancelled(context.Background(), fs.namespace, fs.jobName) @@ -155,10 +155,8 @@ func (fs *fileSystemBR) StartRestore(snapshotID string, target AccessPoint) erro return errors.New("file system data path is not initialized") } - volMode := getPersistentVolumeMode(target) - go func() { - err := fs.uploaderProv.RunRestore(fs.ctx, snapshotID, target.ByPath, volMode, fs) + err := fs.uploaderProv.RunRestore(fs.ctx, snapshotID, target.ByPath, target.VolMode, fs) if err == provider.ErrorCanceled { fs.callbacks.OnCancelled(context.Background(), fs.namespace, fs.jobName) @@ -172,13 +170,6 @@ func (fs *fileSystemBR) StartRestore(snapshotID string, target AccessPoint) erro return nil } -func getPersistentVolumeMode(source AccessPoint) uploader.PersistentVolumeMode { - if source.ByBlock != "" { - return uploader.PersistentVolumeBlock - } - return uploader.PersistentVolumeFilesystem -} - // UpdateProgress which implement ProgressUpdater interface to update progress status func (fs *fileSystemBR) UpdateProgress(p *uploader.Progress) { if fs.callbacks.OnProgress != nil { diff --git a/pkg/datapath/types.go b/pkg/datapath/types.go index 431fccc5dc8..e26cf948233 100644 --- a/pkg/datapath/types.go +++ b/pkg/datapath/types.go @@ -53,7 +53,7 @@ type Callbacks struct { // AccessPoint represents an access point that has been exposed to a data path instance type AccessPoint struct { ByPath string - ByBlock string + VolMode uploader.PersistentVolumeMode } // AsyncBR is the interface for asynchronous data path methods diff --git a/pkg/exposer/csi_snapshot.go b/pkg/exposer/csi_snapshot.go index e0842f5f132..a0492d5233c 100644 --- a/pkg/exposer/csi_snapshot.go +++ b/pkg/exposer/csi_snapshot.go @@ -233,9 +233,12 @@ func (e *csiSnapshotExposer) CleanUp(ctx context.Context, ownerObject corev1.Obj } func getVolumeModeByAccessMode(accessMode string) (corev1.PersistentVolumeMode, error) { - if accessMode == AccessModeFileSystem { + switch accessMode { + case AccessModeFileSystem: return corev1.PersistentVolumeFilesystem, nil - } else { + case AccessModeBlock: + return corev1.PersistentVolumeBlock, nil + default: return "", errors.Errorf("unsupported access mode %s", accessMode) } } @@ -356,6 +359,7 @@ func (e *csiSnapshotExposer) createBackupPod(ctx context.Context, ownerObject co } var gracePeriod int64 = 0 + volumeMounts, volumeDevices := kube.MakePodPVCAttachment(volumeName, backupPVC.Spec.VolumeMode) pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -379,10 +383,8 @@ func (e *csiSnapshotExposer) createBackupPod(ctx context.Context, ownerObject co Image: podInfo.image, ImagePullPolicy: corev1.PullNever, Command: []string{"/velero-helper", "pause"}, - VolumeMounts: []corev1.VolumeMount{{ - Name: volumeName, - MountPath: "/" + volumeName, - }}, + VolumeMounts: volumeMounts, + VolumeDevices: volumeDevices, }, }, ServiceAccountName: podInfo.serviceAccount, diff --git a/pkg/exposer/generic_restore.go b/pkg/exposer/generic_restore.go index ca5cd68a3fa..0868aba4758 100644 --- a/pkg/exposer/generic_restore.go +++ b/pkg/exposer/generic_restore.go @@ -82,7 +82,7 @@ func (e *genericRestoreExposer) Expose(ctx context.Context, ownerObject corev1.O return errors.Errorf("Target PVC %s/%s has already been bound, abort", sourceNamespace, targetPVCName) } - restorePod, err := e.createRestorePod(ctx, ownerObject, hostingPodLabels, selectedNode) + restorePod, err := e.createRestorePod(ctx, ownerObject, targetPVC, hostingPodLabels, selectedNode) if err != nil { return errors.Wrapf(err, "error to create restore pod") } @@ -247,7 +247,8 @@ func (e *genericRestoreExposer) RebindVolume(ctx context.Context, ownerObject co return nil } -func (e *genericRestoreExposer) createRestorePod(ctx context.Context, ownerObject corev1.ObjectReference, label map[string]string, selectedNode string) (*corev1.Pod, error) { +func (e *genericRestoreExposer) createRestorePod(ctx context.Context, ownerObject corev1.ObjectReference, targetPVC *corev1.PersistentVolumeClaim, + label map[string]string, selectedNode string) (*corev1.Pod, error) { restorePodName := ownerObject.Name restorePVCName := ownerObject.Name @@ -260,6 +261,7 @@ func (e *genericRestoreExposer) createRestorePod(ctx context.Context, ownerObjec } var gracePeriod int64 = 0 + volumeMounts, volumeDevices := kube.MakePodPVCAttachment(volumeName, targetPVC.Spec.VolumeMode) pod := &corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ @@ -283,10 +285,8 @@ func (e *genericRestoreExposer) createRestorePod(ctx context.Context, ownerObjec Image: podInfo.image, ImagePullPolicy: corev1.PullNever, Command: []string{"/velero-helper", "pause"}, - VolumeMounts: []corev1.VolumeMount{{ - Name: volumeName, - MountPath: "/" + volumeName, - }}, + VolumeMounts: volumeMounts, + VolumeDevices: volumeDevices, }, }, ServiceAccountName: podInfo.serviceAccount, diff --git a/pkg/exposer/host_path.go b/pkg/exposer/host_path.go index 458667d9233..94dc4503c33 100644 --- a/pkg/exposer/host_path.go +++ b/pkg/exposer/host_path.go @@ -26,11 +26,13 @@ import ( ctrlclient "sigs.k8s.io/controller-runtime/pkg/client" "github.com/vmware-tanzu/velero/pkg/datapath" + "github.com/vmware-tanzu/velero/pkg/uploader" "github.com/vmware-tanzu/velero/pkg/util/filesystem" "github.com/vmware-tanzu/velero/pkg/util/kube" ) var getVolumeDirectory = kube.GetVolumeDirectory +var getVolumeMode = kube.GetVolumeMode var singlePathMatch = kube.SinglePathMatch // GetPodVolumeHostPath returns a path that can be accessed from the host for a given volume of a pod @@ -45,7 +47,17 @@ func GetPodVolumeHostPath(ctx context.Context, pod *corev1.Pod, volumeName strin logger.WithField("volDir", volDir).Info("Got volume dir") - pathGlob := fmt.Sprintf("/host_pods/%s/volumes/*/%s", string(pod.GetUID()), volDir) + volMode, err := getVolumeMode(ctx, logger, pod, volumeName, cli) + if err != nil { + return datapath.AccessPoint{}, errors.Wrapf(err, "error getting volume mode for volume %s in pod %s", volumeName, pod.Name) + } + + volSubDir := "volumes" + if volMode == uploader.PersistentVolumeBlock { + volSubDir = "volumeDevices" + } + + pathGlob := fmt.Sprintf("/host_pods/%s/%s/*/%s", string(pod.GetUID()), volSubDir, volDir) logger.WithField("pathGlob", pathGlob).Debug("Looking for path matching glob") path, err := singlePathMatch(pathGlob, fs, logger) @@ -56,6 +68,7 @@ func GetPodVolumeHostPath(ctx context.Context, pod *corev1.Pod, volumeName strin logger.WithField("path", path).Info("Found path matching glob") return datapath.AccessPoint{ - ByPath: path, + ByPath: path, + VolMode: volMode, }, nil } diff --git a/pkg/exposer/host_path_test.go b/pkg/exposer/host_path_test.go index f71518d2d78..1022dffd30b 100644 --- a/pkg/exposer/host_path_test.go +++ b/pkg/exposer/host_path_test.go @@ -29,17 +29,19 @@ import ( velerov1api "github.com/vmware-tanzu/velero/pkg/apis/velero/v1" "github.com/vmware-tanzu/velero/pkg/builder" velerotest "github.com/vmware-tanzu/velero/pkg/test" + "github.com/vmware-tanzu/velero/pkg/uploader" "github.com/vmware-tanzu/velero/pkg/util/filesystem" ) func TestGetPodVolumeHostPath(t *testing.T) { tests := []struct { - name string - getVolumeDirFunc func(context.Context, logrus.FieldLogger, *corev1.Pod, string, ctrlclient.Client) (string, error) - pathMatchFunc func(string, filesystem.Interface, logrus.FieldLogger) (string, error) - pod *corev1.Pod - pvc string - err string + name string + getVolumeDirFunc func(context.Context, logrus.FieldLogger, *corev1.Pod, string, ctrlclient.Client) (string, error) + getVolumeModeFunc func(context.Context, logrus.FieldLogger, *corev1.Pod, string, ctrlclient.Client) (uploader.PersistentVolumeMode, error) + pathMatchFunc func(string, filesystem.Interface, logrus.FieldLogger) (string, error) + pod *corev1.Pod + pvc string + err string }{ { name: "get volume dir fail", @@ -55,6 +57,9 @@ func TestGetPodVolumeHostPath(t *testing.T) { getVolumeDirFunc: func(context.Context, logrus.FieldLogger, *corev1.Pod, string, ctrlclient.Client) (string, error) { return "", nil }, + getVolumeModeFunc: func(context.Context, logrus.FieldLogger, *corev1.Pod, string, ctrlclient.Client) (uploader.PersistentVolumeMode, error) { + return uploader.PersistentVolumeFilesystem, nil + }, pathMatchFunc: func(string, filesystem.Interface, logrus.FieldLogger) (string, error) { return "", errors.New("fake-error-2") }, @@ -62,6 +67,18 @@ func TestGetPodVolumeHostPath(t *testing.T) { pvc: "fake-pvc-1", err: "error identifying unique volume path on host for volume fake-pvc-1 in pod fake-pod-2: fake-error-2", }, + { + name: "get block volume dir success", + getVolumeDirFunc: func(context.Context, logrus.FieldLogger, *corev1.Pod, string, ctrlclient.Client) ( + string, error) { + return "fake-pvc-1", nil + }, + pathMatchFunc: func(string, filesystem.Interface, logrus.FieldLogger) (string, error) { + return "/host_pods/fake-pod-1-id/volumeDevices/kubernetes.io~csi/fake-pvc-1-id", nil + }, + pod: builder.ForPod(velerov1api.DefaultNamespace, "fake-pod-1").Result(), + pvc: "fake-pvc-1", + }, } for _, test := range tests { @@ -70,12 +87,18 @@ func TestGetPodVolumeHostPath(t *testing.T) { getVolumeDirectory = test.getVolumeDirFunc } + if test.getVolumeModeFunc != nil { + getVolumeMode = test.getVolumeModeFunc + } + if test.pathMatchFunc != nil { singlePathMatch = test.pathMatchFunc } _, err := GetPodVolumeHostPath(context.Background(), test.pod, test.pvc, nil, nil, velerotest.NewLogger()) - assert.EqualError(t, err, test.err) + if test.err != "" || err != nil { + assert.EqualError(t, err, test.err) + } }) } } diff --git a/pkg/exposer/types.go b/pkg/exposer/types.go index 253256eb97f..21c473366df 100644 --- a/pkg/exposer/types.go +++ b/pkg/exposer/types.go @@ -22,6 +22,7 @@ import ( const ( AccessModeFileSystem = "by-file-system" + AccessModeBlock = "by-block-device" ) // ExposeResult defines the result of expose. diff --git a/pkg/install/daemonset.go b/pkg/install/daemonset.go index b139f81242d..8e74e16da1a 100644 --- a/pkg/install/daemonset.go +++ b/pkg/install/daemonset.go @@ -86,6 +86,14 @@ func DaemonSet(namespace string, opts ...podTemplateOption) *appsv1.DaemonSet { }, }, }, + { + Name: "host-plugins", + VolumeSource: corev1.VolumeSource{ + HostPath: &corev1.HostPathVolumeSource{ + Path: "/var/lib/kubelet/plugins", + }, + }, + }, { Name: "scratch", VolumeSource: corev1.VolumeSource{ @@ -102,13 +110,20 @@ func DaemonSet(namespace string, opts ...podTemplateOption) *appsv1.DaemonSet { "/velero", }, Args: daemonSetArgs, - + SecurityContext: &corev1.SecurityContext{ + Privileged: &c.privilegedNodeAgent, + }, VolumeMounts: []corev1.VolumeMount{ { Name: "host-pods", MountPath: "/host_pods", MountPropagation: &mountPropagationMode, }, + { + Name: "host-plugins", + MountPath: "/var/lib/kubelet/plugins", + MountPropagation: &mountPropagationMode, + }, { Name: "scratch", MountPath: "/scratch", diff --git a/pkg/install/daemonset_test.go b/pkg/install/daemonset_test.go index 762e95b16b3..017d5004d22 100644 --- a/pkg/install/daemonset_test.go +++ b/pkg/install/daemonset_test.go @@ -35,7 +35,7 @@ func TestDaemonSet(t *testing.T) { ds = DaemonSet("velero", WithSecret(true)) assert.Equal(t, 7, len(ds.Spec.Template.Spec.Containers[0].Env)) - assert.Equal(t, 3, len(ds.Spec.Template.Spec.Volumes)) + assert.Equal(t, 4, len(ds.Spec.Template.Spec.Volumes)) ds = DaemonSet("velero", WithFeatures([]string{"foo,bar,baz"})) assert.Len(t, ds.Spec.Template.Spec.Containers[0].Args, 3) diff --git a/pkg/install/deployment.go b/pkg/install/deployment.go index 993d4b16f38..917f7d9f115 100644 --- a/pkg/install/deployment.go +++ b/pkg/install/deployment.go @@ -47,6 +47,7 @@ type podTemplateConfig struct { serviceAccountName string uploaderType string defaultSnapshotMoveData bool + privilegedNodeAgent bool } func WithImage(image string) podTemplateOption { @@ -149,6 +150,12 @@ func WithServiceAccountName(sa string) podTemplateOption { } } +func WithPrivilegedNodeAgent() podTemplateOption { + return func(c *podTemplateConfig) { + c.privilegedNodeAgent = true + } +} + func Deployment(namespace string, opts ...podTemplateOption) *appsv1.Deployment { // TODO: Add support for server args c := &podTemplateConfig{ diff --git a/pkg/install/resources.go b/pkg/install/resources.go index a029ecc4a5a..9e2e8e4dab9 100644 --- a/pkg/install/resources.go +++ b/pkg/install/resources.go @@ -240,6 +240,7 @@ type VeleroOptions struct { SecretData []byte RestoreOnly bool UseNodeAgent bool + PrivilegedNodeAgent bool UseVolumeSnapshots bool BSLConfig map[string]string VSLConfig map[string]string @@ -374,6 +375,9 @@ func AllResources(o *VeleroOptions) *unstructured.UnstructuredList { if len(o.Features) > 0 { dsOpts = append(dsOpts, WithFeatures(o.Features)) } + if o.PrivilegedNodeAgent { + dsOpts = append(dsOpts, WithPrivilegedNodeAgent()) + } ds := DaemonSet(o.Namespace, dsOpts...) if err := appendUnstructured(resources, ds); err != nil { fmt.Printf("error appending DaemonSet %s: %s\n", ds.GetName(), err.Error()) diff --git a/pkg/podvolume/backupper.go b/pkg/podvolume/backupper.go index e7cfa2e5cbe..78c2b6d651d 100644 --- a/pkg/podvolume/backupper.go +++ b/pkg/podvolume/backupper.go @@ -200,10 +200,11 @@ func (b *backupper) BackupPodVolumes(backup *velerov1api.Backup, pod *corev1api. b.resultsLock.Unlock() var ( - errs []error - podVolumeBackups []*velerov1api.PodVolumeBackup - podVolumes = make(map[string]corev1api.Volume) - mountedPodVolumes = sets.String{} + errs []error + podVolumeBackups []*velerov1api.PodVolumeBackup + podVolumes = make(map[string]corev1api.Volume) + mountedPodVolumes = sets.String{} + attachedPodDevices = sets.String{} ) pvcSummary := NewPVCBackupSummary() @@ -233,6 +234,9 @@ func (b *backupper) BackupPodVolumes(backup *velerov1api.Backup, pod *corev1api. for _, volumeMount := range container.VolumeMounts { mountedPodVolumes.Insert(volumeMount.Name) } + for _, volumeDevice := range container.VolumeDevices { + attachedPodDevices.Insert(volumeDevice.Name) + } } repoIdentifier := "" @@ -268,6 +272,15 @@ func (b *backupper) BackupPodVolumes(backup *velerov1api.Backup, pod *corev1api. continue } + // check if volume is a block volume + if attachedPodDevices.Has(volumeName) { + msg := fmt.Sprintf("volume %s declared in pod %s/%s is a block volume. Block volumes are not supported for fs backup, skipping", + volumeName, pod.Namespace, pod.Name) + log.Warn(msg) + pvcSummary.addSkipped(volumeName, msg) + continue + } + // volumes that are not mounted by any container should not be backed up, because // its directory is not created if !mountedPodVolumes.Has(volumeName) { diff --git a/pkg/uploader/kopia/block_backup.go b/pkg/uploader/kopia/block_backup.go new file mode 100644 index 00000000000..a637925a491 --- /dev/null +++ b/pkg/uploader/kopia/block_backup.go @@ -0,0 +1,55 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kopia + +import ( + "os" + "syscall" + + "github.com/kopia/kopia/fs" + "github.com/kopia/kopia/fs/virtualfs" + "github.com/pkg/errors" +) + +const ErrNotPermitted = "operation not permitted" + +func getLocalBlockEntry(sourcePath string) (fs.Entry, error) { + source, err := resolveSymlink(sourcePath) + if err != nil { + return nil, errors.Wrap(err, "resolveSymlink") + } + + fileInfo, err := os.Lstat(source) + if err != nil { + return nil, errors.Wrapf(err, "unable to get the source device information %s", source) + } + + if (fileInfo.Sys().(*syscall.Stat_t).Mode & syscall.S_IFMT) != syscall.S_IFBLK { + return nil, errors.Errorf("source path %s is not a block device", source) + } + + device, err := os.Open(source) + if err != nil { + if os.IsPermission(err) || err.Error() == ErrNotPermitted { + return nil, errors.Wrapf(err, "no permission to open the source device %s, make sure that node agent is running in privileged mode", source) + } + return nil, errors.Wrapf(err, "unable to open the source device %s", source) + } + + sf := virtualfs.StreamingFileFromReader(source, device) + return virtualfs.NewStaticDirectory(source, []fs.Entry{sf}), nil +} diff --git a/pkg/uploader/kopia/block_restore.go b/pkg/uploader/kopia/block_restore.go new file mode 100644 index 00000000000..25d11ee24e8 --- /dev/null +++ b/pkg/uploader/kopia/block_restore.go @@ -0,0 +1,99 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kopia + +import ( + "context" + "io" + "os" + "path/filepath" + "syscall" + + "github.com/kopia/kopia/fs" + "github.com/kopia/kopia/snapshot/restore" + "github.com/pkg/errors" +) + +type BlockOutput struct { + *restore.FilesystemOutput + + targetFileName string +} + +var _ restore.Output = &BlockOutput{} + +const bufferSize = 128 * 1024 + +func (o *BlockOutput) WriteFile(ctx context.Context, relativePath string, remoteFile fs.File) error { + remoteReader, err := remoteFile.Open(ctx) + if err != nil { + return errors.Wrapf(err, "failed to open remote file %s", remoteFile.Name()) + } + defer remoteReader.Close() + + targetFile, err := os.Create(o.targetFileName) + if err != nil { + return errors.Wrapf(err, "failed to open file %s", o.targetFileName) + } + defer targetFile.Close() + + buffer := make([]byte, bufferSize) + + readData := true + for readData { + bytesToWrite, err := remoteReader.Read(buffer) + if err != nil { + if err != io.EOF { + return errors.Wrapf(err, "failed to read data from remote file %s", o.targetFileName) + } + readData = false + } + + if bytesToWrite > 0 { + offset := 0 + for bytesToWrite > 0 { + if bytesWritten, err := targetFile.Write(buffer[offset:bytesToWrite]); err == nil { + bytesToWrite -= bytesWritten + offset += bytesWritten + } else { + return errors.Wrapf(err, "failed to write data to file %s", o.targetFileName) + } + } + } + } + + return nil +} + +func (o *BlockOutput) BeginDirectory(ctx context.Context, relativePath string, e fs.Directory) error { + var err error + o.targetFileName, err = filepath.EvalSymlinks(o.TargetPath) + if err != nil { + return errors.Wrapf(err, "unable to evaluate symlinks for %s", o.targetFileName) + } + + fileInfo, err := os.Lstat(o.targetFileName) + if err != nil { + return errors.Wrapf(err, "unable to get the target device information for %s", o.TargetPath) + } + + if (fileInfo.Sys().(*syscall.Stat_t).Mode & syscall.S_IFMT) != syscall.S_IFBLK { + return errors.Errorf("target file %s is not a block device", o.TargetPath) + } + + return nil +} diff --git a/pkg/uploader/kopia/snapshot.go b/pkg/uploader/kopia/snapshot.go index 02129c9f0ba..27ac842532c 100644 --- a/pkg/uploader/kopia/snapshot.go +++ b/pkg/uploader/kopia/snapshot.go @@ -131,25 +131,22 @@ func Backup(ctx context.Context, fsUploader SnapshotUploader, repoWriter repo.Re if fsUploader == nil { return nil, false, errors.New("get empty kopia uploader") } - - if volMode == uploader.PersistentVolumeBlock { - return nil, false, errors.New("unable to handle block storage") - } - - dir, err := filepath.Abs(sourcePath) + source, err := filepath.Abs(sourcePath) if err != nil { return nil, false, errors.Wrapf(err, "Invalid source path '%s'", sourcePath) } - // to be consistent with restic when backup empty dir returns one error for upper logic handle - dirs, err := os.ReadDir(dir) - if err != nil { - return nil, false, errors.Wrapf(err, "Unable to read dir in path %s", dir) - } else if len(dirs) == 0 { - return nil, true, nil + if volMode == uploader.PersistentVolumeFilesystem { + // to be consistent with restic when backup empty dir returns one error for upper logic handle + dirs, err := os.ReadDir(source) + if err != nil { + return nil, false, errors.Wrapf(err, "Unable to read dir in path %s", source) + } else if len(dirs) == 0 { + return nil, true, nil + } } - dir = filepath.Clean(dir) + source = filepath.Clean(source) sourceInfo := snapshot.SourceInfo{ UserName: udmrepo.GetRepoUser(), @@ -157,16 +154,25 @@ func Backup(ctx context.Context, fsUploader SnapshotUploader, repoWriter repo.Re Path: filepath.Clean(realSource), } if realSource == "" { - sourceInfo.Path = dir + sourceInfo.Path = source } - rootDir, err := getLocalFSEntry(dir) - if err != nil { - return nil, false, errors.Wrap(err, "Unable to get local filesystem entry") + var sourceEntry fs.Entry + + if volMode == uploader.PersistentVolumeBlock { + sourceEntry, err = getLocalBlockEntry(source) + if err != nil { + return nil, false, errors.Wrap(err, "unable to get local block device entry") + } + } else { + sourceEntry, err = getLocalFSEntry(source) + if err != nil { + return nil, false, errors.Wrap(err, "unable to get local filesystem entry") + } } kopiaCtx := kopia.SetupKopiaLog(ctx, log) - snapID, snapshotSize, err := SnapshotSource(kopiaCtx, repoWriter, fsUploader, sourceInfo, rootDir, forceFull, parentSnapshot, tags, log, "Kopia Uploader") + snapID, snapshotSize, err := SnapshotSource(kopiaCtx, repoWriter, fsUploader, sourceInfo, sourceEntry, forceFull, parentSnapshot, tags, log, "Kopia Uploader") if err != nil { return nil, false, err } @@ -348,7 +354,8 @@ func findPreviousSnapshotManifest(ctx context.Context, rep repo.Repository, sour } // Restore restore specific sourcePath with given snapshotID and update progress -func Restore(ctx context.Context, rep repo.RepositoryWriter, progress *Progress, snapshotID, dest string, log logrus.FieldLogger, cancleCh chan struct{}) (int64, int32, error) { +func Restore(ctx context.Context, rep repo.RepositoryWriter, progress *Progress, snapshotID, dest string, volMode uploader.PersistentVolumeMode, + log logrus.FieldLogger, cancleCh chan struct{}) (int64, int32, error) { log.Info("Start to restore...") kopiaCtx := kopia.SetupKopiaLog(ctx, log) @@ -370,7 +377,7 @@ func Restore(ctx context.Context, rep repo.RepositoryWriter, progress *Progress, return 0, 0, errors.Wrapf(err, "Unable to resolve path %v", dest) } - output := &restore.FilesystemOutput{ + fsOutput := &restore.FilesystemOutput{ TargetPath: path, OverwriteDirectories: true, OverwriteFiles: true, @@ -378,11 +385,18 @@ func Restore(ctx context.Context, rep repo.RepositoryWriter, progress *Progress, IgnorePermissionErrors: true, } - err = output.Init(ctx) + err = fsOutput.Init(ctx) if err != nil { return 0, 0, errors.Wrap(err, "error to init output") } + var output restore.Output = fsOutput + if volMode == uploader.PersistentVolumeBlock { + output = &BlockOutput{ + FilesystemOutput: fsOutput, + } + } + stat, err := restoreEntryFunc(kopiaCtx, rep, output, rootEntry, restore.Options{ Parallel: runtime.NumCPU(), RestoreDirEntryAtDepth: math.MaxInt32, diff --git a/pkg/uploader/kopia/snapshot_test.go b/pkg/uploader/kopia/snapshot_test.go index 232ea92c4d2..65ec22136ad 100644 --- a/pkg/uploader/kopia/snapshot_test.go +++ b/pkg/uploader/kopia/snapshot_test.go @@ -23,6 +23,7 @@ import ( "time" "github.com/kopia/kopia/fs" + "github.com/kopia/kopia/fs/virtualfs" "github.com/kopia/kopia/repo" "github.com/kopia/kopia/repo/manifest" "github.com/kopia/kopia/snapshot" @@ -594,11 +595,11 @@ func TestBackup(t *testing.T) { expectedError: errors.New("Unable to read dir"), }, { - name: "Unable to handle block mode", + name: "Source path is not a block device", sourcePath: "/", tags: nil, volMode: uploader.PersistentVolumeBlock, - expectedError: errors.New("unable to handle block storage"), + expectedError: errors.New("source path / is not a block device"), }, } @@ -660,6 +661,7 @@ func TestRestore(t *testing.T) { expectedBytes int64 expectedCount int32 expectedError error + volMode uploader.PersistentVolumeMode } // Define test cases @@ -697,6 +699,46 @@ func TestRestore(t *testing.T) { snapshotID: "snapshot-123", expectedError: nil, }, + { + name: "Expect block volume successful", + filesystemEntryFunc: func(ctx context.Context, rep repo.Repository, rootID string, consistentAttributes bool) (fs.Entry, error) { + return snapshotfs.EntryFromDirEntry(rep, &snapshot.DirEntry{Type: snapshot.EntryTypeFile}), nil + }, + restoreEntryFunc: func(ctx context.Context, rep repo.Repository, output restore.Output, rootEntry fs.Entry, options restore.Options) (restore.Stats, error) { + return restore.Stats{}, nil + }, + snapshotID: "snapshot-123", + expectedError: nil, + volMode: uploader.PersistentVolumeBlock, + }, + { + name: "Unable to evaluate symlinks for block volume", + filesystemEntryFunc: func(ctx context.Context, rep repo.Repository, rootID string, consistentAttributes bool) (fs.Entry, error) { + return snapshotfs.EntryFromDirEntry(rep, &snapshot.DirEntry{Type: snapshot.EntryTypeFile}), nil + }, + restoreEntryFunc: func(ctx context.Context, rep repo.Repository, output restore.Output, rootEntry fs.Entry, options restore.Options) (restore.Stats, error) { + err := output.BeginDirectory(ctx, "fake-dir", virtualfs.NewStaticDirectory("fake-dir", nil)) + return restore.Stats{}, err + }, + snapshotID: "snapshot-123", + expectedError: errors.New("unable to evaluate symlinks for"), + volMode: uploader.PersistentVolumeBlock, + dest: "/wrong-dest", + }, + { + name: "Target file is not a block device", + filesystemEntryFunc: func(ctx context.Context, rep repo.Repository, rootID string, consistentAttributes bool) (fs.Entry, error) { + return snapshotfs.EntryFromDirEntry(rep, &snapshot.DirEntry{Type: snapshot.EntryTypeFile}), nil + }, + restoreEntryFunc: func(ctx context.Context, rep repo.Repository, output restore.Output, rootEntry fs.Entry, options restore.Options) (restore.Stats, error) { + err := output.BeginDirectory(ctx, "fake-dir", virtualfs.NewStaticDirectory("fake-dir", nil)) + return restore.Stats{}, err + }, + snapshotID: "snapshot-123", + expectedError: errors.New("target file /tmp is not a block device"), + volMode: uploader.PersistentVolumeBlock, + dest: "/tmp", + }, } em := &manifest.EntryMetadata{ @@ -706,6 +748,10 @@ func TestRestore(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + if tc.volMode == "" { + tc.volMode = uploader.PersistentVolumeFilesystem + } + if tc.invalidManifestType { em.Labels[manifest.TypeLabelKey] = "" } else { @@ -725,7 +771,7 @@ func TestRestore(t *testing.T) { repoWriterMock.On("OpenObject", mock.Anything, mock.Anything).Return(em, nil) progress := new(Progress) - bytesRestored, fileCount, err := Restore(context.Background(), repoWriterMock, progress, tc.snapshotID, tc.dest, logrus.New(), nil) + bytesRestored, fileCount, err := Restore(context.Background(), repoWriterMock, progress, tc.snapshotID, tc.dest, tc.volMode, logrus.New(), nil) // Check if the returned error matches the expected error if tc.expectedError != nil { diff --git a/pkg/uploader/provider/kopia.go b/pkg/uploader/provider/kopia.go index dd32173b82f..706393362d6 100644 --- a/pkg/uploader/provider/kopia.go +++ b/pkg/uploader/provider/kopia.go @@ -128,11 +128,6 @@ func (kp *kopiaProvider) RunBackup( return "", false, errors.New("path is empty") } - // For now, error on block mode - if volMode == uploader.PersistentVolumeBlock { - return "", false, errors.New("unable to currently support block mode") - } - log := kp.log.WithFields(logrus.Fields{ "path": path, "realSource": realSource, @@ -214,10 +209,6 @@ func (kp *kopiaProvider) RunRestore( "volumePath": volumePath, }) - if volMode == uploader.PersistentVolumeBlock { - return errors.New("unable to currently support block mode") - } - repoWriter := kopia.NewShimRepo(kp.bkRepo) progress := new(kopia.Progress) progress.InitThrottle(restoreProgressCheckInterval) @@ -235,7 +226,7 @@ func (kp *kopiaProvider) RunRestore( // We use the cancel channel to control the restore cancel, so don't pass a context with cancel to Kopia restore. // Otherwise, Kopia restore will not response to the cancel control but return an arbitrary error. // Kopia restore cancel is not designed as well as Kopia backup which uses the context to control backup cancel all the way. - size, fileCount, err := RestoreFunc(context.Background(), repoWriter, progress, snapshotID, volumePath, log, restoreCancel) + size, fileCount, err := RestoreFunc(context.Background(), repoWriter, progress, snapshotID, volumePath, volMode, log, restoreCancel) if err != nil { return errors.Wrapf(err, "Failed to run kopia restore") diff --git a/pkg/uploader/provider/kopia_test.go b/pkg/uploader/provider/kopia_test.go index 944cdbcceb8..c38d370ce35 100644 --- a/pkg/uploader/provider/kopia_test.go +++ b/pkg/uploader/provider/kopia_test.go @@ -94,12 +94,12 @@ func TestRunBackup(t *testing.T) { notError: false, }, { - name: "error on vol mode", + name: "success to backup block mode volume", hookBackupFunc: func(ctx context.Context, fsUploader kopia.SnapshotUploader, repoWriter repo.RepositoryWriter, sourcePath string, realSource string, forceFull bool, parentSnapshot string, volMode uploader.PersistentVolumeMode, tags map[string]string, log logrus.FieldLogger) (*uploader.SnapshotInfo, bool, error) { - return nil, true, nil + return &uploader.SnapshotInfo{}, false, nil }, volMode: uploader.PersistentVolumeBlock, - notError: false, + notError: true, }, } for _, tc := range testCases { @@ -125,31 +125,31 @@ func TestRunRestore(t *testing.T) { testCases := []struct { name string - hookRestoreFunc func(ctx context.Context, rep repo.RepositoryWriter, progress *kopia.Progress, snapshotID, dest string, log logrus.FieldLogger, cancleCh chan struct{}) (int64, int32, error) + hookRestoreFunc func(ctx context.Context, rep repo.RepositoryWriter, progress *kopia.Progress, snapshotID, dest string, volMode uploader.PersistentVolumeMode, log logrus.FieldLogger, cancleCh chan struct{}) (int64, int32, error) notError bool volMode uploader.PersistentVolumeMode }{ { name: "normal restore", - hookRestoreFunc: func(ctx context.Context, rep repo.RepositoryWriter, progress *kopia.Progress, snapshotID, dest string, log logrus.FieldLogger, cancleCh chan struct{}) (int64, int32, error) { + hookRestoreFunc: func(ctx context.Context, rep repo.RepositoryWriter, progress *kopia.Progress, snapshotID, dest string, volMode uploader.PersistentVolumeMode, log logrus.FieldLogger, cancleCh chan struct{}) (int64, int32, error) { return 0, 0, nil }, notError: true, }, { name: "failed to restore", - hookRestoreFunc: func(ctx context.Context, rep repo.RepositoryWriter, progress *kopia.Progress, snapshotID, dest string, log logrus.FieldLogger, cancleCh chan struct{}) (int64, int32, error) { + hookRestoreFunc: func(ctx context.Context, rep repo.RepositoryWriter, progress *kopia.Progress, snapshotID, dest string, volMode uploader.PersistentVolumeMode, log logrus.FieldLogger, cancleCh chan struct{}) (int64, int32, error) { return 0, 0, errors.New("failed to restore") }, notError: false, }, { - name: "failed to restore block mode", - hookRestoreFunc: func(ctx context.Context, rep repo.RepositoryWriter, progress *kopia.Progress, snapshotID, dest string, log logrus.FieldLogger, cancleCh chan struct{}) (int64, int32, error) { - return 0, 0, errors.New("failed to restore") + name: "normal block mode restore", + hookRestoreFunc: func(ctx context.Context, rep repo.RepositoryWriter, progress *kopia.Progress, snapshotID, dest string, volMode uploader.PersistentVolumeMode, log logrus.FieldLogger, cancleCh chan struct{}) (int64, int32, error) { + return 0, 0, nil }, volMode: uploader.PersistentVolumeBlock, - notError: false, + notError: true, }, } diff --git a/pkg/uploader/provider/restic_test.go b/pkg/uploader/provider/restic_test.go index f2203d7bdd8..62f44d04f3f 100644 --- a/pkg/uploader/provider/restic_test.go +++ b/pkg/uploader/provider/restic_test.go @@ -212,6 +212,9 @@ func TestResticRunRestore(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + if tc.volMode == "" { + tc.volMode = uploader.PersistentVolumeFilesystem + } resticRestoreCMDFunc = tc.hookResticRestoreFunc if tc.volMode == "" { tc.volMode = uploader.PersistentVolumeFilesystem diff --git a/pkg/util/kube/pvc_pv.go b/pkg/util/kube/pvc_pv.go index 2af818d909f..1a393b3ab28 100644 --- a/pkg/util/kube/pvc_pv.go +++ b/pkg/util/kube/pvc_pv.go @@ -316,3 +316,23 @@ func WaitPVBound(ctx context.Context, pvGetter corev1client.CoreV1Interface, pvN func IsPVCBound(pvc *corev1api.PersistentVolumeClaim) bool { return pvc.Spec.VolumeName != "" } + +// MakePodPVCAttachment returns the volume mounts and devices for a pod needed to attach a PVC +func MakePodPVCAttachment(volumeName string, volumeMode *corev1api.PersistentVolumeMode) ([]corev1api.VolumeMount, []corev1api.VolumeDevice) { + var volumeMounts []corev1api.VolumeMount = nil + var volumeDevices []corev1api.VolumeDevice = nil + + if volumeMode != nil && *volumeMode == corev1api.PersistentVolumeBlock { + volumeDevices = []corev1api.VolumeDevice{{ + Name: volumeName, + DevicePath: "/" + volumeName, + }} + } else { + volumeMounts = []corev1api.VolumeMount{{ + Name: volumeName, + MountPath: "/" + volumeName, + }} + } + + return volumeMounts, volumeDevices +} diff --git a/pkg/util/kube/utils.go b/pkg/util/kube/utils.go index 3d8d4e3ef1c..e1ed48dba28 100644 --- a/pkg/util/kube/utils.go +++ b/pkg/util/kube/utils.go @@ -35,6 +35,7 @@ import ( corev1client "k8s.io/client-go/kubernetes/typed/core/v1" "sigs.k8s.io/controller-runtime/pkg/client" + "github.com/vmware-tanzu/velero/pkg/uploader" "github.com/vmware-tanzu/velero/pkg/util/filesystem" ) @@ -50,6 +51,8 @@ const ( KubeAnnSelectedNode = "volume.kubernetes.io/selected-node" ) +var ErrorPodVolumeIsNotPVC = errors.New("pod volume is not a PVC") + // NamespaceAndName returns a string in the format / func NamespaceAndName(objMeta metav1.Object) string { if objMeta.GetNamespace() == "" { @@ -122,6 +125,57 @@ func EnsureNamespaceExistsAndIsReady(namespace *corev1api.Namespace, client core // where the specified volume lives. // For volumes with a CSIVolumeSource, append "/mount" to the directory name. func GetVolumeDirectory(ctx context.Context, log logrus.FieldLogger, pod *corev1api.Pod, volumeName string, cli client.Client) (string, error) { + pvc, pv, volume, err := GetPodPVCVolume(ctx, log, pod, volumeName, cli) + if err != nil { + // This case implies the administrator created the PV and attached it directly, without PVC. + // Note that only one VolumeSource can be populated per Volume on a pod + if err == ErrorPodVolumeIsNotPVC { + if volume.VolumeSource.CSI != nil { + return volume.Name + "/mount", nil + } + return volume.Name, nil + } + return "", errors.WithStack(err) + } + + // Most common case is that we have a PVC VolumeSource, and we need to check the PV it points to for a CSI source. + // PV's been created with a CSI source. + isProvisionedByCSI, err := isProvisionedByCSI(log, pv, cli) + if err != nil { + return "", errors.WithStack(err) + } + if isProvisionedByCSI { + if pv.Spec.VolumeMode != nil && *pv.Spec.VolumeMode == corev1api.PersistentVolumeBlock { + return pvc.Spec.VolumeName, nil + } + return pvc.Spec.VolumeName + "/mount", nil + } + + return pvc.Spec.VolumeName, nil +} + +// GetVolumeMode gets the uploader.PersistentVolumeMode of the volume. +func GetVolumeMode(ctx context.Context, log logrus.FieldLogger, pod *corev1api.Pod, volumeName string, cli client.Client) ( + uploader.PersistentVolumeMode, error) { + _, pv, _, err := GetPodPVCVolume(ctx, log, pod, volumeName, cli) + + if err != nil { + if err == ErrorPodVolumeIsNotPVC { + return uploader.PersistentVolumeFilesystem, nil + } + return "", errors.WithStack(err) + } + + if pv.Spec.VolumeMode != nil && *pv.Spec.VolumeMode == corev1api.PersistentVolumeBlock { + return uploader.PersistentVolumeBlock, nil + } + return uploader.PersistentVolumeFilesystem, nil +} + +// GetPodPVCVolume gets the PVC, PV and volume for a pod volume name. +// Returns pod volume in case of ErrorPodVolumeIsNotPVC error +func GetPodPVCVolume(ctx context.Context, log logrus.FieldLogger, pod *corev1api.Pod, volumeName string, cli client.Client) ( + *corev1api.PersistentVolumeClaim, *corev1api.PersistentVolume, *corev1api.Volume, error) { var volume *corev1api.Volume for i := range pod.Spec.Volumes { @@ -132,41 +186,26 @@ func GetVolumeDirectory(ctx context.Context, log logrus.FieldLogger, pod *corev1 } if volume == nil { - return "", errors.New("volume not found in pod") + return nil, nil, nil, errors.New("volume not found in pod") } - // This case implies the administrator created the PV and attached it directly, without PVC. - // Note that only one VolumeSource can be populated per Volume on a pod if volume.VolumeSource.PersistentVolumeClaim == nil { - if volume.VolumeSource.CSI != nil { - return volume.Name + "/mount", nil - } - return volume.Name, nil + return nil, nil, volume, ErrorPodVolumeIsNotPVC // There is a pod volume but it is not a PVC } - // Most common case is that we have a PVC VolumeSource, and we need to check the PV it points to for a CSI source. pvc := &corev1api.PersistentVolumeClaim{} err := cli.Get(ctx, client.ObjectKey{Namespace: pod.Namespace, Name: volume.VolumeSource.PersistentVolumeClaim.ClaimName}, pvc) if err != nil { - return "", errors.WithStack(err) + return nil, nil, nil, errors.WithStack(err) } pv := &corev1api.PersistentVolume{} err = cli.Get(ctx, client.ObjectKey{Name: pvc.Spec.VolumeName}, pv) if err != nil { - return "", errors.WithStack(err) + return nil, nil, nil, errors.WithStack(err) } - // PV's been created with a CSI source. - isProvisionedByCSI, err := isProvisionedByCSI(log, pv, cli) - if err != nil { - return "", errors.WithStack(err) - } - if isProvisionedByCSI { - return pvc.Spec.VolumeName + "/mount", nil - } - - return pvc.Spec.VolumeName, nil + return pvc, pv, volume, nil } // isProvisionedByCSI function checks whether this is a CSI PV by annotation. diff --git a/pkg/util/kube/utils_test.go b/pkg/util/kube/utils_test.go index 6edf0843513..31110dc8c03 100644 --- a/pkg/util/kube/utils_test.go +++ b/pkg/util/kube/utils_test.go @@ -38,6 +38,7 @@ import ( "github.com/vmware-tanzu/velero/pkg/builder" velerotest "github.com/vmware-tanzu/velero/pkg/test" + "github.com/vmware-tanzu/velero/pkg/uploader" ) func TestNamespaceAndName(t *testing.T) { @@ -164,6 +165,13 @@ func TestGetVolumeDirectorySuccess(t *testing.T) { pv: builder.ForPersistentVolume("a-pv").CSI("csi.test.com", "provider-volume-id").Result(), want: "a-pv/mount", }, + { + name: "Block CSI volume with a PVC/PV does not append '/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").CSI("csi.test.com", "provider-volume-id").VolumeMode(corev1.PersistentVolumeBlock).Result(), + want: "a-pv", + }, { name: "CSI volume mounted without a PVC appends '/mount' to the volume name", pod: builder.ForPod("ns-1", "my-pod").Volumes(builder.ForVolume("my-vol").CSISource("csi.test.com").Result()).Result(), @@ -211,6 +219,54 @@ func TestGetVolumeDirectorySuccess(t *testing.T) { } } +// TestGetVolumeModeSuccess tests the GetVolumeMode function +func TestGetVolumeModeSuccess(t *testing.T) { + tests := []struct { + name string + pod *corev1.Pod + pvc *corev1.PersistentVolumeClaim + pv *corev1.PersistentVolume + want uploader.PersistentVolumeMode + }{ + { + name: "Filesystem PVC volume", + 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").VolumeMode(corev1.PersistentVolumeFilesystem).Result(), + want: uploader.PersistentVolumeFilesystem, + }, + { + name: "Block PVC volume", + 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").VolumeMode(corev1.PersistentVolumeBlock).Result(), + want: uploader.PersistentVolumeBlock, + }, + { + name: "Pod volume without a PVC", + pod: builder.ForPod("ns-1", "my-pod").Volumes(builder.ForVolume("my-vol").Result()).Result(), + want: uploader.PersistentVolumeFilesystem, + }, + } + + for _, tc := range tests { + clientBuilder := fake.NewClientBuilder() + + if tc.pvc != nil { + clientBuilder = clientBuilder.WithObjects(tc.pvc) + } + if tc.pv != nil { + clientBuilder = clientBuilder.WithObjects(tc.pv) + } + + // Function under test + mode, err := GetVolumeMode(context.Background(), logrus.StandardLogger(), tc.pod, tc.pod.Spec.Volumes[0].Name, clientBuilder.Build()) + + require.NoError(t, err) + assert.Equal(t, tc.want, mode) + } +} + func TestIsV1Beta1CRDReady(t *testing.T) { tests := []struct { name string diff --git a/site/content/docs/main/csi-snapshot-data-movement.md b/site/content/docs/main/csi-snapshot-data-movement.md index 667be0a48c6..b694da9f3a3 100644 --- a/site/content/docs/main/csi-snapshot-data-movement.md +++ b/site/content/docs/main/csi-snapshot-data-movement.md @@ -75,24 +75,10 @@ To mount the correct hostpath to pods volumes, run the node-agent pod in `privil oc adm policy add-scc-to-user privileged -z velero -n velero ``` -2. Modify the DaemonSet yaml to request a privileged mode: - - ```diff - @@ -67,3 +67,5 @@ spec: - value: /credentials/cloud - - name: VELERO_SCRATCH_DIR - value: /scratch - + securityContext: - + privileged: true +2. Install Velero with the '--privileged-node-agent' option to request a privileged mode: + ``` - - or - - ```shell - oc patch ds/node-agent \ - --namespace velero \ - --type json \ - -p '[{"op":"add","path":"/spec/template/spec/containers/0/securityContext","value": { "privileged": true}}]' + velero install --use-node-agent --privileged-node-agent ``` If node-agent is not running in a privileged mode, it will not be able to access snapshot volumes within the mounted diff --git a/site/content/docs/main/customize-installation.md b/site/content/docs/main/customize-installation.md index 0d8621685d7..a4e2299c8a4 100644 --- a/site/content/docs/main/customize-installation.md +++ b/site/content/docs/main/customize-installation.md @@ -23,6 +23,14 @@ By default, `velero install` does not install Velero's [File System Backup][3]. If you've already run `velero install` without the `--use-node-agent` flag, you can run the same command again, including the `--use-node-agent` flag, to add the file system backup to your existing install. +## CSI Snapshot Data Movement + +Velero node-agent is required by CSI snapshot data movement when Velero built-in data mover is used. By default, `velero install` does not install Velero's node-agent. To enable it, specify the `--use-node-agent` flag. + +For some use cases, Velero node-agent requires to run under privileged mode. For example, when backing up block volumes, it is required to allow the node-agent to access the block device. To enable it set velero install flags `--privileged-node-agent`. + +If you've already run `velero install` without the `--use-node-agent` or `--privileged-node-agent` flag, you can run the same command again, including the `--use-node-agent` or `--privileged-node-agent` flag, to add CSI snapshot data movement to your existing install. + ## Default Pod Volume backup to file system backup By default, `velero install` does not enable the use of File System Backup (FSB) to take backups of all pod volumes. You must apply an [annotation](file-system-backup.md/#using-opt-in-pod-volume-backup) to every pod which contains volumes for Velero to use FSB for the backup. diff --git a/site/content/docs/main/file-system-backup.md b/site/content/docs/main/file-system-backup.md index 108daede441..30021806be9 100644 --- a/site/content/docs/main/file-system-backup.md +++ b/site/content/docs/main/file-system-backup.md @@ -111,24 +111,10 @@ To mount the correct hostpath to pods volumes, run the node-agent pod in `privil oc adm policy add-scc-to-user privileged -z velero -n velero ``` -2. Modify the DaemonSet yaml to request a privileged mode: - - ```diff - @@ -67,3 +67,5 @@ spec: - value: /credentials/cloud - - name: VELERO_SCRATCH_DIR - value: /scratch - + securityContext: - + privileged: true +2. Install Velero with the '--privileged-node-agent' option to request a privileged mode: + ``` - - or - - ```shell - oc patch ds/node-agent \ - --namespace velero \ - --type json \ - -p '[{"op":"add","path":"/spec/template/spec/containers/0/securityContext","value": { "privileged": true}}]' + velero install --use-node-agent --privileged-node-agent ``` diff --git a/tilt-resources/examples/node-agent.yaml b/tilt-resources/examples/node-agent.yaml index d5c10fc47ee..835ba297ff1 100644 --- a/tilt-resources/examples/node-agent.yaml +++ b/tilt-resources/examples/node-agent.yaml @@ -49,6 +49,9 @@ spec: - mountPath: /host_pods mountPropagation: HostToContainer name: host-pods + - mountPath: /var/lib/kubelet/plugins + mountPropagation: HostToContainer + name: host-plugins - mountPath: /scratch name: scratch - mountPath: /credentials @@ -60,6 +63,9 @@ spec: - hostPath: path: /var/lib/kubelet/pods name: host-pods + - hostPath: + path: /var/lib/kubelet/plugins + name: host-plugins - emptyDir: {} name: scratch - name: cloud-credentials From 11745809c444123c4d5f0394824140e0961587fe Mon Sep 17 00:00:00 2001 From: yanggang Date: Tue, 19 Sep 2023 19:28:27 +0800 Subject: [PATCH 05/10] Add missing file licences and do some clean works. Signed-off-by: yanggang --- .../volume-snapshot-data-movement.md | 2 +- internal/resourcemodifiers/resource_modifiers.go | 15 +++++++++++++++ .../resourcemodifiers/resource_modifiers_test.go | 15 +++++++++++++++ .../resource_modifiers_validator.go | 15 +++++++++++++++ .../resource_modifiers_validator_test.go | 15 +++++++++++++++ internal/resourcepolicies/resource_policies.go | 15 +++++++++++++++ .../resourcepolicies/resource_policies_test.go | 15 +++++++++++++++ internal/resourcepolicies/volume_resources.go | 15 +++++++++++++++ .../resourcepolicies/volume_resources_test.go | 15 +++++++++++++++ .../volume_resources_validator.go | 15 +++++++++++++++ .../volume_resources_validator_test.go | 15 +++++++++++++++ pkg/apis/velero/v2alpha1/data_download_types.go | 2 +- pkg/controller/download_request_controller.go | 4 ++-- 13 files changed, 154 insertions(+), 4 deletions(-) diff --git a/design/volume-snapshot-data-movement/volume-snapshot-data-movement.md b/design/volume-snapshot-data-movement/volume-snapshot-data-movement.md index 6a88b8b48e4..674284433c2 100644 --- a/design/volume-snapshot-data-movement/volume-snapshot-data-movement.md +++ b/design/volume-snapshot-data-movement/volume-snapshot-data-movement.md @@ -397,7 +397,7 @@ Target volume information includes PVC and PV that represents the volume and the The data mover information and backup repository information are the same with DataUpload CRD. DataDownload CRD defines the same status as DataUpload CRD with nearly the same meanings. -Below is the full spec of DataUpload CRD: +Below is the full spec of DataDownload CRD: ``` apiVersion: apiextensions.k8s.io/v1alpha1 kind: CustomResourceDefinition diff --git a/internal/resourcemodifiers/resource_modifiers.go b/internal/resourcemodifiers/resource_modifiers.go index dbcd8e7ba42..b5d3047ef6d 100644 --- a/internal/resourcemodifiers/resource_modifiers.go +++ b/internal/resourcemodifiers/resource_modifiers.go @@ -1,3 +1,18 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ package resourcemodifiers import ( diff --git a/internal/resourcemodifiers/resource_modifiers_test.go b/internal/resourcemodifiers/resource_modifiers_test.go index c2150bbc871..a8e61dae4f2 100644 --- a/internal/resourcemodifiers/resource_modifiers_test.go +++ b/internal/resourcemodifiers/resource_modifiers_test.go @@ -1,3 +1,18 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ package resourcemodifiers import ( diff --git a/internal/resourcemodifiers/resource_modifiers_validator.go b/internal/resourcemodifiers/resource_modifiers_validator.go index 24fe0dbb2e5..aca3fe923af 100644 --- a/internal/resourcemodifiers/resource_modifiers_validator.go +++ b/internal/resourcemodifiers/resource_modifiers_validator.go @@ -1,3 +1,18 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ package resourcemodifiers import ( diff --git a/internal/resourcemodifiers/resource_modifiers_validator_test.go b/internal/resourcemodifiers/resource_modifiers_validator_test.go index a46c4653071..4011ba0d2df 100644 --- a/internal/resourcemodifiers/resource_modifiers_validator_test.go +++ b/internal/resourcemodifiers/resource_modifiers_validator_test.go @@ -1,3 +1,18 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ package resourcemodifiers import ( diff --git a/internal/resourcepolicies/resource_policies.go b/internal/resourcepolicies/resource_policies.go index 956a06753c5..0c2c9b8082e 100644 --- a/internal/resourcepolicies/resource_policies.go +++ b/internal/resourcepolicies/resource_policies.go @@ -1,3 +1,18 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ package resourcepolicies import ( diff --git a/internal/resourcepolicies/resource_policies_test.go b/internal/resourcepolicies/resource_policies_test.go index 51b2a4f75f4..005d1e12b65 100644 --- a/internal/resourcepolicies/resource_policies_test.go +++ b/internal/resourcepolicies/resource_policies_test.go @@ -1,3 +1,18 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ package resourcepolicies import ( diff --git a/internal/resourcepolicies/volume_resources.go b/internal/resourcepolicies/volume_resources.go index fc6fdb7d53a..7f281ea5e64 100644 --- a/internal/resourcepolicies/volume_resources.go +++ b/internal/resourcepolicies/volume_resources.go @@ -1,3 +1,18 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ package resourcepolicies import ( diff --git a/internal/resourcepolicies/volume_resources_test.go b/internal/resourcepolicies/volume_resources_test.go index 66d7996699c..c73bb692d8f 100644 --- a/internal/resourcepolicies/volume_resources_test.go +++ b/internal/resourcepolicies/volume_resources_test.go @@ -1,3 +1,18 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ package resourcepolicies import ( diff --git a/internal/resourcepolicies/volume_resources_validator.go b/internal/resourcepolicies/volume_resources_validator.go index 7b4d0b815d4..15aba2f949f 100644 --- a/internal/resourcepolicies/volume_resources_validator.go +++ b/internal/resourcepolicies/volume_resources_validator.go @@ -1,3 +1,18 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ package resourcepolicies import ( diff --git a/internal/resourcepolicies/volume_resources_validator_test.go b/internal/resourcepolicies/volume_resources_validator_test.go index 92fd9a60b58..8518065807e 100644 --- a/internal/resourcepolicies/volume_resources_validator_test.go +++ b/internal/resourcepolicies/volume_resources_validator_test.go @@ -1,3 +1,18 @@ +/* +Copyright The Velero Contributors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ package resourcepolicies import ( diff --git a/pkg/apis/velero/v2alpha1/data_download_types.go b/pkg/apis/velero/v2alpha1/data_download_types.go index 4776a8272eb..bd18a815a3d 100644 --- a/pkg/apis/velero/v2alpha1/data_download_types.go +++ b/pkg/apis/velero/v2alpha1/data_download_types.go @@ -56,7 +56,7 @@ type DataDownloadSpec struct { OperationTimeout metav1.Duration `json:"operationTimeout"` } -// TargetPVCSpec is the specification for a target PVC. +// TargetVolumeSpec is the specification for a target PVC. type TargetVolumeSpec struct { // PVC is the name of the target PVC that is created by Velero restore PVC string `json:"pvc"` diff --git a/pkg/controller/download_request_controller.go b/pkg/controller/download_request_controller.go index 1f3de3955e6..699b4d164fa 100644 --- a/pkg/controller/download_request_controller.go +++ b/pkg/controller/download_request_controller.go @@ -144,7 +144,7 @@ func (r *downloadRequestReconciler) Reconcile(ctx context.Context, req ctrl.Requ log.WithError(err).Error("fail to get restore for DownloadRequest") return ctrl.Result{}, nil } - log.Warnf("Fail to get restore for DownloadRequest %s. Retry later.", err.Error()) + log.Warnf("fail to get restore for DownloadRequest %s. Retry later.", err.Error()) return ctrl.Result{}, errors.WithStack(err) } backupName = restore.Spec.BackupName @@ -172,7 +172,7 @@ func (r *downloadRequestReconciler) Reconcile(ctx context.Context, req ctrl.Requ log.Errorf("BSL for DownloadRequest cannot be found") return ctrl.Result{}, nil } - log.Warnf("Fail to get BSL for DownloadRequest: %s", err.Error()) + log.Warnf("fail to get BSL for DownloadRequest: %s", err.Error()) return ctrl.Result{}, errors.WithStack(err) } From 0d79afe049983dc883f80e79551ed0afb86450ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Wenkai=20Yin=28=E5=B0=B9=E6=96=87=E5=BC=80=29?= Date: Sat, 30 Sep 2023 00:19:51 +0800 Subject: [PATCH 06/10] Replace the base image with paketobuildpacks image (#6883) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the base image with paketobuildpacks image Fixes #6851 Signed-off-by: Wenkai Yin(尹文开) --- Dockerfile | 4 ++-- changelogs/unreleased/6883-ywk253100 | 1 + hack/docker-push.sh | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/6883-ywk253100 diff --git a/Dockerfile b/Dockerfile index b853cb89d27..576d7367dcb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -70,7 +70,7 @@ RUN mkdir -p /output/usr/bin && \ go clean -modcache -cache # Velero image packing section -FROM gcr.io/distroless/base-nossl-debian11:nonroot +FROM paketobuildpacks/run-jammy-tiny:latest LABEL maintainer="Xun Jiang " @@ -78,5 +78,5 @@ COPY --from=velero-builder /output / COPY --from=restic-builder /output / -USER nonroot:nonroot +USER cnb:cnb diff --git a/changelogs/unreleased/6883-ywk253100 b/changelogs/unreleased/6883-ywk253100 new file mode 100644 index 00000000000..bc8d80b92ef --- /dev/null +++ b/changelogs/unreleased/6883-ywk253100 @@ -0,0 +1 @@ +Replace the base image with paketobuildpacks image \ No newline at end of file diff --git a/hack/docker-push.sh b/hack/docker-push.sh index d675bffd347..e503358c9f8 100755 --- a/hack/docker-push.sh +++ b/hack/docker-push.sh @@ -89,7 +89,7 @@ else fi if [[ -z "$BUILDX_PLATFORMS" ]]; then - BUILDX_PLATFORMS="linux/amd64,linux/arm64,linux/arm/v7,linux/ppc64le" + BUILDX_PLATFORMS="linux/amd64,linux/arm64" fi # Debugging info From fd67ecb6885af1b2c864620e00eb2cee88412013 Mon Sep 17 00:00:00 2001 From: Yang Gang Date: Fri, 29 Sep 2023 17:23:12 +0100 Subject: [PATCH 07/10] Code clean for backup cmd client. (#6750) Signed-off-by: yanggang --- pkg/cmd/cli/backup/create.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/pkg/cmd/cli/backup/create.go b/pkg/cmd/cli/backup/create.go index c6ef3977808..4925179dc0d 100644 --- a/pkg/cmd/cli/backup/create.go +++ b/pkg/cmd/cli/backup/create.go @@ -174,11 +174,6 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto return fmt.Errorf("either a 'selector' or an 'or-selector' can be specified, but not both") } - client, err := f.KubebuilderWatchClient() - if err != nil { - return err - } - // Ensure that unless FromSchedule is set, args contains a backup name if o.FromSchedule == "" && len(args) != 1 { return fmt.Errorf("a backup name is required, unless you are creating based on a schedule") @@ -197,7 +192,7 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto if o.StorageLocation != "" { location := &velerov1api.BackupStorageLocation{} - if err := client.Get(context.Background(), kbclient.ObjectKey{ + if err := o.client.Get(context.Background(), kbclient.ObjectKey{ Namespace: f.Namespace(), Name: o.StorageLocation, }, location); err != nil { @@ -207,7 +202,7 @@ func (o *CreateOptions) Validate(c *cobra.Command, args []string, f client.Facto for _, loc := range o.SnapshotLocations { snapshotLocation := new(velerov1api.VolumeSnapshotLocation) - if err := o.client.Get(context.TODO(), kbclient.ObjectKey{Namespace: f.Namespace(), Name: loc}, snapshotLocation); err != nil { + if err := o.client.Get(context.Background(), kbclient.ObjectKey{Namespace: f.Namespace(), Name: loc}, snapshotLocation); err != nil { return err } } From c51b5998450bc7c1b79d3b29dd53044c65337cd2 Mon Sep 17 00:00:00 2001 From: Tiger Kaovilai Date: Fri, 29 Sep 2023 13:55:32 -0400 Subject: [PATCH 08/10] Fix code-standards url rendering for `https://developercertificate.org/)` Signed-off-by: Tiger Kaovilai --- site/content/docs/main/code-standards.md | 2 +- site/content/docs/v1.10/code-standards.md | 2 +- site/content/docs/v1.11/code-standards.md | 2 +- site/content/docs/v1.12/code-standards.md | 2 +- site/content/docs/v1.2.0/code-standards.md | 2 +- site/content/docs/v1.3.0/code-standards.md | 2 +- site/content/docs/v1.3.1/code-standards.md | 2 +- site/content/docs/v1.3.2/code-standards.md | 2 +- site/content/docs/v1.4/code-standards.md | 2 +- site/content/docs/v1.7/code-standards.md | 2 +- site/content/docs/v1.9/code-standards.md | 2 +- 11 files changed, 11 insertions(+), 11 deletions(-) diff --git a/site/content/docs/main/code-standards.md b/site/content/docs/main/code-standards.md index c12317981a4..732389cf445 100644 --- a/site/content/docs/main/code-standards.md +++ b/site/content/docs/main/code-standards.md @@ -108,7 +108,7 @@ Signed-off-by: Joe Beda This can easily be done with the `--signoff` option to `git commit`. -By doing this you state that you can certify the following (from https://developercertificate.org/): +By doing this you state that you can certify the following (from [https://developercertificate.org/](https://developercertificate.org/)): ``` Developer Certificate of Origin diff --git a/site/content/docs/v1.10/code-standards.md b/site/content/docs/v1.10/code-standards.md index 00b9e751251..256ddbcfa8f 100644 --- a/site/content/docs/v1.10/code-standards.md +++ b/site/content/docs/v1.10/code-standards.md @@ -108,7 +108,7 @@ Signed-off-by: Joe Beda This can easily be done with the `--signoff` option to `git commit`. -By doing this you state that you can certify the following (from https://developercertificate.org/): +By doing this you state that you can certify the following (from [https://developercertificate.org/](https://developercertificate.org/)): ``` Developer Certificate of Origin diff --git a/site/content/docs/v1.11/code-standards.md b/site/content/docs/v1.11/code-standards.md index 582d92902f8..e3f7c127a79 100644 --- a/site/content/docs/v1.11/code-standards.md +++ b/site/content/docs/v1.11/code-standards.md @@ -108,7 +108,7 @@ Signed-off-by: Joe Beda This can easily be done with the `--signoff` option to `git commit`. -By doing this you state that you can certify the following (from https://developercertificate.org/): +By doing this you state that you can certify the following (from [https://developercertificate.org/](https://developercertificate.org/)): ``` Developer Certificate of Origin diff --git a/site/content/docs/v1.12/code-standards.md b/site/content/docs/v1.12/code-standards.md index d4d41e2e9e2..3370354db5a 100644 --- a/site/content/docs/v1.12/code-standards.md +++ b/site/content/docs/v1.12/code-standards.md @@ -108,7 +108,7 @@ Signed-off-by: Joe Beda This can easily be done with the `--signoff` option to `git commit`. -By doing this you state that you can certify the following (from https://developercertificate.org/): +By doing this you state that you can certify the following (from [https://developercertificate.org/](https://developercertificate.org/)): ``` Developer Certificate of Origin diff --git a/site/content/docs/v1.2.0/code-standards.md b/site/content/docs/v1.2.0/code-standards.md index 7b57ae0ad63..d5258154625 100644 --- a/site/content/docs/v1.2.0/code-standards.md +++ b/site/content/docs/v1.2.0/code-standards.md @@ -83,7 +83,7 @@ Signed-off-by: Joe Beda This can easily be done with the `--signoff` option to `git commit`. -By doing this you state that you can certify the following (from https://developercertificate.org/): +By doing this you state that you can certify the following (from [https://developercertificate.org/](https://developercertificate.org/)): ``` Developer Certificate of Origin diff --git a/site/content/docs/v1.3.0/code-standards.md b/site/content/docs/v1.3.0/code-standards.md index ed9c12fee6c..bd5b6a327e3 100644 --- a/site/content/docs/v1.3.0/code-standards.md +++ b/site/content/docs/v1.3.0/code-standards.md @@ -83,7 +83,7 @@ Signed-off-by: Joe Beda This can easily be done with the `--signoff` option to `git commit`. -By doing this you state that you can certify the following (from https://developercertificate.org/): +By doing this you state that you can certify the following (from [https://developercertificate.org/](https://developercertificate.org/)): ``` Developer Certificate of Origin diff --git a/site/content/docs/v1.3.1/code-standards.md b/site/content/docs/v1.3.1/code-standards.md index f76152a1589..6c9a1e81efc 100644 --- a/site/content/docs/v1.3.1/code-standards.md +++ b/site/content/docs/v1.3.1/code-standards.md @@ -82,7 +82,7 @@ Signed-off-by: Joe Beda This can easily be done with the `--signoff` option to `git commit`. -By doing this you state that you can certify the following (from https://developercertificate.org/): +By doing this you state that you can certify the following (from [https://developercertificate.org/](https://developercertificate.org/)): ``` Developer Certificate of Origin diff --git a/site/content/docs/v1.3.2/code-standards.md b/site/content/docs/v1.3.2/code-standards.md index 0987630f37d..acbeae2511b 100644 --- a/site/content/docs/v1.3.2/code-standards.md +++ b/site/content/docs/v1.3.2/code-standards.md @@ -83,7 +83,7 @@ Signed-off-by: Joe Beda This can easily be done with the `--signoff` option to `git commit`. -By doing this you state that you can certify the following (from https://developercertificate.org/): +By doing this you state that you can certify the following (from [https://developercertificate.org/](https://developercertificate.org/)): ``` Developer Certificate of Origin diff --git a/site/content/docs/v1.4/code-standards.md b/site/content/docs/v1.4/code-standards.md index 8ad95ae84f1..9c3543cae52 100644 --- a/site/content/docs/v1.4/code-standards.md +++ b/site/content/docs/v1.4/code-standards.md @@ -101,7 +101,7 @@ Signed-off-by: Joe Beda This can easily be done with the `--signoff` option to `git commit`. -By doing this you state that you can certify the following (from https://developercertificate.org/): +By doing this you state that you can certify the following (from [https://developercertificate.org/](https://developercertificate.org/)): ``` Developer Certificate of Origin diff --git a/site/content/docs/v1.7/code-standards.md b/site/content/docs/v1.7/code-standards.md index 80dbbe81334..978107101e0 100644 --- a/site/content/docs/v1.7/code-standards.md +++ b/site/content/docs/v1.7/code-standards.md @@ -108,7 +108,7 @@ Signed-off-by: Joe Beda This can easily be done with the `--signoff` option to `git commit`. -By doing this you state that you can certify the following (from https://developercertificate.org/): +By doing this you state that you can certify the following (from [https://developercertificate.org/](https://developercertificate.org/)): ``` Developer Certificate of Origin diff --git a/site/content/docs/v1.9/code-standards.md b/site/content/docs/v1.9/code-standards.md index 3d3ea252b73..ed82aaf1582 100644 --- a/site/content/docs/v1.9/code-standards.md +++ b/site/content/docs/v1.9/code-standards.md @@ -108,7 +108,7 @@ Signed-off-by: Joe Beda This can easily be done with the `--signoff` option to `git commit`. -By doing this you state that you can certify the following (from https://developercertificate.org/): +By doing this you state that you can certify the following (from [https://developercertificate.org/](https://developercertificate.org/)): ``` Developer Certificate of Origin From 13019b943a34a32799a6b8fec2f7bedcec46f2be Mon Sep 17 00:00:00 2001 From: Raghuram Devarakonda Date: Mon, 2 Oct 2023 09:57:11 -0400 Subject: [PATCH 09/10] Document pod volume host path setting for Nutanix. (#6902) Signed-off-by: Raghuram Devarakonda --- .../docs/main/csi-snapshot-data-movement.md | 18 +++++++++++++++++- site/content/docs/main/file-system-backup.md | 19 ++++++++++++++++++- .../docs/v1.12/csi-snapshot-data-movement.md | 17 ++++++++++++++++- site/content/docs/v1.12/file-system-backup.md | 19 ++++++++++++++++++- 4 files changed, 69 insertions(+), 4 deletions(-) diff --git a/site/content/docs/main/csi-snapshot-data-movement.md b/site/content/docs/main/csi-snapshot-data-movement.md index b694da9f3a3..7a68821ab7a 100644 --- a/site/content/docs/main/csi-snapshot-data-movement.md +++ b/site/content/docs/main/csi-snapshot-data-movement.md @@ -41,7 +41,7 @@ velero install --use-node-agent ### Configure Node Agent DaemonSet spec After installation, some PaaS/CaaS platforms based on Kubernetes also require modifications the node-agent DaemonSet spec. -The steps in this section are only needed if you are installing on RancherOS, OpenShift, VMware Tanzu Kubernetes Grid +The steps in this section are only needed if you are installing on RancherOS, Nutanix, OpenShift, VMware Tanzu Kubernetes Grid Integrated Edition (formerly VMware Enterprise PKS), or Microsoft Azure. @@ -63,6 +63,22 @@ hostPath: path: /opt/rke/var/lib/kubelet/pods ``` +**Nutanix** + +Update the host path for volumes in the node-agent DaemonSet in the Velero namespace from `/var/lib/kubelet/pods` to +`/var/nutanix/var/lib/kubelet`. + +```yaml +hostPath: + path: /var/lib/kubelet/pods +``` + +to + +```yaml +hostPath: + path: /var/nutanix/var/lib/kubelet +``` **OpenShift** diff --git a/site/content/docs/main/file-system-backup.md b/site/content/docs/main/file-system-backup.md index 30021806be9..9fe6e97aee6 100644 --- a/site/content/docs/main/file-system-backup.md +++ b/site/content/docs/main/file-system-backup.md @@ -77,7 +77,7 @@ backup which created the backup repository, then Velero will not be able to conn ### Configure Node Agent DaemonSet spec After installation, some PaaS/CaaS platforms based on Kubernetes also require modifications the node-agent DaemonSet spec. -The steps in this section are only needed if you are installing on RancherOS, OpenShift, VMware Tanzu Kubernetes Grid +The steps in this section are only needed if you are installing on RancherOS, Nutanix, OpenShift, VMware Tanzu Kubernetes Grid Integrated Edition (formerly VMware Enterprise PKS), or Microsoft Azure. @@ -99,6 +99,23 @@ hostPath: path: /opt/rke/var/lib/kubelet/pods ``` +**Nutanix** + + +Update the host path for volumes in the node-agent DaemonSet in the Velero namespace from `/var/lib/kubelet/pods` to +`/var/nutanix/var/lib/kubelet`. + +```yaml +hostPath: + path: /var/lib/kubelet/pods +``` + +to + +```yaml +hostPath: + path: /var/nutanix/var/lib/kubelet +``` **OpenShift** diff --git a/site/content/docs/v1.12/csi-snapshot-data-movement.md b/site/content/docs/v1.12/csi-snapshot-data-movement.md index 667be0a48c6..1fa4026380f 100644 --- a/site/content/docs/v1.12/csi-snapshot-data-movement.md +++ b/site/content/docs/v1.12/csi-snapshot-data-movement.md @@ -41,7 +41,7 @@ velero install --use-node-agent ### Configure Node Agent DaemonSet spec After installation, some PaaS/CaaS platforms based on Kubernetes also require modifications the node-agent DaemonSet spec. -The steps in this section are only needed if you are installing on RancherOS, OpenShift, VMware Tanzu Kubernetes Grid +The steps in this section are only needed if you are installing on RancherOS, Nutanix, OpenShift, VMware Tanzu Kubernetes Grid Integrated Edition (formerly VMware Enterprise PKS), or Microsoft Azure. @@ -62,7 +62,22 @@ to hostPath: path: /opt/rke/var/lib/kubelet/pods ``` +**Nutanix** +Update the host path for volumes in the node-agent DaemonSet in the Velero namespace from `/var/lib/kubelet/pods` to +`/var/nutanix/var/lib/kubelet`. + +```yaml +hostPath: + path: /var/lib/kubelet/pods +``` + +to + +```yaml +hostPath: + path: /var/nutanix/var/lib/kubelet +``` **OpenShift** diff --git a/site/content/docs/v1.12/file-system-backup.md b/site/content/docs/v1.12/file-system-backup.md index 108daede441..ab87a3befcc 100644 --- a/site/content/docs/v1.12/file-system-backup.md +++ b/site/content/docs/v1.12/file-system-backup.md @@ -77,7 +77,7 @@ backup which created the backup repository, then Velero will not be able to conn ### Configure Node Agent DaemonSet spec After installation, some PaaS/CaaS platforms based on Kubernetes also require modifications the node-agent DaemonSet spec. -The steps in this section are only needed if you are installing on RancherOS, OpenShift, VMware Tanzu Kubernetes Grid +The steps in this section are only needed if you are installing on RancherOS, Nutanix, OpenShift, VMware Tanzu Kubernetes Grid Integrated Edition (formerly VMware Enterprise PKS), or Microsoft Azure. @@ -99,6 +99,23 @@ hostPath: path: /opt/rke/var/lib/kubelet/pods ``` +**Nutanix** + + +Update the host path for volumes in the node-agent DaemonSet in the Velero namespace from `/var/lib/kubelet/pods` to +`/var/nutanix/var/lib/kubelet`. + +```yaml +hostPath: + path: /var/lib/kubelet/pods +``` + +to + +```yaml +hostPath: + path: /var/nutanix/var/lib/kubelet +``` **OpenShift** From 7f73acab1644f7170779b7225a5d7336b20f0868 Mon Sep 17 00:00:00 2001 From: Guang Jiong Lou <7991675+27149chen@users.noreply.github.com> Date: Wed, 4 Oct 2023 11:59:09 +0800 Subject: [PATCH 10/10] Proposal to support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers (#6797) * Proposal to support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers Signed-off-by: lou * add changelog Signed-off-by: lou * add conditional patches Signed-off-by: lou * update design Signed-off-by: lou * update after review Signed-off-by: lou * update after review Signed-off-by: lou --------- Signed-off-by: lou --- changelogs/unreleased/6797-27149chen | 1 + ...atch-and-strategic-in-resource-modifier.md | 193 ++++++++++++++++++ 2 files changed, 194 insertions(+) create mode 100644 changelogs/unreleased/6797-27149chen create mode 100644 design/merge-patch-and-strategic-in-resource-modifier.md diff --git a/changelogs/unreleased/6797-27149chen b/changelogs/unreleased/6797-27149chen new file mode 100644 index 00000000000..463017fc5d5 --- /dev/null +++ b/changelogs/unreleased/6797-27149chen @@ -0,0 +1 @@ +Proposal to support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers \ No newline at end of file diff --git a/design/merge-patch-and-strategic-in-resource-modifier.md b/design/merge-patch-and-strategic-in-resource-modifier.md new file mode 100644 index 00000000000..0259ca64250 --- /dev/null +++ b/design/merge-patch-and-strategic-in-resource-modifier.md @@ -0,0 +1,193 @@ +# Proposal to Support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers + +- [Proposal to Support JSON Merge Patch and Strategic Merge Patch in Resource Modifiers](#proposal-to-support-json-merge-patch-and-strategic-merge-patch-in-resource-modifiers) + - [Abstract](#abstract) + - [Goals](#goals) + - [Non Goals](#non-goals) + - [User Stories](#user-stories) + - [Scenario 1](#scenario-1) + - [Scenario 2](#scenario-2) + - [Detailed Design](#detailed-design) + - [How to choose the right patch type](#how-to-choose-the-right-patch-type) + - [New Field MergePatches](#new-field-mergepatches) + - [New Field StrategicPatches](#new-field-strategicpatches) + - [Conditional Patches in ALL Patch Types](#conditional-patches-in-all-patch-types) + - [Wildcard Support for GroupResource](#wildcard-support-for-groupresource) + - [Helper Command to Generate Merge Patch and Strategic Merge Patch](#helper-command-to-generate-merge-patch-and-strategic-merge-patch) + - [Security Considerations](#security-considerations) + - [Compatibility](#compatibility) + - [Implementation](#implementation) + - [Future Enhancements](#future-enhancements) + - [Open Issues](#open-issues) + +## Abstract +Velero introduced the concept of Resource Modifiers in v1.12.0. This feature allows the user to specify a configmap with a set of rules to modify the resources during restore. The user can specify the filters to select the resources and then specify the JSON Patch to apply on the resource. This feature is currently limited to the operations supported by JSON Patch RFC. +This proposal is to add support for JSON Merge Patch and Strategic Merge Patch in the Resource Modifiers. This will allow the user to use the same configmap to apply JSON Merge Patch and Strategic Merge Patch on the resources during restore. + +## Goals +- Allow the user to specify a JSON patch, JSON Merge Patch or Strategic Merge Patch for modification. +- Allow the user to specify multiple JSON Patch, JSON Merge Patch or Strategic Merge Patch. +- Allow the user to specify mixed JSON Patch, JSON Merge Patch and Strategic Merge Patch in the same configmap. + +## Non Goals +- Deprecating the existing RestoreItemAction plugins for standard substitutions(like changing the namespace, changing the storage class, etc.) + +## User Stories + +### Scenario 1 +- Alice has some Pods and part of them have an annotation `{"for": "bar"}`. +- Alice wishes to restore these Pods to a different cluster without this annotation. +- Alice can use this feature to remove this annotation during restore. + +### Scenario 2 +- Bob has a Pod with several containers and one container with name nginx has an image `repo1/nginx`. +- Bob wishes to restore this Pod to a different cluster, but new cluster can not access repo1, so he pushes the image to repo2. +- Bob can use this feature to update the image of container nginx to `repo2/nginx` during restore. + +## Detailed Design +- The design and approach is inspired by kubectl patch command and [this doc](https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/). +- New fields `MergePatches` and `StrategicPatches` will be added to the `ResourceModifierRule` struct to support all three patch types. +- Only one of the three patch types can be specified in a single `ResourceModifierRule`. +- Add wildcard support for `groupResource` in `conditions` struct. +- The workflow to create Resource Modifier ConfigMap and reference it in RestoreSpec will remain the same as described in document [Resource Modifiers](https://github.com/vmware-tanzu/velero/blob/main/site/content/docs/main/restore-resource-modifiers.md). + +### How to choose the right patch type +- [JSON Merge Patch](https://datatracker.ietf.org/doc/html/rfc7386) is a naively simple format, with limited usability. Probably it is a good choice if you are building something small, with very simple JSON Schema. +- [JSON Patch](https://datatracker.ietf.org/doc/html/rfc6902) is a more complex format, but it is applicable to any JSON documents. For a comparison of JSON patch and JSON merge patch, see [JSON Patch and JSON Merge Patch](https://erosb.github.io/post/json-patch-vs-merge-patch/). +- Strategic Merge Patch is a Kubernetes defined patch type, mainly used to process resources of type list. You can replace/merge a list, add/remove items from a list by key, change the order of items in a list, etc. Strategic merge patch is not supported for custom resources. For more details, see [this doc](https://kubernetes.io/docs/tasks/manage-kubernetes-objects/update-api-object-kubectl-patch/). + +### New Field MergePatches +MergePatches is a list to specify the merge patches to be applied on the resource. The merge patches will be applied in the order specified in the configmap. A subsequent patch is applied in order and if multiple patches are specified for the same path, the last patch will override the previous patches. + +Example of MergePatches in ResourceModifierRule +```yaml +version: v1 +resourceModifierRules: +- conditions: + groupResource: pods + namespaces: + - ns1 + mergePatches: + - patchData: | + { + "metadata": { + "annotations": { + "foo": null + } + } + } +``` +- The above configmap will apply the Merge Patch to all the pods in namespace ns1 and remove the annotation `foo` from the pods. +- Both json and yaml format are supported for the patchData. + +### New Field StrategicPatches +StrategicPatches is a list to specify the strategic merge patches to be applied on the resource. The strategic merge patches will be applied in the order specified in the configmap. A subsequent patch is applied in order and if multiple patches are specified for the same path, the last patch will override the previous patches. + +Example of StrategicPatches in ResourceModifierRule +```yaml +version: v1 +resourceModifierRules: +- conditions: + groupResource: pods + resourceNameRegex: "^my-pod$" + namespaces: + - ns1 + strategicPatches: + - patchData: | + { + "spec": { + "containers": [ + { + "name": "nginx", + "image": "repo2/nginx" + } + ] + } + } +``` +- The above configmap will apply the Strategic Merge Patch to the pod with name my-pod in namespace ns1 and update the image of container nginx to `repo2/nginx`. +- Both json and yaml format are supported for the patchData. + +### Conditional Patches in ALL Patch Types +Since JSON Merge Patch and Strategic Merge Patch do not support conditional patches, we will use the `test` operation of JSON Patch to support conditional patches in all patch types by adding it to `Conditions` struct in `ResourceModifierRule`. + +Example of test in conditions +```yaml +version: v1 +resourceModifierRules: +- conditions: + groupResource: persistentvolumeclaims.storage.k8s.io + matches: + - path: "/spec/storageClassName" + value: "premium" + mergePatches: + - patchData: | + { + "metadata": { + "annotations": { + "foo": null + } + } + } +``` +- The above configmap will apply the Merge Patch to all the PVCs in all namespaces with storageClassName premium and remove the annotation `foo` from the PVCs. +- You can specify multiple rules in the `matches` list. The patch will be applied only if all the matches are satisfied. + +### Wildcard Support for GroupResource +The user can specify a wildcard for groupResource in the conditions' struct. This will allow the user to apply the patches for all the resources of a particular group or all resources in all groups. For example, `*.apps` will apply to all the resources in the `apps` group, `*` will apply to all the resources in all groups. + +### Helper Command to Generate Merge Patch and Strategic Merge Patch +The patchData of Strategic Merge Patch is sometimes a bit complex for user to write. We can provide a helper command to generate the patchData for Strategic Merge Patch. The command will take the original resource and the modified resource as input and generate the patchData. +It can also be used in JSON Merge Patch. + +Here is a sample code snippet to achieve this: +```go +package main + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +func main() { + pod := &corev1.Pod{ + Spec: corev1.PodSpec{ + Containers: []corev1.Container{ + { + Name: "web", + Image: "nginx", + }, + }, + }, + } + newPod := pod.DeepCopy() + patch := client.StrategicMergeFrom(pod) + newPod.Spec.Containers[0].Image = "nginx1" + + data, _ := patch.Data(newPod) + fmt.Println(string(data)) + // Output: + // {"spec":{"$setElementOrder/containers":[{"name":"web"}],"containers":[{"image":"nginx1","name":"web"}]}} +} +``` + +## Security Considerations +No security impact. + +## Compatibility +Compatible with current Resource Modifiers. + +## Implementation +- Use "github.com/evanphx/json-patch" to support JSON Merge Patch. +- Use "k8s.io/apimachinery/pkg/util/strategicpatch" to support Strategic Merge Patch. +- Use glob to support wildcard for `groupResource` in `conditions` struct. +- Use `test` operation of JSON Patch to calculate the `matches` in `conditions` struct. + +## Future enhancements +- add a Velero subcommand to generate/validate the patchData for Strategic Merge Patch and JSON Merge Patch. +- add jq support for more complex conditions or patches, to meet the situations that the current conditions or patches can not handle. like [this issue](https://github.com/vmware-tanzu/velero/issues/6344) + +## Open Issues +N/A