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

Add E2E test cases for backup VolumeInfo feature. #7396

Merged
merged 2 commits into from
Feb 23, 2024

Conversation

blackpiglet
Copy link
Contributor

@blackpiglet blackpiglet commented Feb 6, 2024

Thank you for contributing to Velero!

Please add a summary of your change

Does your change fix a particular issue?

Fixes #7388

Please indicate you've done the following:

  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Created a changelog file or added /kind changelog-not-required as a comment on this pull request.
  • Updated the corresponding documentation in site/content/docs/main.

@blackpiglet blackpiglet added kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes and removed has-e2e-tests labels Feb 6, 2024
@blackpiglet blackpiglet self-assigned this Feb 6, 2024
Copy link

codecov bot commented Feb 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (174c10f) 61.64% compared to head (ef5c2ed) 61.64%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7396   +/-   ##
=======================================
  Coverage   61.64%   61.64%           
=======================================
  Files         262      262           
  Lines       28480    28480           
=======================================
  Hits        17556    17556           
  Misses       9691     9691           
  Partials     1233     1233           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@blackpiglet blackpiglet force-pushed the backup_volumeinfo_e2e branch 5 times, most recently from 8dee79a to 40b9af8 Compare February 7, 2024 09:26
@blackpiglet blackpiglet marked this pull request as ready for review February 7, 2024 10:06
@blackpiglet blackpiglet requested review from danfengliu and removed request for anshulahuja98 February 7, 2024 10:06
@blackpiglet
Copy link
Contributor Author

E2E test passed on developing AWS, Azure, and GCP environments. Ready for review.

f.BackupName,
BackupObjectsPrefix+"/"+f.BackupName,
)

Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to print volume info file content for case improvement in future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added.

if strings.Contains(v.VeleroCfg.Features, "EnableCSI") {
if strings.Contains(v.CaseBaseName, "native-snapshot") {
fmt.Printf("Skip native snapshot case %s when the CSI feature is enabled.\n", v.CaseBaseName)
Skip("Skip due to vSphere CSI driver long time issue of Static provisioning")
Copy link
Contributor

Choose a reason for hiding this comment

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

Skip reason is not right here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified.

BackupVolumeInfo{
SnapshotVolumes: true,
TestCase: TestCase{
CaseBaseName: "csi-snapshot-volumeinfo",
Copy link
Contributor

Choose a reason for hiding this comment

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

This test case should be skipped on vSphere pipeline since we're not going test Velero CSI plugin on vSphere cluster in near future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Add the check in the base test case.

@@ -189,3 +194,59 @@ func IsSnapshotExisted(cloudProvider, cloudCredentialsFile, bslBucket, bslConfig
}
return nil
}

func GetVolumeInfoMetadataContent(
cloudProvider,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should use objectStoreProvider as input when getting volumeinfo.json.gz from an object store, cloudProvider might not same with objectStoreProvider when having test on Kind or other local clusters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified.

@blackpiglet blackpiglet force-pushed the backup_volumeinfo_e2e branch 3 times, most recently from c6f6901 to 3151ace Compare February 22, 2024 05:58
// No need to install the VSC for vsphere environment.
if v.VeleroCfg.CloudProvider != "vsphere" {
// Install VolumeSnapshotClass
Expect(KubectlApplyByFile(v.Ctx, fmt.Sprintf("../testdata/volume-snapshot-class/%s.yaml", v.VeleroCfg.CloudProvider))).To(Succeed(), "Failed to install VolumeSnapshotClass")
Copy link
Contributor

Choose a reason for hiding this comment

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

VolumeSnapshotClass installation should be moved to Velero installation part, because other test cases with CSI snapshot test steps will also need VolumeSnapshotClass if it is not installed before E2E execution. This test case can not guarantee CSI plugin is passed to the E2E test, currently that depends on CI to pass CSI plugin and EnableCSI flag to E2E test, so we assume we can not have native-snapshot and CSI-snapshot in the same test execution, it's not as design for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified.

Xun Jiang and others added 2 commits February 22, 2024 16:18
Signed-off-by: Xun Jiang <blackpigletbruce@gmail.com>
Signed-off-by: Xun Jiang <blackpigletbruce@gmail.com>
@danfengliu danfengliu merged commit 82f8481 into vmware-tanzu:main Feb 23, 2024
66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-e2e-2tests has-e2e-tests kind/changelog-not-required PR does not require a user changelog. Often for docs, website, or build changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DataDownload fails during restore when SnapshotMoveData: true for empty PVC workload
3 participants