Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Sep 8, 2020

Conversation

pawanpraka1
Copy link
Contributor

@pawanpraka1 pawanpraka1 commented Aug 10, 2020

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

Adding backup and restore support for ZFS-LocalPV. The PR in the ZFS-LocalPV repo is here : openebs/zfs-localpv#162

What this PR does?:
A new provider(openebs.io/zfspv-blockstore) has been added to
support backup and restore for ZFS-LocalPV. We can use below
VolumeSnapshotLocation to backup/restore the data to/from the cloud.

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

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

test case: backup the full namespace, delete it, and restore it and verify the data.

Any additional information for your reviewer?:

  • Restore in a namespace where volume already exist, will fail.

I am using local repo in go mod as ZFS-LocalPV side changes are not merged. Will update that once changes are merged.

github.com/openebs/zfs-localpv => /home/pawan/Desktop/openebs/src/github.com/openebs/zfs-localpv

To Do:

  • velero schedule(incremental) backup and restore

There are few changes related to it in this PR, will support the full incremental backup and restore after PR #99

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:

pkg/zfs/plugin/backup.go Outdated Show resolved Hide resolved
pkg/zfs/plugin/zfs.go Outdated Show resolved Hide resolved
pkg/zfs/plugin/zfs.go Outdated Show resolved Hide resolved
pkg/zfs/plugin/zfs.go Outdated Show resolved Hide resolved
pkg/zfs/plugin/restore.go Outdated Show resolved Hide resolved
pkg/zfs/plugin/backup.go Show resolved Hide resolved
pkg/zfs/plugin/backup.go Outdated Show resolved Hide resolved
pkg/zfs/plugin/zfs.go Outdated Show resolved Hide resolved
pkg/zfs/plugin/zfs.go Outdated Show resolved Hide resolved
pkg/zfs/plugin/zfs.go Show resolved Hide resolved
Copy link
Member

@mynktl mynktl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good. Given few comments about formatting.

}

func (p *Plugin) createBackup(vol *apis.ZFSVolume, schdname, snapname string) (string, error) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unnecessary leading newline

)

func (p *Plugin) createVolume(pvname string, bkpname string, bkpZV *apis.ZFSVolume) (*apis.ZFSVolume, error) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unnecessary new line

}

func (p *Plugin) checkRestoreStatus(snapname string) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unnecessary new line

}

func (p *Plugin) restoreZFSVolume(pvname, bkpname string) (*apis.ZFSVolume, error) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: unnecessary new line


func (p *Plugin) checkRestoreStatus(snapname string) {

for true {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for true {
for {

var vol *apis.ZFSVolume = nil

if len (volList.Items) > 1 {
return nil, errors.Errorf("zfs: error can not have more than one source volume %s", pvname, bkpname)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missing indentifier for bkpname in Errorf.

pawanpraka1 and others added 3 commits September 4, 2020 22:03
A new provider(openebs.io/zfspv-blockstore) has been added to
support backup and restore for ZFS-LocalPV. We can use below
VolumeSnapshotLocation to backup/restore the data to/from the cloud.

```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
```

Signed-off-by: Pawan <pawan@mayadata.io>
Signed-off-by: Pawan <pawan@mayadata.io>
Signed-off-by: Pawan <pawan@mayadata.io>
Signed-off-by: Pawan <pawan@mayadata.io>
Signed-off-by: Pawan <pawan@mayadata.io>
@@ -83,7 +83,7 @@ all: build
container: all
@echo ">> building container"
@cp Dockerfile _output/Dockerfile
docker build -t $(IMAGE):$(IMAGE_TAG) ${DBUILD_ARGS} -f _output/Dockerfile _output
@sudo docker build -t $(IMAGE):$(IMAGE_TAG) ${DBUILD_ARGS} -f _output/Dockerfile _output
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these changes really required? sudo?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, needed for docker.

@kmova kmova merged commit 5ed3848 into openebs:master Sep 8, 2020
@pawanpraka1 pawanpraka1 added feature and removed Enhancement issue/pr is a feature request labels Sep 9, 2020
@pawanpraka1 pawanpraka1 added this to the 2.1 milestone Sep 9, 2020
@pawanpraka1 pawanpraka1 deleted the backup branch September 9, 2020 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants