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

Add support for smart PVC cloning #695

Closed
wants to merge 17 commits into from
Closed

Add support for smart PVC cloning #695

wants to merge 17 commits into from

Conversation

Madhu-1
Copy link
Collaborator

@Madhu-1 Madhu-1 commented Oct 22, 2019

This PR adds the support for rbd to create smart cloning from a PVC, to create a new image from an existing image we are following below steps

Create a PVC from PVC

  • Create a temporary snapshot from the parent volume
  • Clone a new image from a temporary snapshot with options --rbd-default-clone-format 2 --image-feature layering,deep-flatten
  • Delete temporary snapshot created
  • Create a snapshot with requested Name from cloned volume
  • Create a clone from a snapshot with user-provided image features

Delete a PVC

  • Move the image to the trash
  • Add a task to remove the image from the trash

example commands:-

1) rbd snap create <RBD image for src k8s volume>@<random snap name>
2) rbd clone --rbd-default-clone-format 2 --image-feature
layering,deep-flatten <RBD image for src  k8s volume>@<random snap <RBD image for temporary snap image>
3) rbd snap rm <RBD image for src k8s volume>@<random snap name>
4) rbd snap create <RBD image for temporary snap image>@<random snap name> <k8s snap name>
5) rbd clone --rbd-default-clone-format 2 --image-feature <k8s dst vol config>
<RBD image for temporary snap image>@<random snap name> <RBD image for k8s dst vol>
6) rbd snap rm <RBD image for temporary snap image>@<random snap name>

Fixes: #675

Note: This PR is built on top of #693, will squash/remove commits once #693 is merged

Madhu-1 added 17 commits October 18, 2019 10:40
@Madhu-1
Revert "Skip snapshot testing in CI"
91d9dd4 
This reverts commit 90fef91.
@Madhu-1
update Travis to bionic
c792cbf 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Add E2E for snapshot cloning
dfde2af 
refractor E2E for cephfs and rbd

Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Add flatten for cloned rbd image
3ef00b0 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Add support for standalone snapshot creation
fa46815 
with the current implementation in ceph-csi, it's not possible to
delete the cloned volume if the snapshot is present
due to the child linking, to remove this dependency
we had a discussion and come up with an idea to separate
out the clone and snapshot, so that we can delete the
snapshot and cloned image in any order.

the steps followed to create an independent snapshot as follows

* Create  a temporary snapshot from the parent volume
* Clone a new image from a temporary snapshot with options
`--rbd-default-clone-format 2 --image-feature layering,deep-flatten`
* Delete temporary snapshot created
* Create a snapshot with requested Name

* Clone a new image from the snapshot with user-provided options
* Check the depth of the image as the maximum number of nested volume
clones can be (max 16 can be changed based on the configuration)
if the depth is reached flatten the newly cloned image

* Delete the cloned image (earlier we were removing the image with `rbd rm`
command with the  new design we will be moving the images to the trash)
same applies for normal volume deletion also

* Delete the temporary cloned image which was created for a snapshot
* Delete the snapshot

example commands:-
```
1) rbd snap create <RBD image for src k8s volume>@<random snap name>
2) rbd clone --rbd-default-clone-format 2 --image-feature
layering,deep-flatten <RBD image for src  k8s volume>@<random snap <RBD image for temporary snap image>
3) rbd snap rm <RBD image for src k8s volume>@<random snap name>
4) rbd snap create <RBD image for temporary snap image>@<random snap name> <k8s snap name>
5) rbd clone --rbd-default-clone-format 2 --image-feature <k8s dst vol config>
<RBD image for temporary snap image>@<random snap name> <RBD image for k8s dst vol>
```

Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Added Validation for clone depth flags
a8950c1 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Remove realpath installation
015f628 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Dump CSI logs if E2E fails
d5c62d8 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Address review comments
8116828 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Add support for LegacySnapshot deletion
88c94b5 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
update snapshot sidecar image in minikube.sh
4d7957b 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Fix leader election flag in deployment
c012134 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Add support of smart cloning
3bb6f3a 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
update external-provisioner from v1.3.0 to v1.4.0
ddbe231 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Add sample yaml file for smart cloning
edd271f 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
Add E2E for smart cloning
079ee41 
Signed-off-by: Madhu Rajanna <[email protected]>
@Madhu-1
drop this commit during rebase
d4c7867 
Signed-off-by: Madhu Rajanna <[email protected]>
nixpanic added a commit to nixpanic/ceph-csi that referenced this pull request Oct 30, 2019
@nixpanic
[unFIXed] rbd: clone from a VolumeSource:Volume
726387b 
STEP: creating mount and staging directories
STEP: creating a volume from source snapshot
• Failure [1.001 seconds]
Controller Service [Controller Server]
/home/vagrant/go/src/github.com/kubernetes-csi/csi-test/pkg/sanity/tests.go:44
  CreateVolume
  /home/vagrant/go/src/github.com/kubernetes-csi/csi-test/pkg/sanity/controller.go:390
    should fail when the volume source volume is not found [It]
    /home/vagrant/go/src/github.com/kubernetes-csi/csi-test/pkg/sanity/controller.go:827

    Expected
        <codes.Code>: 3
    to equal
        <codes.Code>: 5

    /home/vagrant/go/src/github.com/kubernetes-csi/csi-test/pkg/sanity/controller.go:846

See-also: ceph#695
nixpanic
nixpanic reviewed Oct 30, 2019
View reviewed changes
pkg/rbd/clone.go

volumeID := volumeSource.GetVolumeId()
if volumeID == "" {
return status.Error(codes.InvalidArgument, "volume content source volume ID cannot be empty")
Copy link
Member

Choose a reason for hiding this comment

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

This should return status.NotFound in case cloning from a Volume is intended, but the reference to the SourceVolume is incorrect.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

will take care of it, same applies for the snapshot source also

@Madhu-1 Madhu-1 mentioned this pull request Dec 6, 2019
Closed
@humblec humblec mentioned this pull request Feb 26, 2020
Closed
14 tasks
@humblec
Copy link
Collaborator

humblec commented Mar 3, 2020

@Madhu-1 can you revisit this PR ?

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Mar 3, 2020

this will be based on #693, once it is merged we will revisit this one

@humblec humblec mentioned this pull request Mar 17, 2020
Closed
6 tasks
@humblec
Copy link
Collaborator

humblec commented Mar 17, 2020

[status update]
Last week we had a meeting around snapshot/clone design/implementation we introduced recently with this PR, to support further use cases like Bakcup/DR , Data replication..etc. The main reason behind this meeting was to avoid a revamp again with snapshot/clone implementation while we start supporting these use cases too in near future. Atm, we are listing down the requirements to support it along with possible changes/enhancements we need with current implementation/design. Further technical notes will be followed/pointed here to those discussion. Considering we are at code freeze of v2.1.0 and the state we are with this features, the decision is to defer it to next month release (v3.0.0 #865 ) and release v2.1.0 as planned with other bug fixes and enhancements.

@ShyamsundarR
Copy link
Contributor

[status update]
Last week we had a meeting around snapshot/clone design/implementation we introduced recently with this PR, to support further use cases like Bakcup/DR , Data replication..etc. The main reason behind this meeting was to avoid a revamp again with snapshot/clone implementation while we start supporting these use cases too in near future. Atm, we are listing down the requirements to support it along with possible changes/enhancements we need with current implementation/design. Further technical notes will be followed/pointed here to those discussion. Considering we are at code freeze of v2.1.0 and the state we are with this features, the decision is to defer it to next month release (v3.0.0 #865 ) and release v2.1.0 as planned with other bug fixes and enhancements.

Details of other use-cases is present here.

The decision is to continue with the current design and implementation as it does not impede the future uses as understood at present.

nixpanic added a commit to nixpanic/ceph-csi that referenced this pull request Mar 20, 2020
@nixpanic
[unFIXed] rbd: clone from a VolumeSource:Volume
8abcff9 
STEP: creating mount and staging directories
STEP: creating a volume from source snapshot
• Failure [1.001 seconds]
Controller Service [Controller Server]
/home/vagrant/go/src/github.com/kubernetes-csi/csi-test/pkg/sanity/tests.go:44
  CreateVolume
  /home/vagrant/go/src/github.com/kubernetes-csi/csi-test/pkg/sanity/controller.go:390
    should fail when the volume source volume is not found [It]
    /home/vagrant/go/src/github.com/kubernetes-csi/csi-test/pkg/sanity/controller.go:827

    Expected
        <codes.Code>: 3
    to equal
        <codes.Code>: 5

    /home/vagrant/go/src/github.com/kubernetes-csi/csi-test/pkg/sanity/controller.go:846

See-also: ceph#695
@nixpanic nixpanic added this to the release-3.0.0 milestone Apr 9, 2020
@nixpanic nixpanic added enhancement New feature or request component/rbd Issues related to RBD labels Apr 19, 2020
@mergify
Copy link
Contributor

mergify bot commented May 21, 2020

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

@Madhu-1 Madhu-1 mentioned this pull request Jul 8, 2020
Merged
@mergify mergify bot closed this in #1213 Jul 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nixpanic nixpanic nixpanic left review comments

Assignees
No one assigned
Labels
component/rbd Issues related to RBD enhancement New feature or request
Projects
None yet
Milestone
release-3.0.0
Development

Successfully merging this pull request may close these issues.

Add support for smart cloning in rbd
4 participants
@Madhu-1 @humblec @ShyamsundarR @nixpanic