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

support nbd on euler or arm #4291

Merged
merged 1 commit into from
Mar 15, 2024
Merged

support nbd on euler or arm #4291

merged 1 commit into from
Mar 15, 2024

Conversation

muxuelanKK
Copy link

Describe what this PR does

For euler or arm,command “rbd device list --format=json --device-type=nbd” can be response slowly, often more than 5 miniutes. So we use rbd-nbd list-mapped instead.

Is there anything that requires special attention

Do you have any questions?

Is the change backward compatible?

Are there concerns around backward compatibility?

Provide any external context for the change, if any.

@nixpanic nixpanic requested a review from pkalever November 28, 2023 13:36
@nixpanic nixpanic added the component/rbd Issues related to RBD label Nov 28, 2023
Copy link

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in two weeks if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Dec 28, 2023
@nixpanic nixpanic removed the stale label Jan 4, 2024
@nixpanic
Copy link
Member

nixpanic commented Jan 4, 2024

I think this looks reasonable. Could you update the title of the commit to

rbd: support nbd on euler or arm

so that commitlint will accept it? You'll need to force-push that change to your branch that you used to create this PR from.

Thanks!

isUnmap := CheckSliceContains(cmdArgs, "unmap")
if !isUnmap {
if !strings.Contains(userOptions, useNbdNetlink) {
cmdArgs = append(cmdArgs, "--options", useNbdNetlink)
cmdArgs = append(cmdArgs, "--"+useNbdNetlink)
Copy link
Member

Choose a reason for hiding this comment

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

My main concern with this patch is that the --options parameter seems to be dropped. Is that intentional, and does it keep working with the currently maintained Ceph versions?

@nixpanic
Copy link
Member

nixpanic commented Jan 5, 2024

Inspired by this PR, I've created #4364 which will cause a little conflict, sorry!

Copy link
Contributor

mergify bot commented Jan 11, 2024

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

1 similar comment
Copy link
Contributor

mergify bot commented Feb 6, 2024

This pull request now has conflicts with the target branch. Could you please resolve conflicts and force push the corrected changes? 🙏

@nixpanic
Copy link
Member

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

@nixpanic nixpanic requested a review from a team February 19, 2024 14:53
@nixpanic
Copy link
Member

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

This passed, so I think all still works. Ideally it is more efficient now too.

@pkalever can you have a quick look and check if it doesn't break anything?

Copy link
Member

@nixpanic nixpanic left a comment

Choose a reason for hiding this comment

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

CI job passed, so functionality should be OK.

@nixpanic nixpanic requested a review from a team March 14, 2024 08:30
@nixpanic
Copy link
Member

@Mergifyio rebase

Copy link
Contributor

mergify bot commented Mar 14, 2024

rebase

✅ Branch has been successfully rebased

@nixpanic
Copy link
Member

@Mergifyio queue

Copy link
Contributor

mergify bot commented Mar 14, 2024

queue

🛑 The pull request has been removed from the queue default

The queue 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 14, 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/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/k8s-e2e-external-storage/1.27

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Mar 14, 2024
@nixpanic
Copy link
Member

@Mergifyio rebase

Signed-off-by: muxuelan <muxuelan@cmss.chinamobile.com>
Copy link
Contributor

mergify bot commented Mar 15, 2024

rebase

✅ Branch has been successfully rebased

@nixpanic
Copy link
Member

@Mergifyio queue

Copy link
Contributor

mergify bot commented Mar 15, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 4f04748

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Mar 15, 2024
@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.29

@ceph-csi-bot
Copy link
Collaborator

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

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot ceph-csi-bot removed the ok-to-test Label to trigger E2E tests label Mar 15, 2024
@mergify mergify bot merged commit 4f04748 into ceph:devel Mar 15, 2024
34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/rbd Issues related to RBD
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants