Skip to content

Add e2e/integration tests for image snapshot #929

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
Merged
merged 3 commits into from
Apr 1, 2022
Merged

Add e2e/integration tests for image snapshot #929

merged 3 commits into from
Apr 1, 2022

Conversation

luohao
Copy link
Contributor

@luohao luohao commented Mar 16, 2022

What type of PR is this?

/kind feature

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #924

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

N/A

@mattcary
Copy link
Contributor

/assign
/ok-to-test

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. kind/feature Categorizes issue or PR as related to a new feature. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 16, 2022
@k8s-ci-robot
Copy link
Contributor

Hi @luohao. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

@k8s-ci-robot k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Mar 16, 2022
@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Mar 16, 2022
@luohao luohao force-pushed the hluo/image-integration-tests branch from 7bd75d6 to a40772f Compare March 16, 2022 16:56
@luohao
Copy link
Contributor Author

luohao commented Mar 16, 2022

/test pull-gcp-compute-persistent-disk-csi-driver-e2e

@luohao
Copy link
Contributor Author

luohao commented Mar 16, 2022

/retest

@luohao luohao force-pushed the hluo/image-integration-tests branch from a40772f to b15acd6 Compare March 16, 2022 18:51
@luohao
Copy link
Contributor Author

luohao commented Mar 16, 2022

/retest

@luohao luohao force-pushed the hluo/image-integration-tests branch from b15acd6 to 50045ff Compare March 16, 2022 21:18
@luohao
Copy link
Contributor Author

luohao commented Mar 16, 2022

/retest

@luohao luohao force-pushed the hluo/image-integration-tests branch from 50045ff to ea2f074 Compare March 18, 2022 17:12
@luohao
Copy link
Contributor Author

luohao commented Mar 18, 2022

/retest

@luohao
Copy link
Contributor Author

luohao commented Mar 18, 2022

/test pull-gcp-compute-persistent-disk-csi-driver-e2e

mattcary
mattcary reviewed Mar 18, 2022
View reviewed changes
test/e2e/tests/single_zone_e2e_test.go Outdated
Expect(err).To(BeNil(), "Could not get snapshot from cloud directly")
Expect(snapshot.Name).To(Equal(snapshotName))

err = wait.Poll(10*time.Second, 3*time.Minute, func() (bool, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you've seen disk image creation take up to 4 minutes? Maybe should set timeout to 5 min to decrease test flakiness.

test/run-k8s-integration-ci.sh
@@ -29,7 +29,7 @@ readonly run_intree_plugin_tests=${RUN_INTREE_PLUGIN_TESTS:-false}
readonly use_kubetest2=${USE_KUBETEST2:-true}
readonly test_pd_labels=${TEST_PD_LABELS:-true}
readonly migration_test=${MIGRATION_TEST:-false}
readonly test_disk_image_snapshot=${TEST_DISK_IMAGE_SNAPSHOT:-false}
readonly test_disk_image_snapshot=${TEST_DISK_IMAGE_SNAPSHOT:-true}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use multiple snapshot class files with the test flag? If so, we should just set the CI to run both the conventional & disk image snapshots.

@luohao luohao force-pushed the hluo/image-integration-tests branch from ea2f074 to 21f2dc6 Compare March 18, 2022 19:31
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 18, 2022
@luohao
Copy link
Contributor Author

luohao commented Mar 18, 2022

/retest

@luohao luohao force-pushed the hluo/image-integration-tests branch from 21f2dc6 to e93645e Compare March 18, 2022 23:19
@luohao
Copy link
Contributor Author

luohao commented Mar 18, 2022

/retest

@luohao luohao force-pushed the hluo/image-integration-tests branch from f6ab3fb to 598af14 Compare March 28, 2022 18:17
@luohao
Copy link
Contributor Author

luohao commented Mar 28, 2022

/retest

1 similar comment
@luohao
Copy link
Contributor Author

luohao commented Mar 28, 2022

/retest

@luohao
Copy link
Contributor Author

luohao commented Mar 28, 2022

/test pull-gcp-compute-persistent-disk-csi-driver-e2e

@luohao
Copy link
Contributor Author

luohao commented Mar 28, 2022

/retest

@luohao
Copy link
Contributor Author

luohao commented Mar 28, 2022

/test pull-gcp-compute-persistent-disk-csi-driver-kubernetes-integration

1 similar comment
@luohao
Copy link
Contributor Author

luohao commented Mar 28, 2022

/test pull-gcp-compute-persistent-disk-csi-driver-kubernetes-integration

@luohao luohao force-pushed the hluo/image-integration-tests branch from d65c583 to 7d9dc0d Compare March 29, 2022 02:12
@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 29, 2022
@luohao
Fix image creation and clone
aaebd25 
This patch fixed two issues in image creation and clone:
- add forceCreate flag to avoid image creation failures when source disk
  is attached
- fix the missing content source in CreateVolume response when restoring
  an image
@luohao luohao force-pushed the hluo/image-integration-tests branch from 7d9dc0d to 8076b27 Compare March 29, 2022 02:15
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Mar 29, 2022
@luohao
Copy link
Contributor Author

luohao commented Mar 29, 2022

/retest

@luohao luohao force-pushed the hluo/image-integration-tests branch from 8076b27 to fc50ff8 Compare March 29, 2022 04:02
@luohao luohao marked this pull request as ready for review March 29, 2022 04:02
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Mar 29, 2022
@k8s-ci-robot k8s-ci-robot requested a review from saad-ali March 29, 2022 04:02
@luohao
Copy link
Contributor Author

luohao commented Mar 29, 2022

/test pull-gcp-compute-persistent-disk-csi-driver-e2e

@mattcary
Copy link
Contributor

Can you edit driver-config.go and configs/test-config-template.in so that the driver name includes the snapshot storage class, or some reasonable abbreviation? Then we can see what snapshot class was used in the tests as it appears in the [Driver: XXX] tag in the test output.

@luohao
Copy link
Contributor Author

luohao commented Mar 30, 2022

@mattcary I updated the driver name by appending the snapshotclass name to it(when no snapshot class is specified, it appends no-volumesnapshotclass to it).

The driver name(csi-gcepd-sc-standard--pd-volumesnapshotclass) looks ok to me(originally I thought the name might be too long).

Can you take a look, please? Thanks!

@mattcary
Copy link
Contributor

mattcary commented Apr 1, 2022

/lgtm
/approve

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Apr 1, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: luohao, mattcary

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 1, 2022
@k8s-ci-robot k8s-ci-robot merged commit 9681ab1 into kubernetes-sigs:master Apr 1, 2022
mattcary added a commit that referenced this pull request Apr 1, 2022
@mattcary
Merge pull request #955 from mattcary/cp-image
633e466 
Cherry-pick of #929
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mattcary mattcary mattcary left review comments

@saikat-royc saikat-royc Awaiting requested review from saikat-royc

@saad-ali saad-ali Awaiting requested review from saad-ali

Assignees

@mattcary mattcary

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/feature Categorizes issue or PR as related to a new feature. lgtm "Looks good to me", indicates that a PR is ready to be merged. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support disk image snapshots
3 participants
@luohao @mattcary @k8s-ci-robot