From 317433252db7679a61c8587b566aa0ace0e21f4d Mon Sep 17 00:00:00 2001 From: Jing Xu Date: Mon, 17 Sep 2018 16:23:36 -0700 Subject: [PATCH] Fix restore size type in snapshot content Change the restore size to int64 for snapshot content. Also add the code to update the restore size for snapshot content. --- pkg/apis/volumesnapshot/v1alpha1/types.go | 4 +- .../v1alpha1/zz_generated.deepcopy.go | 4 +- pkg/controller/framework_test.go | 4 +- pkg/controller/snapshot_controller.go | 38 ++++++++++++++++--- pkg/controller/snapshot_create_test.go | 12 +++--- 5 files changed, 43 insertions(+), 19 deletions(-) diff --git a/pkg/apis/volumesnapshot/v1alpha1/types.go b/pkg/apis/volumesnapshot/v1alpha1/types.go index 19694cb39..2d0b07d04 100644 --- a/pkg/apis/volumesnapshot/v1alpha1/types.go +++ b/pkg/apis/volumesnapshot/v1alpha1/types.go @@ -114,10 +114,8 @@ type VolumeSnapshotStatus struct { // TODO: After TypedLocalObjectReference is merged into the in-tree core API, this will be replaced. type TypedLocalObjectReference struct { // Name of the referent. - // +optional Name string `json:"name,omitempty" protobuf:"bytes,1,opt,name=name"` // Kind of the referent. - // +optional Kind string `json:"kind,omitempty" protobuf:"bytes,2,opt,name=kind"` } @@ -241,5 +239,5 @@ type CSIVolumeSnapshotSource struct { // larger than the RestoreSize if it is specified. If RestoreSize is set to nil, it means // that the storage plugin does not have this information available. // +optional - RestoreSize *resource.Quantity `json:"restoreSize" protobuf:"bytes,4,opt,name=restoreSize"` + RestoreSize *int64 `json:"restoreSize,omitempty" protobuf:"bytes,4,opt,name=restoreSize"` } diff --git a/pkg/apis/volumesnapshot/v1alpha1/zz_generated.deepcopy.go b/pkg/apis/volumesnapshot/v1alpha1/zz_generated.deepcopy.go index 2f7d25b9f..0072c1d92 100644 --- a/pkg/apis/volumesnapshot/v1alpha1/zz_generated.deepcopy.go +++ b/pkg/apis/volumesnapshot/v1alpha1/zz_generated.deepcopy.go @@ -36,8 +36,8 @@ func (in *CSIVolumeSnapshotSource) DeepCopyInto(out *CSIVolumeSnapshotSource) { } if in.RestoreSize != nil { in, out := &in.RestoreSize, &out.RestoreSize - x := (*in).DeepCopy() - *out = &x + *out = new(int64) + **out = **in } return } diff --git a/pkg/controller/framework_test.go b/pkg/controller/framework_test.go index 983895d2a..b3c115302 100644 --- a/pkg/controller/framework_test.go +++ b/pkg/controller/framework_test.go @@ -743,7 +743,7 @@ func newTestController(kubeClient kubernetes.Interface, clientset clientset.Inte } // newContent returns a new content with given attributes -func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, size *resource.Quantity, creationTime *int64) *crdv1.VolumeSnapshotContent { +func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, size *int64, creationTime *int64) *crdv1.VolumeSnapshotContent { content := crdv1.VolumeSnapshotContent{ ObjectMeta: metav1.ObjectMeta{ Name: name, @@ -780,7 +780,7 @@ func newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToS return &content } -func newContentArray(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, size *resource.Quantity, creationTime *int64) []*crdv1.VolumeSnapshotContent { +func newContentArray(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName string, size *int64, creationTime *int64) []*crdv1.VolumeSnapshotContent { return []*crdv1.VolumeSnapshotContent{ newContent(name, className, snapshotHandle, volumeUID, volumeName, boundToSnapshotUID, boundToSnapshotName, size, creationTime), } diff --git a/pkg/controller/snapshot_controller.go b/pkg/controller/snapshot_controller.go index 38da92c74..e9ef5e72e 100644 --- a/pkg/controller/snapshot_controller.go +++ b/pkg/controller/snapshot_controller.go @@ -170,7 +170,7 @@ func (ctrl *csiSnapshotController) syncReadySnapshot(snapshot *crdv1.VolumeSnaps return fmt.Errorf("Cannot convert object from snapshot content store to VolumeSnapshotContent %q!?: %#v", snapshot.Spec.SnapshotContentName, obj) } - glog.V(5).Infof("syncCompleteSnapshot[%s]: VolumeSnapshotContent %q found", snapshotKey(snapshot), content.Name) + glog.V(5).Infof("syncReadySnapshot[%s]: VolumeSnapshotContent %q found", snapshotKey(snapshot), content.Name) if !IsSnapshotBound(snapshot, content) { // snapshot is bound but content is not bound to snapshot correctly if err = ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotMisbound", "VolumeSnapshotContent is not bound to the VolumeSnapshot correctly"); err != nil { @@ -212,7 +212,7 @@ func (ctrl *csiSnapshotController) syncUnreadySnapshot(snapshot *crdv1.VolumeSna // snapshot is already bound correctly, check the status and update if it is ready. glog.V(5).Infof("Check and update snapshot %s status", uniqueSnapshotName) - if err = ctrl.checkandUpdateSnapshotStatus(snapshot, content); err != nil { + if err = ctrl.checkandUpdateBoundSnapshotStatus(snapshot, content); err != nil { return err } return nil @@ -321,11 +321,11 @@ func (ctrl *csiSnapshotController) createSnapshot(snapshot *crdv1.VolumeSnapshot return nil } -func (ctrl *csiSnapshotController) checkandUpdateSnapshotStatus(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) error { +func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatus(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) error { glog.V(5).Infof("checkandUpdateSnapshotStatus[%s] started", snapshotKey(snapshot)) opName := fmt.Sprintf("check-%s[%s]", snapshotKey(snapshot), string(snapshot.UID)) ctrl.scheduleOperation(opName, func() error { - snapshotObj, err := ctrl.checkandUpdateSnapshotStatusOperation(snapshot, content) + snapshotObj, err := ctrl.checkandUpdateBoundSnapshotStatusOperation(snapshot, content) if err != nil { ctrl.updateSnapshotErrorStatusWithEvent(snapshot, v1.EventTypeWarning, "SnapshotCheckandUpdateFailed", fmt.Sprintf("Failed to check and update snapshot: %v", err)) glog.Errorf("checkandUpdateSnapshotStatus [%s]: error occured %v", snapshotKey(snapshot), err) @@ -420,7 +420,7 @@ func (ctrl *csiSnapshotController) checkandBindSnapshotContent(snapshot *crdv1.V return nil } -func (ctrl *csiSnapshotController) checkandUpdateSnapshotStatusOperation(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshot, error) { +func (ctrl *csiSnapshotController) checkandUpdateBoundSnapshotStatusOperation(snapshot *crdv1.VolumeSnapshot, content *crdv1.VolumeSnapshotContent) (*crdv1.VolumeSnapshot, error) { status, _, size, err := ctrl.handler.GetSnapshotStatus(content) if err != nil { return nil, fmt.Errorf("failed to check snapshot status %s with error %v", snapshot.Name, err) @@ -430,6 +430,10 @@ func (ctrl *csiSnapshotController) checkandUpdateSnapshotStatusOperation(snapsho if err != nil { return nil, err } + err = ctrl.updateSnapshotContentSize(content, size) + if err != nil { + return nil, err + } return newSnapshot, nil } @@ -521,7 +525,7 @@ func (ctrl *csiSnapshotController) createSnapshotOperation(snapshot *crdv1.Volum Driver: driverName, SnapshotHandle: snapshotID, CreationTime: ×tamp, - RestoreSize: resource.NewQuantity(size, resource.BinarySI), + RestoreSize: &size, }, }, VolumeSnapshotClassName: &(class.Name), @@ -637,6 +641,28 @@ func (ctrl *csiSnapshotController) bindandUpdateVolumeSnapshot(snapshotContent * return snapshotCopy, nil } +// updateSnapshotContentSize update the restore size for snapshot content +func (ctrl *csiSnapshotController) updateSnapshotContentSize(content *crdv1.VolumeSnapshotContent, size int64) error { + if content.Spec.VolumeSnapshotSource.CSI == nil || size == 0 { + return nil + } + if content.Spec.VolumeSnapshotSource.CSI.RestoreSize != nil && *content.Spec.VolumeSnapshotSource.CSI.RestoreSize > 0 { + return nil + } + contentClone := content.DeepCopy() + contentClone.Spec.VolumeSnapshotSource.CSI.RestoreSize = &size + _, err := ctrl.clientset.VolumesnapshotV1alpha1().VolumeSnapshotContents().Update(contentClone) + if err != nil { + return newControllerUpdateError(content.Name, err.Error()) + } + + _, err = ctrl.storeContentUpdate(contentClone) + if err != nil { + glog.Errorf("failed to update content store %v", err) + } + return nil +} + // UpdateSnapshotStatus converts snapshot status to crdv1.VolumeSnapshotCondition func (ctrl *csiSnapshotController) updateSnapshotStatus(snapshot *crdv1.VolumeSnapshot, csistatus *csi.SnapshotStatus, createdAt, size int64, bound bool) (*crdv1.VolumeSnapshot, error) { glog.V(5).Infof("updating VolumeSnapshot[]%s, set status %v, timestamp %v", snapshotKey(snapshot), csistatus, createdAt) diff --git a/pkg/controller/snapshot_create_test.go b/pkg/controller/snapshot_create_test.go index 57b8be807..d900015a3 100644 --- a/pkg/controller/snapshot_create_test.go +++ b/pkg/controller/snapshot_create_test.go @@ -66,7 +66,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", getSize(defaultSize), &timeNow), + expectedContents: newContentArray("snapcontent-snapuid6-1", classGold, "sid6-1", "pv-uid6-1", "volume6-1", "snapuid6-1", "snap6-1", &defaultSize, &timeNow), 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), @@ -93,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", getSize(defaultSize), &timeNow), + expectedContents: newContentArray("snapcontent-snapuid6-2", classSilver, "sid6-2", "pv-uid6-2", "volume6-2", "snapuid6-2", "snap6-2", &defaultSize, &timeNow), 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), @@ -120,7 +120,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", getSize(defaultSize), &timeNow), + expectedContents: newContentArray("snapcontent-snapuid6-3", validSecretClass, "sid6-3", "pv-uid6-3", "volume6-3", "snapuid6-3", "snap6-3", &defaultSize, &timeNow), 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), @@ -149,7 +149,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", getSize(defaultSize), &timeNow), + expectedContents: newContentArray("snapcontent-snapuid6-4", emptySecretClass, "sid6-4", "pv-uid6-4", "volume6-4", "snapuid6-4", "snap6-4", &defaultSize, &timeNow), 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), @@ -178,7 +178,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", getSize(defaultSize), &timeNow), + expectedContents: newContentArray("snapcontent-snapuid6-5", classGold, "sid6-5", "pv-uid6-5", "volume6-5", "snapuid6-5", "snap6-5", &defaultSize, &timeNow), 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), @@ -205,7 +205,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", getSize(defaultSize), &timeNow), + expectedContents: newContentArray("snapcontent-snapuid6-6", classGold, "sid6-6", "pv-uid6-6", "volume6-6", "snapuid6-6", "snap6-6", &defaultSize, &timeNow), 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),