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

rbd: add additional space for encrypted volumes #4582

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from
Open

rbd: add additional space for encrypted volumes #4582

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
851ebb0
rbd: add additional space for encrypted volumes
iPraveenParihar Mar 28, 2024
d756a9d
e2e: test to verify the encrypted image size
iPraveenParihar Apr 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
111 changes: 111 additions & 0 deletions e2e/rbd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,12 @@ import (

. "github.com/onsi/ginkgo/v2"
v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/apimachinery/pkg/util/wait"
clientset "k8s.io/client-go/kubernetes"
"k8s.io/cloud-provider/volume/helpers"
"k8s.io/kubernetes/test/e2e/framework"
e2edebug "k8s.io/kubernetes/test/e2e/framework/debug"
e2epod "k8s.io/kubernetes/test/e2e/framework/pod"
Expand Down Expand Up @@ -1984,6 +1986,115 @@ var _ = Describe("RBD", func() {
}
})

By("create a encrypted block pvc and verify the image size", func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Check snapshot process and size of created snapshot too in the same flow since we had a problem with restore?

err := deleteResource(rbdExamplePath + "storageclass.yaml")
if err != nil {
framework.Failf("failed to delete storageclass: %v", err)
}
err = createRBDStorageClass(
f.ClientSet,
f,
defaultSCName,
nil,
map[string]string{"encrypted": "true", "encryptionType": util.EncryptionTypeBlock.String()},
deletePolicy)
if err != nil {
framework.Failf("failed to create storageclass: %v", err)
}

var (
imageSize uint64
resizeImageSize uint64
sizeInBytes int64
)

//nolint:goconst // The string "1Gi" is used multiple times in rbd.go, so it's not a const value.
pvcSize := "1Gi"
if sizeInBytes, err = helpers.RoundUpToB(resource.MustParse(pvcSize)); err != nil {
framework.Failf("failed to parse pvc size: %v", err)
}
imageSize = uint64(sizeInBytes) + util.GetLuksHeaderSize()

resizePVCSize := "2Gi"
if sizeInBytes, err = helpers.RoundUpToB(resource.MustParse(resizePVCSize)); err != nil {
framework.Failf("failed to parse resize pvc size: %v", err)
}
resizeImageSize = uint64(sizeInBytes) + util.GetLuksHeaderSize()

pvc, err := loadPVC(rawPvcPath)
if err != nil {
framework.Failf("failed to load PVC: %v", err)
}
pvc.Namespace = f.UniqueName
pvc.Spec.Resources.Requests[v1.ResourceStorage] = resource.MustParse(pvcSize)

app, err := loadApp(rawAppPath)
if err != nil {
framework.Failf("failed to load application: %v", err)
}
app.Labels = map[string]string{"app": "rbd-pod-block-encrypted"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

labelKey:="app"
labelVal:= "rbd-pod-block-encrypted"
app.Labels = map[string]string{labelKey:labelVal}

opt := metav1.ListOptions{
	LabelSelector: fmt.Sprintf("%s=%s",labelKey,labelVal)
}

using like above is less error prone, if we check it in one place we don't need to change it in other place


app.Namespace = f.UniqueName
err = createPVCAndApp("", f, pvc, app, deployTimeout)
if err != nil {
framework.Failf("failed to create PVC and application: %v", err)
}

pvc, err = getPersistentVolumeClaim(f.ClientSet, pvc.Namespace, pvc.Name)
if err != nil {
framework.Failf("failed to get pvc: %v", err)
}

opt := metav1.ListOptions{
LabelSelector: "app=rbd-pod-block-encrypted",
}

// validate created backend rbd images
err = validateImageSize(f, pvc, imageSize)
if err != nil {
framework.Failf("failed to validate image size: %v", err)
}

err = checkDeviceSize(app, f, &opt, pvcSize)
if err != nil {
framework.Failf("failed to validate device size: %v", err)
}

err = expandPVCSize(f.ClientSet, pvc, resizePVCSize, deployTimeout)
if err != nil {
framework.Failf("failed to expand pvc size: %v", err)
}
// wait for application pod to come up after resize
err = waitForPodInRunningState(app.Name, app.Namespace, f.ClientSet, deployTimeout, noError)
if err != nil {
framework.Failf("timeout waiting for pod to be in running state: %v", err)
}

// validate created backend rbd images
err = validateImageSize(f, pvc, resizeImageSize)
if err != nil {
framework.Failf("failed to validate image size after resize: %v", err)
}

err = checkDeviceSize(app, f, &opt, resizePVCSize)
if err != nil {
framework.Failf("failed to validate device size after resize: %v", err)
}

err = deletePVCAndApp("", f, pvc, app)
if err != nil {
framework.Failf("failed to delete pvc and app: %v", err)
}
Comment on lines +2084 to +2087
Copy link
Collaborator

Choose a reason for hiding this comment

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

please add E2E for PVC-PVC clone and snapshot restore as well

err = deleteResource(rbdExamplePath + "storageclass.yaml")
if err != nil {
framework.Failf("failed to delete storageclass: %v", err)
}
err = createRBDStorageClass(f.ClientSet, f, defaultSCName, nil, nil, deletePolicy)
if err != nil {
framework.Failf("failed to create storageclass: %v", err)
}
})

ByFileAndBlockEncryption("create a PVC and bind it to an app using rbd-nbd mounter with encryption", func(
validator encryptionValidateFunc, _ validateFunc, encType util.EncryptionType,
) {
Expand Down
20 changes: 20 additions & 0 deletions e2e/rbd_helper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1067,6 +1067,7 @@ type imageInfo struct {
StripeUnit int `json:"stripe_unit"`
StripeCount int `json:"stripe_count"`
ObjectSize int `json:"object_size"`
Size int64 `json:"size"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we use uint64 here?

}

// getImageInfo queries rbd about the given image and returns its metadata, and returns
Expand Down Expand Up @@ -1126,3 +1127,22 @@ func validateStripe(f *framework.Framework,

return nil
}

// validateImageSize validates the size of the image.
func validateImageSize(f *framework.Framework, pvc *v1.PersistentVolumeClaim, imageSize uint64) error {
imageData, err := getImageInfoFromPVC(pvc.Namespace, pvc.Name, f)
if err != nil {
return err
}

imgInfo, err := getImageInfo(f, imageData.imageName, defaultRBDPool)
if err != nil {
return err
}

if uint64(imgInfo.Size) != imageSize {
return fmt.Errorf("image %s size %d does not match expected %d", imgInfo.Name, imgInfo.Size, imageSize)
}

return nil
}
5 changes: 5 additions & 0 deletions internal/rbd/controllerserver.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1217,6 +1217,11 @@ func (cs *ControllerServer) CreateSnapshot(
return nil, status.Error(codes.Internal, err.Error())
}

// remove the extra size of the LUKS header, if the volume is block encrypted.
if vol.isBlockEncrypted() {
vol.VolSize -= int64(util.GetLuksHeaderSize())
}

return &csi.CreateSnapshotResponse{
Snapshot: &csi.Snapshot{
SizeBytes: vol.VolSize,
Expand Down
24 changes: 21 additions & 3 deletions internal/rbd/rbd_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,8 +443,14 @@ func createImage(ctx context.Context, pOpts *rbdVolume, cr *util.Credentials) er
return fmt.Errorf("failed to get IOContext: %w", err)
}

err = librbd.CreateImage(pOpts.ioctx, pOpts.RbdImageName,
uint64(util.RoundOffVolSize(pOpts.VolSize)*helpers.MiB), options)
size := uint64(util.RoundOffVolSize(pOpts.VolSize) * helpers.MiB)

// add the extra size of the LUKS header to the image size if block encryption is enabled.
if pOpts.isBlockEncrypted() {
size += util.GetLuksHeaderSize()
}

err = librbd.CreateImage(pOpts.ioctx, pOpts.RbdImageName, size, options)
if err != nil {
return fmt.Errorf("failed to create rbd image: %w", err)
}
Expand Down Expand Up @@ -1598,6 +1604,11 @@ func (ri *rbdImage) getImageInfo() error {
// TODO: can rv.VolSize not be a uint64? Or initialize it to -1?
ri.VolSize = int64(imageInfo.Size)

// remove the extra size of the LUKS header, if the volume is block encrypted.
if ri.isBlockEncrypted() {
ri.VolSize -= int64(util.GetLuksHeaderSize())
}

features, err := image.GetFeatures()
if err != nil {
return err
Expand Down Expand Up @@ -1846,7 +1857,14 @@ func (ri *rbdImage) resize(newSize int64) error {
}
defer image.Close()

err = image.Resize(uint64(util.RoundOffVolSize(newSize) * helpers.MiB))
size := uint64(util.RoundOffVolSize(newSize) * helpers.MiB)

// add the extra size of the LUKS header to the image size if block encryption is enabled.
if ri.isBlockEncrypted() {
size += util.GetLuksHeaderSize()
}

err = image.Resize(size)
if err != nil {
return err
}
Expand Down
18 changes: 17 additions & 1 deletion internal/util/cryptsetup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,22 @@ import (
"os/exec"
"strconv"
"strings"

"k8s.io/cloud-provider/volume/helpers"
)

// Limit memory used by Argon2i PBKDF to 32 MiB.
const cryptsetupPBKDFMemoryLimit = 32 << 10 // 32768 KiB
const (
cryptsetupPBKDFMemoryLimit = 32 << 10 // 32768 KiB
luks2MetadataSize = 32 << 7 // 4096 KiB
luks2KeySlotsSize = 32 << 8 // 8192 KiB
)

func GetLuksHeaderSize() uint64 {
size := uint64((((2 * luks2MetadataSize) + luks2KeySlotsSize) * helpers.KiB))

return size
}
Comment on lines +36 to +40
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add unit testing for this helper?

Copy link
Collaborator

Choose a reason for hiding this comment

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

please add unit test for it.


// LuksFormat sets up volume as an encrypted LUKS partition.
func LuksFormat(devicePath, passphrase string) (string, string, error) {
Expand All @@ -37,6 +49,10 @@ func LuksFormat(devicePath, passphrase string) (string, string, error) {
"luks2",
"--hash",
"sha256",
"--luks2-metadata-size",
strconv.Itoa(luks2MetadataSize)+"k",
"--luks2-keyslots-size",
strconv.Itoa(luks2KeySlotsSize)+"k",
"--pbkdf-memory",
strconv.Itoa(cryptsetupPBKDFMemoryLimit),
devicePath,
Expand Down