From 97bde72f6071611bc0f04f8a9d9cd5775468e948 Mon Sep 17 00:00:00 2001 From: ameade <847570+ameade@users.noreply.github.com> Date: Tue, 10 Aug 2021 21:13:10 -0400 Subject: [PATCH] Return correct CSI snapshot size Snapshots now have the restore size set to the lower of the requested size of the volume and the actual snapshot size. Closes #611 --- CHANGELOG.md | 1 + frontend/csi/controller_server.go | 25 ++++++++++++++++++- storage_drivers/ontap/api/ontap_zapi.go | 26 ++++++++++++++++++++ storage_drivers/ontap/ontap_nas.go | 7 +++--- storage_drivers/ontap/ontap_nas_flexgroup.go | 6 ++--- storage_drivers/ontap/ontap_san.go | 4 +-- utils/utils.go | 8 ++++++ utils/utils_test.go | 23 ++++++++++++----- 8 files changed, 84 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 54cc00d94..ef4faeb60 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ **Fixes:** - Fixed custom YAML installer issue with different image (Issue [#613](https://github.com/NetApp/trident/issues/613)). +- Fixed snapshot size calculation (Issue [#611](https://github.com/NetApp/trident/issues/611)). **Enhancements:** diff --git a/frontend/csi/controller_server.go b/frontend/csi/controller_server.go index fd43f9dba..36d02fe49 100644 --- a/frontend/csi/controller_server.go +++ b/frontend/csi/controller_server.go @@ -906,9 +906,32 @@ func (p *Plugin) getCSISnapshotFromTridentSnapshot( Logc(ctx).WithField("time", snapshot.Created).Error("Could not parse RFC3339 snapshot time.") createdSeconds = time.Now() } + volume, err := p.orchestrator.GetVolume(ctx, snapshot.Config.VolumeName) + if err != nil { + Logc(ctx).WithFields(log.Fields{ + "volume": snapshot.Config.VolumeName, + }).Error("Could not find volume.") + return nil, err + } + volCapacityString, err := utils.ConvertSizeToBytes(volume.Config.Size) + if err != nil { + Logc(ctx).WithFields(log.Fields{ + "volume": volume.Config.InternalName, + "size": volume.Config.Size, + }).Error("Could not parse volume size.") + return nil, err + } + volCapacity, err := strconv.ParseInt(volCapacityString, 10, 64) + if err != nil { + Logc(ctx).WithFields(log.Fields{ + "volume": volume.Config.InternalName, + "size": volume.Config.Size, + }).Error("Could not parse volume size.") + return nil, err + } return &csi.Snapshot{ - SizeBytes: snapshot.SizeBytes, + SizeBytes: utils.MinInt64(snapshot.SizeBytes, volCapacity), SnapshotId: storage.MakeSnapshotID(snapshot.Config.VolumeName, snapshot.Config.Name), SourceVolumeId: snapshot.Config.VolumeName, CreationTime: ×tamp.Timestamp{Seconds: createdSeconds.Unix()}, diff --git a/storage_drivers/ontap/api/ontap_zapi.go b/storage_drivers/ontap/api/ontap_zapi.go index 09923b0ee..a6eb1a2b5 100644 --- a/storage_drivers/ontap/api/ontap_zapi.go +++ b/storage_drivers/ontap/api/ontap_zapi.go @@ -1035,6 +1035,20 @@ func (d Client) FlexGroupExists(ctx context.Context, name string) (bool, error) return true, nil } +// FlexGroupUsedSize retrieves the used space of the specified volume +func (d Client) FlexGroupUsedSize(name string) (int, error) { + volAttrs, err := d.FlexGroupGet(name) + if err != nil { + return 0, err + } + if volAttrs == nil { + return 0, fmt.Errorf("error getting used space for FlexGroup: %v", name) + } + + volSpaceAttrs := volAttrs.VolumeSpaceAttributes() + return volSpaceAttrs.SizeUsed() - volSpaceAttrs.SizeUsedBySnapshots(), nil +} + // FlexGroupSize retrieves the size of the specified volume func (d Client) FlexGroupSize(name string) (int, error) { volAttrs, err := d.FlexGroupGet(name) @@ -1482,6 +1496,18 @@ func (d Client) VolumeExists(ctx context.Context, name string) (bool, error) { return true, nil } +// VolumeUsedSize retrieves the used bytes of the specified volume +func (d Client) VolumeUsedSize(name string) (int, error) { + + volAttrs, err := d.VolumeGet(name) + if err != nil { + return 0, err + } + volSpaceAttrs := volAttrs.VolumeSpaceAttributes() + + return volSpaceAttrs.SizeUsed() - volSpaceAttrs.SizeUsedBySnapshots(), nil +} + // VolumeSize retrieves the size of the specified volume func (d Client) VolumeSize(name string) (int, error) { diff --git a/storage_drivers/ontap/ontap_nas.go b/storage_drivers/ontap/ontap_nas.go index 6b8db55dd..b3db62271 100644 --- a/storage_drivers/ontap/ontap_nas.go +++ b/storage_drivers/ontap/ontap_nas.go @@ -598,7 +598,7 @@ func (d *NASStorageDriver) GetSnapshot(ctx context.Context, snapConfig *storage. defer Logc(ctx).WithFields(fields).Debug("<<<< GetSnapshot") } - return GetSnapshot(ctx, snapConfig, &d.Config, d.API, d.API.VolumeSize) + return GetSnapshot(ctx, snapConfig, &d.Config, d.API, d.API.VolumeUsedSize) } // Return the list of snapshots associated with the specified volume @@ -615,8 +615,7 @@ func (d *NASStorageDriver) GetSnapshots(ctx context.Context, volConfig *storage. Logc(ctx).WithFields(fields).Debug(">>>> GetSnapshots") defer Logc(ctx).WithFields(fields).Debug("<<<< GetSnapshots") } - - return GetSnapshots(ctx, volConfig, &d.Config, d.API, d.API.VolumeSize) + return GetSnapshots(ctx, volConfig, &d.Config, d.API, d.API.VolumeUsedSize) } // CreateSnapshot creates a snapshot for the given volume @@ -638,7 +637,7 @@ func (d *NASStorageDriver) CreateSnapshot( defer Logc(ctx).WithFields(fields).Debug("<<<< CreateSnapshot") } - return CreateSnapshot(ctx, snapConfig, &d.Config, d.API, d.API.VolumeSize) + return CreateSnapshot(ctx, snapConfig, &d.Config, d.API, d.API.VolumeUsedSize) } // RestoreSnapshot restores a volume (in place) from a snapshot. diff --git a/storage_drivers/ontap/ontap_nas_flexgroup.go b/storage_drivers/ontap/ontap_nas_flexgroup.go index e9dedcb35..8dde01c0d 100644 --- a/storage_drivers/ontap/ontap_nas_flexgroup.go +++ b/storage_drivers/ontap/ontap_nas_flexgroup.go @@ -813,7 +813,7 @@ func (d *NASFlexGroupStorageDriver) GetSnapshot( defer Logc(ctx).WithFields(fields).Debug("<<<< GetSnapshot") } - return GetSnapshot(ctx, snapConfig, &d.Config, d.API, d.API.FlexGroupSize) + return GetSnapshot(ctx, snapConfig, &d.Config, d.API, d.API.FlexGroupUsedSize) } // Return the list of snapshots associated with the specified volume @@ -831,7 +831,7 @@ func (d *NASFlexGroupStorageDriver) GetSnapshots( defer Logc(ctx).WithFields(fields).Debug("<<<< GetSnapshots") } - return GetSnapshots(ctx, volConfig, &d.Config, d.API, d.API.FlexGroupSize) + return GetSnapshots(ctx, volConfig, &d.Config, d.API, d.API.FlexGroupUsedSize) } // CreateSnapshot creates a snapshot for the given volume @@ -853,7 +853,7 @@ func (d *NASFlexGroupStorageDriver) CreateSnapshot( defer Logc(ctx).WithFields(fields).Debug("<<<< CreateSnapshot") } - return CreateSnapshot(ctx, snapConfig, &d.Config, d.API, d.API.FlexGroupSize) + return CreateSnapshot(ctx, snapConfig, &d.Config, d.API, d.API.FlexGroupUsedSize) } // RestoreSnapshot restores a volume (in place) from a snapshot. diff --git a/storage_drivers/ontap/ontap_san.go b/storage_drivers/ontap/ontap_san.go index c732f134f..6bbb04f00 100644 --- a/storage_drivers/ontap/ontap_san.go +++ b/storage_drivers/ontap/ontap_san.go @@ -774,7 +774,7 @@ func (d *SANStorageDriver) GetSnapshot(ctx context.Context, snapConfig *storage. defer Logc(ctx).WithFields(fields).Debug("<<<< GetSnapshot") } - return GetSnapshot(ctx, snapConfig, &d.Config, d.API, d.API.VolumeSize) + return GetSnapshot(ctx, snapConfig, &d.Config, d.API, d.API.LunSize) } // Return the list of snapshots associated with the specified volume @@ -792,7 +792,7 @@ func (d *SANStorageDriver) GetSnapshots(ctx context.Context, volConfig *storage. defer Logc(ctx).WithFields(fields).Debug("<<<< GetSnapshots") } - return GetSnapshots(ctx, volConfig, &d.Config, d.API, d.API.VolumeSize) + return GetSnapshots(ctx, volConfig, &d.Config, d.API, d.API.LunSize) } // CreateSnapshot creates a snapshot for the given volume diff --git a/utils/utils.go b/utils/utils.go index 982fa6713..d8b2b0b22 100644 --- a/utils/utils.go +++ b/utils/utils.go @@ -622,3 +622,11 @@ func SplitString(_ context.Context, s, sep string) []string { return strings.Split(s, sep) } + +// MinInt64 returns the lower of the two integers specified +func MinInt64(a, b int64) int64 { + if a < b { + return a + } + return b +} diff --git a/utils/utils_test.go b/utils/utils_test.go index e7e4f9167..975e18c69 100644 --- a/utils/utils_test.go +++ b/utils/utils_test.go @@ -471,8 +471,7 @@ func TestGetNFSVersionFromMountOptions(t *testing.T) { } } -const multipathConf = -` +const multipathConf = ` defaults { user_friendly_names yes find_multipaths no @@ -553,13 +552,13 @@ func TestSplitString(t *testing.T) { ctx := context.TODO() stringList := SplitString(ctx, "a,b, c", ",") - assert.Equal(t, []string{"a","b"," c"}, stringList) + assert.Equal(t, []string{"a", "b", " c"}, stringList) stringList = SplitString(ctx, "a,b,c", ",") - assert.Equal(t, []string{"a","b","c"}, stringList) + assert.Equal(t, []string{"a", "b", "c"}, stringList) stringList = SplitString(ctx, "a,b,c", "") - assert.Equal(t, []string{"a",",","b",",","c"}, stringList) + assert.Equal(t, []string{"a", ",", "b", ",", "c"}, stringList) stringList = SplitString(ctx, "a,b,c", ";") assert.Equal(t, []string{"a,b,c"}, stringList) @@ -574,5 +573,17 @@ func TestSplitString(t *testing.T) { assert.Equal(t, []string{" "}, stringList) stringList = SplitString(ctx, ";a;b", ";") - assert.Equal(t, []string{"","a","b"}, stringList) + assert.Equal(t, []string{"", "a", "b"}, stringList) +} + +func TestMinInt64(t *testing.T) { + log.Debug("Running TestMinInt64...") + assert.Equal(t, int64(2), MinInt64(2, 3)) + assert.Equal(t, int64(2), MinInt64(3, 2)) + assert.Equal(t, int64(-2), MinInt64(-2, 3)) + assert.Equal(t, int64(-2), MinInt64(3, -2)) + assert.Equal(t, int64(-3), MinInt64(3, -3)) + assert.Equal(t, int64(-3), MinInt64(-3, 3)) + assert.Equal(t, int64(0), MinInt64(0, 0)) + assert.Equal(t, int64(2), MinInt64(2, 2)) }