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.

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 the test for ephemeral volume in e2e #246

Merged
merged 1 commit into from
Aug 25, 2022

Conversation

riya-singhal31
Copy link
Contributor

@riya-singhal31 riya-singhal31 commented Aug 24, 2022

This PR adds the test for ephemeral volume, by creating a pod for ephemeral volume and confirming its lifetime.
Also creates snapshot and clone for the ephemeral volume.

Signed-off-by: riya-singhal31 [email protected]

@riya-singhal31 riya-singhal31 changed the title test: added the test for ephemeral volume in e2e [WIP]test: added the test for ephemeral volume in e2e Aug 24, 2022
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 24, 2022
@riya-singhal31 riya-singhal31 force-pushed the ephemeral-test branch 2 times, most recently from d3da6a5 to b6e050d Compare August 24, 2022 14:01
resources:
requests:
storage: 1Gi
storageClassName: %s
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
storageClassName: %s
storageClassName: %s
volumeMode: Block

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done


func ephemeralTest() {
Describe("Ephemeral Volume Tests", func() {
var pvc *k8sv1.PersistentVolumeClaim
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var pvc *k8sv1.PersistentVolumeClaim
var pvc *k8sv1.PersistentVolumeClaim = &k8sv1.PersistentVolumeClaim{}

fmt.Printf("Pod %s is running\n", ephemeralPod.Name)

By("Creating a Snapshot of the pvc")
snapshotYaml := fmt.Sprintf(volumeSnapshotYAMLTemplate, "ephemeralfilepvc-snapshot", testNamespace, snapshotClass, "ephemeral-filepod-generic-ephemeral-volume")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create separate YAMLs for each resource you create in this test in the ephemeral_tests dir. It makes it easy to keep track of which file is used in which test.
Rename ephemeral_test dir to ephemeral_tests.

- ReadWriteOnce
resources:
requests:
storage: 1Gi
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PVC Clone operations will fail because the size in the pvcCloneYAMLTemplate does not match the size in the ephemeral volume spec. Please create a different yaml in the ephemeral_tests dir.

By("Verifying that the Snapshot is ready")
Eventually(func() bool {
err := crClient.Get(ctx, types.NamespacedName{Name: snapshot.Name, Namespace: snapshot.Namespace}, snapshot)
return err == nil && *snapshot.Status.ReadyToUse
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return err == nil && *snapshot.Status.ReadyToUse
return err == nil && snapshot.Status != nil && *snapshot.Status.ReadyToUse

By("Verifying that the Snapshot is ready")
Eventually(func() bool {
err := crClient.Get(ctx, types.NamespacedName{Name: snapshot.Name, Namespace: snapshot.Namespace}, snapshot)
return err == nil && *snapshot.Status.ReadyToUse
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return err == nil && *snapshot.Status.ReadyToUse
return err == nil && snapshot.Status != nil && *snapshot.Status.ReadyToUse

@riya-singhal31 riya-singhal31 force-pushed the ephemeral-test branch 2 times, most recently from 4ebff69 to 1462926 Compare August 25, 2022 05:31
@riya-singhal31 riya-singhal31 changed the title [WIP]test: added the test for ephemeral volume in e2e test: added the test for ephemeral volume in e2e Aug 25, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 25, 2022
@riya-singhal31 riya-singhal31 changed the title test: added the test for ephemeral volume in e2e [WIP]test: added the test for ephemeral volume in e2e Aug 25, 2022
@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 25, 2022
@@ -26,4 +26,5 @@ var _ = AfterSuite(func() {
var _ = Describe("LVMO e2e tests", func() {
Context("LVMCluster reconciliation", validateResources)
Context("PVC tests", pvcTest)
Context("Ephemeral tests", ephemeralTest)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Context("Ephemeral tests", ephemeralTest)
Context("Ephemeral volume tests", ephemeralTest)

@riya-singhal31 riya-singhal31 changed the title [WIP]test: added the test for ephemeral volume in e2e test: added the test for ephemeral volume in e2e Aug 25, 2022
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 25, 2022
Copy link
Contributor

@nbalacha nbalacha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@riya-singhal31 riya-singhal31 force-pushed the ephemeral-test branch 2 times, most recently from 3e32301 to f1590cd Compare August 25, 2022 06:31
@riya-singhal31
Copy link
Contributor Author

Please update

https://github.com/red-hat-storage/lvm-operator/blob/a99277e6e488db82f43faac58201cedeca0f4ff4/.github/workflows/conf/yamllint.yaml#L7

to ignore the newly added template yamls .

Thanks for mentioning. Updated.

Comment on lines 16 to 35
//go:embed testdata/ephemeral_tests/pod-ephemeral-volume-device.yaml
var podEphemeralBlockYAMLTemplate string

//go:embed testdata/ephemeral_tests/pod-ephemeral-volume-mount.yaml
var podEphemeralFSYAMLTemplate string

//go:embed testdata/ephemeral_tests/ephemeral-volume-snapshot.yaml
var ephemeralVolumeSnapshotYAMLTemplate string

//go:embed testdata/ephemeral_tests/ephemeral-clone.yaml
var ephemeralPvcCloneYAMLTemplate string

//go:embed testdata/ephemeral_tests/ephemeral-snapshot-restore.yaml
var ephemeralPvcSnapshotRestoreYAMLTemplate string

//go:embed testdata/ephemeral_tests/pod-volume-mount-template.yaml
var podFSYAMLTemplate string

//go:embed testdata/ephemeral_tests/pod-volume-device-template.yaml
var podBlockYAMLTemplate string

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of multiple variable declarations, create a var block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, updated.

Comment on lines +68 to +76
return err == nil && ephemeralPod.Status.Phase == k8sv1.PodRunning
}, timeout, interval).Should(BeTrue())
fmt.Printf("Pod %s is running\n", ephemeralPod.Name)

By("Creating a Snapshot of the pvc")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Write some data in the parent pvc and verify it later on cloned/restored volumes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's another PR doing this check. #235
Will modify once that gets merged.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Aug 25, 2022
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Aug 25, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: nbalacha, riya-singhal31

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 25, 2022
@openshift-merge-robot openshift-merge-robot merged commit a6d1875 into openshift:main Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants