feat(zfspv): adding backup and restore support #162

pawanpraka1 · 2020-06-25T08:21:55Z

Why is this PR required? What issue does it fix?:

This is the PR to handle backup and restore request at ZFS-LocalPV side. There will be changes at the velero plugin side also and the work is in progress for that.

Here velero plugin will create the backup CR to create a backup and ZFS-LocalPV driver will watch for that CR and create the back and transfer the data to the mentioned localtion. In the similar way to restore the data from the remote location, velero plugin will create the restore CR with the remote location information where the data is and ZFS-LocalPV will restore from that location.

How to use?:

Steps to use velero plugin for ZFS-LocalPV are :

install velero

velero install --provider aws --bucket velero --secret-file /home/pawan/pawan/credentials-minio --plugins velero/velero-plugin-for-aws:v1.0.0-beta.1 --backup-location-config region=minio,s3ForcePathStyle="true",s3Url=http://minio.velero.svc:9000 --use-volume-snapshots=true --use-restic

add openebs plugin

velero plugin add openebs/velero-plugin:latest

Create the volumesnapshot location :

for backup :-

apiVersion: velero.io/v1
kind: VolumeSnapshotLocation
metadata:
  name: default
  namespace: velero
spec:
  provider: openebs.io/zfspv-blockstore
  config:
    bucket: velero
    prefix: zfs
    namespace: openebs # this is namespace where ZFS-LocalPV creates all the CRs, passed as OPENEBS_NAMESPACE env in the ZFS-LocalPV deployment
    provider: aws
    region: minio
    s3ForcePathStyle: "true"
    s3Url: http://minio.velero.svc:9000

Create backup

velero backup create my-backup --snapshot-volumes --include-namespaces=velero-ns --volume-snapshot-locations=default --storage-location=default

Create Schedule

velero create schedule newschedule  --schedule="*/1 * * * *" --snapshot-volumes --include-namespaces=velero-ns --volume-snapshot-locations=aws-local-default --storage-location=default

Restore from backup

velero restore create --from-backup my-backup --restore-volumes=true --namespace-mappings velero-ns:ns-2

The PR in the Velero repo is here : openebs/velero-plugin#102

Does this PR require any upgrade changes?:

no

If the changes in this PR are manually verified, list down the scenarios covered and commands you used for testing with logs:

velero full backup and restore
velero schedule(full) backup and restore

apiVersion: velero.io/v1
kind: VolumeSnapshotLocation
metadata:
  name: default
  namespace: velero
spec:
  provider: openebs.io/zfspv-blockstore
  config:
    bucket: velero
    prefix: zfs
    namespace: openebs
    provider: aws
    region: minio
    s3ForcePathStyle: "true"
    s3Url: http://minio.velero.svc:9000

tested by doing backing up the full namespace, deleted it, and restored it and verified the data.

Any additional information for your reviewer?:

added support for full backup
added support for scheduled(full) backup

Limitation:

if restoring in a different cluster, it should have same nodes and same zpool present.

To Do:

incremental backup is supported fully. incremental restore is supported partially(with few limitation). Need to add full support for scheduled(incremental) restore.

Checklist:

Fixes #
PR Title follows the convention of <type>(<scope>): <subject>
Has the change log section been updated?
Commit has unit tests
Commit has integration tests
(Optional) Are upgrade changes included in this PR? If not, mention the issue/PR to track:
(Optional) If documentation changes are required, which issue on https://github.com/openebs/openebs-docs is used to track them:

codecov-commenter · 2020-06-25T08:35:22Z

Codecov Report

Merging #162 into master will decrease coverage by 0.06%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##           master    #162      +/-   ##
=========================================
- Coverage    9.88%   9.82%   -0.07%     
=========================================
  Files          20      20              
  Lines        1163    1171       +8     
=========================================
  Hits          115     115              
- Misses       1047    1055       +8     
  Partials        1       1

Impacted Files	Coverage Δ
pkg/driver/agent.go	`0.00% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5e645b...247cee0. Read the comment docs.

pawanpraka1 · 2020-09-07T06:56:38Z

@kmova I am already working on that part #200.

kmova · 2020-09-07T06:59:03Z

deploy/yamls/zfsbackup-crd.yaml

+                is
+              minLength: 1
+              type: string
+            prevSnapName:


q_: Are there names in sync with the proposed refactoring changes planned in velero plugin?

proposed refactoring changes planned in velero plugin

not aware of this, can you share the proposal link.

kmova · 2020-09-07T07:29:45Z

pkg/driver/agent.go

+	go func() {
+		err := backup.Start(&ControllerMutex, stopCh)
+		if err != nil {
+			klog.Fatalf("Failed to start ZFS volume snapshot management controller: %s", err.Error())


n_: log message can probably say volume snapshot backup controller and the below one as volume snapshot restore controller.

kmova · 2020-09-07T07:35:30Z

pkg/zfs/volume.go

@@ -63,7 +67,7 @@ var (
 func init() {

 	OpenEBSNamespace = os.Getenv(OpenEBSNamespaceKey)
-	if OpenEBSNamespace == "" {
+	if OpenEBSNamespace == "" && os.Getenv("OPENEBS_NODE_DRIVER") != "" {


Is namespace only required for node driver?

node-driver env can be "agent" or "controller". So it is needed if we are using ZFS-LocalPV driver.

pkg/zfs/volume.go

pkg/zfs/zfs_util.go

pkg/mgmt/restore/start.go

pkg/mgmt/restore/restore.go

kmova · 2020-09-07T08:10:37Z

pkg/mgmt/restore/restore.go

@@ -0,0 +1,246 @@
+/*


Can we add a doc.go to this folder that lists the various low-level cases that are supported and any limitations?

Few things that are not clear by going through the code easily are:

how the restore differentiate between zvol and zfs datasets?

how does the file-system information get passed on to the restore process?

For example:

The restore flow is as follows:

plugin creates a restore PV (is this correct?)

plugin create a ZFS restore CR to restore the data from remote backup into the restore PV?

restore controller (on node) keeps a a watch for new CRs associated with the node id.

if status != init or marked for deletion, restore controller will execute the zfs recv | create command.

Limitations:

If the restore fails due to network issues? Will it be re-attempted?

If the restore doesn't have the specified backup, will it marked as failed?

Similar to the need for nodeID being same, the expectation is also that capacity should be available on the dest node/pool?

What happens when the same volume is restored twice or if the dest pool already has a volume with the same name.

pkg/mgmt/backup/backup.go

mynktl

changes looks good.

mynktl · 2020-09-07T05:01:32Z

pkg/driver/agent.go

+	go func() {
+		err := backup.Start(&ControllerMutex, stopCh)
+		if err != nil {
+			klog.Fatalf("Failed to start ZFS volume snapshot management controller: %s", err.Error())


log message needs to be changed for backup and restart start function error handling.

mynktl · 2020-09-07T06:50:14Z

pkg/mgmt/backup/backup.go

+				klog.Infof("backup %s done %s@%s prevsnap [%s]", bkp.Name, bkp.Spec.VolumeName, bkp.Spec.SnapName, bkp.Spec.PrevSnapName)
+				err = zfs.UpdateBkpInfo(bkp, apis.BKPZFSStatusDone)
+			} else {
+				klog.Errorf("backup %s failed %s@%s", bkp.Name, bkp.Spec.VolumeName, bkp.Spec.SnapName)


better to log error here for debugging

mynktl · 2020-09-07T06:52:18Z

pkg/mgmt/backup/backup.go

+func (c *BkpController) addBkp(obj interface{}) {
+	bkp, ok := obj.(*apis.ZFSBackup)
+	if !ok {
+		runtime.HandleError(fmt.Errorf("Couldn't get bkp object %#v", obj))


nit: better to use full form instead of bkp

mynktl · 2020-09-07T06:52:46Z

pkg/mgmt/backup/backup.go

+	if zfs.NodeID != bkp.Spec.OwnerNodeID {
+		return
+	}
+	klog.Infof("Got add event for Bkp %s snap %s@%s", bkp.Name, bkp.Spec.VolumeName, bkp.Spec.SnapName)


can be move to debug.

klog I guess does not have debug. May be later we can add verbose to these logs.

mynktl · 2020-09-07T10:06:32Z

pkg/mgmt/backup/backup.go

+		}
+	} else {
+		// if status is init then it means we are creating the zfs backup.
+		if bkp.Status == apis.BKPZFSStatusInit {


With this condition backup resource needs to be created with init status. Should we add check here to execute backup if status is empty also? Or we need to mark empty status as invalid.

nit: else / if can be merged here

will do the validation in the CR. working on adding validation schema for this.

mynktl · 2020-09-07T10:12:56Z

pkg/mgmt/restore/restore.go

+	// ZFSRestore should be deleted. Check if deletion timestamp is set
+	if !c.isDeletionCandidate(rstr) {
+		// if finalizer is not set then it means we are creating
+		// the zfs backup.


comment needs to be changed.

Here adding Backup and Restore controller, which will be watching for the events. The velero plugin will create a Backup CR to create a backup with the remote location information, the controller will send the data to that remote location. In the same way, the velero plugin will create a Restore CR to restore the volume from the the remote location and the restore controller will restore the data. Steps to use velero plugin for ZFS-LocalPV are : 1. install velero 2. add openebs plugin velero plugin add openebs/velero-plugin:latest 3. Create the volumesnapshot location : for full backup :- ```yaml apiVersion: velero.io/v1 kind: VolumeSnapshotLocation metadata: name: default namespace: velero spec: provider: openebs.io/zfspv-blockstore config: bucket: velero prefix: zfs namespace: openebs provider: aws region: minio s3ForcePathStyle: "true" s3Url: http://minio.velero.svc:9000 ``` for incremental backup :- ```yaml apiVersion: velero.io/v1 kind: VolumeSnapshotLocation metadata: name: default namespace: velero spec: provider: openebs.io/zfspv-blockstore config: bucket: velero prefix: zfs backup: incremental namespace: openebs provider: aws region: minio s3ForcePathStyle: "true" s3Url: http://minio.velero.svc:9000 ``` 4. Create backup velero backup create my-backup --snapshot-volumes --include-namespaces=velero-ns --volume-snapshot-locations=aws-cloud-default --storage-location=default 5. Create Schedule velero create schedule newschedule --schedule="*/1 * * * *" --snapshot-volumes --include-namespaces=velero-ns --volume-snapshot-locations=aws-local-default --storage-location=default 6. Restore from backup velero restore create --from-backup my-backup --restore-volumes=true --namespace-mappings velero-ns:ns1 Signed-off-by: Pawan <pawan@mayadata.io>

kmova · 2020-09-08T06:38:55Z

pkg/mgmt/backup/doc.go

+
+- Backup controller (on node) keeps a watch for new CRs associated with the node id. This node ID will be same as the Node ID present in the ZFSVolume resource.
+
+- The Backup controller will take a snapshot and then send the data to the remote location.


This one probably needs little clarity. remote location here refers to velero controller that receives the data and then pushes to S3.

right, updated it.

mynktl · 2020-09-08T07:18:25Z

pkg/mgmt/backup/doc.go

+
+- It will save the namespace information where the pvc is created also while taking the backup. Plugin will use this info if restoring without a namespace mapping to find if volume has already been restored.
+
+- plugin then creates the ZFSBackup CR with the destination volume and remote location where the data needs to be send.


better to mention, status should be init

mynktl · 2020-09-08T07:19:37Z

pkg/mgmt/backup/doc.go

+
+- Backup controller (on node) keeps a watch for new CRs associated with the node id. This node ID will be same as the Node ID present in the ZFSVolume resource.
+
+- The Backup controller will take a snapshot and then send the data to the remote location.


better to remove send the data to remote location since it is part of next step and may create confusion.

Signed-off-by: Pawan <pawan@mayadata.io>

pawanpraka1 added enhancement Add new functionality to existing feature pr/hold-merge hold the merge. labels Jun 25, 2020

pawanpraka1 added this to the v0.9 milestone Jun 25, 2020

pawanpraka1 requested a review from kmova June 25, 2020 08:21

pawanpraka1 force-pushed the backup branch from e7a7b7d to 4d9a38b Compare June 25, 2020 10:03

pawanpraka1 force-pushed the backup branch 2 times, most recently from b9e893f to f90925c Compare June 29, 2020 10:10

pawanpraka1 force-pushed the backup branch from f90925c to cfde926 Compare July 13, 2020 14:20

pawanpraka1 modified the milestones: v0.9, v0.10 Jul 16, 2020

pawanpraka1 force-pushed the backup branch 2 times, most recently from f8248c3 to 6e9f607 Compare August 10, 2020 15:01

pawanpraka1 modified the milestones: v0.9.1, v1.0.0 Aug 14, 2020

pawanpraka1 force-pushed the backup branch 4 times, most recently from 97976fe to 6e0145a Compare August 17, 2020 06:13

pawanpraka1 mentioned this pull request Aug 18, 2020

feat(velero): velero plugin for backup and restore for ZFS-LocalPV openebs/velero-plugin#102

Merged

7 tasks

pawanpraka1 removed the pr/hold-merge hold the merge. label Aug 18, 2020

pawanpraka1 force-pushed the backup branch from 779fc32 to fe85b8a Compare August 18, 2020 10:45