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

cephfs: add CSI-Addons support to the cephfs #3991

Merged
merged 1 commit into from
Jul 14, 2023
Merged

cephfs: add CSI-Addons support to the cephfs #3991

merged 1 commit into from
Jul 14, 2023

Conversation

riya-singhal31
Copy link
Contributor

this commit adds CSI-Addons support to the
cephfs provisioner

Updates: #3969

@mergify mergify bot added the component/cephfs Issues related to CephFS label Jul 11, 2023
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.

This doesn't look complete, please make sure you also implement the services for completeness.

Comment on lines 80 to 97
},
&identity.Capability{
Type: &identity.Capability_ReclaimSpace_{
ReclaimSpace: &identity.Capability_ReclaimSpace{
Type: identity.Capability_ReclaimSpace_OFFLINE,
},
},
}, &identity.Capability{
Type: &identity.Capability_NetworkFence_{
NetworkFence: &identity.Capability_NetworkFence{
Type: identity.Capability_NetworkFence_NETWORK_FENCE,
},
},
}, &identity.Capability{
Type: &identity.Capability_VolumeReplication_{
VolumeReplication: &identity.Capability_VolumeReplication{
Type: identity.Capability_VolumeReplication_VOLUME_REPLICATION,
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we support this for cephfs csi driver?

Comment on lines 113 to 117
Type: &identity.Capability_ReclaimSpace_{
ReclaimSpace: &identity.Capability_ReclaimSpace{
Type: identity.Capability_ReclaimSpace_ONLINE,
},
},
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we support reclaimspace for cephfs?

@riya-singhal31
Copy link
Contributor Author

Thanks for the review Madhu, updated the capabilities supported by cephfs.
PTAL.

// setupCSIAddonsServer creates a new CSI-Addons Server on the given (URL)
// endpoint. The supported CSI-Addons operations get registered as their own
// services.
func (fs *Driver) setupCSIAddonsServer(conf *util.Config) error {
Copy link
Member

Choose a reason for hiding this comment

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

Looks quite complete to me, but you still need to call this function from the main Run() one somewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review Niels, just missed it. Updated. PTAL.

@riya-singhal31
Copy link
Contributor Author

riya-singhal31 commented Jul 14, 2023

The below shows that the csi-addons command is running in the csi-addons-sidecar container of a cephfs provisioner pod, as expected.

[ndevos@ibm.com ~]$ oc -n openshift-storage rsh -c csi-addons csi-cephfsplugin-jwtfd
sh-5.1# csi-addons -h
  -drivername string
        name of the CSI driver
  -endpoint string
        CSI-Addons endpoint (default "unix:///tmp/csi-addons.sock")
  -legacy
        use legacy format for old Kubernetes versions
  -operation string
        csi-addons operation
  -persistentvolume string
        name of the PersistentVolume
  -stagingpath string
        staging path (default "/var/lib/kubelet/plugins/kubernetes.io/csi/")

The following operations are supported:
 - GetIdentity
 - GetCapabilities
 - Probe
 - ControllerReclaimSpace
 - NodeReclaimSpace
sh-5.1# csi-addons -endpoint unix:///csi/csi-addons.sock -operation GetIdentity
identity: name:"openshift-storage.cephfs.csi.ceph.com"  vendor_version:"canary"
sh-5.1# csi-addons -endpoint unix:///csi/csi-addons.sock -operation GetCapabilities
capabilities:

Thank you @nixpanic for helping out.
cc: @Madhu-1

@riya-singhal31
Copy link
Contributor Author

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Jul 14, 2023

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at d432421

this commit adds CSI-Addons support to the
cephfs provisioner

Signed-off-by: riya-singhal31 <rsinghal@redhat.com>
@mergify mergify bot added the ok-to-test Label to trigger E2E tests label Jul 14, 2023
@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

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

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

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

@ceph-csi-bot
Copy link
Collaborator

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

@ceph-csi-bot
Copy link
Collaborator

/test ci/centos/mini-e2e/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/upgrade-tests-cephfs

@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 Jul 14, 2023
@mergify mergify bot merged commit d432421 into ceph:devel Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/cephfs Issues related to CephFS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants