-
Notifications
You must be signed in to change notification settings - Fork 550
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
Conversation
/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:
|
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.
@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?
@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. |
@Mergifyio rebase |
✅ Branch has been successfully rebased |
c097524
to
d0e18fb
Compare
/test ci/centos/k8s-e2e-external-storage/1.29 |
/test ci/centos/upgrade-tests-cephfs |
/test ci/centos/mini-e2e-helm/k8s-1.29 |
/test ci/centos/upgrade-tests-rbd |
/test ci/centos/mini-e2e/k8s-1.29 |
/test ci/centos/k8s-e2e-external-storage/1.30 |
/test ci/centos/mini-e2e-helm/k8s-1.30 |
/test ci/centos/k8s-e2e-external-storage/1.31 |
/test ci/centos/mini-e2e/k8s-1.30 |
/test ci/centos/mini-e2e-helm/k8s-1.31 |
/test ci/centos/mini-e2e/k8s-1.31 |
return vgsc, nil | ||
} | ||
|
||
func (rvgs *rbdVolumeGroupSnapshot) ValidateResourcesForCreate(vgs *groupsnapapi.VolumeGroupSnapshot) 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.
good to have comment for function description, maybe describe the validation criteria.
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.
Comments are provided in the interface at e2e/volumegroupsnapshot_base.go
.
Tested with RBD VolumeGroupSnapshot in PR#4502. |
@Mergifyio rebase |
Signed-off-by: Niels de Vos <[email protected]>
Signed-off-by: Niels de Vos <[email protected]>
✅ Branch has been successfully rebased |
5090c83
to
5ada0cb
Compare
@Mergifyio queue |
✅ The pull request has been merged automaticallyThe pull request has been merged automatically at c451997 |
/test ci/centos/k8s-e2e-external-storage/1.31 |
/test ci/centos/k8s-e2e-external-storage/1.30 |
/test ci/centos/mini-e2e-helm/k8s-1.31 |
/test ci/centos/mini-e2e-helm/k8s-1.30 |
/test ci/centos/k8s-e2e-external-storage/1.29 |
/test ci/centos/mini-e2e/k8s-1.31 |
/test ci/centos/mini-e2e/k8s-1.30 |
/test ci/centos/mini-e2e-helm/k8s-1.29 |
/test ci/centos/upgrade-tests-cephfs |
/test ci/centos/mini-e2e/k8s-1.29 |
/test ci/centos/upgrade-tests-rbd |
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 unrelatedfailure (please report the failure too!)