From 26f95af09c76df419affc59c280ea0835a4a65de Mon Sep 17 00:00:00 2001 From: zhucan Date: Tue, 30 Apr 2019 16:39:12 +0800 Subject: [PATCH] fix TODO use time.Time for createSnapshot & solute conflict --- Gopkg.lock | 2 - pkg/controller/csi_handler.go | 16 ++--- pkg/controller/framework_test.go | 29 ++++---- pkg/controller/snapshot_controller.go | 29 ++++---- pkg/controller/snapshot_create_test.go | 97 +++++++++++++------------- pkg/controller/snapshot_ready_test.go | 30 ++++---- pkg/snapshotter/snapshotter.go | 38 ++++------ pkg/snapshotter/snapshotter_test.go | 9 +-- 8 files changed, 121 insertions(+), 129 deletions(-) diff --git a/Gopkg.lock b/Gopkg.lock index 2c17f2478..95ed8acaf 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -62,7 +62,6 @@ "ptypes", "ptypes/any", "ptypes/duration", - "ptypes/timestamp", "ptypes/wrappers", ] pruneopts = "NUT" @@ -754,7 +753,6 @@ "github.com/container-storage-interface/spec/lib/go/csi", "github.com/golang/mock/gomock", "github.com/golang/protobuf/ptypes", - "github.com/golang/protobuf/ptypes/timestamp", "github.com/kubernetes-csi/csi-lib-utils/connection", "github.com/kubernetes-csi/csi-lib-utils/leaderelection", "github.com/kubernetes-csi/csi-lib-utils/rpc", diff --git a/pkg/controller/csi_handler.go b/pkg/controller/csi_handler.go index 2abee0978..3b3a9000a 100644 --- a/pkg/controller/csi_handler.go +++ b/pkg/controller/csi_handler.go @@ -30,9 +30,9 @@ import ( // Handler is responsible for handling VolumeSnapshot events from informer. type Handler interface { - CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, int64, int64, bool, error) + CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, time.Time, int64, bool, error) DeleteSnapshot(content *crdv1.VolumeSnapshotContent, snapshotterCredentials map[string]string) error - GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (bool, int64, int64, error) + GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (bool, time.Time, int64, error) } // csiHandler is a handler that calls CSI to create/delete volume snapshot. @@ -58,18 +58,18 @@ func NewCSIHandler( } } -func (handler *csiHandler) CreateSnapshot(snapshot *crdv1.VolumeSnapshot, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, int64, int64, bool, error) { +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) { ctx, cancel := context.WithTimeout(context.Background(), handler.timeout) defer cancel() snapshotName, err := makeSnapshotName(handler.snapshotNamePrefix, string(snapshot.UID), handler.snapshotNameUUIDLength) if err != nil { - return "", "", 0, 0, false, err + return "", "", time.Time{}, 0, false, err } newParameters, err := removePrefixedParameters(parameters) if err != nil { - return "", "", 0, 0, false, fmt.Errorf("failed to remove CSI Parameters of prefixed keys: %v", err) + return "", "", time.Time{}, 0, false, fmt.Errorf("failed to remove CSI Parameters of prefixed keys: %v", err) } return handler.snapshotter.CreateSnapshot(ctx, snapshotName, volume, newParameters, snapshotterCredentials) } @@ -89,16 +89,16 @@ func (handler *csiHandler) DeleteSnapshot(content *crdv1.VolumeSnapshotContent, return nil } -func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (bool, int64, int64, error) { +func (handler *csiHandler) GetSnapshotStatus(content *crdv1.VolumeSnapshotContent) (bool, time.Time, int64, error) { if content.Spec.CSI == nil { - return false, 0, 0, fmt.Errorf("CSISnapshot not defined in spec") + return false, time.Time{}, 0, fmt.Errorf("CSISnapshot not defined in spec") } ctx, cancel := context.WithTimeout(context.Background(), handler.timeout) defer cancel() csiSnapshotStatus, timestamp, size, err := handler.snapshotter.GetSnapshotStatus(ctx, content.Spec.CSI.SnapshotHandle) if err != nil { - return false, 0, 0, fmt.Errorf("failed to list snapshot content %s: %q", content.Name, err) + return false, time.Time{}, 0, fmt.Errorf("failed to list snapshot content %s: %q", content.Name, err) } return csiSnapshotStatus, timestamp, size, nil diff --git a/pkg/controller/framework_test.go b/pkg/controller/framework_test.go index a9ad4832e..48a0d28af 100644 --- a/pkg/controller/framework_test.go +++ b/pkg/controller/framework_test.go @@ -21,6 +21,7 @@ import ( "errors" "fmt" "reflect" + sysruntime "runtime" "strconv" "strings" "sync" @@ -1126,7 +1127,7 @@ type listCall struct { snapshotID string // information to return readyToUse bool - createTime int64 + createTime time.Time size int64 err error } @@ -1144,12 +1145,12 @@ type createCall struct { parameters map[string]string secrets map[string]string // information to return - driverName string - snapshotId string - timestamp int64 - size int64 - readyToUse bool - err error + driverName string + snapshotId string + creationTime time.Time + size int64 + readyToUse bool + err error } // Fake SnapShotter implementation that check that Attach/Detach is called @@ -1164,10 +1165,10 @@ type fakeSnapshotter struct { t *testing.T } -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) { +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) { if f.createCallCounter >= len(f.createCalls) { f.t.Errorf("Unexpected CSI Create Snapshot call: snapshotName=%s, volume=%v, index: %d, calls: %+v", snapshotName, volume.Name, f.createCallCounter, f.createCalls) - return "", "", 0, 0, false, fmt.Errorf("unexpected call") + return "", "", time.Time{}, 0, false, fmt.Errorf("unexpected call") } call := f.createCalls[f.createCallCounter] f.createCallCounter++ @@ -1194,10 +1195,10 @@ func (f *fakeSnapshotter) CreateSnapshot(ctx context.Context, snapshotName strin } if err != nil { - return "", "", 0, 0, false, fmt.Errorf("unexpected call") + return "", "", time.Time{}, 0, false, fmt.Errorf("unexpected call") } - return call.driverName, call.snapshotId, call.timestamp, call.size, call.readyToUse, call.err + return call.driverName, call.snapshotId, call.creationTime, call.size, call.readyToUse, call.err } 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, return call.err } -func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, int64, int64, error) { +func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, time.Time, int64, error) { if f.listCallCounter >= len(f.listCalls) { f.t.Errorf("Unexpected CSI list Snapshot call: snapshotID=%s, index: %d, calls: %+v", snapshotID, f.createCallCounter, f.createCalls) - return false, 0, 0, fmt.Errorf("unexpected call") + return false, time.Time{}, 0, fmt.Errorf("unexpected call") } call := f.listCalls[f.listCallCounter] f.listCallCounter++ @@ -1241,7 +1242,7 @@ func (f *fakeSnapshotter) GetSnapshotStatus(ctx context.Context, snapshotID stri } if err != nil { - return false, 0, 0, fmt.Errorf("unexpected call") + return false, time.Time{}, 0, fmt.Errorf("unexpected call") } return call.readyToUse, call.createTime, call.size, call.err diff --git a/pkg/controller/snapshot_controller.go b/pkg/controller/snapshot_controller.go index 3d30844af..28704de22 100644 --- a/pkg/controller/snapshot_controller.go +++ b/pkg/controller/snapshot_controller.go @@ -367,7 +367,6 @@ func (ctrl *csiSnapshotController) createSnapshot(snapshot *crdv1.VolumeSnapshot // We will get an "snapshot update" event soon, this is not a big error klog.V(4).Infof("createSnapshot [%s]: cannot update internal cache: %v", snapshotKey(snapshotObj), updateErr) } - return nil }) return nil @@ -556,7 +555,7 @@ func (ctrl *csiSnapshotController) getCreateSnapshotInput(snapshot *crdv1.Volume func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshot, error) { var err error - var timestamp int64 + var creationTime time.Time var size int64 var readyToUse = false var driverName string @@ -564,7 +563,7 @@ func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(sn if snapshot.Spec.Source == nil { klog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: checking whether snapshot [%s] is pre-bound to content [%s]", snapshot.Name, content.Name) - readyToUse, timestamp, size, err = ctrl.handler.GetSnapshotStatus(content) + readyToUse, creationTime, size, err = ctrl.handler.GetSnapshotStatus(content) if err != nil { klog.Errorf("checkandUpdateBoundSnapshotStatusOperation: failed to call get snapshot status to check whether snapshot is ready to use %q", err) return nil, err @@ -577,18 +576,18 @@ func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(sn if err != nil { return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err) } - driverName, snapshotID, timestamp, size, readyToUse, err = ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials) + driverName, snapshotID, creationTime, size, readyToUse, err = ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials) if err != nil { klog.Errorf("checkandUpdateBoundSnapshotStatusOperation: failed to call create snapshot to check whether the snapshot is ready to use %q", err) return nil, err } } - klog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: driver %s, snapshotId %s, timestamp %d, size %d, readyToUse %t", driverName, snapshotID, timestamp, size, readyToUse) + klog.V(5).Infof("checkandUpdateBoundSnapshotStatusOperation: driver %s, snapshotId %s, creationTime %v, size %d, readyToUse %t", driverName, snapshotID, creationTime, size, readyToUse) - if timestamp == 0 { - timestamp = time.Now().UnixNano() + if creationTime.IsZero() { + creationTime = time.Now() } - newSnapshot, err := ctrl.updateSnapshotStatus(snapshot, readyToUse, timestamp, size, IsSnapshotBound(snapshot, content)) + newSnapshot, err := ctrl.updateSnapshotStatus(snapshot, readyToUse, creationTime, size, IsSnapshotBound(snapshot, content)) if err != nil { return nil, err } @@ -617,17 +616,18 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum return nil, fmt.Errorf("failed to get input parameters to create snapshot %s: %q", snapshot.Name, err) } - driverName, snapshotID, timestamp, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials) + driverName, snapshotID, creationTime, size, readyToUse, err := ctrl.handler.CreateSnapshot(snapshot, volume, class.Parameters, snapshotterCredentials) if err != nil { return nil, fmt.Errorf("failed to take snapshot of the volume, %s: %q", volume.Name, err) } - klog.V(5).Infof("Created snapshot: driver %s, snapshotId %s, timestamp %d, size %d, readyToUse %t", driverName, snapshotID, timestamp, size, readyToUse) + + klog.V(5).Infof("Created snapshot: driver %s, snapshotId %s, creationTime %v, size %d, readyToUse %t", driverName, snapshotID, creationTime, size, readyToUse) var newSnapshot *crdv1.VolumeSnapshot - // Update snapshot status with timestamp + // Update snapshot status with creationTime for i := 0; i < ctrl.createSnapshotContentRetryCount; i++ { klog.V(5).Infof("createSnapshot [%s]: trying to update snapshot creation timestamp", snapshotKey(snapshot)) - newSnapshot, err = ctrl.updateSnapshotStatus(snapshot, readyToUse, timestamp, size, false) + newSnapshot, err = ctrl.updateSnapshotStatus(snapshot, readyToUse, creationTime, size, false) if err == nil { break } @@ -651,6 +651,7 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum class.DeletionPolicy = new(crdv1.DeletionPolicy) *class.DeletionPolicy = crdv1.VolumeSnapshotContentDelete } + timestamp := creationTime.UnixNano() snapshotContent := &crdv1.VolumeSnapshotContent{ ObjectMeta: metav1.ObjectMeta{ Name: contentName, @@ -804,12 +805,12 @@ func (ctrl *csiSnapshotController) updateSnapshotContentSize(content *crdv1.Volu } // UpdateSnapshotStatus converts snapshot status to crdv1.VolumeSnapshotCondition -func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSnapshot, readyToUse bool, createdAt, size int64, bound bool) (*crdv1.VolumeSnapshot, error) { +func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSnapshot, readyToUse bool, createdAt time.Time, size int64, bound bool) (*crdv1.VolumeSnapshot, error) { klog.V(5).Infof("updating VolumeSnapshot[]%s, readyToUse %v, timestamp %v", snapshotKey(snapshot), readyToUse, createdAt) status := snapshot.Status change := false timeAt := &metav1.Time{ - Time: time.Unix(0, createdAt), + Time: createdAt, } snapshotClone := snapshot.DeepCopy() diff --git a/pkg/controller/snapshot_create_test.go b/pkg/controller/snapshot_create_test.go index bfb925a6d..53358b238 100644 --- a/pkg/controller/snapshot_create_test.go +++ b/pkg/controller/snapshot_create_test.go @@ -27,10 +27,11 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) -var timeNow = time.Now().UnixNano() +var timeNow = time.Now() +var timeNowStamp = timeNow.UnixNano() var metaTimeNowUnix = &metav1.Time{ - Time: time.Unix(0, timeNow), + Time: timeNow, } var defaultSize int64 = 1000 @@ -68,7 +69,7 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-1 - successful create snapshot with snapshot class gold", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-1", classGold, "sid6-1", "pv-uid6-1", "volume6-1", "snapuid6-1", "snap6-1", &deletePolicy, &defaultSize, &timeNow, false), + expectedContents: newContentArray("snapcontent-snapuid6-1", classGold, "sid6-1", "pv-uid6-1", "volume6-1", "snapuid6-1", "snap6-1", &deletePolicy, &defaultSize, &timeNowStamp, false), initialSnapshots: newSnapshotArray("snap6-1", classGold, "", "snapuid6-1", "claim6-1", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-1", classGold, "snapcontent-snapuid6-1", "snapuid6-1", "claim6-1", false, nil, metaTimeNowUnix, getSize(defaultSize)), initialClaims: newClaimArray("claim6-1", "pvc-uid6-1", "1Gi", "volume6-1", v1.ClaimBound, &classEmpty), @@ -79,11 +80,11 @@ func TestCreateSnapshotSync(t *testing.T) { volume: newVolume("volume6-1", "pv-uid6-1", "pv-handle6-1", "1Gi", "pvc-uid6-1", "claim6-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), parameters: map[string]string{"param1": "value1"}, // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid6-1", - timestamp: timeNow, - readyToUse: true, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "sid6-1", + creationTime: timeNow, + readyToUse: true, }, }, errors: noerrors, @@ -92,7 +93,7 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-2 - successful create snapshot with snapshot class silver", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-2", classSilver, "sid6-2", "pv-uid6-2", "volume6-2", "snapuid6-2", "snap6-2", &deletePolicy, &defaultSize, &timeNow, false), + expectedContents: newContentArray("snapcontent-snapuid6-2", classSilver, "sid6-2", "pv-uid6-2", "volume6-2", "snapuid6-2", "snap6-2", &deletePolicy, &defaultSize, &timeNowStamp, false), initialSnapshots: newSnapshotArray("snap6-2", classSilver, "", "snapuid6-2", "claim6-2", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-2", classSilver, "snapcontent-snapuid6-2", "snapuid6-2", "claim6-2", false, nil, metaTimeNowUnix, getSize(defaultSize)), initialClaims: newClaimArray("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty), @@ -103,11 +104,11 @@ func TestCreateSnapshotSync(t *testing.T) { volume: newVolume("volume6-2", "pv-uid6-2", "pv-handle6-2", "1Gi", "pvc-uid6-2", "claim6-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), parameters: map[string]string{"param2": "value2"}, // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid6-2", - timestamp: timeNow, - readyToUse: true, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "sid6-2", + creationTime: timeNow, + readyToUse: true, }, }, errors: noerrors, @@ -116,7 +117,7 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-3 - successful create snapshot with snapshot class valid-secret-class", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-3", validSecretClass, "sid6-3", "pv-uid6-3", "volume6-3", "snapuid6-3", "snap6-3", &deletePolicy, &defaultSize, &timeNow, false), + expectedContents: newContentArray("snapcontent-snapuid6-3", validSecretClass, "sid6-3", "pv-uid6-3", "volume6-3", "snapuid6-3", "snap6-3", &deletePolicy, &defaultSize, &timeNowStamp, false), initialSnapshots: newSnapshotArray("snap6-3", validSecretClass, "", "snapuid6-3", "claim6-3", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-3", validSecretClass, "snapcontent-snapuid6-3", "snapuid6-3", "claim6-3", false, nil, metaTimeNowUnix, getSize(defaultSize)), initialClaims: newClaimArray("claim6-3", "pvc-uid6-3", "1Gi", "volume6-3", v1.ClaimBound, &classEmpty), @@ -129,11 +130,11 @@ func TestCreateSnapshotSync(t *testing.T) { parameters: class5Parameters, secrets: map[string]string{"foo": "bar"}, // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid6-3", - timestamp: timeNow, - readyToUse: true, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "sid6-3", + creationTime: timeNow, + readyToUse: true, }, }, errors: noerrors, @@ -142,7 +143,7 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-4 - successful create snapshot with snapshot class empty-secret-class", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-4", emptySecretClass, "sid6-4", "pv-uid6-4", "volume6-4", "snapuid6-4", "snap6-4", &deletePolicy, &defaultSize, &timeNow, false), + expectedContents: newContentArray("snapcontent-snapuid6-4", emptySecretClass, "sid6-4", "pv-uid6-4", "volume6-4", "snapuid6-4", "snap6-4", &deletePolicy, &defaultSize, &timeNowStamp, false), initialSnapshots: newSnapshotArray("snap6-4", emptySecretClass, "", "snapuid6-4", "claim6-4", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-4", emptySecretClass, "snapcontent-snapuid6-4", "snapuid6-4", "claim6-4", false, nil, metaTimeNowUnix, getSize(defaultSize)), initialClaims: newClaimArray("claim6-4", "pvc-uid6-4", "1Gi", "volume6-4", v1.ClaimBound, &classEmpty), @@ -155,11 +156,11 @@ func TestCreateSnapshotSync(t *testing.T) { parameters: class4Parameters, secrets: map[string]string{}, // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid6-4", - timestamp: timeNow, - readyToUse: true, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "sid6-4", + creationTime: timeNow, + readyToUse: true, }, }, errors: noerrors, @@ -168,7 +169,7 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-5 - successful create snapshot with status uploading", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-5", classGold, "sid6-5", "pv-uid6-5", "volume6-5", "snapuid6-5", "snap6-5", &deletePolicy, &defaultSize, &timeNow, false), + expectedContents: newContentArray("snapcontent-snapuid6-5", classGold, "sid6-5", "pv-uid6-5", "volume6-5", "snapuid6-5", "snap6-5", &deletePolicy, &defaultSize, &timeNowStamp, false), initialSnapshots: newSnapshotArray("snap6-5", classGold, "", "snapuid6-5", "claim6-5", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-5", classGold, "snapcontent-snapuid6-5", "snapuid6-5", "claim6-5", false, nil, metaTimeNowUnix, getSize(defaultSize)), initialClaims: newClaimArray("claim6-5", "pvc-uid6-5", "1Gi", "volume6-5", v1.ClaimBound, &classEmpty), @@ -179,11 +180,11 @@ func TestCreateSnapshotSync(t *testing.T) { volume: newVolume("volume6-5", "pv-uid6-5", "pv-handle6-5", "1Gi", "pvc-uid6-5", "claim6-5", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), parameters: map[string]string{"param1": "value1"}, // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid6-5", - timestamp: timeNow, - readyToUse: true, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "sid6-5", + creationTime: timeNow, + readyToUse: true, }, }, errors: noerrors, @@ -192,7 +193,7 @@ func TestCreateSnapshotSync(t *testing.T) { { name: "6-6 - successful create snapshot with status error uploading", initialContents: nocontents, - expectedContents: newContentArray("snapcontent-snapuid6-6", classGold, "sid6-6", "pv-uid6-6", "volume6-6", "snapuid6-6", "snap6-6", &deletePolicy, &defaultSize, &timeNow, false), + expectedContents: newContentArray("snapcontent-snapuid6-6", classGold, "sid6-6", "pv-uid6-6", "volume6-6", "snapuid6-6", "snap6-6", &deletePolicy, &defaultSize, &timeNowStamp, false), initialSnapshots: newSnapshotArray("snap6-6", classGold, "", "snapuid6-6", "claim6-6", false, nil, nil, nil), expectedSnapshots: newSnapshotArray("snap6-6", classGold, "snapcontent-snapuid6-6", "snapuid6-6", "claim6-6", false, nil, metaTimeNowUnix, getSize(defaultSize)), initialClaims: newClaimArray("claim6-6", "pvc-uid6-6", "1Gi", "volume6-6", v1.ClaimBound, &classEmpty), @@ -203,11 +204,11 @@ func TestCreateSnapshotSync(t *testing.T) { volume: newVolume("volume6-6", "pv-uid6-6", "pv-handle6-6", "1Gi", "pvc-uid6-6", "claim6-6", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), parameters: map[string]string{"param1": "value1"}, // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid6-6", - timestamp: timeNow, - readyToUse: true, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "sid6-6", + creationTime: timeNow, + readyToUse: true, }, }, errors: noerrors, @@ -318,11 +319,11 @@ func TestCreateSnapshotSync(t *testing.T) { volume: newVolume("volume7-8", "pv-uid7-8", "pv-handle7-8", "1Gi", "pvc-uid7-8", "claim7-8", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), parameters: map[string]string{"param1": "value1"}, // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid7-8", - timestamp: timeNow, - readyToUse: true, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "sid7-8", + creationTime: timeNow, + readyToUse: true, }, }, errors: []reactorError{ @@ -349,11 +350,11 @@ func TestCreateSnapshotSync(t *testing.T) { volume: newVolume("volume7-9", "pv-uid7-9", "pv-handle7-9", "1Gi", "pvc-uid7-9", "claim7-9", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty), parameters: map[string]string{"param1": "value1"}, // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid7-9", - timestamp: timeNow, - readyToUse: true, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "sid7-9", + creationTime: timeNow, + readyToUse: true, }, }, errors: []reactorError{ diff --git a/pkg/controller/snapshot_ready_test.go b/pkg/controller/snapshot_ready_test.go index b495af781..1ccb6bd90 100644 --- a/pkg/controller/snapshot_ready_test.go +++ b/pkg/controller/snapshot_ready_test.go @@ -79,10 +79,10 @@ func TestSync(t *testing.T) { parameters: class5Parameters, secrets: map[string]string{"foo": "bar"}, // information to return - driverName: mockDriverName, - snapshotId: "sid2-3", - timestamp: timeNow, - readyToUse: false, + driverName: mockDriverName, + snapshotId: "sid2-3", + creationTime: timeNow, + readyToUse: false, }, }, errors: noerrors, @@ -114,10 +114,10 @@ func TestSync(t *testing.T) { parameters: class5Parameters, secrets: map[string]string{"foo": "bar"}, // information to return - driverName: mockDriverName, - snapshotId: "sid2-5", - timestamp: timeNow, - readyToUse: true, + driverName: mockDriverName, + snapshotId: "sid2-5", + creationTime: timeNow, + readyToUse: true, }, }, errors: noerrors, @@ -125,8 +125,8 @@ func TestSync(t *testing.T) { }, { name: "2-6 - snapshot bound to prebound content correctly, status ready false -> true, ref.UID '' -> 'snapuid2-6'", - initialContents: newContentArray("content2-6", validSecretClass, "sid2-6", noVolume, noVolume, noBoundUID, "snap2-6", &deletePolicy, nil, &timeNow, false), - expectedContents: newContentArray("content2-6", validSecretClass, "sid2-6", noVolume, noVolume, "snapuid2-6", "snap2-6", &deletePolicy, nil, &timeNow, false), + initialContents: newContentArray("content2-6", validSecretClass, "sid2-6", noVolume, noVolume, noBoundUID, "snap2-6", &deletePolicy, nil, &timeNowStamp, false), + expectedContents: newContentArray("content2-6", validSecretClass, "sid2-6", noVolume, noVolume, "snapuid2-6", "snap2-6", &deletePolicy, nil, &timeNowStamp, false), initialSnapshots: newSnapshotArray("snap2-6", validSecretClass, "content2-6", "snapuid2-6", noClaim, false, nil, metaTimeNow, nil), expectedSnapshots: newSnapshotArray("snap2-6", validSecretClass, "content2-6", "snapuid2-6", noClaim, true, nil, metaTimeNow, nil), expectedListCalls: []listCall{ @@ -178,11 +178,11 @@ func TestSync(t *testing.T) { parameters: class5Parameters, secrets: map[string]string{"foo": "bar"}, // information to return - driverName: mockDriverName, - size: defaultSize, - snapshotId: "sid2-8", - timestamp: timeNow, - readyToUse: true, + driverName: mockDriverName, + size: defaultSize, + snapshotId: "sid2-8", + creationTime: timeNow, + readyToUse: true, }, }, errors: []reactorError{ diff --git a/pkg/snapshotter/snapshotter.go b/pkg/snapshotter/snapshotter.go index ee5031dfc..ff0f64eef 100644 --- a/pkg/snapshotter/snapshotter.go +++ b/pkg/snapshotter/snapshotter.go @@ -19,10 +19,10 @@ package snapshotter import ( "context" "fmt" + "time" "github.com/container-storage-interface/spec/lib/go/csi" "github.com/golang/protobuf/ptypes" - "github.com/golang/protobuf/ptypes/timestamp" csirpc "github.com/kubernetes-csi/csi-lib-utils/rpc" "google.golang.org/grpc" @@ -34,13 +34,13 @@ import ( // Snapshotter implements CreateSnapshot/DeleteSnapshot operations against a remote CSI driver. type Snapshotter interface { // CreateSnapshot creates a snapshot for a volume - CreateSnapshot(ctx context.Context, snapshotName string, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (driverName string, snapshotId string, timestamp int64, size int64, readyToUse bool, err error) + CreateSnapshot(ctx context.Context, snapshotName string, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (driverName string, snapshotId string, timestamp time.Time, size int64, readyToUse bool, err error) // DeleteSnapshot deletes a snapshot from a volume DeleteSnapshot(ctx context.Context, snapshotID string, snapshotterCredentials map[string]string) (err error) // GetSnapshotStatus returns if a snapshot is ready to use, creation time, and restore size. - GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, int64, int64, error) + GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, time.Time, int64, error) } type snapshot struct { @@ -53,17 +53,17 @@ func NewSnapshotter(conn *grpc.ClientConn) Snapshotter { } } -func (s *snapshot) CreateSnapshot(ctx context.Context, snapshotName string, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, int64, int64, bool, error) { +func (s *snapshot) CreateSnapshot(ctx context.Context, snapshotName string, volume *v1.PersistentVolume, parameters map[string]string, snapshotterCredentials map[string]string) (string, string, time.Time, int64, bool, error) { klog.V(5).Infof("CSI CreateSnapshot: %s", snapshotName) if volume.Spec.CSI == nil { - return "", "", 0, 0, false, fmt.Errorf("CSIPersistentVolumeSource not defined in spec") + return "", "", time.Time{}, 0, false, fmt.Errorf("CSIPersistentVolumeSource not defined in spec") } client := csi.NewControllerClient(s.conn) driverName, err := csirpc.GetDriverName(ctx, s.conn) if err != nil { - return "", "", 0, 0, false, err + return "", "", time.Time{}, 0, false, err } req := csi.CreateSnapshotRequest{ @@ -75,13 +75,13 @@ func (s *snapshot) CreateSnapshot(ctx context.Context, snapshotName string, volu rsp, err := client.CreateSnapshot(ctx, &req) if err != nil { - return "", "", 0, 0, false, err + return "", "", time.Time{}, 0, false, err } klog.V(5).Infof("CSI CreateSnapshot: %s driver name [%s] snapshot ID [%s] time stamp [%d] size [%d] readyToUse [%v]", snapshotName, driverName, rsp.Snapshot.SnapshotId, rsp.Snapshot.CreationTime, rsp.Snapshot.SizeBytes, rsp.Snapshot.ReadyToUse) - creationTime, err := timestampToUnixTime(rsp.Snapshot.CreationTime) + creationTime, err := ptypes.Timestamp(rsp.Snapshot.CreationTime) if err != nil { - return "", "", 0, 0, false, err + return "", "", time.Time{}, 0, false, err } return driverName, rsp.Snapshot.SnapshotId, creationTime, rsp.Snapshot.SizeBytes, rsp.Snapshot.ReadyToUse, nil } @@ -101,7 +101,7 @@ func (s *snapshot) DeleteSnapshot(ctx context.Context, snapshotID string, snapsh return nil } -func (s *snapshot) GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, int64, int64, error) { +func (s *snapshot) GetSnapshotStatus(ctx context.Context, snapshotID string) (bool, time.Time, int64, error) { client := csi.NewControllerClient(s.conn) req := csi.ListSnapshotsRequest{ @@ -110,26 +110,16 @@ func (s *snapshot) GetSnapshotStatus(ctx context.Context, snapshotID string) (bo rsp, err := client.ListSnapshots(ctx, &req) if err != nil { - return false, 0, 0, err + return false, time.Time{}, 0, err } if rsp.Entries == nil || len(rsp.Entries) == 0 { - return false, 0, 0, fmt.Errorf("can not find snapshot for snapshotID %s", snapshotID) + return false, time.Time{}, 0, fmt.Errorf("can not find snapshot for snapshotID %s", snapshotID) } - creationTime, err := timestampToUnixTime(rsp.Entries[0].Snapshot.CreationTime) + creationTime, err := ptypes.Timestamp(rsp.Entries[0].Snapshot.CreationTime) if err != nil { - return false, 0, 0, err + return false, time.Time{}, 0, err } return rsp.Entries[0].Snapshot.ReadyToUse, creationTime, rsp.Entries[0].Snapshot.SizeBytes, nil } - -func timestampToUnixTime(t *timestamp.Timestamp) (int64, error) { - time, err := ptypes.Timestamp(t) - if err != nil { - return -1, err - } - // TODO: clean this up, we probably don't need this translation layer - // and can just use time.Time - return time.UnixNano(), nil -} diff --git a/pkg/snapshotter/snapshotter_test.go b/pkg/snapshotter/snapshotter_test.go index dbe973d38..588030ae0 100644 --- a/pkg/snapshotter/snapshotter_test.go +++ b/pkg/snapshotter/snapshotter_test.go @@ -21,6 +21,7 @@ import ( "fmt" "reflect" "testing" + "time" "github.com/container-storage-interface/spec/lib/go/csi" "github.com/golang/mock/gomock" @@ -118,7 +119,7 @@ func TestCreateSnapshot(t *testing.T) { type snapshotResult struct { driverName string snapshotId string - timestamp int64 + timestamp time.Time size int64 readyToUse bool } @@ -127,7 +128,7 @@ func TestCreateSnapshot(t *testing.T) { size: 1000, driverName: driverName, snapshotId: defaultID, - timestamp: createTime.UnixNano(), + timestamp: createTime, readyToUse: true, } @@ -376,7 +377,7 @@ func TestGetSnapshotStatus(t *testing.T) { injectError codes.Code expectError bool expectReady bool - expectCreateAt int64 + expectCreateAt time.Time expectSize int64 }{ { @@ -386,7 +387,7 @@ func TestGetSnapshotStatus(t *testing.T) { output: defaultResponse, expectError: false, expectReady: true, - expectCreateAt: createTime.UnixNano(), + expectCreateAt: createTime, expectSize: size, }, {