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

Add support for cephfs helm chat #218

Merged
merged 5 commits into from
Feb 25, 2019
Merged

Add support for cephfs helm chat #218

merged 5 commits into from
Feb 25, 2019

Conversation

Madhu-1
Copy link
Collaborator

@Madhu-1 Madhu-1 commented Feb 21, 2019

This PR adds the support for cephfs helm chat

logs

[🎩]mrajanna@localhost ceph-csi $]hlm install  --name "ceph-csi-cephfs" deploy/cephfs/helm/
NAME:   ceph-csi-cephfs
LAST DEPLOYED: Thu Feb 21 10:42:40 2019
NAMESPACE: default
STATUS: DEPLOYED

RESOURCES:
==> v1/ServiceAccount
NAME                         SECRETS  AGE
ceph-csi-cephfs-attacher     1        1s
ceph-csi-cephfs-nodeplugin   1        1s
ceph-csi-cephfs-provisioner  1        1s

==> v1/ClusterRole
NAME                         AGE
ceph-csi-cephfs-attacher     1s
ceph-csi-cephfs-nodeplugin   1s
ceph-csi-cephfs-provisioner  1s

==> v1/ClusterRoleBinding
NAME                         AGE
ceph-csi-cephfs-attacher     1s
ceph-csi-cephfs-nodeplugin   1s
ceph-csi-cephfs-provisioner  1s

==> v1/Service
NAME                         TYPE       CLUSTER-IP    EXTERNAL-IP  PORT(S)    AGE
ceph-csi-cephfs-attacher     ClusterIP  10.233.6.190  <none>       12345/TCP  1s
ceph-csi-cephfs-provisioner  ClusterIP  10.233.45.41  <none>       12345/TCP  1s

==> v1beta2/DaemonSet
NAME                        DESIRED  CURRENT  READY  UP-TO-DATE  AVAILABLE  NODE SELECTOR  AGE
ceph-csi-cephfs-nodeplugin  3        3        0      3           0          <none>         1s

==> v1beta1/StatefulSet
NAME                         DESIRED  CURRENT  AGE
ceph-csi-cephfs-attacher     1        1        1s
ceph-csi-cephfs-provisioner  1        1        1s

==> v1/Pod(related)
NAME                              READY  STATUS             RESTARTS  AGE
ceph-csi-cephfs-nodeplugin-7nhgw  0/2    ContainerCreating  0         0s
ceph-csi-cephfs-nodeplugin-bhtpw  0/2    ContainerCreating  0         1s
ceph-csi-cephfs-nodeplugin-x9jsw  0/2    ContainerCreating  0         0s
ceph-csi-cephfs-attacher-0        0/1    ContainerCreating  0         1s
ceph-csi-cephfs-provisioner-0     0/2    ContainerCreating  0         0s


NOTES:
Examples on how to configure a storage class and start using the driver are here:
https://github.com/ceph/ceph-csi/tree/csi-v1.0/examples/cephfs

[🎩︎]mrajanna@localhost ceph-csi $]kube get  po
NAME                               READY   STATUS    RESTARTS   AGE
ceph-csi-cephfs-attacher-0         1/1     Running   0          13s
ceph-csi-cephfs-nodeplugin-7nhgw   2/2     Running   0          12s
ceph-csi-cephfs-nodeplugin-bhtpw   2/2     Running   0          13s
ceph-csi-cephfs-nodeplugin-x9jsw   2/2     Running   0          12s
ceph-csi-cephfs-provisioner-0      2/2     Running   0          12s
[🎩︎]mrajanna@localhost ceph-csi $]hlm list
NAME           	REVISION	UPDATED                 	STATUS  	CHART                	APP VERSION	NAMESPACE
ceph-csi-cephfs	1       	Thu Feb 21 10:42:40 2019	DEPLOYED	ceph-csi-cephfs-0.4.0	1.0.0      	default  
[🎩︎]mrajanna@localhost ceph-csi $]hlm delete ceph-csi-cephfs
release "ceph-csi-cephfs" deleted

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Feb 21, 2019

@rootfs @kfox1111 PTAL

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Feb 21, 2019

@gman0 PTAL

Copy link
Contributor

@kfox1111 kfox1111 left a comment

Choose a reason for hiding this comment

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

All in all, this looks really good. Thanks for contributing it.

There are two files removed that I think you didn't intend to:
deploy/rbd/helm/templates/provisioner-role.yaml
deploy/rbd/helm/templates/provisioner-rolebinding.yaml

A few other comments inline


```bash
helm install --namespace "ceph-csi-rbd" --name "ceph-csi-rbd" ceph-csi/ceph-csi-rbd
helm install --name "ceph-csi-rbd" ceph-csi/ceph-csi-rbd
Copy link
Contributor

Choose a reason for hiding this comment

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

Why remove the namespace? This puts it in "default"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

current templates in kubernetes folder deploy everything in the default namespace, to keep consistency between both I deleted helm deployment in a different namespace

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think installing it in "default" is a good recommendation for the users. Please undo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

undo done

resources: ["volumeattachments"]
verbs: ["get", "list", "watch", "update"]
- apiGroups: [""]
resources: ["configmaps"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please double check the permssions for cephfs? rbd needs configmap reading, but I'm not sure cephfs does.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. I'll talk with rootfs about it. But I don't think that should block this pr.

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Feb 21, 2019

All in all, this looks really good. Thanks for contributing it.

There are two files removed that I think you didn't intend to:
deploy/rbd/helm/templates/provisioner-role.yaml
deploy/rbd/helm/templates/provisioner-rolebinding.yaml

as we are creating cluster-role and binding cluster-role to a service-account, a normal role is not required, tested this and everything is working fine

@kfox1111
Copy link
Contributor

deploy/rbd/helm/templates/provisioner-role.yaml
deploy/rbd/helm/templates/provisioner-rolebinding.yaml

as we are creating cluster-role and binding cluster-role to a service-account, a normal role is not required, tested this and everything is working fine

You tested out the rbd driver without those two files and provisioning rbd volumes still worked?

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Feb 22, 2019

deploy/rbd/helm/templates/provisioner-role.yaml
deploy/rbd/helm/templates/provisioner-rolebinding.yaml

as we are creating cluster-role and binding cluster-role to a service-account, a normal role is not required, tested this and everything is working fine

You tested out the rbd driver without those two files and provisioning rbd volumes still worked?

here are the logs

[🎩]mrajanna@localhost ceph-csi $]hlm install --namespace "ceph-csi-rbd" --name "ceph-csi-rbd" deploy/rbd/helm/
NAME:   ceph-csi-rbd
LAST DEPLOYED: Fri Feb 22 10:08:42 2019
NAMESPACE: ceph-csi-rbd
STATUS: DEPLOYED

RESOURCES:
==> v1beta1/StatefulSet
NAME                      DESIRED  CURRENT  AGE
ceph-csi-rbd-attacher     1        1        1s
ceph-csi-rbd-provisioner  1        1        1s

==> v1/Pod(related)
NAME                           READY  STATUS             RESTARTS  AGE
ceph-csi-rbd-nodeplugin-9jsfb  0/2    ContainerCreating  0         0s
ceph-csi-rbd-nodeplugin-rfsmp  0/2    ContainerCreating  0         0s
ceph-csi-rbd-nodeplugin-xbp9j  0/2    ContainerCreating  0         0s
ceph-csi-rbd-attacher-0        0/1    ContainerCreating  0         0s
ceph-csi-rbd-provisioner-0     0/3    ContainerCreating  0         0s

==> v1/ServiceAccount
NAME                      SECRETS  AGE
ceph-csi-rbd-attacher     1        1s
ceph-csi-rbd-nodeplugin   1        1s
ceph-csi-rbd-provisioner  1        1s

==> v1/ClusterRole
NAME                      AGE
ceph-csi-rbd-attacher     1s
ceph-csi-rbd-nodeplugin   1s
ceph-csi-rbd-provisioner  1s

==> v1/ClusterRoleBinding
NAME                      AGE
ceph-csi-rbd-attacher     1s
ceph-csi-rbd-nodeplugin   1s
ceph-csi-rbd-provisioner  1s

==> v1/Service
NAME                      TYPE       CLUSTER-IP    EXTERNAL-IP  PORT(S)    AGE
ceph-csi-rbd-attacher     ClusterIP  10.233.6.108  <none>       12345/TCP  1s
ceph-csi-rbd-provisioner  ClusterIP  10.233.43.73  <none>       12345/TCP  1s

==> v1beta2/DaemonSet
NAME                     DESIRED  CURRENT  READY  UP-TO-DATE  AVAILABLE  NODE SELECTOR  AGE
ceph-csi-rbd-nodeplugin  3        3        0      3           0          <none>         1s


NOTES:
Examples on how to configure a storage class and start using the driver are here:
https://github.com/ceph/ceph-csi/tree/csi-v1.0/examples/rbd


[🎩︎]mrajanna@localhost rbd $]kube get po -nceph-csi-rbd
NAME                            READY   STATUS    RESTARTS   AGE
ceph-csi-rbd-attacher-0         1/1     Running   0          5m5s
ceph-csi-rbd-nodeplugin-9jsfb   2/2     Running   1          5m5s
ceph-csi-rbd-nodeplugin-rfsmp   2/2     Running   1          5m5s
ceph-csi-rbd-nodeplugin-xbp9j   2/2     Running   1          5m5s
ceph-csi-rbd-provisioner-0      3/3     Running   0          5m5s
[🎩︎]mrajanna@localhost rbd $]kube get pvc
NAME      STATUS   VOLUME                                     CAPACITY   ACCESS MODES   STORAGECLASS   AGE
rbd-pvc   Bound    pvc-588b1039-365c-11e9-9088-5254003a618c   1Gi        RWO            csi-rbd        47s

@kfox1111
Copy link
Contributor

I think it will start, but it won't function properly without permissions to read/write the configmaps.

The rbd helm rbac is a little different then the static manifests. The security is tighter as it only has permissions to the configmaps in the specific namespace rather then for the entire cluster. Its a change I wouild like to propose to the static manifests too, but haven't had time yet.

@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Feb 25, 2019

I think it will start, but it won't function properly without permissions to read/write the configmaps.

means volume mount won't work as expected?

The rbd helm rbac is a little different then the static manifests. The security is tighter as it only has permissions to the configmaps in the specific namespace rather then for the entire cluster. Its a change I wouild like to propose to the static manifests too, but haven't had time yet.

i am not aware of this issue.

to make this PR completed, do i need to bring back deploy/rbd/helm/templates/provisioner-role.yaml deploy/rbd/helm/templates/provisioner-rolebinding.yaml files?

@kfox1111
Copy link
Contributor

Yeah, I don't think it will work properly to provision the volumes without the role.

I think it best if the PR only added the new chart, not touched the other chart. Its mergeable in its current state without the rbd changes.

Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
Signed-off-by: Madhu Rajanna <madhupr007@gmail.com>
@Madhu-1
Copy link
Collaborator Author

Madhu-1 commented Feb 25, 2019

@kfox1111 reverted back the changes for role and role binding

@kfox1111
Copy link
Contributor

/lgtm

@rootfs
Copy link
Member

rootfs commented Feb 25, 2019

thanks @kfox1111

@rootfs rootfs merged commit cb68b68 into ceph:csi-v1.0 Feb 25, 2019
wilmardo pushed a commit to wilmardo/ceph-csi that referenced this pull request Jul 29, 2019
Add support for cephfs helm chat
nixpanic pushed a commit to nixpanic/ceph-csi that referenced this pull request Nov 22, 2023
Syncing latest changes from upstream devel for ceph-csi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants