Skip to content

Commit 3c03339

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #119 from zhucan/68
fix TODO use time.Time for createSnapshot & solute conflict
2 parents a1b3374 + 26f95af commit 3c03339

File tree

8 files changed

+120
-128
lines changed

8 files changed

+120
-128
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: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1291,7 +1291,7 @@ type listCall struct {
12911291
snapshotID string
12921292
// information to return
12931293
readyToUse bool
1294-
createTime int64
1294+
createTime time.Time
12951295
size int64
12961296
err error
12971297
}
@@ -1309,12 +1309,12 @@ type createCall struct {
13091309
parameters map[string]string
13101310
secrets map[string]string
13111311
// information to return
1312-
driverName string
1313-
snapshotId string
1314-
timestamp int64
1315-
size int64
1316-
readyToUse bool
1317-
err error
1312+
driverName string
1313+
snapshotId string
1314+
creationTime time.Time
1315+
size int64
1316+
readyToUse bool
1317+
err error
13181318
}
13191319

13201320
// Fake SnapShotter implementation that check that Attach/Detach is called
@@ -1329,10 +1329,10 @@ type fakeSnapshotter struct {
13291329
t *testing.T
13301330
}
13311331

1332-
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) {
1332+
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) {
13331333
if f.createCallCounter >= len(f.createCalls) {
13341334
f.t.Errorf("Unexpected CSI Create Snapshot call: snapshotName=%s, volume=%v, index: %d, calls: %+v", snapshotName, volume.Name, f.createCallCounter, f.createCalls)
1335-
return "", "", 0, 0, false, fmt.Errorf("unexpected call")
1335+
return "", "", time.Time{}, 0, false, fmt.Errorf("unexpected call")
13361336
}
13371337
call := f.createCalls[f.createCallCounter]
13381338
f.createCallCounter++
@@ -1359,10 +1359,10 @@ func (f *fakeSnapshotter) CreateSnapshot(ctx context.Context, snapshotName strin
13591359
}
13601360

13611361
if err != nil {
1362-
return "", "", 0, 0, false, fmt.Errorf("unexpected call")
1362+
return "", "", time.Time{}, 0, false, fmt.Errorf("unexpected call")
13631363
}
13641364

1365-
return call.driverName, call.snapshotId, call.timestamp, call.size, call.readyToUse, call.err
1365+
return call.driverName, call.snapshotId, call.creationTime, call.size, call.readyToUse, call.err
13661366
}
13671367

13681368
func (f *fakeSnapshotter) DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) error {
@@ -1391,10 +1391,10 @@ func (f *fakeSnapshotter) DeleteSnapshot(ctx context.Context, snapshotID string,
13911391
return call.err
13921392
}
13931393

1394-
func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, int64, int64, error) {
1394+
func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, time.Time, int64, error) {
13951395
if f.listCallCounter >= len(f.listCalls) {
13961396
f.t.Errorf("Unexpected CSI list Snapshot call: snapshotID=%s, index: %d, calls: %+v", snapshotID, f.createCallCounter, f.createCalls)
1397-
return false, 0, 0, fmt.Errorf("unexpected call")
1397+
return false, time.Time{}, 0, fmt.Errorf("unexpected call")
13981398
}
13991399
call := f.listCalls[f.listCallCounter]
14001400
f.listCallCounter++
@@ -1406,7 +1406,7 @@ func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID stri
14061406
}
14071407

14081408
if err != nil {
1409-
return false, 0, 0, fmt.Errorf("unexpected call")
1409+
return false, time.Time{}, 0, fmt.Errorf("unexpected call")
14101410
}
14111411

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

‎pkg/controller/snapshot_controller.go‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -563,15 +563,15 @@ func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.Volume
563563

564564
func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshot, error) {
565565
var err error
566-
var timestamp int64
566+
var creationTime time.Time
567567
var size int64
568568
var readyToUse = false
569569
var driverName string
570570
var snapshotID string
571571

572572
if snapshot.Spec.Source == nil {
573573
klog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: checking whether snapshot [%s] is pre-bound to content [%s]", snapshot.Name, content.Name)
574-
readyToUse, timestamp, size, err = ctrl.handler.GetSnapshotStatus(content)
574+
readyToUse, creationTime, size, err = ctrl.handler.GetSnapshotStatus(content)
575575
if err != nil {
576576
klog.Errorf("checkandUpdateBoundSnapshotStatusOperation: failed to call get snapshot status to check whether snapshot is ready to use %q", err)
577577
return nil, err
@@ -584,18 +584,18 @@ func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(sn
584584
if err != nil {
585585
return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
586586
}
587-
driverName, snapshotID, timestamp, size, readyToUse, err = ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
587+
driverName, snapshotID, creationTime, size, readyToUse, err = ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
588588
if err != nil {
589589
klog.Errorf("checkandUpdateBoundSnapshotStatusOperation: failed to call create snapshot to check whether the snapshot is ready to use %q", err)
590590
return nil, err
591591
}
592592
}
593-
klog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: driver %s, snapshotId %s, timestamp %d, size %d, readyToUse %t", driverName, snapshotID, timestamp, size, readyToUse)
593+
klog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: driver %s, snapshotId %s, creationTime %v, size %d, readyToUse %t", driverName, snapshotID, creationTime, size, readyToUse)
594594

595-
if timestamp == 0 {
596-
timestamp = time.Now().UnixNano()
595+
if creationTime.IsZero() {
596+
creationTime = time.Now()
597597
}
598-
newSnapshot, err := ctrl.updateSnapshotStatus(snapshot, readyToUse, timestamp, size, IsSnapshotBound(snapshot, content))
598+
newSnapshot, err := ctrl.updateSnapshotStatus(snapshot, readyToUse, creationTime, size, IsSnapshotBound(snapshot, content))
599599
if err != nil {
600600
return nil, err
601601
}
@@ -632,17 +632,18 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
632632
return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err)
633633
}
634634

635-
driverName, snapshotID, timestamp, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
635+
driverName, snapshotID, creationTime, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials)
636636
if err != nil {
637637
return nil, fmt.Errorf("failed to take snapshot of the volume, %s: %q", volume.Name, err)
638638
}
639-
klog.V(5).Infof("Created snapshot: driver %s, snapshotId %s, timestamp %d, size %d, readyToUse %t", driverName, snapshotID, timestamp, size, readyToUse)
639+
640+
klog.V(5).Infof("Created snapshot: driver %s, snapshotId %s, creationTime %v, size %d, readyToUse %t", driverName, snapshotID, creationTime, size, readyToUse)
640641

641642
var newSnapshot *crdv1.VolumeSnapshot
642-
// Update snapshot status with timestamp
643+
// Update snapshot status with creationTime
643644
for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ {
644645
klog.V(5).Infof("createSnapshot [%s]: trying to update snapshot creation timestamp", snapshotKey(snapshot))
645-
newSnapshot, err = ctrl.updateSnapshotStatus(snapshot, readyToUse, timestamp, size, false)
646+
newSnapshot, err = ctrl.updateSnapshotStatus(snapshot, readyToUse, creationTime, size, false)
646647
if err == nil {
647648
break
648649
}
@@ -666,6 +667,7 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum
666667
class.DeletionPolicy = new(crdv1.DeletionPolicy)
667668
*class.DeletionPolicy = crdv1.VolumeSnapshotContentDelete
668669
}
670+
timestamp := creationTime.UnixNano()
669671
snapshotContent := &crdv1.VolumeSnapshotContent{
670672
ObjectMeta: metav1.ObjectMeta{
671673
Name: contentName,
@@ -819,12 +821,12 @@ func (ctrl *csiSnapshotController) updateSnapshotContentSize(content *crdv1.Volu
819821
}
820822

821823
// UpdateSnapshotStatus converts snapshot status to crdv1.VolumeSnapshotCondition
822-
func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSnapshot, readyToUse bool, createdAt, size int64, bound bool) (*crdv1.VolumeSnapshot, error) {
824+
func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSnapshot, readyToUse bool, createdAt time.Time, size int64, bound bool) (*crdv1.VolumeSnapshot, error) {
823825
klog.V(5).Infof("updating VolumeSnapshot[]%s, readyToUse %v, timestamp %v", snapshotKey(snapshot), readyToUse, createdAt)
824826
status := snapshot.Status
825827
change := false
826828
timeAt := &metav1.Time{
827-
Time: time.Unix(0, createdAt),
829+
Time: createdAt,
828830
}
829831

830832
snapshotClone := snapshot.DeepCopy()

0 commit comments

Comments
 (0)