Skip to content

Commit 01df026

Browse files
jingxu97jingxu97
committed
Fix updating restore size issue
This PR fixed the issue of not updating the snapshot restore size after snapshot is created. Before snapshot is ready, the returned size might not be accurate. So we need to keep updating the snapshot size during checking the snapshot status.
1 parent 1f658f2 commit 01df026

File tree

4 files changed

+30
-23
lines changed

4 files changed

+30
-23
lines changed

‎pkg/connection/connection.go‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ type CSIConnection interface {
5151
// DeleteSnapshot deletes a snapshot from a volume
5252
DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) (err error)
5353

54-
// GetSnapshotStatus lists snapshot from a volume
55-
GetSnapshotStatus(ctx context.Context, snapshotID string) (*csi.SnapshotStatus, int64, error)
54+
// GetSnapshotStatus returns a snapshot's status, creation time, and restore size.
55+
GetSnapshotStatus(ctx context.Context, snapshotID string) (*csi.SnapshotStatus, int64, int64, error)
5656

5757
// Probe checks that the CSI driver is ready to process requests
5858
Probe(ctx context.Context) error
@@ -232,7 +232,7 @@ func (c *csiConnection) DeleteSnapshot(ctx context.Context, snapshotID string, s
232232
return nil
233233
}
234234

235-
func (c *csiConnection) GetSnapshotStatus(ctx context.Context, snapshotID string) (*csi.SnapshotStatus, int64, error) {
235+
func (c *csiConnection) GetSnapshotStatus(ctx context.Context, snapshotID string) (*csi.SnapshotStatus, int64, int64, error) {
236236
client := csi.NewControllerClient(c.conn)
237237

238238
req := csi.ListSnapshotsRequest{
@@ -241,14 +241,14 @@ func (c *csiConnection) GetSnapshotStatus(ctx context.Context, snapshotID string
241241

242242
rsp, err := client.ListSnapshots(ctx, &req)
243243
if err != nil {
244-
return nil, 0, err
244+
return nil, 0, 0, err
245245
}
246246

247247
if rsp.Entries == nil || len(rsp.Entries) == 0 {
248-
return nil, 0, fmt.Errorf("can not find snapshot for snapshotID %s", snapshotID)
248+
return nil, 0, 0, fmt.Errorf("can not find snapshot for snapshotID %s", snapshotID)
249249
}
250250

251-
return rsp.Entries[0].Snapshot.Status, rsp.Entries[0].Snapshot.CreatedAt, nil
251+
return rsp.Entries[0].Snapshot.Status, rsp.Entries[0].Snapshot.CreatedAt, rsp.Entries[0].Snapshot.SizeBytes, nil
252252
}
253253

254254
func (c *csiConnection) Close() error {

‎pkg/connection/connection_test.go‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -658,6 +658,7 @@ func TestDeleteSnapshot(t *testing.T) {
658658
func TestGetSnapshotStatus(t *testing.T) {
659659
defaultID := "testid"
660660
createdAt := time.Now().UnixNano()
661+
size := int64(1000)
661662

662663
defaultRequest := &csi.ListSnapshotsRequest{
663664
SnapshotId: defaultID,
@@ -668,7 +669,7 @@ func TestGetSnapshotStatus(t *testing.T) {
668669
{
669670
Snapshot: &csi.Snapshot{
670671
Id: defaultID,
671-
SizeBytes: 1000,
672+
SizeBytes: size,
672673
SourceVolumeId: "volumeid",
673674
CreatedAt: createdAt,
674675
Status: &csi.SnapshotStatus{
@@ -689,6 +690,7 @@ func TestGetSnapshotStatus(t *testing.T) {
689690
expectError bool
690691
expectStatus *csi.SnapshotStatus
691692
expectCreateAt int64
693+
expectSize int64
692694
}{
693695
{
694696
name: "success",
@@ -701,6 +703,7 @@ func TestGetSnapshotStatus(t *testing.T) {
701703
Details: "success",
702704
},
703705
expectCreateAt: createdAt,
706+
expectSize: size,
704707
},
705708
{
706709
name: "gRPC transient error",
@@ -741,7 +744,7 @@ func TestGetSnapshotStatus(t *testing.T) {
741744
controllerServer.EXPECT().ListSnapshots(gomock.Any(), in).Return(out, injectedErr).Times(1)
742745
}
743746

744-
status, createTime, err := csiConn.GetSnapshotStatus(context.Background(), test.snapshotID)
747+
status, createTime, size, err := csiConn.GetSnapshotStatus(context.Background(), test.snapshotID)
745748
if test.expectError && err == nil {
746749
t.Errorf("test %q: Expected error, got none", test.name)
747750
}
@@ -754,6 +757,9 @@ func TestGetSnapshotStatus(t *testing.T) {
754757
if test.expectCreateAt != createTime {
755758
t.Errorf("test %q: expected createTime: %v, got: %v", test.name, test.expectCreateAt, createTime)
756759
}
760+
if test.expectedSize != size {
761+
t.Errorf("test %q: expected size: %v, got: %v", test.name, test.expectSize, createTime)
762+
}
757763
}
758764
}
759765

‎pkg/controller/csi_handler.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import (
3232
type Handler interface {
3333
CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, int64, int64, *csi.SnapshotStatus, error)
3434
DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error
35-
GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (*csi.SnapshotStatus, int64, error)
35+
GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (*csi.SnapshotStatus,int64, int64, error)
3636
}
3737

3838
// csiHandler is a handler that calls CSI to create/delete volume snapshot.
@@ -84,18 +84,19 @@ func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent,
8484
return nil
8585
}
8686

87-
func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (*csi.SnapshotStatus, int64, error) {
87+
func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (*csi.SnapshotStatus, int64, int64, error) {
8888
if content.Spec.CSI == nil {
89-
return nil, 0, fmt.Errorf("CSISnapshot not defined in spec")
89+
return nil, 0, 0, fmt.Errorf("CSISnapshot not defined in spec")
9090
}
9191
ctx, cancel := context.WithTimeout(context.Background(), handler.timeout)
9292
defer cancel()
9393

94-
csiSnapshotStatus, timestamp, err := handler.csiConnection.GetSnapshotStatus(ctx, content.Spec.CSI.SnapshotHandle)
94+
csiSnapshotStatus, timestamp, size, err := handler.csiConnection.GetSnapshotStatus(ctx, content.Spec.CSI.SnapshotHandle)
9595
if err != nil {
96-
return nil, 0, fmt.Errorf("failed to list snapshot data %s: %q", content.Name, err)
96+
return nil, 0, 0, fmt.Errorf("failed to list snapshot data %s: %q", content.Name, err)
9797
}
98-
return csiSnapshotStatus, timestamp, nil
98+
return csiSnapshotStatus, timestamp, size, nil
99+
99100
}
100101

101102
func makeSnapshotName(prefix, snapshotUID string, snapshotNameUUIDLength int) (string, error) {

‎pkg/controller/snapshot_controller.go‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -425,12 +425,12 @@ func (ctrl *csiSnapshotController) checkandBindSnapshotContent(snapshot *crdv1.V
425425
}
426426

427427
func (ctrl *csiSnapshotController) checkandUpdateSnapshotStatusOperation(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshot, error) {
428-
status, _, err := ctrl.handler.GetSnapshotStatus(content)
428+
status, _, size, err := ctrl.handler.GetSnapshotStatus(content)
429429
if err != nil {
430430
return nil, fmt.Errorf("failed to check snapshot status %s with error %v", snapshot.Name, err)
431431
}
432-
433-
newSnapshot, err := ctrl.updateSnapshotStatus(snapshot, status, time.Now(), nil, IsSnapshotBound(snapshot, content))
432+
timestamp := time.Now().UnixNano()
433+
newSnapshot, err := ctrl.updateSnapshotStatus(snapshot, status, timestamp, size, IsSnapshotBound(snapshot, content))
434434
if err != nil {
435435
return nil, err
436436
}
@@ -494,7 +494,7 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
494494
// Update snapshot status with timestamp
495495
for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ {
496496
glog.V(5).Infof("createSnapshot [%s]: trying to update snapshot creation timestamp", snapshotKey(snapshot))
497-
newSnapshot, err = ctrl.updateSnapshotStatus(snapshot, csiSnapshotStatus, time.Unix(0, timestamp), resource.NewQuantity(size, resource.BinarySI), false)
497+
newSnapshot, err = ctrl.updateSnapshotStatus(snapshot, csiSnapshotStatus, timestamp, size, false)
498498
if err == nil {
499499
break
500500
}
@@ -642,12 +642,12 @@ func (ctrl *csiSnapshotController) bindandUpdateVolumeSnapshot(snapshotContent *
642642
}
643643

644644
// UpdateSnapshotStatus converts snapshot status to crdv1.VolumeSnapshotCondition
645-
func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSnapshot, csistatus *csi.SnapshotStatus, timestamp time.Time, size *resource.Quantity, bound bool) (*crdv1.VolumeSnapshot, error) {
646-
glog.V(5).Infof("updating VolumeSnapshot[]%s, set status %v, timestamp %v", snapshotKey(snapshot), csistatus, timestamp)
645+
func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSnapshot, csistatus *csi.SnapshotStatus, createdAt, size int64, bound bool) (*crdv1.VolumeSnapshot, error) {
646+
glog.V(5).Infof("updating VolumeSnapshot[]%s, set status %v, timestamp %v", snapshotKey(snapshot), csistatus, createdAt)
647647
status := snapshot.Status
648648
change := false
649649
timeAt := &metav1.Time{
650-
Time: timestamp,
650+
Time: time.Unix(0, createdAt),
651651
}
652652

653653
snapshotClone := snapshot.DeepCopy()
@@ -680,8 +680,8 @@ func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSn
680680
}
681681
}
682682
if change {
683-
if size != nil {
684-
status.RestoreSize = size
683+
if size > 0 {
684+
status.RestoreSize = resource.NewQuantity(size, resource.BinarySI)
685685
}
686686
snapshotClone.Status = status
687687
newSnapshotObj, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshots(snapshotClone.Namespace).Update(snapshotClone)

0 commit comments

Comments
 (0)