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.

Sign up for GitHub

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

Jump to bottom

feat(snapshot,restore): support for local snapshot #53

Merged
merged 11 commits into from
Mar 27, 2020
Merged

feat(snapshot,restore): support for local snapshot #53

merged 11 commits into from
Mar 27, 2020

Conversation

mynktl
Copy link
Contributor

@mynktl mynktl commented Mar 19, 2020 •
edited by kmova
Loading

Ref: openebs/openebs#2844

Changes:

  • Added support for local snapshot restore of CStor volume.

Sample volumeSnapshotLocation:

spec:
  config:
    local: "true"
  provider: openebs.io/cstor-blockstore

This was referenced Mar 19, 2020
Closed
Closed
@mynktl mynktl requested a review from vishnuitta March 19, 2020 09:21
@mynktl mynktl added this to the 1.9 milestone Mar 19, 2020
@kmova kmova mentioned this pull request Mar 19, 2020
Closed
@mynktl mynktl requested a review from kmova March 20, 2020 06:49
vishnuitta
vishnuitta reviewed Mar 25, 2020
View reviewed changes
README.md Outdated
@@ -62,6 +62,17 @@ spec:
provider: <GCP_OR_AWS>
region: <AWS_REGION>
```

For local snapshot of CStor volume through velero, Refere `example/06-local-volumesnapshotlocation.yaml` to configure `VolumeSnapshotLocation`.
Copy link
Contributor

Choose a reason for hiding this comment

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

to be consistent with 'cloud backup', should we call this as 'local backup'? before changing, lets get opinion from @kmova as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That may create confusion with local copy of the snapshot.

Copy link
Member

Choose a reason for hiding this comment

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

For creating local backup using cStor Volume snapshot feature, refer example/06-local-volumesnapshotlocation.yaml to configure VolumeSnapshotLocation.

vishnuitta
vishnuitta reviewed Mar 25, 2020
View reviewed changes
README.md Show resolved Hide resolved
vishnuitta
vishnuitta reviewed Mar 25, 2020
View reviewed changes
README.md Show resolved Hide resolved
vishnuitta
vishnuitta reviewed Mar 25, 2020
View reviewed changes
pkg/cstor/cstor.go Outdated Show resolved Hide resolved
vishnuitta
vishnuitta approved these changes Mar 25, 2020
View reviewed changes
Copy link
Contributor

@vishnuitta vishnuitta left a comment

Choose a reason for hiding this comment

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

Few things like adding comments, testcases are required. Overall flow looks to be good, however one more round of review need to be done.
APIs are changed however backup compatibility is maintained.
This involves good amount of changes with respect to approach as well. Now, storage classes with 'WatiForFirstConsumer' may not work. So, its better to get into the early cycles of testing.
Approving this in order to get into early cycles of testing.

@mynktl
Copy link
Contributor Author

mynktl commented Mar 26, 2020 •
edited
Loading

Created an issue to track test-cases #54.

@mynktl mynktl mentioned this pull request Mar 26, 2020
Merged
kmova
kmova reviewed Mar 26, 2020
View reviewed changes
README.md Outdated
@@ -50,7 +50,7 @@ Run the following command to install development image of OpenEBS velero-plugin
This command will add an init container to Velero deployment to install the OpenEBS velero-plugin.

## Configuring snapshot location
To take a backup of CStor volume through Velero, configure `VolumeSnapshotLocation` with provider `openebs.io/cstor-blockstore`. Sample YAML file for volumesnapshotlocation can be found at `example/06-volumesnapshotlocation.yaml`.
To take a cloud backup of CStor volume through Velero, configure `VolumeSnapshotLocation` with provider `openebs.io/cstor-blockstore`. Sample YAML file for volumesnapshotlocation can be found at `example/06-volumesnapshotlocation.yaml`.
Copy link
Member

Choose a reason for hiding this comment

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

To take remote backup of cStor volume snapshot to cloud or S3 compatible storage, configure VolumeSnapshotLocation with provider openebs.io/cstor-blockstore.

Can you provide samples for popular use cases like:

  • Minio
  • AWS S3
  • GCP buckets.

They could be different commented out YAMLs in the same snapshotlocation example yaml.

Copy link
Member

Choose a reason for hiding this comment

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

Throughout this document - mention remote/cloud backup and local backup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

kmova
kmova reviewed Mar 26, 2020
View reviewed changes
example/06-local-volumesnapshotlocation.yaml
apiVersion: velero.io/v1
kind: VolumeSnapshotLocation
metadata:
name: <LOCATION_NAME>
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment above on example valid location name string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

kmova
kmova reviewed Mar 26, 2020
View reviewed changes
example/06-local-volumesnapshotlocation.yaml
spec:
provider: openebs.io/cstor-blockstore
config:
namespace: <OPENEBS_NAMESPACE>
Copy link
Member

Choose a reason for hiding this comment

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

it is ok to repeat the comment here on what the namespace means.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

kmova
kmova reviewed Mar 26, 2020
View reviewed changes
example/06-volumesnapshotlocation.yaml
@@ -24,6 +24,6 @@ spec:
bucket: <YOUR_BUCKET>
prefix: <PREFIX_FOR_BACKUP_NAME>
backupPathPrefix: <PREFIX_FOR_BACKUP_PATH>
provider: <GCP_OR_AWS>
provider: <gcp_OR_aws>
Copy link
Member

Choose a reason for hiding this comment

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

If there are any know gotchas around permissions or how the buckets are created, please add them as comments in this YAML.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

@kmova
Copy link
Member

kmova commented Mar 26, 2020

Gave some minor comments @mynktl - also can you confirm if a test for local backup and restore can be added in this PR or planned for another PR?

@kmova kmova requested review from avishnu and qiell March 26, 2020 18:11
@mynktl
Copy link
Contributor Author

mynktl commented Mar 27, 2020

Hi @kmova
I have addressed the review comments. For test case We can add it in another PR. As of now I've created an issue to track this. #54.

@kmova kmova merged commit 3e46633 into openebs:master Mar 27, 2020
@mynktl mynktl deleted the local_snapshot branch October 16, 2020 09:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vishnuitta vishnuitta vishnuitta approved these changes

@kmova kmova kmova approved these changes

@avishnu avishnu Awaiting requested review from avishnu

@qiell qiell Awaiting requested review from qiell

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.9
Development

Successfully merging this pull request may close these issues.
3 participants
@mynktl @kmova @vishnuitta