Skip to content

build: move e2e dependencies into e2e/go.mod #5188

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 5 commits into from
Mar 7, 2025
Merged

build: move e2e dependencies into e2e/go.mod #5188

merged 5 commits into from
Mar 7, 2025

Conversation

@nixpanic
Copy link
Member

@nixpanic nixpanic commented Mar 4, 2025

Several packages are only used while running the e2e suite. These
packages are less important to update, as the they can not influence the
final executable that is part of the Ceph-CSI container-image.

By moving these dependencies out of the main Ceph-CSI go.mod, it is
easier to identify if a reported CVE affects Ceph-CSI, or only the
testing (like most of the Kubernetes CVEs).

@mergify mergify bot added component/testing Additional test cases or CI work component/build Issues and PRs related to compiling Ceph-CSI labels Mar 4, 2025
@nixpanic nixpanic force-pushed the e2e-module branch 2 times, most recently from ddc411b to 08805a0 Compare March 4, 2025 16:44
@nixpanic nixpanic marked this pull request as ready for review March 4, 2025 17:05
@nixpanic nixpanic requested review from a team, Rakshith-R and iPraveenParihar March 4, 2025 17:05
Rakshith-R
Rakshith-R reviewed Mar 5, 2025
View reviewed changes
e2e/rbd.go
"time"

"github.com/ceph/ceph-csi/internal/util"
"github.com/ceph/ceph-csi/pkg/util/crypto"
Copy link
Contributor

Choose a reason for hiding this comment

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

This line should have been included in the previous commit.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah! yes.

@nixpanic nixpanic requested a review from Rakshith-R March 6, 2025 08:12
@black-dragon74
Copy link
Member

If we do not have plans to use pkg for anything else, may I suggest a top level utils instead?

@nixpanic
Copy link
Member Author

nixpanic commented Mar 6, 2025

If we do not have plans to use pkg for anything else, may I suggest a top level utils instead?

pkg is a common name for go projects, so I decided to take that. In the future, we may be able to move the internal rbd.Manager to a public pkg package too, so that other tools (rook cli, or other debugging/troubleshooting tools) can use it.

@nixpanic nixpanic requested review from a team and black-dragon74 March 6, 2025 09:08
black-dragon74
black-dragon74 previously approved these changes Mar 6, 2025
View reviewed changes
Rakshith-R
Rakshith-R previously approved these changes Mar 6, 2025
View reviewed changes
@nixpanic
Copy link
Member Author

nixpanic commented Mar 6, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Mar 6, 2025 •
edited
Loading

queue

🛑 The pull request has been removed from the queue default

The merge conditions cannot be satisfied due to failing checks.

You can take a look at Queue: Embarked in merge queue check runs for more details.

In case of a failure due to a flaky test, you should first retrigger the CI.
Then, re-embark the pull request into the merge queue by posting the comment
@mergifyio refresh on the pull request.

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Mar 6, 2025
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@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/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e-helm/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-helm/k8s-1.32

@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/k8s-1.31

@iPraveenParihar
Copy link
Contributor

/retest ci/centos/mini-e2e/k8s-1.32

Failed with:

[FAILED] failed to validate omap count for rados ls --pool=myfs-metadata --namespace=csi | grep -v default | grep -v csi.volume.group. | grep -c ^csi.volume.: expected omap object count 4, got 5

logs

@nixpanic, all jobs have failed at the CephFS test verifying that ceph-fuse recovery works for new pods. The NFS test failures are due to stale entries from the failed CephFS test case, so they can be ignored.

I've seen this test case fail previously with mount-utils versions later than v0.29.3 (reference: #4633). To address this, I had added k8s.io/mount-utils v0.29.3 in the replace section, but it's missing from the main go.mod file, which I suspect is causing the failure. Could you try adding the replace directive in the main go.mod file and see if that resolves the issue?

nixpanic added 5 commits March 7, 2025 14:20
@nixpanic
util: move EncryptionType(s) to pkg/util/crypto
538b964 
Signed-off-by: Niels de Vos <[email protected]>
@nixpanic
util: move kernel version functions to pkg/util/kernel
1a5cc20 
Signed-off-by: Niels de Vos <[email protected]>
@nixpanic
build: move e2e dependencies into e2e/go.mod
43b26e6 
Several packages are only used while running the e2e suite. These
packages are less important to update, as the they can not influence the
final executable that is part of the Ceph-CSI container-image.

By moving these dependencies out of the main Ceph-CSI go.mod, it is
easier to identify if a reported CVE affects Ceph-CSI, or only the
testing (like most of the Kubernetes CVEs).

Signed-off-by: Niels de Vos <[email protected]>
@nixpanic
ci: add e2e module to Dependabot configuration
9721b20 
Signed-off-by: Niels de Vos <[email protected]>
@nixpanic
build: no need to list all go modules
6e38e54 
It is not clear why listing all modules is useful. Some modules have
internal versions (Kubernetes modules), and the command reports those as
a warning with an error exit code.

Signed-off-by: Niels de Vos <[email protected]>
@nixpanic
Copy link
Member Author

nixpanic commented Mar 7, 2025

@nixpanic, all jobs have failed at the CephFS test verifying that ceph-fuse recovery works for new pods. The NFS test failures are due to stale entries from the failed CephFS test case, so they can be ignored.

I've seen this test case fail previously with mount-utils versions later than v0.29.3 (reference: #4633). To address this, I had added k8s.io/mount-utils v0.29.3 in the replace section, but it's missing from the main go.mod file, which I suspect is causing the failure. Could you try adding the replace directive in the main go.mod file and see if that resolves the issue?

Thanks for the suggestion, @iPraveenParihar! The replacing of mount-utils is done in the e2e/go.mod file, but not anymore in main go.mod. I have added that now again, lets see ho things go 🤞

@nixpanic nixpanic added the ok-to-test Label to trigger E2E tests label Mar 7, 2025
@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-cephfs

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/upgrade-tests-rbd

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@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-helm/k8s-1.30

@ceph-csi-bot
Copy link
Collaborator

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

@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.30

@ceph-csi-bot
Copy link
Collaborator

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

@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 Mar 7, 2025
@nixpanic nixpanic requested a review from a team March 7, 2025 15:09
@nixpanic
Copy link
Member Author

nixpanic commented Mar 7, 2025

Thanks for the suggestion, @iPraveenParihar! The replacing of mount-utils is done in the e2e/go.mod file, but not anymore in main go.mod. I have added that now again, lets see ho things go 🤞

Seems to pass with the old mount-utils package!

@Rakshith-R
Copy link
Contributor

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Mar 7, 2025 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 5e19ed5

@mergify mergify bot merged commit 5e19ed5 into ceph:devel Mar 7, 2025
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@black-dragon74 black-dragon74 black-dragon74 approved these changes

@Rakshith-R Rakshith-R Rakshith-R approved these changes

@iPraveenParihar iPraveenParihar Awaiting requested review from iPraveenParihar

Assignees

No one assigned

Labels

component/build Issues and PRs related to compiling Ceph-CSI component/testing Additional test cases or CI work

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@nixpanic @black-dragon74 @ceph-csi-bot @iPraveenParihar @Rakshith-R