Skip to content

Commit

Permalink
Adds fixed overhead for LUKS volumes
Browse files Browse the repository at this point in the history
---------

Co-authored-by: joe webster <[email protected]>
  • Loading branch information
reederc42 and jwebster7 authored Aug 6, 2024
1 parent 2bf1d51 commit 4f24ac7
Show file tree
Hide file tree
Showing 8 changed files with 199 additions and 10 deletions.
7 changes: 7 additions & 0 deletions frontend/csi/controller_helpers/kubernetes/import.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"github.com/netapp/trident/frontend/csi"
. "github.com/netapp/trident/logging"
"github.com/netapp/trident/storage"
"github.com/netapp/trident/utils"
"github.com/netapp/trident/utils/errors"
)

Expand Down Expand Up @@ -111,6 +112,12 @@ func (h *helper) ImportVolume(

dataSize := h.getDataSizeFromTotalSize(ctx, totalSize, snapshotReserve)

if luksAnnotation, ok := claim.GetObjectMeta().GetAnnotations()[AnnLUKSEncryption]; ok {
if utils.ParseBool(luksAnnotation) {
dataSize -= utils.LUKSMetadataSize
}
}

if claim.Spec.Resources.Requests == nil {
claim.Spec.Resources.Requests = v1.ResourceList{}
}
Expand Down
27 changes: 27 additions & 0 deletions frontend/csi/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,3 +465,30 @@ func TestEnsureLUKSVolumePassphrase_NoCorrectPassphraseProvided(t *testing.T) {
assert.Error(t, err)
mockCtrl.Finish()
}

func TestParseBool(t *testing.T) {
tests := []struct {
b string
expected bool
}{
{
b: "true",
expected: true,
},
{
b: "false",
expected: false,
},
{
b: "not a value",
expected: false,
},
}

for _, test := range tests {
t.Run(test.b, func(t *testing.T) {
actual := utils.ParseBool(test.b)
assert.Equal(t, test.expected, actual)
})
}
}
45 changes: 45 additions & 0 deletions storage_drivers/ontap/ontap_common.go
Original file line number Diff line number Diff line change
Expand Up @@ -3473,6 +3473,51 @@ func ConstructPoolForLabels(nameTemplate string, labels map[string]string) *stor
return pool
}

func subtractUintFromSizeString(size string, val uint64) (string, error) {
sizeBytesString, _ := utils.ConvertSizeToBytes(size)
sizeBytes, err := strconv.ParseUint(sizeBytesString, 10, 64)
if err != nil {
return "", fmt.Errorf("invalid size string: %v", err)
}
if val > sizeBytes {
return "", fmt.Errorf("right-hand term too large")
}
return strconv.FormatUint(sizeBytes-val, 10), nil
}

// adds LUKS overhead iff luksEncryption is a true boolean string. Logs an error if the luksEncryption string is not a boolean.
func incrementWithLUKSMetadataIfLUKSEnabled(ctx context.Context, size uint64, luksEncryption string) uint64 {
isLUKS, err := strconv.ParseBool(luksEncryption)
if err != nil && luksEncryption != "" {
Logc(ctx).WithError(err).Debug("Could not parse luksEncryption string.")
}

if isLUKS {
return size + utils.LUKSMetadataSize
}

return size
}

// removes LUKS overhead iff luksEncryption is a true boolean string. Logs an error if the luksEncryption string is not a boolean or size is too small.
func decrementWithLUKSMetadataIfLUKSEnabled(ctx context.Context, size uint64, luksEncryption string) uint64 {
isLUKS, err := strconv.ParseBool(luksEncryption)
if err != nil && luksEncryption != "" {
Logc(ctx).WithError(err).Debug("Could not parse luksEncryption string.")
}

if utils.LUKSMetadataSize > size {
Logc(ctx).WithError(err).WithField("size", size).Error("Size too small to subtract LUKS metadata.")
return 0
}

if isLUKS {
return size - utils.LUKSMetadataSize
}

return size
}

// deleteAutomaticSnapshot deletes a snapshot that was created automatically during volume clone creation.
// An automatic snapshot is detected by the presence of CloneSourceSnapshotInternal in the volume config
// while CloneSourceSnapshot is not set. This method is called after the volume has been deleted, and it
Expand Down
60 changes: 60 additions & 0 deletions storage_drivers/ontap/ontap_common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"context"
"encoding/json"
"fmt"
"math/rand"
"os"
"strconv"
"testing"
"time"

Expand Down Expand Up @@ -6360,6 +6362,64 @@ func TestConstructPoolForLabels(t *testing.T) {
assert.Equal(t, sa.NewLabelOffer(nil), pool.Attributes()["labels"])
}

func TestSubtractUintFromSizeString(t *testing.T) {
units := []string{"", "MB", "MiB", "GB", "GiB"}
count := 100
maxValue := int64(1000000000)

// Test for invalid size string
_, err := subtractUintFromSizeString("not a size", 0)
assert.ErrorContains(t, err, "invalid size")

// Fuzz tests
for range count {
sizeValue := rand.Int63n(maxValue + 1)
size := strconv.FormatInt(sizeValue, 10) + units[rand.Intn(len(units))]
val := uint64(rand.Int63n(maxValue))

sizeBytesString, err := utils.ConvertSizeToBytes(size)
assert.NoError(t, err)
sizeBytes, _ := strconv.ParseUint(sizeBytesString, 10, 64)

newSizeBytesString, err := subtractUintFromSizeString(size, val)

if val > sizeBytes {
assert.ErrorContains(t, err, "too large")
} else {
assert.NoError(t, err)

expected := strconv.FormatUint(sizeBytes-val, 10)
assert.Equal(t, expected, newSizeBytesString)
}
}
}

func TestIncrementWithLUKSMetadataIfLUKSEnabled(t *testing.T) {
size := uint64(1000000000)

actual := incrementWithLUKSMetadataIfLUKSEnabled(context.Background(), size, "true")
assert.Greater(t, actual, size)

actual = incrementWithLUKSMetadataIfLUKSEnabled(context.Background(), size, "false")
assert.Equal(t, actual, size)

actual = incrementWithLUKSMetadataIfLUKSEnabled(context.Background(), size, "blue")
assert.Equal(t, actual, size)
}

func TestDecrementWithLUKSMetadataIfLUKSEnabled(t *testing.T) {
size := uint64(1000000000)

actual := decrementWithLUKSMetadataIfLUKSEnabled(context.Background(), size, "true")
assert.Less(t, actual, size)

actual = decrementWithLUKSMetadataIfLUKSEnabled(context.Background(), size, "false")
assert.Equal(t, actual, size)

actual = decrementWithLUKSMetadataIfLUKSEnabled(context.Background(), size, "blue")
assert.Equal(t, actual, size)
}

func TestDeleteAutomaticSnapshot(t *testing.T) {
type parameters struct {
volConfig storage.VolumeConfig
Expand Down
22 changes: 21 additions & 1 deletion storage_drivers/ontap/ontap_san.go
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,11 @@ func (d *SANStorageDriver) Create(
}
lunSizeBytes := GetVolumeSize(requestedSizeBytes, storagePool.InternalAttributes()[Size])

// Add a constant overhead for LUKS volumes to account for LUKS metadata. This overhead is
// part of the LUN but is not reported to the orchestrator.
reportedSize := lunSizeBytes
lunSizeBytes = incrementWithLUKSMetadataIfLUKSEnabled(ctx, lunSizeBytes, luksEncryption)

lunSize := strconv.FormatUint(lunSizeBytes, 10)
// Get the flexvol size based on the snapshot reserve
flexvolSize := drivers.CalculateVolumeSizeBytes(ctx, name, lunSizeBytes, snapshotReserveInt)
Expand Down Expand Up @@ -378,7 +383,7 @@ func (d *SANStorageDriver) Create(
}

// Update config to reflect values used to create volume
volConfig.Size = strconv.FormatUint(lunSizeBytes, 10)
volConfig.Size = strconv.FormatUint(reportedSize, 10)
volConfig.SpaceReserve = spaceReserve
volConfig.SnapshotPolicy = snapshotPolicy
volConfig.SnapshotReserve = snapshotReserve
Expand Down Expand Up @@ -692,6 +697,14 @@ func (d *SANStorageDriver) Import(ctx context.Context, volConfig *storage.Volume
// Use the LUN size
volConfig.Size = lunInfo.Size

if utils.ParseBool(volConfig.LUKSEncryption) {
newSize, err := subtractUintFromSizeString(volConfig.Size, utils.LUKSMetadataSize)
if err != nil {
return err
}
volConfig.Size = newSize
}

// Rename the volume or LUN if Trident will manage its lifecycle
if !volConfig.ImportNotManaged {
if lunInfo.Name != targetPath {
Expand Down Expand Up @@ -1348,12 +1361,16 @@ func (d *SANStorageDriver) Resize(
return fmt.Errorf("requested size %d is less than existing volume size %d", requestedSizeBytes, lunSizeBytes)
}

// Add a constant overhead for LUKS volumes to account for LUKS metadata.
requestedSizeBytes = incrementWithLUKSMetadataIfLUKSEnabled(ctx, requestedSizeBytes, volConfig.LUKSEncryption)

snapshotReserveInt, err := getSnapshotReserveFromOntap(ctx, name, d.API.VolumeInfo)
if err != nil {
Logc(ctx).WithField("name", name).Errorf("Could not get the snapshot reserve percentage for volume")
}

newFlexvolSize := drivers.CalculateVolumeSizeBytes(ctx, name, requestedSizeBytes, snapshotReserveInt)

newFlexvolSize = uint64(LUNMetadataBufferMultiplier * float64(newFlexvolSize))

sameLUNSize := utils.VolumeSizeWithinTolerance(int64(requestedSizeBytes), int64(currentLunSize),
Expand Down Expand Up @@ -1413,6 +1430,9 @@ func (d *SANStorageDriver) Resize(
}
}

// LUKS metadata size is not reported so remove it from LUN size
returnSize = decrementWithLUKSMetadataIfLUKSEnabled(ctx, returnSize, volConfig.LUKSEncryption)

volConfig.Size = strconv.FormatUint(returnSize, 10)
return nil
}
Expand Down
39 changes: 30 additions & 9 deletions storage_drivers/ontap/ontap_san_economy.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,14 +447,6 @@ func (d *SANEconomyStorageDriver) Create(
}
sizeBytes = GetVolumeSize(sizeBytes, storagePool.InternalAttributes()[Size])

lunSize := strconv.FormatUint(sizeBytes, 10)

if _, _, checkVolumeSizeLimitsError := drivers.CheckVolumeSizeLimits(
ctx, sizeBytes, d.Config.CommonStorageDriverConfig,
); checkVolumeSizeLimitsError != nil {
return checkVolumeSizeLimitsError
}

// Ensure LUN name isn't too long
if len(name) > maxLunNameLength {
return fmt.Errorf("volume %s name exceeds the limit of %d characters", name, maxLunNameLength)
Expand Down Expand Up @@ -482,6 +474,19 @@ func (d *SANEconomyStorageDriver) Create(
luksEncryption = storagePool.InternalAttributes()[LUKSEncryption]
)

// Add a constant overhead for LUKS volumes to account for LUKS metadata. This overhead is
// part of the LUN but is not reported to the orchestrator.
reportedSize := sizeBytes
sizeBytes = incrementWithLUKSMetadataIfLUKSEnabled(ctx, sizeBytes, luksEncryption)

lunSize := strconv.FormatUint(sizeBytes, 10)

if _, _, checkVolumeSizeLimitsError := drivers.CheckVolumeSizeLimits(
ctx, sizeBytes, d.Config.CommonStorageDriverConfig,
); checkVolumeSizeLimitsError != nil {
return checkVolumeSizeLimitsError
}

snapshotReserveInt, err := GetSnapshotReserve(snapshotPolicy, snapshotReserve)
if err != nil {
return fmt.Errorf("invalid value for snapshotReserve: %v", err)
Expand Down Expand Up @@ -510,7 +515,7 @@ func (d *SANEconomyStorageDriver) Create(
}

// Update config to reflect values used to create volume
volConfig.Size = strconv.FormatUint(sizeBytes, 10)
volConfig.Size = strconv.FormatUint(reportedSize, 10)
volConfig.SpaceReserve = spaceReserve
volConfig.SnapshotPolicy = snapshotPolicy
volConfig.SnapshotReserve = snapshotReserve
Expand Down Expand Up @@ -646,6 +651,8 @@ func (d *SANEconomyStorageDriver) Create(
Logc(ctx).WithField("error", err).Error("Failed to determine LUN size")
return err
}
// Remove LUKS metadata size from actual size of LUN
actualSize = decrementWithLUKSMetadataIfLUKSEnabled(ctx, actualSize, luksEncryption)
volConfig.Size = strconv.FormatUint(actualSize, 10)

// Save the fstype in a LUN attribute so we know what to do in Attach
Expand Down Expand Up @@ -883,6 +890,14 @@ func (d *SANEconomyStorageDriver) Import(
}
volConfig.Size = extantLUN.Size

if utils.ParseBool(volConfig.LUKSEncryption) {
newSize, err := subtractUintFromSizeString(volConfig.Size, utils.LUKSMetadataSize)
if err != nil {
return err
}
volConfig.Size = newSize
}

if volConfig.ImportNotManaged {
// Volume/LUN import is not managed by Trident
if !strings.HasPrefix(flexvol.Name, d.FlexvolNamePrefix()) {
Expand Down Expand Up @@ -2152,6 +2167,10 @@ func (d *SANEconomyStorageDriver) Resize(ctx context.Context, volConfig *storage
Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace(">>>> Resize")
defer Logd(ctx, d.Name(), d.Config.DebugTraceFlags["method"]).WithFields(fields).Trace("<<<< Resize")

// Add a constant overhead for LUKS volumes to account for LUKS metadata. This overhead is
// part of the LUN but is not reported to the orchestrator.
sizeBytes = incrementWithLUKSMetadataIfLUKSEnabled(ctx, sizeBytes, volConfig.LUKSEncryption)

// Generic user-facing message
resizeError := errors.New("storage driver failed to resize the volume")

Expand Down Expand Up @@ -2251,6 +2270,8 @@ func (d *SANEconomyStorageDriver) Resize(ctx context.Context, volConfig *storage
Logc(ctx).WithField("error", err).Error("LUN resize failed.")
return fmt.Errorf("volume resize failed")
}
// LUKS metadata is not reported to orchestrator
returnSize = decrementWithLUKSMetadataIfLUKSEnabled(ctx, returnSize, volConfig.LUKSEncryption)

volConfig.Size = strconv.FormatUint(returnSize, 10)

Expand Down
3 changes: 3 additions & 0 deletions utils/devices.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@ import (
"github.com/netapp/trident/utils/errors"
)

// LUKS2 requires ~16MiB for overhead. Default to 18MiB just in case.
const LUKSMetadataSize = 18874368

const (
luksDevicePrefix = "luks-"
devPrefix = "/dev/"
Expand Down
6 changes: 6 additions & 0 deletions utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -1089,6 +1089,12 @@ func GetFormattedBool(value string) (string, error) {
return strconv.FormatBool(valBool), nil
}

// ParseBool wraps strconv.ParseBool to suppress errors. Returns false if strconv.ParseBool would return an error.
func ParseBool(b string) bool {
v, _ := strconv.ParseBool(b)
return v
}

func shiftTextRight(text string, count int) string {
if text == "" {
return ""
Expand Down

0 comments on commit 4f24ac7

Please sign in to comment.