feat(snapshot): add snapshot support for LVM PV #12

akhilerm · 2021-01-11T06:16:37Z

Why is this PR required? What issue does it fix?:
This PR adds support for LVM snapshot to the lvm-localPV CSI driver. The snapshots created will be readonly (as opposed to the default ReadWrite). Also, once snapshots are created for a volume, resize will not work for those volumes, since LVM does't support that.

To create a snapshot, create a snapshot class as given below and then create a volumesnapshot resource

kind: VolumeSnapshotClass
apiVersion: snapshot.storage.k8s.io/v1
metadata:
  name: lvm-localpv-snapclass
  annotations:
    snapshot.storage.kubernetes.io/is-default-class: "true"
driver: local.csi.openebs.io
deletionPolicy: Delete
---
apiVersion: snapshot.storage.k8s.io/v1
kind: VolumeSnapshot
metadata:
  name: lvm-localpv-snap
spec:
  volumeSnapshotClassName: lvm-localpv-snapclass
  source:
    persistentVolumeClaimName: <pvc-name>

What this PR does?:

adds LVMSnapshot CRDs
add snapshot controller to watch for LVMSnapshot CRs
adds the volumesnapshot related CRDs from storage.k8s.io to the deployment
use container images from k8s.gcr.io for CSI components

Limitation
Volumes with snapshots cannot be resized, as LVM does not support online resize of origin volumes with a snapshot. ControllerExpandVolume will error out if the volume to resized has any active snapshots.

Does this PR require any upgrade changes?:
dm-snapshot kernel module should be loaded for snapshot to work

If the changes in this PR are manually verified, list down the scenarios covered::

Snapshot creation
Try to resize volume with snapshot (will error out the volume expansion)
resize should work after snapshots are removed
create multiple snapshots for the same volume

Any additional information for your reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs

Checklist:

Fixes add snapshot support for lvm-localpv #10
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:

deploy/yamls/lvm-driver.yaml

pkg/driver/agent.go

pkg/apis/openebs.io/lvm/v1alpha1/lvmvolume.go

pkg/mgmt/snapshot/snapshot.go

pkg/driver/controller.go

Signed-off-by: Akhil Mohan <[email protected]>

akhilerm · 2021-02-04T11:46:20Z

@pawanpraka1 Updated the NodeExpandVolume to error out if the volume contains any snapshots

pawanpraka1 · 2021-02-04T12:57:49Z

@akhilerm we need to block that in ControllerExpandVolume. Also test this and see if behavior is fine.

Signed-off-by: Akhil Mohan <[email protected]>

akhilerm · 2021-02-04T14:18:10Z

@akhilerm we need to block that in ControllerExpandVolume. Also test this and see if behavior is fine.

Done. @pawanpraka1. Yes , tested and its working fine.

One issue I faced is in the following test case:

Create PVC of 4Gi
Create a snapshot
Resize PVC to 5Gi
Resize fails because of active snapshot
Delete the snapshot
But the controller logs is not having the ControllerExpandVolume call again.
Even after deleting the snapshot the resize is not happening.

pawanpraka1 · 2021-02-04T14:58:42Z

@akhilerm it uses exponential backoff to retry the next time. You might need to wait enough to get the call again?

Signed-off-by: Akhil Mohan <[email protected]>

akhilerm · 2021-02-05T04:52:14Z

@pawanpraka1 Changing the grpc error code worked.

pkg/driver/controller.go

Signed-off-by: Akhil Mohan <[email protected]>

codecov-io · 2021-02-05T09:50:13Z

Codecov Report

Merging #12 (07ac6ea) into master (b370f3c) will decrease coverage by 0.18%.
The diff coverage is 0.00%.

@@            Coverage Diff            @@
##           master     #12      +/-   ##
=========================================
- Coverage    1.41%   1.22%   -0.19%     
=========================================
  Files          11      11              
  Lines         708     818     +110     
=========================================
  Hits           10      10              
- Misses        698     808     +110

Impacted Files	Coverage Δ
pkg/driver/agent.go	`0.00% <0.00%> (ø)`
pkg/driver/controller.go	`0.77% <0.00%> (-0.30%)`	⬇️

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 b370f3c...07ac6ea. Read the comment docs.

Signed-off-by: Akhil Mohan <[email protected]>

deploy/yamls/lvm-driver.yaml

Signed-off-by: Akhil Mohan <[email protected]>

pawanpraka1

looks good.

akhilerm marked this pull request as ready for review January 25, 2021 05:51

akhilerm requested review from kmova and pawanpraka1 January 25, 2021 05:51

akhilerm force-pushed the snapshot-clone branch 2 times, most recently from 98a5dbf to d0d48a1 Compare January 29, 2021 06:08

akhilerm commented Jan 29, 2021

View reviewed changes

deploy/yamls/lvm-driver.yaml Show resolved Hide resolved

kmova reviewed Jan 30, 2021

View reviewed changes

pkg/driver/agent.go Outdated Show resolved Hide resolved

akhilerm force-pushed the snapshot-clone branch from 848399e to 2f5b805 Compare January 31, 2021 14:42

akhilerm requested a review from kmova February 1, 2021 04:31

pawanpraka1 reviewed Feb 1, 2021

View reviewed changes

pkg/apis/openebs.io/lvm/v1alpha1/lvmvolume.go Outdated Show resolved Hide resolved

pkg/mgmt/snapshot/snapshot.go Outdated Show resolved Hide resolved

pkg/driver/controller.go Show resolved Hide resolved

akhilerm added 17 commits February 3, 2021 14:47

updated license template

92d603b

Signed-off-by: Akhil Mohan <[email protected]>

add lvm snapshot crd

d210f18

Signed-off-by: Akhil Mohan <[email protected]>

generate clientset for snapshot

f0819b5

Signed-off-by: Akhil Mohan <[email protected]>

add builder methods for LVM snapshot

0a12c7e

Signed-off-by: Akhil Mohan <[email protected]>

added lvm_util functions for creating snapshot

3579fee

Signed-off-by: Akhil Mohan <[email protected]>

add changelog

5535d7d

Signed-off-by: Akhil Mohan <[email protected]>

change snapshotID

8053664

Signed-off-by: Akhil Mohan <[email protected]>

make snapshot capacity same as source volume capacity

184b9f7

Signed-off-by: Akhil Mohan <[email protected]>

add snapshot crds

bee1f79

Signed-off-by: Akhil Mohan <[email protected]>

fix review comments

4cac22c

Signed-off-by: Akhil Mohan <[email protected]>

remove snapname from volumeinfo

cf9a410

Signed-off-by: Akhil Mohan <[email protected]>

add verify snapshot request

2f0225b

Signed-off-by: Akhil Mohan <[email protected]>

add sample snapshot yaml

c0a51d9

Signed-off-by: Akhil Mohan <[email protected]>

fix delete snapshot

defd854

Signed-off-by: Akhil Mohan <[email protected]>

update crds

27c3609

Signed-off-by: Akhil Mohan <[email protected]>

fix lint issues

23505cf

Signed-off-by: Akhil Mohan <[email protected]>

add crds

95e5413

Signed-off-by: Akhil Mohan <[email protected]>

akhilerm force-pushed the snapshot-clone branch from d177336 to 95e5413 Compare February 3, 2021 09:17

remove snapshot- prefix from snapname

d7e8ab4

Signed-off-by: Akhil Mohan <[email protected]>

fix lvm snap name

84b15b2

Signed-off-by: Akhil Mohan <[email protected]>

block resize on volumes with active snapshot

48cfaf9

Signed-off-by: Akhil Mohan <[email protected]>

akhilerm added 2 commits February 4, 2021 19:18

move snapshot check from nodeexpand to controllerexpand call

d4f51ce

Signed-off-by: Akhil Mohan <[email protected]>

fix logs

e7df29a

Signed-off-by: Akhil Mohan <[email protected]>

change error code

808e0ac

Signed-off-by: Akhil Mohan <[email protected]>

pawanpraka1 reviewed Feb 5, 2021

View reviewed changes

pkg/driver/controller.go Outdated Show resolved Hide resolved

akhilerm added 2 commits February 5, 2021 12:26

add snapshot request validation

3b6dcbb

Signed-off-by: Akhil Mohan <[email protected]>

use snapshot status instead of finalizers

07ac6ea

Signed-off-by: Akhil Mohan <[email protected]>

update readme

d9a9d54

Signed-off-by: Akhil Mohan <[email protected]>

akhilerm requested a review from pawanpraka1 February 5, 2021 10:01

pawanpraka1 reviewed Feb 5, 2021

View reviewed changes

deploy/yamls/lvm-driver.yaml Outdated Show resolved Hide resolved

pawanpraka1 reviewed Feb 5, 2021

View reviewed changes

deploy/yamls/lvm-driver.yaml Outdated Show resolved Hide resolved

pawanpraka1 reviewed Feb 5, 2021

View reviewed changes

deploy/yamls/lvm-driver.yaml Outdated Show resolved Hide resolved

update registry from quay to gcr

f476ee9

Signed-off-by: Akhil Mohan <[email protected]>

akhilerm requested a review from pawanpraka1 February 5, 2021 11:31

replace zpool with volume group

8fcab81

Signed-off-by: Akhil Mohan <[email protected]>

pawanpraka1 approved these changes Feb 8, 2021

View reviewed changes

pawanpraka1 merged commit 7e0bd8a into openebs:master Feb 8, 2021

pawanpraka1 added this to the v0.2 milestone Feb 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(snapshot): add snapshot support for LVM PV #12

feat(snapshot): add snapshot support for LVM PV #12

akhilerm commented Jan 11, 2021 •

edited

Loading

akhilerm commented Feb 4, 2021

pawanpraka1 commented Feb 4, 2021 •

edited

Loading

akhilerm commented Feb 4, 2021

pawanpraka1 commented Feb 4, 2021

akhilerm commented Feb 5, 2021

codecov-io commented Feb 5, 2021

pawanpraka1 left a comment

feat(snapshot): add snapshot support for LVM PV #12

feat(snapshot): add snapshot support for LVM PV #12

Conversation

akhilerm commented Jan 11, 2021 • edited Loading

akhilerm commented Feb 4, 2021

pawanpraka1 commented Feb 4, 2021 • edited Loading

akhilerm commented Feb 4, 2021

pawanpraka1 commented Feb 4, 2021

akhilerm commented Feb 5, 2021

codecov-io commented Feb 5, 2021

Codecov Report

pawanpraka1 left a comment

Choose a reason for hiding this comment

akhilerm commented Jan 11, 2021 •

edited

Loading

pawanpraka1 commented Feb 4, 2021 •

edited

Loading