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

cleanup: simplify rbdGetDeviceList() #4364

Merged
merged 1 commit into from
Jan 11, 2024

Conversation

nixpanic
Copy link
Member

@nixpanic nixpanic commented Jan 5, 2024

The rbdGetDeviceList() function uses two very similar types for
converting krbd and NBD device information from JSON. There is no need
to use this distinction, and callers of rbdGetDeviceList() should not
need to care about it either.

By introducing a deviceInfo interface with Get-functions, the
rbdGetDeviceList() function becomes a little simpler, with a clearly
defined API for the returned list.

@mergify mergify bot added the cleanup label Jan 5, 2024
@nixpanic nixpanic added the component/rbd Issues related to RBD label Jan 5, 2024
@nixpanic
Copy link
Member Author

nixpanic commented Jan 5, 2024

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

GetRadosNamespace() string
GetDevice() string
}

// rbdDeviceInfo strongly typed JSON spec for rbd device list output (of type krbd).
type rbdDeviceInfo struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

We no longer have to save ID ?
It's not used anywhere ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, it is not used anywhere for attaching/mounting.

@nixpanic
Copy link
Member Author

@Mergifyio rebase

The `rbdGetDeviceList()` function uses two very similar types for
converting krbd and NBD device information from JSON. There is no need
to use this distinction, and callers of `rbdGetDeviceList()` should not
need to care about it either.

By introducing a `deviceInfo` interface with Get-functions, the
`rbdGetDeviceList()` function becomes a little simpler, with a clearly
defined API for the returned list.

Signed-off-by: Niels de Vos <[email protected]>
Copy link
Contributor

mergify bot commented Jan 11, 2024

rebase

✅ Branch has been successfully rebased

@nixpanic
Copy link
Member Author

@Mergifyio queue

Copy link
Contributor

mergify bot commented Jan 11, 2024

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 3bf5c0e

@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Jan 11, 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/k8s-e2e-external-storage/1.28

@ceph-csi-bot
Copy link
Collaborator

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

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

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

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

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

Successfully merging this pull request may close these issues.

4 participants