Skip to content

Commit

Permalink
test: fix tests that failed after using patch to update objects
Browse files Browse the repository at this point in the history 
Signed-off-by: hoyho <[email protected]>
  • Loading branch information
@hoyho
hoyho committed Jun 18, 2024
1 parent 11790ef commit da5424f
Show file tree
Hide file tree
Showing 3 changed files with 149 additions and 117 deletions.
28 changes: 25 additions & 3 deletions pkg/common-controller/framework_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -287,10 +287,13 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, nil, err
}

err = json.Unmarshal(modified, content)
//json.Unmarshal will not clear the fields that are not present in the patch
var newContent = crdv1.VolumeSnapshotContent{}
err = json.Unmarshal(modified, &newContent)
if err != nil {
return true, nil, err
}
content = &newContent

storedVer, _ := strconv.Atoi(content.ResourceVersion)
content.ResourceVersion = strconv.Itoa(storedVer + 1)
Expand Down Expand Up @@ -351,10 +354,13 @@ func (r *snapshotReactor) React(action core.Action) (handled bool, ret runtime.O
return true, nil, err
}

err = json.Unmarshal(modified, storedSnapshot)
//json.Unmarshal will not clear the fields that are not present in the patch
var newSnapshot = crdv1.VolumeSnapshot{}
err = json.Unmarshal(modified, &newSnapshot)
if err != nil {
return true, nil, err
}
storedSnapshot = &newSnapshot

storedVer, _ := strconv.Atoi(storedSnapshot.ResourceVersion)
storedSnapshot.ResourceVersion = strconv.Itoa(storedVer + 1)
Expand Down Expand Up @@ -1047,6 +1053,14 @@ func newSnapshot(
if targetContentName != "" {
snapshot.Spec.Source.VolumeSnapshotContentName = &targetContentName
}

// Perform marshal and unmarshal to simulate the real behavior of the API server
// for example, field like metav1.Time percision should be normalized

Check failure on line 1058 in pkg/common-controller/framework_test.go

View workflow job for this annotation

GitHub Actions / Check for spelling errors 

percision ==> precision
buf, _ := json.Marshal(snapshot)
normalized := &crdv1.VolumeSnapshot{}
_ = json.Unmarshal(buf, normalized)
snapshot = *normalized

if withAllFinalizers {
return withSnapshotFinalizers([]*crdv1.VolumeSnapshot{&snapshot}, utils.VolumeSnapshotAsSourceFinalizer, utils.VolumeSnapshotBoundFinalizer)[0]
}
Expand Down Expand Up @@ -1201,10 +1215,18 @@ func newVolumeArray(name, volumeUID, volumeHandle, capacity, boundToClaimUID, bo
}

func newVolumeError(message string) *crdv1.VolumeSnapshotError {
return &crdv1.VolumeSnapshotError{
snapErrIn := &crdv1.VolumeSnapshotError{
Time: &metav1.Time{},
Message: &message,
}

// Marshal and unmarshal to simulate the real behavior of the API server
// for example, metav1.Time percision should be normalized

Check failure on line 1224 in pkg/common-controller/framework_test.go

View workflow job for this annotation

GitHub Actions / Check for spelling errors 

percision ==> precision
buf, _ := json.Marshal(snapErrIn)
snapErrOut := &crdv1.VolumeSnapshotError{}
_ = json.Unmarshal(buf, snapErrOut)

return snapErrOut
}

func testSyncSnapshot(ctrl *csiSnapshotCommonController, reactor *snapshotReactor, test controllerTest) error {
Expand Down
62 changes: 36 additions & 26 deletions pkg/common-controller/snapshot_create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,16 @@ var (
retainPolicy = crdv1.VolumeSnapshotContentRetain
)

func newTruePtr() *bool {
var b bool = true
return &b
}

func newFalsePtr() *bool {
var b bool = false
return &b
}

// Test single call to SyncSnapshot, expecting create snapshot to happen.
// 1. Fill in the controller with initial data
// 2. Call the SyncSnapshot *once*.
Expand All @@ -53,8 +63,8 @@ func TestCreateSnapshotSync(t *testing.T) {
name: "6-1 - successful create snapshot with snapshot class gold",
initialContents: nocontents,
expectedContents: newContentArrayNoStatus("snapcontent-snapuid6-1", "snapuid6-1", "snap6-1", "sid6-1", classGold, "", "pv-handle6-1", deletionPolicy, nil, nil, false, false),
initialSnapshots: newSnapshotArray("snap6-1", "snapuid6-1", "claim6-1", "", classGold, "", &False, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap6-1", "snapuid6-1", "claim6-1", "", classGold, "snapcontent-snapuid6-1", &False, nil, nil, nil, false, true, nil),
initialSnapshots: newSnapshotArray("snap6-1", "snapuid6-1", "claim6-1", "", classGold, "", newFalsePtr(), nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap6-1", "snapuid6-1", "claim6-1", "", classGold, "snapcontent-snapuid6-1", newFalsePtr(), nil, nil, nil, false, true, nil),
initialClaims: newClaimArray("claim6-1", "pvc-uid6-1", "1Gi", "volume6-1", v1.ClaimBound, &classGold),
initialVolumes: newVolumeArray("volume6-1", "pv-uid6-1", "pv-handle6-1", "1Gi", "pvc-uid6-1", "claim6-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classGold),
errors: noerrors,
Expand All @@ -68,8 +78,8 @@ func TestCreateSnapshotSync(t *testing.T) {
"snapshot.storage.kubernetes.io/deletion-secret-name": "secret",
"snapshot.storage.kubernetes.io/deletion-secret-namespace": "default",
}),
initialSnapshots: newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", validSecretClass, "", &False, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", validSecretClass, "snapcontent-snapuid6-2", &False, nil, nil, nil, false, true, nil),
initialSnapshots: newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", validSecretClass, "", newFalsePtr(), nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap6-2", "snapuid6-2", "claim6-2", "", validSecretClass, "snapcontent-snapuid6-2", newFalsePtr(), nil, nil, nil, false, true, nil),
initialClaims: newClaimArray("claim6-2", "pvc-uid6-2", "1Gi", "volume6-2", v1.ClaimBound, &classEmpty),
initialVolumes: newVolumeArray("volume6-2", "pv-uid6-2", "pv-handle6-2", "1Gi", "pvc-uid6-2", "claim6-2", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
initialSecrets: []*v1.Secret{secret()}, // no initial secret created
Expand All @@ -80,8 +90,8 @@ func TestCreateSnapshotSync(t *testing.T) {
name: "7-1 - fail to create snapshot with non-existing snapshot class",
initialContents: nocontents,
expectedContents: nocontents,
initialSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-1: \"volumesnapshotclass.snapshot.storage.k8s.io \\\"non-existing\\\" not found\""), false, true, nil),
initialSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", newFalsePtr(), nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-1", "snapuid7-1", "claim7-1", "", classNonExisting, "", newFalsePtr(), nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-1: \"volumesnapshotclass.snapshot.storage.k8s.io \\\"non-existing\\\" not found\""), false, true, nil),
initialClaims: newClaimArray("claim7-1", "pvc-uid7-1", "1Gi", "volume7-1", v1.ClaimBound, &classEmpty),
initialVolumes: newVolumeArray("volume7-1", "pv-uid7-1", "pv-handle7-1", "1Gi", "pvc-uid7-1", "claim7-1", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
expectedEvents: []string{"Warning SnapshotContentCreationFailed"},
Expand All @@ -93,8 +103,8 @@ func TestCreateSnapshotSync(t *testing.T) {
name: "7-3 - fail to create snapshot without snapshot class ",
initialContents: nocontents,
expectedContents: nocontents,
initialSnapshots: newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", &False, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-3: \"failed to take snapshot snap7-3 without a snapshot class\""), false, true, nil),
initialSnapshots: newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", newFalsePtr(), nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-3", "snapuid7-3", "claim7-3", "", "", "", newFalsePtr(), nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-3: \"failed to take snapshot snap7-3 without a snapshot class\""), false, true, nil),
initialClaims: newClaimArray("claim7-3", "pvc-uid7-3", "1Gi", "volume7-3", v1.ClaimBound, &classEmpty),
initialVolumes: newVolumeArray("volume7-3", "pv-uid7-3", "pv-handle7-3", "1Gi", "pvc-uid7-3", "claim7-3", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
expectedEvents: []string{"Warning SnapshotContentCreationFailed"},
Expand All @@ -106,8 +116,8 @@ func TestCreateSnapshotSync(t *testing.T) {
name: "7-4 - fail create snapshot with no-existing claim",
initialContents: nocontents,
expectedContents: nocontents,
initialSnapshots: newSnapshotArray("snap7-4", "snapuid7-4", "claim7-4", "", classGold, "", &False, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-4", "snapuid7-4", "claim7-4", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error snapshot controller failed to update snap7-4 on API server: cannot get claim from snapshot"), false, true, nil),
initialSnapshots: newSnapshotArray("snap7-4", "snapuid7-4", "claim7-4", "", classGold, "", newFalsePtr(), nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-4", "snapuid7-4", "claim7-4", "", classGold, "", newFalsePtr(), nil, nil, newVolumeError("Failed to create snapshot content with error snapshot controller failed to update snap7-4 on API server: cannot get claim from snapshot"), false, true, nil),
initialVolumes: newVolumeArray("volume7-4", "pv-uid7-4", "pv-handle7-4", "1Gi", "pvc-uid7-4", "claim7-4", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classGold),
expectedEvents: []string{"Warning SnapshotContentCreationFailed"},
errors: noerrors,
Expand All @@ -118,8 +128,8 @@ func TestCreateSnapshotSync(t *testing.T) {
name: "7-5 - fail create snapshot with no-existing volume",
initialContents: nocontents,
expectedContents: nocontents,
initialSnapshots: newSnapshotArray("snap7-5", "snapuid7-5", "claim7-5", "", classGold, "", &False, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-5", "snapuid7-5", "claim7-5", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-5: \"failed to retrieve PV volume7-5 from the API server: \\\"cannot find volume volume7-5\\\"\""), false, true, nil),
initialSnapshots: newSnapshotArray("snap7-5", "snapuid7-5", "claim7-5", "", classGold, "", newFalsePtr(), nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-5", "snapuid7-5", "claim7-5", "", classGold, "", newFalsePtr(), nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-5: \"failed to retrieve PV volume7-5 from the API server: \\\"cannot find volume volume7-5\\\"\""), false, true, nil),
initialClaims: newClaimArray("claim7-5", "pvc-uid7-5", "1Gi", "volume7-5", v1.ClaimBound, &classGold),
expectedEvents: []string{"Warning SnapshotContentCreationFailed"},
errors: noerrors,
Expand All @@ -131,8 +141,8 @@ func TestCreateSnapshotSync(t *testing.T) {
name: "7-6 - fail create snapshot with claim that is not yet bound",
initialContents: nocontents,
expectedContents: nocontents,
initialSnapshots: newSnapshotArray("snap7-6", "snapuid7-6", "claim7-6", "", classGold, "", &False, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-6", "snapuid7-6", "claim7-6", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-6: \"the PVC claim7-6 is not yet bound to a PV, will not attempt to take a snapshot\""), false, true, nil),
initialSnapshots: newSnapshotArray("snap7-6", "snapuid7-6", "claim7-6", "", classGold, "", newFalsePtr(), nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-6", "snapuid7-6", "claim7-6", "", classGold, "", newFalsePtr(), nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-6: \"the PVC claim7-6 is not yet bound to a PV, will not attempt to take a snapshot\""), false, true, nil),
initialClaims: newClaimArray("claim7-6", "pvc-uid7-6", "1Gi", "", v1.ClaimPending, &classGold),
expectedEvents: []string{"Warning SnapshotContentCreationFailed"},
errors: noerrors,
Expand All @@ -144,8 +154,8 @@ func TestCreateSnapshotSync(t *testing.T) {
name: "7-7 - remove pvc finalizer failed",
initialContents: newContentArray("snapcontent-snapuid7-7", "snapuid7-7", "snap7-7", "sid7-7", classGold, "", "pv-handle7-7", deletionPolicy, nil, nil, false),
expectedContents: newContentArray("snapcontent-snapuid7-7", "snapuid7-7", "snap7-7", "sid7-7", classGold, "", "pv-handle7-7", deletionPolicy, nil, nil, false),
initialSnapshots: newSnapshotArray("snap7-7", "snapuid7-7", "claim7-7", "", classGold, "snapcontent-snapuid7-7", &True, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-7", "snapuid7-7", "claim7-7", "", classGold, "snapcontent-snapuid7-7", &True, nil, nil, nil, false, true, nil),
initialSnapshots: newSnapshotArray("snap7-7", "snapuid7-7", "claim7-7", "", classGold, "snapcontent-snapuid7-7", newTruePtr(), nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-7", "snapuid7-7", "claim7-7", "", classGold, "snapcontent-snapuid7-7", newTruePtr(), nil, nil, nil, false, true, nil),
initialClaims: newClaimArrayFinalizer("claim7-7", "pvc-uid7-7", "1Gi", "volume7-7", v1.ClaimBound, &classGold),
initialVolumes: newVolumeArray("volume7-7", "pv-uid7-7", "pv-handle7-7", "1Gi", "pvc-uid7-7", "claim7-7", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classGold),
errors: []reactorError{
Expand All @@ -160,15 +170,15 @@ func TestCreateSnapshotSync(t *testing.T) {
name: "7-9 - fail create snapshot due to cannot update snapshot status, and failure cannot be recorded either due to additional status update failure.",
initialContents: nocontents,
expectedContents: newContentArrayNoStatus("snapcontent-snapuid7-9", "snapuid7-9", "snap7-9", "sid7-9", classGold, "", "pv-handle7-9", deletionPolicy, nil, nil, false, false),
initialSnapshots: newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "", &False, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "", &False, nil, nil, nil, false, true, nil),
initialSnapshots: newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "", newFalsePtr(), nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-9", "snapuid7-9", "claim7-9", "", classGold, "", newFalsePtr(), nil, nil, nil, false, true, nil),
initialClaims: newClaimArray("claim7-9", "pvc-uid7-9", "1Gi", "volume7-9", v1.ClaimBound, &classGold),
initialVolumes: newVolumeArray("volume7-9", "pv-uid7-9", "pv-handle7-9", "1Gi", "pvc-uid7-9", "claim7-9", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classGold),
errors: []reactorError{
{"update", "volumesnapshots", errors.New("mock update error")},
{"update", "volumesnapshots", errors.New("mock update error")},
{"update", "volumesnapshots", errors.New("mock update error")},
{"update", "volumesnapshots", errors.New("mock update error")},
{"patch", "volumesnapshots", errors.New("mock patch error")},
{"patch", "volumesnapshots", errors.New("mock patch error")},
{"patch", "volumesnapshots", errors.New("mock patch error")},
{"patch", "volumesnapshots", errors.New("mock patch error")},
},
expectSuccess: false,
test: testSyncSnapshot,
Expand All @@ -177,8 +187,8 @@ func TestCreateSnapshotSync(t *testing.T) {
name: "7-10 - fail create snapshot with invalid secret",
initialContents: nocontents,
expectedContents: nocontents,
initialSnapshots: newSnapshotArray("snap7-10", "snapuid7-10", "claim7-10", "", invalidSecretClass, "", &False, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-10", "snapuid7-10", "claim7-10", "", invalidSecretClass, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-10: \"failed to get name and namespace template from params: either name and namespace for Snapshotter secrets specified, Both must be specified\""), false, true, nil),
initialSnapshots: newSnapshotArray("snap7-10", "snapuid7-10", "claim7-10", "", invalidSecretClass, "", newFalsePtr(), nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-10", "snapuid7-10", "claim7-10", "", invalidSecretClass, "", newFalsePtr(), nil, nil, newVolumeError("Failed to create snapshot content with error failed to get input parameters to create snapshot snap7-10: \"failed to get name and namespace template from params: either name and namespace for Snapshotter secrets specified, Both must be specified\""), false, true, nil),
initialClaims: newClaimArray("claim7-10", "pvc-uid7-10", "1Gi", "volume7-10", v1.ClaimBound, &classEmpty),
initialVolumes: newVolumeArray("volume7-10", "pv-uid7-10", "pv-handle7-10", "1Gi", "pvc-uid7-10", "claim7-10", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classEmpty),
initialSecrets: []*v1.Secret{}, // no initial secret created
Expand All @@ -188,8 +198,8 @@ func TestCreateSnapshotSync(t *testing.T) {
name: "7-11 - fail create snapshot due to cannot save snapshot content",
initialContents: nocontents,
expectedContents: nocontents,
initialSnapshots: newSnapshotArray("snap7-11", "snapuid7-11", "claim7-11", "", classGold, "", &False, nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-11", "snapuid7-11", "claim7-11", "", classGold, "", &False, nil, nil, newVolumeError("Failed to create snapshot content with error snapshot controller failed to update default/snap7-11 on API server: mock create error"), false, true, nil),
initialSnapshots: newSnapshotArray("snap7-11", "snapuid7-11", "claim7-11", "", classGold, "", newFalsePtr(), nil, nil, nil, false, true, nil),
expectedSnapshots: newSnapshotArray("snap7-11", "snapuid7-11", "claim7-11", "", classGold, "", newFalsePtr(), nil, nil, newVolumeError("Failed to create snapshot content with error snapshot controller failed to update default/snap7-11 on API server: mock create error"), false, true, nil),
initialClaims: newClaimArray("claim7-11", "pvc-uid7-11", "1Gi", "volume7-11", v1.ClaimBound, &classGold),
initialVolumes: newVolumeArray("volume7-11", "pv-uid7-11", "pv-handle7-11", "1Gi", "pvc-uid7-11", "claim7-11", v1.VolumeBound, v1.PersistentVolumeReclaimDelete, classGold),
errors: []reactorError{
Expand Down
Loading

0 comments on commit da5424f

Please sign in to comment.