From 796e6b6c44ea05ea1b90bb5b57b952db9e0fdeed Mon Sep 17 00:00:00 2001 From: Rakshith R Date: Mon, 10 Mar 2025 15:15:45 +0530 Subject: [PATCH] rbd: use ListChildrenAttributes() instead of ListChildren() This commit modifies listSnapAndChildren() to make use of ListChildrenAttributes() instead of ListChildren() which allows us to filter out images in trash. This commit also order the alive images so that temp clone images are followed by images backing volume snapshots so that temp clone images are flattened first. Signed-off-by: Rakshith R --- go.mod | 4 +- go.sum | 6 +++ internal/rbd/clone.go | 9 +++- internal/rbd/controllerserver.go | 13 +++++- internal/rbd/rbd_util.go | 43 ++++++++++++++---- vendor/github.com/aws/smithy-go/CHANGELOG.md | 10 +++++ .../aws/smithy-go/go_module_metadata.go | 2 +- .../github.com/ceph/go-ceph/rbd/encryption.go | 38 +++++++++++----- .../ceph/go-ceph/rbd/encryption_load2.go | 9 ++-- .../ceph/go-ceph/rbd/encryption_opt_luks.go | 17 ++++--- .../ceph/go-ceph/rbd/snapshot_nautilus.go | 44 +++++++++++++++++++ vendor/modules.txt | 8 ++-- 12 files changed, 163 insertions(+), 40 deletions(-) diff --git a/go.mod b/go.mod index 95f35b818..0a18db35b 100644 --- a/go.mod +++ b/go.mod @@ -14,7 +14,7 @@ require ( github.com/aws/aws-sdk-go v1.55.6 github.com/aws/aws-sdk-go-v2/service/sts v1.33.17 github.com/ceph/ceph-csi/api v0.0.0-00010101000000-000000000000 - github.com/ceph/go-ceph v0.32.0 + github.com/ceph/go-ceph v0.32.1-0.20250307053135-38b9676b1d4e github.com/container-storage-interface/spec v1.11.0 github.com/csi-addons/kubernetes-csi-addons v0.12.0 github.com/csi-addons/spec v0.2.1-0.20241104111131-27825f744db5 @@ -78,7 +78,7 @@ require ( github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.34 // indirect github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.3 // indirect github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.15 // indirect - github.com/aws/smithy-go v1.22.2 // indirect + github.com/aws/smithy-go v1.22.3 // indirect github.com/beorn7/perks v1.0.1 // indirect github.com/blang/semver/v4 v4.0.0 // indirect github.com/cenkalti/backoff/v4 v4.3.0 // indirect diff --git a/go.sum b/go.sum index ff9ea9ebb..9d91eb5b1 100644 --- a/go.sum +++ b/go.sum @@ -122,6 +122,8 @@ github.com/aws/aws-sdk-go-v2/service/sts v1.33.17 h1:PZV5W8yk4OtH1JAuhV2PXwwO9v5 github.com/aws/aws-sdk-go-v2/service/sts v1.33.17/go.mod h1:cQnB8CUnxbMU82JvlqjKR2HBOm3fe9pWorWBza6MBJ4= github.com/aws/smithy-go v1.22.2 h1:6D9hW43xKFrRx/tXXfAlIZc4JI+yQe6snnWcQyxSyLQ= github.com/aws/smithy-go v1.22.2/go.mod h1:irrKGvNn1InZwb2d7fkIRNucdfwR8R+Ts3wxYa/cJHg= +github.com/aws/smithy-go v1.22.3 h1:Z//5NuZCSW6R4PhQ93hShNbyBbn8BWCmCVCt+Q8Io5k= +github.com/aws/smithy-go v1.22.3/go.mod h1:t1ufH5HMublsJYulve2RKmHDC15xu1f26kHCp/HgceI= github.com/benbjohnson/clock v1.1.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA= github.com/beorn7/perks v1.0.1 h1:VlbKKnNfV8bJzeqoa4cOKqO6bYr3WgKZxO8Z16+hsOM= github.com/beorn7/perks v1.0.1/go.mod h1:G2ZrVWU2WbWT9wwq4/hrbKbnv/1ERSJQ0ibhJ6rlkpw= @@ -136,6 +138,10 @@ github.com/cenkalti/backoff/v4 v4.3.0/go.mod h1:Y3VNntkOUPxTVeUxJ/G5vcM//AlwfmyY github.com/census-instrumentation/opencensus-proto v0.2.1/go.mod h1:f6KPmirojxKA12rnyqOA5BBL4O983OfeGPqjHWSTneU= github.com/ceph/go-ceph v0.32.0 h1:iXRUGdPmH7h9Vf/WA1Dg3Wo1tgL7gcUbylfpbxrlGLs= github.com/ceph/go-ceph v0.32.0/go.mod h1:42eoJzyLS3VREzqrg2ot44NtuluQZi55hFRSoLF36GQ= +github.com/ceph/go-ceph v0.32.1-0.20250303071035-7740b94e7f49 h1:0q5Ye65vYS5szvVsH5VILBGTv80p6v+znzPHsjiheDg= +github.com/ceph/go-ceph v0.32.1-0.20250303071035-7740b94e7f49/go.mod h1:FtN9DMlp/aS7VuO0VXdpdFfaetHNwtvn5q8gIhW/hQ4= +github.com/ceph/go-ceph v0.32.1-0.20250307053135-38b9676b1d4e h1:Ykum5wAFYzyUi+yW0qQFDsbP9pAuRKTOf+ttMoo1jZ4= +github.com/ceph/go-ceph v0.32.1-0.20250307053135-38b9676b1d4e/go.mod h1:bIS41nqhGmD7gQVoSBu3IYCwCiVEMeIxFU5qaqmC4a8= github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc= github.com/cespare/xxhash/v2 v2.1.1/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XLFGgcrjCOs= github.com/cespare/xxhash/v2 v2.3.0 h1:UL815xU9SqsFlibzuggzjXhog7bL6oX9BbNZnL2UFvs= diff --git a/internal/rbd/clone.go b/internal/rbd/clone.go index bcf801c45..648d85621 100644 --- a/internal/rbd/clone.go +++ b/internal/rbd/clone.go @@ -20,6 +20,7 @@ import ( "context" "errors" "fmt" + "strings" "github.com/ceph/ceph-csi/internal/util" "github.com/ceph/ceph-csi/internal/util/k8s" @@ -122,7 +123,7 @@ func (rv *rbdVolume) generateTempClone() *rbdVolume { // The temp cloned image name will be always (rbd image name + "-temp") // this name will be always unique, as cephcsi never creates an image with // this format for new rbd images - tempClone.RbdImageName = rv.RbdImageName + "-temp" + tempClone.RbdImageName = rv.RbdImageName + tempImageSuffix return &tempClone } @@ -250,3 +251,9 @@ func (rv *rbdVolume) doSnapClone(ctx context.Context, parentVol *rbdVolume) erro return nil } + +// isTempClonedImage checks whether the image is a temporary cloned image +// by checking the suffix of the image name. +func isTempClonedImage(imageName string) bool { + return strings.HasSuffix(imageName, tempImageSuffix) +} diff --git a/internal/rbd/controllerserver.go b/internal/rbd/controllerserver.go index 846bd7af9..15f49a145 100644 --- a/internal/rbd/controllerserver.go +++ b/internal/rbd/controllerserver.go @@ -590,7 +590,7 @@ func (cs *ControllerServer) repairExistingVolume(ctx context.Context, req *csi.C // are more than the `minSnapshotOnImage` Add a task to flatten all the // temporary cloned images. func flattenTemporaryClonedImages(ctx context.Context, rbdVol *rbdVolume, cr *util.Credentials) error { - snaps, children, err := rbdVol.listSnapAndChildren() + snapAndChildrenInfo, err := rbdVol.listSnapAndChildren() if err != nil { if errors.Is(err, util.ErrImageNotFound) { return status.Error(codes.InvalidArgument, err.Error()) @@ -598,6 +598,17 @@ func flattenTemporaryClonedImages(ctx context.Context, rbdVol *rbdVolume, cr *ut return status.Error(codes.Internal, err.Error()) } + children := make([]string, 0) + // order the temp clone images first followed by the volume snapshot + // images so that the temp clone images are flattened first. + // This is done in order to: + // - increase number of snapshots which can be supported for + // changed block tracking(rbd snap diff). + // - favor scalability since multiple PVCs can be restored from same + // volumesnapshot versus just one PVC-PVC clone. + children = append(children, snapAndChildrenInfo.TempCloneChildren...) + children = append(children, snapAndChildrenInfo.VolumeSnapshotChildren...) + snaps := snapAndChildrenInfo.SnapInfoList if len(snaps) > int(maxSnapshotsOnImage) { log.DebugLog( diff --git a/internal/rbd/rbd_util.go b/internal/rbd/rbd_util.go index 7e36b6b1b..8c49ce12a 100644 --- a/internal/rbd/rbd_util.go +++ b/internal/rbd/rbd_util.go @@ -84,6 +84,10 @@ const ( // clusterNameKey cluster Key, set on RBD image. clusterNameKey = "csi.ceph.com/cluster/name" + + // Suffix added to the temp cloned image name. + // This will always be (rbd image name + "-temp"). + tempImageSuffix = "-temp" ) // rbdImage contains common attributes and methods for the rbdVolume and @@ -2025,26 +2029,49 @@ func (ri *rbdImage) DisableDeepFlatten() error { return image.UpdateFeatures(librbd.FeatureDeepFlatten, false) } -// listSnapAndChildren returns list of names of snapshots and child images. -func (ri *rbdImage) listSnapAndChildren() ([]librbd.SnapInfo, []string, error) { +type snapAndChildrenInfo struct { + SnapInfoList []librbd.SnapInfo + TempCloneChildren []string + VolumeSnapshotChildren []string +} + +// listSnapAndChildren returns list of snapshot names, volume snapshot images and +// child temp clone images. Only child images which are not in trash are returned. +func (ri *rbdImage) listSnapAndChildren() (*snapAndChildrenInfo, error) { image, err := ri.open() if err != nil { - return nil, nil, err + return nil, err } defer image.Close() snaps, err := image.GetSnapshotNames() if err != nil { - return nil, nil, err + return nil, err } - // ListChildren() returns pools, images, err. - _, children, err := image.ListChildren() + childImageSpecList, err := image.ListChildrenAttributes() if err != nil { - return nil, nil, err + return nil, err } - return snaps, children, nil + tempCloneChildren := make([]string, 0) + volSnapChildren := make([]string, 0) + for _, child := range childImageSpecList { + if child.Trash { + continue + } + if isTempClonedImage(child.ImageName) { + tempCloneChildren = append(tempCloneChildren, child.ImageName) + } else { + volSnapChildren = append(volSnapChildren, child.ImageName) + } + } + + return &snapAndChildrenInfo{ + SnapInfoList: snaps, + TempCloneChildren: tempCloneChildren, + VolumeSnapshotChildren: volSnapChildren, + }, nil } func (ri *rbdImage) isCompatibleEncryption(dst *rbdImage) error { diff --git a/vendor/github.com/aws/smithy-go/CHANGELOG.md b/vendor/github.com/aws/smithy-go/CHANGELOG.md index de39171cf..4df632dce 100644 --- a/vendor/github.com/aws/smithy-go/CHANGELOG.md +++ b/vendor/github.com/aws/smithy-go/CHANGELOG.md @@ -1,3 +1,13 @@ +# Release (2025-02-17) + +## General Highlights +* **Dependency Update**: Updated to the latest SDK module versions + +## Module Highlights +* `github.com/aws/smithy-go`: v1.22.3 + * **Bug Fix**: Fix HTTP metrics data race. + * **Bug Fix**: Replace usages of deprecated ioutil package. + # Release (2025-01-21) ## General Highlights diff --git a/vendor/github.com/aws/smithy-go/go_module_metadata.go b/vendor/github.com/aws/smithy-go/go_module_metadata.go index a51ceca4c..d12d95891 100644 --- a/vendor/github.com/aws/smithy-go/go_module_metadata.go +++ b/vendor/github.com/aws/smithy-go/go_module_metadata.go @@ -3,4 +3,4 @@ package smithy // goModuleVersion is the tagged release for this module -const goModuleVersion = "1.22.2" +const goModuleVersion = "1.22.3" diff --git a/vendor/github.com/ceph/go-ceph/rbd/encryption.go b/vendor/github.com/ceph/go-ceph/rbd/encryption.go index 1fede749d..3ad0c6061 100644 --- a/vendor/github.com/ceph/go-ceph/rbd/encryption.go +++ b/vendor/github.com/ceph/go-ceph/rbd/encryption.go @@ -61,30 +61,44 @@ type EncryptionOptions interface { } func (opts EncryptionOptionsLUKS1) allocateEncryptionOptions() cEncryptionData { - var cOpts C.rbd_encryption_luks1_format_options_t var retData cEncryptionData - cOpts.alg = C.rbd_encryption_algorithm_t(opts.Alg) + // CBytes allocates memory. it will be freed when cEncryptionData.free is called - cOpts.passphrase = (*C.char)(C.CBytes(opts.Passphrase)) + cPassphrase := (*C.char)(C.CBytes(opts.Passphrase)) + cOptsSize := C.size_t(C.sizeof_rbd_encryption_luks1_format_options_t) + cOpts := (*C.rbd_encryption_luks1_format_options_t)(C.malloc(cOptsSize)) + cOpts.alg = C.rbd_encryption_algorithm_t(opts.Alg) + cOpts.passphrase = cPassphrase cOpts.passphrase_size = C.size_t(len(opts.Passphrase)) - retData.opts = C.rbd_encryption_options_t(&cOpts) - retData.optsSize = C.size_t(C.sizeof_rbd_encryption_luks1_format_options_t) - retData.free = func() { C.free(unsafe.Pointer(cOpts.passphrase)) } + retData.format = C.RBD_ENCRYPTION_FORMAT_LUKS1 + retData.opts = C.rbd_encryption_options_t(cOpts) + retData.optsSize = cOptsSize + retData.free = func() { + C.free(unsafe.Pointer(cOpts.passphrase)) + C.free(unsafe.Pointer(cOpts)) + } return retData } func (opts EncryptionOptionsLUKS2) allocateEncryptionOptions() cEncryptionData { - var cOpts C.rbd_encryption_luks2_format_options_t var retData cEncryptionData - cOpts.alg = C.rbd_encryption_algorithm_t(opts.Alg) + // CBytes allocates memory. it will be freed when cEncryptionData.free is called - cOpts.passphrase = (*C.char)(C.CBytes(opts.Passphrase)) + cPassphrase := (*C.char)(C.CBytes(opts.Passphrase)) + cOptsSize := C.size_t(C.sizeof_rbd_encryption_luks2_format_options_t) + cOpts := (*C.rbd_encryption_luks2_format_options_t)(C.malloc(cOptsSize)) + cOpts.alg = C.rbd_encryption_algorithm_t(opts.Alg) + cOpts.passphrase = cPassphrase cOpts.passphrase_size = C.size_t(len(opts.Passphrase)) - retData.opts = C.rbd_encryption_options_t(&cOpts) - retData.optsSize = C.size_t(C.sizeof_rbd_encryption_luks2_format_options_t) - retData.free = func() { C.free(unsafe.Pointer(cOpts.passphrase)) } + retData.format = C.RBD_ENCRYPTION_FORMAT_LUKS2 + retData.opts = C.rbd_encryption_options_t(cOpts) + retData.optsSize = cOptsSize + retData.free = func() { + C.free(unsafe.Pointer(cOpts.passphrase)) + C.free(unsafe.Pointer(cOpts)) + } return retData } diff --git a/vendor/github.com/ceph/go-ceph/rbd/encryption_load2.go b/vendor/github.com/ceph/go-ceph/rbd/encryption_load2.go index 81f6f3e4f..5d12f97c6 100644 --- a/vendor/github.com/ceph/go-ceph/rbd/encryption_load2.go +++ b/vendor/github.com/ceph/go-ceph/rbd/encryption_load2.go @@ -81,25 +81,22 @@ func (image *Image) EncryptionLoad2(opts []EncryptionOptions) error { } length := len(opts) - cspecs := (*C.rbd_encryption_spec_t)(C.malloc( - C.size_t(C.sizeof_rbd_encryption_spec_t * length))) - specs := unsafe.Slice(cspecs, length) + cspecs := make([]C.rbd_encryption_spec_t, length) freeFuncs := make([]func(), length) for idx, option := range opts { - f := option.(encryptionOptions2).writeEncryptionSpec(&specs[idx]) + f := option.(encryptionOptions2).writeEncryptionSpec(&cspecs[idx]) freeFuncs[idx] = f } defer func() { for _, f := range freeFuncs { f() } - C.free(unsafe.Pointer(cspecs)) }() ret := C.rbd_encryption_load2( image.image, - cspecs, + (*C.rbd_encryption_spec_t)(unsafe.Pointer(&cspecs[0])), C.size_t(length)) return getError(ret) } diff --git a/vendor/github.com/ceph/go-ceph/rbd/encryption_opt_luks.go b/vendor/github.com/ceph/go-ceph/rbd/encryption_opt_luks.go index bfacdde54..363b780cb 100644 --- a/vendor/github.com/ceph/go-ceph/rbd/encryption_opt_luks.go +++ b/vendor/github.com/ceph/go-ceph/rbd/encryption_opt_luks.go @@ -21,15 +21,22 @@ type EncryptionOptionsLUKS struct { } func (opts EncryptionOptionsLUKS) allocateEncryptionOptions() cEncryptionData { - var cOpts C.rbd_encryption_luks_format_options_t var retData cEncryptionData + // CBytes allocates memory. it will be freed when cEncryptionData.free is called - cOpts.passphrase = (*C.char)(C.CBytes(opts.Passphrase)) + cPassphrase := (*C.char)(C.CBytes(opts.Passphrase)) + cOptsSize := C.size_t(C.sizeof_rbd_encryption_luks_format_options_t) + cOpts := (*C.rbd_encryption_luks_format_options_t)(C.malloc(cOptsSize)) + cOpts.passphrase = cPassphrase cOpts.passphrase_size = C.size_t(len(opts.Passphrase)) - retData.opts = C.rbd_encryption_options_t(&cOpts) - retData.optsSize = C.size_t(C.sizeof_rbd_encryption_luks_format_options_t) - retData.free = func() { C.free(unsafe.Pointer(cOpts.passphrase)) } + retData.format = C.RBD_ENCRYPTION_FORMAT_LUKS + retData.opts = C.rbd_encryption_options_t(cOpts) + retData.optsSize = cOptsSize + retData.free = func() { + C.free(unsafe.Pointer(cOpts.passphrase)) + C.free(unsafe.Pointer(cOpts)) + } return retData } diff --git a/vendor/github.com/ceph/go-ceph/rbd/snapshot_nautilus.go b/vendor/github.com/ceph/go-ceph/rbd/snapshot_nautilus.go index ce3e11a16..aeaa06faf 100644 --- a/vendor/github.com/ceph/go-ceph/rbd/snapshot_nautilus.go +++ b/vendor/github.com/ceph/go-ceph/rbd/snapshot_nautilus.go @@ -168,6 +168,50 @@ func (image *Image) ListChildren() (pools []string, images []string, err error) return pools, images, nil } +// ListChildrenAttributes returns an array of struct with the names and ids of the +// images and pools and the trash of the images that are children of the given image. +// +// Implements: +// +// int rbd_list_children3(rbd_image_t image, rbd_linked_image_spec_t *images, +// size_t *max_images); +func (image *Image) ListChildrenAttributes() (imgSpec []ImageSpec, err error) { + if err := image.validate(imageIsOpen); err != nil { + return nil, err + } + var ( + csize C.size_t + children []C.rbd_linked_image_spec_t + ) + retry.WithSizes(16, 4096, func(size int) retry.Hint { + csize = C.size_t(size) + children = make([]C.rbd_linked_image_spec_t, csize) + ret := C.rbd_list_children3( + image.image, + (*C.rbd_linked_image_spec_t)(unsafe.Pointer(&children[0])), + &csize) + err = getErrorIfNegative(ret) + return retry.Size(int(csize)).If(err == errRange) + }) + if err != nil { + return nil, err + } + defer C.rbd_linked_image_spec_list_cleanup((*C.rbd_linked_image_spec_t)(unsafe.Pointer(&children[0])), csize) + + imgSpec = make([]ImageSpec, csize) + for i, child := range children[:csize] { + imgSpec[i] = ImageSpec{ + ImageName: C.GoString(child.image_name), + ImageID: C.GoString(child.image_id), + PoolName: C.GoString(child.pool_name), + PoolNamespace: C.GoString(child.pool_namespace), + PoolID: uint64(child.pool_id), + Trash: bool(child.trash), + } + } + return imgSpec, nil +} + // SetSnapByID updates the rbd image (not the Snapshot) such that the snapshot // is the source of readable data. // diff --git a/vendor/modules.txt b/vendor/modules.txt index 7810c3c90..b63461aa2 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -157,8 +157,8 @@ github.com/aws/aws-sdk-go-v2/service/internal/presigned-url github.com/aws/aws-sdk-go-v2/service/sts github.com/aws/aws-sdk-go-v2/service/sts/internal/endpoints github.com/aws/aws-sdk-go-v2/service/sts/types -# github.com/aws/smithy-go v1.22.2 -## explicit; go 1.21 +# github.com/aws/smithy-go v1.22.3 +## explicit; go 1.22 github.com/aws/smithy-go github.com/aws/smithy-go/auth github.com/aws/smithy-go/auth/bearer @@ -195,8 +195,8 @@ github.com/ceph/ceph-csi/api/deploy/kubernetes/cephfs github.com/ceph/ceph-csi/api/deploy/kubernetes/nfs github.com/ceph/ceph-csi/api/deploy/kubernetes/rbd github.com/ceph/ceph-csi/api/deploy/ocp -# github.com/ceph/go-ceph v0.32.0 -## explicit; go 1.21 +# github.com/ceph/go-ceph v0.32.1-0.20250307053135-38b9676b1d4e +## explicit; go 1.22 github.com/ceph/go-ceph/cephfs github.com/ceph/go-ceph/cephfs/admin github.com/ceph/go-ceph/common/admin/manager