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

Remove child clone volume reference from the parent volume #425

Closed
wants to merge 1 commit into from
Closed

Remove child clone volume reference from the parent volume #425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 31 additions & 0 deletions pkg/rbd/controllerserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package rbd

import (
"fmt"
"strings"

csicommon "github.com/ceph/ceph-csi/pkg/csi-common"
"github.com/ceph/ceph-csi/pkg/util"
Expand Down Expand Up @@ -210,6 +211,12 @@ func (cs *ControllerServer) checkSnapshot(req *csi.CreateVolumeRequest, rbdVol *
if err != nil {
return status.Error(codes.Internal, err.Error())
}

err = flattenImage(rbdVol, req.GetSecrets())
if err != nil {
return status.Error(codes.Internal, err.Error())
}

klog.V(4).Infof("create volume %s from snapshot %s", req.GetName(), rbdSnap.RbdSnapName)
return nil
}
Expand Down Expand Up @@ -272,6 +279,30 @@ func (cs *ControllerServer) DeleteVolume(ctx context.Context, req *csi.DeleteVol
}
}()

// check if rbd image has snapshot
key, err := getKey(rbdVol.AdminID, req.GetSecrets())
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
snaps, err := listSnapshot(rbdVol.Monitors, rbdVol.AdminID, key, rbdVol.Pool,
rbdVol.RbdImageName)
if err != nil {
return nil, status.Error(codes.Internal, err.Error())
}
// reject volume deletion if it has snapshot
if len(snaps) != 0 {
return nil, status.Errorf(codes.FailedPrecondition, "%s is having snapshot,please delete snapshot before deleting volume", req.GetVolumeId())
}

// remove the reference from volume and snapshot
// this will occur if flatten image failed during volume clone
err = flattenImage(rbdVol, req.GetSecrets())
// if the image does not contain reference to snapshot, flattern will return
// image has no parent error

if err != nil && !strings.Contains(err.Error(), "image has no parent") {
return nil, status.Error(codes.Internal, err.Error())
}
// Deleting rbd image
klog.V(4).Infof("deleting image %s", rbdVol.RbdImageName)
if err := deleteImage(rbdVol, rbdVol.AdminID, req.GetSecrets()); err != nil {
Expand Down
62 changes: 45 additions & 17 deletions pkg/rbd/rbd_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,26 @@ func createSnapshot(pOpts *rbdSnapshot, adminID string, credentials map[string]s
return nil
}

// flattenImage remove the reference from the child clone to the parent
// snapshot (refer http://docs.ceph.com/docs/giant/rbd/rbd-snapshot/#flattening-a-cloned-image)
func flattenImage(vol *rbdVolume, credentials map[string]string) error {
var output []byte

key, err := getKey(vol.AdminID, credentials)
if err != nil {
return err
}
klog.V(4).Infof("rbd: flatten image %s using mon %s, pool %s", vol.RbdImageName, vol.Monitors, vol.Pool)
args := []string{"flatten", "--pool", vol.Pool, "--image", vol.RbdImageName, "--id", vol.AdminID, "-m", vol.Monitors, "--key=" + key}

output, err = execCommand("rbd", args)

if err != nil {
Copy link

Choose a reason for hiding this comment

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

This should be expected to fail if the image doesn't have a parent. Additionally, if the image has snapshots and does not have the 'deep-flatten' feature enabled, the snapshots themselves will still be linked to the parent image.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dillaman Thanks for the review,
do I need to explicitly enable deep-flatten feature during clone and create or is it enabled by default

Copy link

Choose a reason for hiding this comment

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

You cannot enable it on existing images, it needs to have been enabled when the image was created. Support for this in krbd wasn't added until v5.1.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dillaman don't we need to enable this feature during clone (cloned image can be used later to snapshot and clone a PVC from it)

Copy link

Choose a reason for hiding this comment

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

@Madhu-1 negative -- deep-flatten isn't required for cloning, layering is the required feature.

Thinking holistically, we added the "rbd trash mv" tool to support cases where you cannot delete a parent image due to linked clones. In the Nautilus release, this is even automated for you via the rbd_move_parent_to_trash_on_remove config option (requires Mimic-or-later cluster). We are also adding automated background operation handling [1].

From an operator's point-of-view, we should never really expose the RBD implementation details for how snapshots and volume from snapshots (RBD clones) work on the backend. If you attempt to delete an image that has linked clones, move it to the trash for later cleanup. If you attempt to delete a snapshot with a linked clone, w/ clone v2 it just works. To support clone v1 (and v2), perhaps k8s snapshots could create a new RBD cloned image (and then it follows the previous rule about 'move-to-trash' upon delete).

[1] https://trello.com/c/JO5BPkRG

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#424

return errors.Wrapf(err, "failed to flatten image, command output: %s", string(output))
}

return nil
}
func unprotectSnapshot(pOpts *rbdSnapshot, adminID string, credentials map[string]string) error {
var output []byte

Expand Down Expand Up @@ -707,18 +727,8 @@ type snapInfo struct {
Timestamp string `json:"timestamp"`
}

/*
getSnapInfo queries rbd about the snapshots of the given image and returns its metadata, and
returns ErrImageNotFound if provided image is not found, and ErrSnapNotFound if provided snap
is not found in the images snapshot list
*/
func getSnapInfo(monitors, adminID, key, poolName, imageName, snapName string) (snapInfo, error) {
// rbd --format=json snap ls [image-spec]

var (
snpInfo snapInfo
snaps []snapInfo
)
func listSnapshot(monitors, adminID, key, poolName, imageName string) ([]snapInfo, error) {
var snaps []snapInfo

stdout, _, err := util.ExecCommand(
"rbd",
Expand All @@ -729,23 +739,41 @@ func getSnapInfo(monitors, adminID, key, poolName, imageName, snapName string) (
"--format="+"json",
"snap", "ls", poolName+"/"+imageName)
if err != nil {
klog.Errorf("failed getting snap (%s) information from image (%s): (%s)",
snapName, poolName+"/"+imageName, err)
klog.Errorf("failed listing snapshot information from image (%s): (%s)",
poolName+"/"+imageName, err)
if strings.Contains(string(stdout), "rbd: error opening image "+imageName+
": (2) No such file or directory") {
return snpInfo, ErrImageNotFound{imageName, err}
return snaps, ErrImageNotFound{imageName, err}
}
return snpInfo, err
return snaps, err
}

err = json.Unmarshal(stdout, &snaps)
if err != nil {
klog.Errorf("failed to parse JSON output of image snap list (%s): (%s)",
poolName+"/"+imageName, err)
return snpInfo, fmt.Errorf("unmarshal failed: %+v. raw buffer response: %s",
return snaps, fmt.Errorf("unmarshal failed: %+v. raw buffer response: %s",
err, string(stdout))
}
return snaps, nil
}

/*
getSnapInfo queries rbd about the snapshots of the given image and returns its metadata, and
returns ErrImageNotFound if provided image is not found, and ErrSnapNotFound if provided snap
is not found in the images snapshot list
*/
func getSnapInfo(monitors, adminID, key, poolName, imageName, snapName string) (snapInfo, error) {
// rbd --format=json snap ls [image-spec]

var (
snpInfo snapInfo
)

snaps, err := listSnapshot(monitors, adminID, key, poolName, imageName)
if err != nil {
return snpInfo, err
}
for _, snap := range snaps {
if snap.Name == snapName {
return snap, nil
Expand Down