Skip to content

Commit

Permalink
Return correct CSI snapshot size
Browse files Browse the repository at this point in the history
Snapshots now have the restore size set to
the lower of the requested size of the volume
and the actual snapshot size.

Closes #611
  • Loading branch information
ameade authored Aug 11, 2021
1 parent fd2bec7 commit 40b3d1b
Show file tree
Hide file tree
Showing 8 changed files with 84 additions and 16 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:**

Expand Down
25 changes: 24 additions & 1 deletion frontend/csi/controller_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: &timestamp.Timestamp{Seconds: createdSeconds.Unix()},
Expand Down
26 changes: 26 additions & 0 deletions storage_drivers/ontap/api/ontap_zapi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {

Expand Down
7 changes: 3 additions & 4 deletions storage_drivers/ontap/ontap_nas.go
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,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)
}

// GetSnapshots returns the list of snapshots associated with the specified volume
Expand All @@ -616,8 +616,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
Expand All @@ -639,7 +638,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.
Expand Down
6 changes: 3 additions & 3 deletions storage_drivers/ontap/ontap_nas_flexgroup.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

// GetSnapshots returns the list of snapshots associated with the specified volume
Expand All @@ -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
Expand All @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions storage_drivers/ontap/ontap_san.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
23 changes: 17 additions & 6 deletions utils/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -471,8 +471,7 @@ func TestGetNFSVersionFromMountOptions(t *testing.T) {
}
}

const multipathConf =
`
const multipathConf = `
defaults {
user_friendly_names yes
find_multipaths no
Expand Down Expand Up @@ -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)
Expand All @@ -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))
}

0 comments on commit 40b3d1b

Please sign in to comment.