Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix TODO use time.Time for createSnapshot #119

Merged
merged 1 commit into from
May 9, 2019
Merged

fix TODO use time.Time for createSnapshot #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions Gopkg.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

16 changes: 8 additions & 8 deletions pkg/controller/csi_handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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)
}
Expand All @@ -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
Expand Down
29 changes: 15 additions & 14 deletions pkg/controller/framework_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"errors"
"fmt"
"reflect"
sysruntime "runtime"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -1126,7 +1127,7 @@ type listCall struct {
snapshotID string
// information to return
readyToUse bool
createTime int64
createTime time.Time
size int64
err error
}
Expand All @@ -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
Expand All @@ -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++
Expand All @@ -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 {
Expand Down Expand Up @@ -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++
Expand All @@ -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
Expand Down
29 changes: 15 additions & 14 deletions pkg/controller/snapshot_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -556,15 +555,15 @@ 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
var snapshotID string

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
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
Loading