Skip to content

Commit 26f95af

Browse files
zhucanzhucan
committed
fix TODO use time.Time for createSnapshot & solute conflict
1 parent ac33959 commit 26f95af

File tree

8 files changed

+121
-129
lines changed

8 files changed

+121
-129
lines changed

‎Gopkg.lock‎

Lines changed: 0 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/controller/csi_handler.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ import (
3030

3131
// Handler is responsible for handling VolumeSnapshot events from informer.
3232
type Handler interface {
33-
CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, int64, int64, bool, error)
33+
CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, time.Time, int64, bool, error)
3434
DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error
35-
GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (bool, int64, int64, error)
35+
GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (bool, time.Time, int64, error)
3636
}
3737

3838
// csiHandler is a handler that calls CSI to create/delete volume snapshot.
@@ -58,18 +58,18 @@ func NewCSIHandler(
5858
}
5959
}
6060

61-
func (handler *csiHandler) CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, int64, int64, bool, error) {
61+
func (handler *csiHandler) CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, time.Time, int64, bool, error) {
6262

6363
ctx, cancel := context.WithTimeout(context.Background(), handler.timeout)
6464
defer cancel()
6565

6666
snapshotName, err := makeSnapshotName(handler.snapshotNamePrefix, string(snapshot.UID), handler.snapshotNameUUIDLength)
6767
if err != nil {
68-
return "", "", 0, 0, false, err
68+
return "", "", time.Time{}, 0, false, err
6969
}
7070
newParameters, err := removePrefixedParameters(parameters)
7171
if err != nil {
72-
return "", "", 0, 0, false, fmt.Errorf("failed to remove CSI Parameters of prefixed keys: %v", err)
72+
return "", "", time.Time{}, 0, false, fmt.Errorf("failed to remove CSI Parameters of prefixed keys: %v", err)
7373
}
7474
return handler.snapshotter.CreateSnapshot(ctx, snapshotName, volume, newParameters, snapshotterCredentials)
7575
}
@@ -89,16 +89,16 @@ func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent,
8989
return nil
9090
}
9191

92-
func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (bool, int64, int64, error) {
92+
func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (bool, time.Time, int64, error) {
9393
if content.Spec.CSI == nil {
94-
return false, 0, 0, fmt.Errorf("CSISnapshot not defined in spec")
94+
return false, time.Time{}, 0, fmt.Errorf("CSISnapshot not defined in spec")
9595
}
9696
ctx, cancel := context.WithTimeout(context.Background(), handler.timeout)
9797
defer cancel()
9898

9999
csiSnapshotStatus, timestamp, size, err := handler.snapshotter.GetSnapshotStatus(ctx, content.Spec.CSI.SnapshotHandle)
100100
if err != nil {
101-
return false, 0, 0, fmt.Errorf("failed to list snapshot content %s: %q", content.Name, err)
101+
return false, time.Time{}, 0, fmt.Errorf("failed to list snapshot content %s: %q", content.Name, err)
102102
}
103103

104104
return csiSnapshotStatus, timestamp, size, nil

‎pkg/controller/framework_test.go‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"errors"
2222
"fmt"
2323
"reflect"
24+
sysruntime "runtime"
2425
"strconv"
2526
"strings"
2627
"sync"
@@ -1126,7 +1127,7 @@ type listCall struct {
11261127
snapshotID string
11271128
// information to return
11281129
readyToUse bool
1129-
createTime int64
1130+
createTime time.Time
11301131
size int64
11311132
err error
11321133
}
@@ -1144,12 +1145,12 @@ type createCall struct {
11441145
parameters map[string]string
11451146
secrets map[string]string
11461147
// information to return
1147-
driverName string
1148-
snapshotId string
1149-
timestamp int64
1150-
size int64
1151-
readyToUse bool
1152-
err error
1148+
driverName string
1149+
snapshotId string
1150+
creationTime time.Time
1151+
size int64
1152+
readyToUse bool
1153+
err error
11531154
}
11541155

11551156
// Fake SnapShotter implementation that check that Attach/Detach is called
@@ -1164,10 +1165,10 @@ type fakeSnapshotter struct {
11641165
t *testing.T
11651166
}
11661167

1167-
func (f *fakeSnapshotter) CreateSnapshot(ctx context.Context, snapshotName string, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, int64, int64, bool, error) {
1168+
func (f *fakeSnapshotter) CreateSnapshot(ctx context.Context, snapshotName string, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, time.Time, int64, bool, error) {
11681169
if f.createCallCounter >= len(f.createCalls) {
11691170
f.t.Errorf("Unexpected CSI Create Snapshot call: snapshotName=%s, volume=%v, index: %d, calls: %+v", snapshotName, volume.Name, f.createCallCounter, f.createCalls)
1170-
return "", "", 0, 0, false, fmt.Errorf("unexpected call")
1171+
return "", "", time.Time{}, 0, false, fmt.Errorf("unexpected call")
11711172
}
11721173
call := f.createCalls[f.createCallCounter]
11731174
f.createCallCounter++
@@ -1194,10 +1195,10 @@ func (f *fakeSnapshotter) CreateSnapshot(ctx context.Context, snapshotName strin
11941195
}
11951196

11961197
if err != nil {
1197-
return "", "", 0, 0, false, fmt.Errorf("unexpected call")
1198+
return "", "", time.Time{}, 0, false, fmt.Errorf("unexpected call")
11981199
}
11991200

1200-
return call.driverName, call.snapshotId, call.timestamp, call.size, call.readyToUse, call.err
1201+
return call.driverName, call.snapshotId, call.creationTime, call.size, call.readyToUse, call.err
12011202
}
12021203

12031204
func (f *fakeSnapshotter) DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) error {
@@ -1226,10 +1227,10 @@ func (f *fakeSnapshotter) DeleteSnapshot(ctx context.Context, snapshotID string,
12261227
return call.err
12271228
}
12281229

1229-
func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, int64, int64, error) {
1230+
func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, time.Time, int64, error) {
12301231
if f.listCallCounter >= len(f.listCalls) {
12311232
f.t.Errorf("Unexpected CSI list Snapshot call: snapshotID=%s, index: %d, calls: %+v", snapshotID, f.createCallCounter, f.createCalls)
1232-
return false, 0, 0, fmt.Errorf("unexpected call")
1233+
return false, time.Time{}, 0, fmt.Errorf("unexpected call")
12331234
}
12341235
call := f.listCalls[f.listCallCounter]
12351236
f.listCallCounter++
@@ -1241,7 +1242,7 @@ func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID stri
12411242
}
12421243

12431244
if err != nil {
1244-
return false, 0, 0, fmt.Errorf("unexpected call")
1245+
return false, time.Time{}, 0, fmt.Errorf("unexpected call")
12451246
}
12461247

12471248
return call.readyToUse, call.createTime, call.size, call.err

‎pkg/controller/snapshot_controller.go‎

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,6 @@ func (ctrl *csiSnapshotController) createSnapshot(snapshot *crdv1.VolumeSnapshot
367367
// We will get an "snapshot update" event soon, this is not a big error
368368
klog.V(4).Infof("createSnapshot [%s]: cannot update internal cache: %v", snapshotKey(snapshotObj), updateErr)
369369
}
370-
371370
return nil
372371
})
373372
return nil
@@ -556,15 +555,15 @@ func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.Volume
556555

557556
func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshot, error) {
558557
var err error
559-
var timestamp int64
558+
var creationTime time.Time
560559
var size int64
561560
var readyToUse = false
562561
var driverName string
563562
var snapshotID string
564563

565564
if snapshot.Spec.Source == nil {
566565
klog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: checking whether snapshot [%s] is pre-bound to content [%s]", snapshot.Name, content.Name)
567-
readyToUse, timestamp, size, err = ctrl.handler.GetSnapshotStatus(content)
566+
readyToUse, creationTime, size, err = ctrl.handler.GetSnapshotStatus(content)
568567
if err != nil {
569568
klog.Errorf("checkandUpdateBoundSnapshotStatusOperation: failed to call get snapshot status to check whether snapshot is ready to use %q", err)
570569
return nil, err
@@ -577,18 +576,18 @@ func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(sn
577576
if err != nil {
578577
return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
579578
}
580-
driverName, snapshotID, timestamp, size, readyToUse, err = ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
579+
driverName, snapshotID, creationTime, size, readyToUse, err = ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
581580
if err != nil {
582581
klog.Errorf("checkandUpdateBoundSnapshotStatusOperation: failed to call create snapshot to check whether the snapshot is ready to use %q", err)
583582
return nil, err
584583
}
585584
}
586-
klog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: driver %s, snapshotId %s, timestamp %d, size %d, readyToUse %t", driverName, snapshotID, timestamp, size, readyToUse)
585+
klog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: driver %s, snapshotId %s, creationTime %v, size %d, readyToUse %t", driverName, snapshotID, creationTime, size, readyToUse)
587586

588-
if timestamp == 0 {
589-
timestamp = time.Now().UnixNano()
587+
if creationTime.IsZero() {
588+
creationTime = time.Now()
590589
}
591-
newSnapshot, err := ctrl.updateSnapshotStatus(snapshot, readyToUse, timestamp, size, IsSnapshotBound(snapshot, content))
590+
newSnapshot, err := ctrl.updateSnapshotStatus(snapshot, readyToUse, creationTime, size, IsSnapshotBound(snapshot, content))
592591
if err != nil {
593592
return nil, err
594593
}
@@ -617,17 +616,18 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
617616
return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
618617
}
619618

620-
driverName, snapshotID, timestamp, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
619+
driverName, snapshotID, creationTime, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
621620
if err != nil {
622621
return nil, fmt.Errorf("failed to take snapshot of the volume, %s: %q", volume.Name, err)
623622
}
624-
klog.V(5).Infof("Created snapshot: driver %s, snapshotId %s, timestamp %d, size %d, readyToUse %t", driverName, snapshotID, timestamp, size, readyToUse)
623+
624+
klog.V(5).Infof("Created snapshot: driver %s, snapshotId %s, creationTime %v, size %d, readyToUse %t", driverName, snapshotID, creationTime, size, readyToUse)
625625

626626
var newSnapshot *crdv1.VolumeSnapshot
627-
// Update snapshot status with timestamp
627+
// Update snapshot status with creationTime
628628
for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ {
629629
klog.V(5).Infof("createSnapshot [%s]: trying to update snapshot creation timestamp", snapshotKey(snapshot))
630-
newSnapshot, err = ctrl.updateSnapshotStatus(snapshot, readyToUse, timestamp, size, false)
630+
newSnapshot, err = ctrl.updateSnapshotStatus(snapshot, readyToUse, creationTime, size, false)
631631
if err == nil {
632632
break
633633
}
@@ -651,6 +651,7 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
651651
class.DeletionPolicy = new(crdv1.DeletionPolicy)
652652
*class.DeletionPolicy = crdv1.VolumeSnapshotContentDelete
653653
}
654+
timestamp := creationTime.UnixNano()
654655
snapshotContent := &crdv1.VolumeSnapshotContent{
655656
ObjectMeta: metav1.ObjectMeta{
656657
Name: contentName,
@@ -804,12 +805,12 @@ func (ctrl *csiSnapshotController) updateSnapshotContentSize(content *crdv1.Volu
804805
}
805806

806807
// UpdateSnapshotStatus converts snapshot status to crdv1.VolumeSnapshotCondition
807-
func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSnapshot, readyToUse bool, createdAt, size int64, bound bool) (*crdv1.VolumeSnapshot, error) {
808+
func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSnapshot, readyToUse bool, createdAt time.Time, size int64, bound bool) (*crdv1.VolumeSnapshot, error) {
808809
klog.V(5).Infof("updating VolumeSnapshot[]%s, readyToUse %v, timestamp %v", snapshotKey(snapshot), readyToUse, createdAt)
809810
status := snapshot.Status
810811
change := false
811812
timeAt := &metav1.Time{
812-
Time: time.Unix(0, createdAt),
813+
Time: createdAt,
813814
}
814815

815816
snapshotClone := snapshot.DeepCopy()

0 commit comments

Comments
 (0)