-
Notifications
You must be signed in to change notification settings - Fork 39
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.
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
test: added check for snapshot and clone creation in e2e #206
Conversation
7c32b3e
to
ef8877d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include a test to restore the snapshot as we...
e2e/testdata.go
Outdated
} | ||
|
||
func GetSampleClone(quantity, name string, volumemode k8sv1.PersistentVolumeMode) *k8sv1.PersistentVolumeClaim { | ||
storageClassName := "odf-lvm-vg1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please pass this as an argument. Then if we change the storageclass name, it only has to be changed in one place. Comment holds for all instances of this usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
ef8877d
to
8c6d726
Compare
e2e/lvm/pvc_test.go
Outdated
@@ -25,6 +26,12 @@ func PVCTest() { | |||
var filepod *k8sv1.Pod | |||
var blockpvc *k8sv1.PersistentVolumeClaim | |||
var blockpod *k8sv1.Pod | |||
var fileSnapshot *snapapi.VolumeSnapshot | |||
var blockSnapshot *snapapi.VolumeSnapshot | |||
var filepvcclone *k8sv1.PersistentVolumeClaim |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The names should follow the camelcase
convention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
8c6d726
to
b1d7953
Compare
b1d7953
to
0d7d3ad
Compare
O/P: [riyasinghal@fedora lvm-operator]$ oc get pvc,pods -n lvm-endtoendtest NAME READY STATUS RESTARTS AGE [riyasinghal@fedora lvm-operator]$ oc get volumesnapshot -n lvm-endtoendtest |
e2e/lvm/pvc_test.go
Outdated
var restoreBlockPod *k8sv1.Pod | ||
|
||
Context("create pvc, pod, snapshots, clones", func() { | ||
It("Verifies the status for filepvc", func() { | ||
By("Creates pvc and pod") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By("Creates pvc and pod") | |
By("Creating pvc and pod") |
e2e/lvm/pvc_test.go
Outdated
filepvc = tests.GetSamplePVC(tests.StorageClass, "5Gi", "lvmfilepvc", k8sv1.PersistentVolumeFilesystem) | ||
filepod = tests.GetSamplePod("lvmfilepod", "lvmfilepvc") | ||
err := tests.DeployManagerObj.GetCrClient().Create(context.TODO(), filepvc) | ||
filePvc = tests.GetSamplePVC(tests.StorageClass, quantity, "lvmfilepvc", k8sv1.PersistentVolumeFilesystem) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filePvc = tests.GetSamplePVC(tests.StorageClass, quantity, "lvmfilepvc", k8sv1.PersistentVolumeFilesystem) | |
filePvc = tests.GetSamplePVC(tests.StorageClass, size, "lvmfilepvc", k8sv1.PersistentVolumeFilesystem) |
Please rename the variable to size. Quantity sounds more like count.
e2e/lvm/pvc_test.go
Outdated
err := tests.DeployManagerObj.GetCrClient().Create(context.TODO(), filepvc) | ||
filePvc = tests.GetSamplePVC(tests.StorageClass, quantity, "lvmfilepvc", k8sv1.PersistentVolumeFilesystem) | ||
filePod = tests.GetSamplePod("lvmfilepod", filePvc.Name) | ||
err := tests.DeployManagerObj.GetCrClient().Create(context.TODO(), filePvc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of calling tests.DeployManagerObj.GetCrClient() everywhere, please save this to a variable in the beginning and use that everywhere.
e2e/lvm/pvc_test.go
Outdated
blockpvc = tests.GetSamplePVC(tests.StorageClass, "5Gi", "lvmblockpvc", k8sv1.PersistentVolumeBlock) | ||
blockpod = tests.GetSamplePod("lvmblockpod", "lvmblockpvc") | ||
err = tests.DeployManagerObj.GetCrClient().Create(context.TODO(), blockpvc) | ||
By("PVC(file system) Should be bound and Pod should be running") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By("PVC(file system) Should be bound and Pod should be running") | |
By("Verifying that the PVC(file system) is bound and the Pod is running") |
e2e/lvm/pvc_test.go
Outdated
}, timeout, interval).Should(BeTrue()) | ||
fmt.Printf("Pod %s is running\n", filePod.Name) | ||
|
||
By("Snapshot for file-pvc") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By("Snapshot for file-pvc") | |
By("Creating a Snapshot of the file-pvc") |
e2e/lvm/pvc_test.go
Outdated
Expect(err).To(BeNil()) | ||
fmt.Printf("Clone PVC %s is deleted\n", blockPvcClone.Name) | ||
|
||
err = tests.DeployManagerObj.GetCrClient().Delete(context.TODO(), blockPvcRestore) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
err = tests.DeployManagerObj.GetCrClient().Delete(context.TODO(), blockPvcRestore) | |
err = tests.DeployManagerObj.GetCrClient().Delete(context.TODO(), restoreBlockPod) |
e2e/lvm/pvc_test.go
Outdated
Expect(err).To(BeNil()) | ||
fmt.Printf("Clone PVC %s is deleted\n", filePvcClone.Name) | ||
|
||
err = tests.DeployManagerObj.GetCrClient().Delete(context.TODO(), filePvcRestore) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
err = tests.DeployManagerObj.GetCrClient().Delete(context.TODO(), filePvcRestore) | |
err = tests.DeployManagerObj.GetCrClient().Delete(context.TODO(), restoreFilePod) |
e2e/testdata.go
Outdated
return vs | ||
} | ||
|
||
func GetSampleClone(quantity, name string, volumemode k8sv1.PersistentVolumeMode, storageClass string) *k8sv1.PersistentVolumeClaim { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func GetSampleClone(quantity, name string, volumemode k8sv1.PersistentVolumeMode, storageClass string) *k8sv1.PersistentVolumeClaim { | |
func GetSampleClone(size, name string, volumemode k8sv1.PersistentVolumeMode, storageClass string) *k8sv1.PersistentVolumeClaim { |
e2e/testdata.go
Outdated
return pvc | ||
} | ||
|
||
func GetRestoreVolume(quantity, name string, storageClass string) *k8sv1.PersistentVolumeClaim { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func GetRestoreVolume(quantity, name string, storageClass string) *k8sv1.PersistentVolumeClaim { | |
func GetRestoreVolume(size, name string, storageClass string) *k8sv1.PersistentVolumeClaim { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can also consider defining resources the way topolvm does for its e2e tests.
e2e/testdata.go
Outdated
return vs | ||
} | ||
|
||
func GetSampleClone(quantity, name string, volumemode k8sv1.PersistentVolumeMode, storageClass string) *k8sv1.PersistentVolumeClaim { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Combine the clone and restore functions into a single one taking a sourceType argument. The rest of the code is the same.
0d7d3ad
to
704166e
Compare
Thanks @nbalacha for your suggestions, have addressed them all. |
704166e
to
4b332bc
Compare
e2e/lvm/pvc_test.go
Outdated
var clonePod *k8sv1.Pod | ||
var restorePvc *k8sv1.PersistentVolumeClaim | ||
var restorePod *k8sv1.Pod | ||
test := tests.DeployManagerObj.GetCrClient() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
test := tests.DeployManagerObj.GetCrClient() | |
client := tests.DeployManagerObj.GetCrClient() |
e2e/lvm/pvc_test.go
Outdated
By("Creating pvc and pod") | ||
pvc = tests.GetSamplePVC(tests.StorageClass, size, "lvmfilepvc", k8sv1.PersistentVolumeFilesystem) | ||
pod = tests.GetSamplePod("lvmfilepod", pvc.Name) | ||
err := test.Create(context.TODO(), pvc) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these resources cleaned up in case of error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the end, we are deleting this namespace only, so anyway all of these will get removed.
e2e/lvm/pvc_test.go
Outdated
fmt.Printf("Snapshot %s is created\n", snapshot.Name) | ||
|
||
By("Creating a clone of the file-pvc") | ||
clonePvc = tests.GetPvc(size, pvc.Name, k8sv1.PersistentVolumeFilesystem, tests.StorageClass, "clone", "PersistentVolumeClaim") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any input should be in terms of the PVC being cloned. For example:
clonePvc = tests.GetPvc(pvc.size, pvc.Name, pvc.AccessMode, tests.StorageClass, "clone")
The last argument can be removed.
If sourceType== clone, it is of type PVC.
If sourceType==snapshot, it is of type VolumeSnapshot.
e2e/lvm/pvc_test.go
Outdated
fmt.Printf("Cloned PVC %s is bound\n", clonePvc.Name) | ||
|
||
By("Restore Snapshot for file-pvc") | ||
restorePvc = tests.GetPvc(size, pvc.Name+"snapshot", k8sv1.PersistentVolumeFilesystem, tests.StorageClass, "restore", "VolumeSnapshot") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Restoring a snapshot will create a PVC, not a snapshot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have passed snapshot as string here(pvc.Name+"snapshot") to specify the dataSource, as this is the format I used to name the snapshot.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can the GetSamplePVC be extended to do what GetPVC is doing?
4b332bc
to
f30be35
Compare
e2e/testdata.go
Outdated
} | ||
|
||
// GetSamplePvc returns restore or clone of the pvc based on the kind(kindName) provided. | ||
func GetSamplePvc(size, name string, volumemode k8sv1.PersistentVolumeMode, storageClass string, sourceType string) *k8sv1.PersistentVolumeClaim { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please refactor to something like this:
func GetSamplePvc(size, name string, volumemode k8sv1.PersistentVolumeMode, storageClass string, sourceType, sourceName string) *k8sv1.PersistentVolumeClaim {
var kindName string
var sourceName string
pvc := &k8sv1.PersistentVolumeClaim{
ObjectMeta: metav1.ObjectMeta{
Name: name,
Namespace: TestNamespace,
},
Spec: k8sv1.PersistentVolumeClaimSpec{
StorageClassName: &storageClass,
AccessModes: []k8sv1.PersistentVolumeAccessMode{k8sv1.ReadWriteOnce},
VolumeMode: &volumemode,
Resources: k8sv1.ResourceRequirements{
Requests: k8sv1.ResourceList{
k8sv1.ResourceStorage: resource.MustParse(size),
},
},
},
}
if sourceType != "" && sourceName != "" {
pvc.Spec.DataSource = &k8sv1.TypedLocalObjectReference{
Name: sourceName,
Kind: sourceType,
},
}
return pvc
}
e2e/testdata.go
Outdated
@@ -61,3 +44,62 @@ func GetSamplePod(name, pvcName string) *k8sv1.Pod { | |||
} | |||
return pod | |||
} | |||
|
|||
// GetSampleVolumeSnapshot creates and returns the VolumeSnapshot for the provided PVC. | |||
func GetSampleVolumeSnapshot(name string, storageClass string) *snapapi.VolumeSnapshot { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func GetSampleVolumeSnapshot(name string, storageClass string) *snapapi.VolumeSnapshot { | |
func GetSampleVolumeSnapshot(name string, storageClass string, sourceName string) *snapapi.VolumeSnapshot { |
e2e/testdata.go
Outdated
vs := &snapapi.VolumeSnapshot{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
// name = name of pvc + snapshot | ||
Name: name + "snapshot", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name: name + "snapshot", | |
Name: name, |
e2e/testdata.go
Outdated
Spec: snapapi.VolumeSnapshotSpec{ | ||
VolumeSnapshotClassName: &storageClass, | ||
Source: snapapi.VolumeSnapshotSource{ | ||
PersistentVolumeClaimName: &name, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PersistentVolumeClaimName: &name, | |
PersistentVolumeClaimName: &sourceName, |
3b9891d
to
2e57965
Compare
1bd9d79
to
534bc51
Compare
e2e/lvm/pvc_test.go
Outdated
fmt.Printf("Snapshot %s is created\n", snapshot.Name) | ||
|
||
By("Creating a clone of the file-pvc") | ||
clonePvc = tests.GetSamplePvc(size, pvc.Name, k8sv1.PersistentVolumeFilesystem, tests.StorageClass, "PersistentVolumeClaim", pvc.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clonePvc = tests.GetSamplePvc(size, pvc.Name, k8sv1.PersistentVolumeFilesystem, tests.StorageClass, "PersistentVolumeClaim", pvc.Name) | |
clonePvc = tests.GetSamplePvc(size, pvc.Name + "-clone", k8sv1.PersistentVolumeFilesystem, tests.StorageClass, "PersistentVolumeClaim", pvc.Name) |
e2e/lvm/pvc_test.go
Outdated
|
||
err = tests.DeployManagerObj.GetCrClient().Delete(context.TODO(), filepvc) | ||
By("Creating a clone of the block-pvc") | ||
clonePvc = tests.GetSamplePvc(size, pvc.Name, k8sv1.PersistentVolumeBlock, tests.StorageClass, "PersistentVolumeClaim", pvc.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clonePvc = tests.GetSamplePvc(size, pvc.Name, k8sv1.PersistentVolumeBlock, tests.StorageClass, "PersistentVolumeClaim", pvc.Name) | |
clonePvc = tests.GetSamplePvc(size, pvc.Name+ "-clone", k8sv1.PersistentVolumeBlock, tests.StorageClass, "PersistentVolumeClaim", pvc.Name) |
e2e/lvm/pvc_test.go
Outdated
fmt.Printf("Cloned PVC %s is bound\n", clonePvc.Name) | ||
|
||
By("Restore Snapshot for file-pvc") | ||
restorePvc = tests.GetSamplePvc(size, pvc.Name, k8sv1.PersistentVolumeFilesystem, tests.StorageClass, snapshot.Name, pvc.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
restorePvc = tests.GetSamplePvc(size, pvc.Name, k8sv1.PersistentVolumeFilesystem, tests.StorageClass, snapshot.Name, pvc.Name) | |
restorePvc = tests.GetSamplePvc(size, pvc.Name + "-restore", k8sv1.PersistentVolumeFilesystem, tests.StorageClass, snapshot.Name, pvc.Name) |
e2e/lvm/pvc_test.go
Outdated
fmt.Printf("Cloned PVC %s is bound\n", clonePvc.Name) | ||
|
||
By("Restore Snapshot for block-pvc") | ||
restorePvc = tests.GetSamplePvc(size, pvc.Name, k8sv1.PersistentVolumeBlock, tests.StorageClass, snapshot.Name, pvc.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
restorePvc = tests.GetSamplePvc(size, pvc.Name, k8sv1.PersistentVolumeBlock, tests.StorageClass, snapshot.Name, pvc.Name) | |
restorePvc = tests.GetSamplePvc(size, pvc.Name+"-restore", k8sv1.PersistentVolumeBlock, tests.StorageClass, snapshot.Name, pvc.Name) |
e2e/testdata.go
Outdated
} | ||
pvc := &k8sv1.PersistentVolumeClaim{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: name + kindName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name: name + kindName, | |
Name: name , |
e2e/testdata.go
Outdated
@@ -61,3 +41,54 @@ func GetSamplePod(name, pvcName string) *k8sv1.Pod { | |||
} | |||
return pod | |||
} | |||
|
|||
// GetSampleVolumeSnapshot creates and returns the VolumeSnapshot for the provided PVC. | |||
func GetSampleVolumeSnapshot(sourceName string, storageClass string) *snapapi.VolumeSnapshot { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
func GetSampleVolumeSnapshot(sourceName string, storageClass string) *snapapi.VolumeSnapshot { | |
func GetSampleVolumeSnapshot(snapshotName, sourceName string, storageClass string) *snapapi.VolumeSnapshot { |
e2e/testdata.go
Outdated
func GetSampleVolumeSnapshot(sourceName string, storageClass string) *snapapi.VolumeSnapshot { | ||
vs := &snapapi.VolumeSnapshot{ | ||
ObjectMeta: metav1.ObjectMeta{ | ||
Name: sourceName, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Name: sourceName, | |
Name: snapshotName, |
534bc51
to
3252962
Compare
Thanks @nbalacha. Addressed all the reviews. |
3252962
to
e815090
Compare
e2e/lvm/pvc_test.go
Outdated
fmt.Printf("Cloned PVC %s is bound\n", clonePvc.Name) | ||
|
||
By("Restore Snapshot for block-pvc") | ||
restorePvc = tests.GetSamplePvc(size, pvc.Name+"-restore", k8sv1.PersistentVolumeBlock, tests.StorageClass, snapshot.Name, pvc.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't look right - shouldn't it be
restorePvc = tests.GetSamplePvc(size, pvc.Name+"-restore", k8sv1.PersistentVolumeBlock, tests.StorageClass, snapshot.Name, pvc.Name) | |
restorePvc = tests.GetSamplePvc(size, pvc.Name+"-restore", k8sv1.PersistentVolumeBlock, tests.StorageClass, "VolumeSnapshot", snapshot.Name) |
e2e/lvm/pvc_test.go
Outdated
fmt.Printf("Cloned PVC %s is bound\n", clonePvc.Name) | ||
|
||
By("Restore Snapshot for file-pvc") | ||
restorePvc = tests.GetSamplePvc(size, pvc.Name+"-restore", k8sv1.PersistentVolumeFilesystem, tests.StorageClass, snapshot.Name, pvc.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
restorePvc = tests.GetSamplePvc(size, pvc.Name+"-restore", k8sv1.PersistentVolumeFilesystem, tests.StorageClass, snapshot.Name, pvc.Name) | |
restorePvc = tests.GetSamplePvc(size, pvc.Name+"-restore", k8sv1.PersistentVolumeFilesystem, tests.StorageClass, "VolumeSnapshot", pvc.Name) |
e2e/testdata.go
Outdated
if sourceType != "" && sourceName != "" { | ||
pvc.Spec.DataSource = &k8sv1.TypedLocalObjectReference{ | ||
Name: sourceName, | ||
Kind: sourceType, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if sourceType != "" && sourceName != "" { | |
pvc.Spec.DataSource = &k8sv1.TypedLocalObjectReference{ | |
Name: sourceName, | |
Kind: sourceType, | |
} | |
} | |
if sourceType != "" && sourceName != "" { | |
if sourceType == "VolumeSnapshot" { | |
pvc.Spec.DataSource = &k8sv1.TypedLocalObjectReference{ | |
Name: sourceName, | |
Kind: sourceType, | |
ApiGroup: "snapshot.storage.k8s.io" | |
} | |
} else { | |
pvc.Spec.DataSource = &k8sv1.TypedLocalObjectReference{ | |
Name: sourceName, | |
Kind: sourceType, | |
} | |
} | |
} |
Signed-off-by: riya-singhal31 <[email protected]>
e815090
to
8d00bb5
Compare
For file system: NAME READY STATUS RESTARTS AGE [riyasinghal@fedora lvm-operator]$ oc get vs -n lvm-endtoendtest For block: NAME READY STATUS RESTARTS AGE [riyasinghal@fedora lvm-operator]$ oc get vs |
@riya-singhal31: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: nbalacha, riya-singhal31, sp98 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
It creates snapshots and clones and check if the cloned PVC is bound or not.
Signed-off-by: riya-singhal31 [email protected]