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

e2e: add tests for RBD VolumeGroupSnapshots #4899

Merged
merged 2 commits into from
Nov 4, 2024

Conversation

nixpanic
Copy link
Member

@nixpanic nixpanic commented Oct 10, 2024

Support for RBD VolumeGroupSnapshots is coming soon. However the implementation depends on changes in librbd, which are currently only available in the Ceph master branch.

Only when librbd in the Ceph-CSI container has support for the new function, the tests should te attempred

Related issues

This is no a 100% strict dependency, as current Ceph releases do not have the required functions yet. It is expected that #4502 gets merged before Ceph has a stable release (non master branch builds).

Sortof-depends-on: #4502

Example YAML files that are used by the test are posted as #4901.


Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)

@nixpanic
Copy link
Member Author

/test ci/centos/mini-e2e-helm/k8s-1.31

@mergify mergify bot added the component/testing Additional test cases or CI work label Oct 10, 2024
@nixpanic
Copy link
Member Author

/test ci/centos/mini-e2e-helm/k8s-1.31

This job is expected to pass, with the VolumeGroupSnapshot test getting skipped.

@nixpanic
Copy link
Member Author

/test ci/centos/mini-e2e-helm/k8s-1.31

This job is expected to pass, with the VolumeGroupSnapshot test getting skipped.

The e2e run contains this note in the logs:

[SKIPPED] librbd does not support required VolumeGroupSnapshot function(s)

Copy link
Collaborator

@Madhu-1 Madhu-1 left a comment

Choose a reason for hiding this comment

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

@nixpanic we dont support running test with different version or the cephcsi driver in other words the e2e and the supported cephcsi version are tied, am not sure having this check helps because we have the required base image, this check becomes obsolute and not holds much value. should we make this changes as part of the PR which is actual testing the code changes?

@nixpanic
Copy link
Member Author

nixpanic commented Oct 15, 2024

@Madhu-1, once this and the VolumeGroupSnapshot PR are merged, we can create a PR that updates the base image to the Ceph main branch version that includes the required features. There is no Ceph release yet that includes them, so for the moment, we can run the tests on stable Ceph-CSI images.

@nixpanic nixpanic requested a review from Madhu-1 October 18, 2024 07:56
Madhu-1
Madhu-1 previously approved these changes Oct 18, 2024
@Madhu-1 Madhu-1 requested a review from a team October 18, 2024 07:57
@nixpanic nixpanic added the ci/skip/multi-arch-build skip building on multiple architectures label Oct 18, 2024
@nixpanic
Copy link
Member Author

@Mergifyio rebase

Copy link
Contributor

mergify bot commented Oct 18, 2024

rebase

✅ Branch has been successfully rebased

@nixpanic nixpanic force-pushed the e2e/rbd/volume-group-snapshot branch from c097524 to d0e18fb Compare October 18, 2024 14:45
@nixpanic nixpanic added the ok-to-test Label to trigger E2E tests label Oct 18, 2024
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.31

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Oct 18, 2024
return vgsc, nil
}

func (rvgs *rbdVolumeGroupSnapshot) ValidateResourcesForCreate(vgs *groupsnapapi.VolumeGroupSnapshot) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

good to have comment for function description, maybe describe the validation criteria.

Copy link
Member Author

Choose a reason for hiding this comment

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

Comments are provided in the interface at e2e/volumegroupsnapshot_base.go.

@mergify mergify bot dismissed Madhu-1’s stale review October 24, 2024 12:57

Pull request has been modified.

@nixpanic
Copy link
Member Author

Tested with RBD VolumeGroupSnapshot in PR#4502.

@nixpanic nixpanic requested a review from a team November 1, 2024 08:47
@Madhu-1 Madhu-1 requested a review from a team November 4, 2024 07:33
@Madhu-1
Copy link
Collaborator

Madhu-1 commented Nov 4, 2024

@Mergifyio rebase
@Mergifyio queue

Copy link
Contributor

mergify bot commented Nov 4, 2024

rebase

✅ Branch has been successfully rebased

@Madhu-1 Madhu-1 force-pushed the e2e/rbd/volume-group-snapshot branch from 5090c83 to 5ada0cb Compare November 4, 2024 13:47
@Madhu-1
Copy link
Collaborator

Madhu-1 commented Nov 4, 2024

@Mergifyio queue

Copy link
Contributor

mergify bot commented Nov 4, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at c451997

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Nov 4, 2024
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/k8s-e2e-external-storage/1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.31

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/k8s-1.29

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Nov 4, 2024
@mergify mergify bot merged commit c451997 into ceph:devel Nov 4, 2024
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci/skip/multi-arch-build skip building on multiple architectures component/testing Additional test cases or CI work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants