search

samba-in-kubernetes / samba-operator

An operator for a Samba as a service on PVCs in kubernetes
Apache License 2.0
120 stars 24 forks source link

Grouped Shares Issues #297

Open koenigsreiter opened 1 year ago

koenigsreiter commented 1 year ago

Hello!

First of all, I really like this project and I'm currently evaluating the usage of it.

I'm quiet interested in the Grouped Shares feature as implemented in PR #240. Based on this I see that it should be possible to create a Grouped share. Now I have two issues and I hope you can help me with this:

  1. Why is it disabled to export two different PVCs under one Server? I see in https://github.com/samba-in-kubernetes/samba-operator/blob/master/internal/planner/compatible.go#L63 that here a check for the PVCs having the same name, but based on the Phase 1 Design doc (https://github.com/samba-in-kubernetes/samba-operator/blob/master/docs/design/crd-proposal-phase1.md#examples) in the third example I see a version where there are two different PVCs with the same group name. Is this now the plan to allow this and it's just not implemented yet, or is there a drift, between the design doc and the actual plan for this implementation?
  2. Is there already a plan, when the grouped share feature will be released? Currently the only way I found to get the CRDs with the additional properties into the cluster is by cloning the repo and doing a kustomize build in the config/default directory.

Best regards and thank you very much for your help in advance!

phlogistonjohn commented 1 year ago

Hello!

First of all, I really like this project and I'm currently evaluating the usage of it.

Thanks!

I'm quiet interested in the Grouped Shares feature as implemented in PR #240. Based on this I see that it should be possible to create a Grouped share. Now I have two issues and I hope you can help me with this:

1. Why is it disabled to export two different PVCs under one Server? I see in https://github.com/samba-in-kubernetes/samba-operator/blob/master/internal/planner/compatible.go#L63 that here a check for the PVCs having the same name, but based on the Phase 1 Design doc (https://github.com/samba-in-kubernetes/samba-operator/blob/master/docs/design/crd-proposal-phase1.md#examples) in the third example I see a version where there are two different PVCs with the same group name. Is this now the plan to allow this and it's just not implemented yet, or is there a drift, between the design doc and the actual plan for this implementation?

It's a bit between drift, shifting priorities, and becoming more aware of other constrains in the system. The primary reason being that it is not possible to add a PVC to an existing pod. Because the operator responds dynamically to new SmbShares and either creates new pods OR updates existing pods we'd only be able to fulfill a SmbShare with different PVCs when the existing Server pod does not exist. Behaving differently when the SmbShare had a backing server vs not - seemed worse than restricting the grouped SmbShares to use the same PVC.

Additionally, when we wrote the design doc we didn't know how much a priority different storage backends might be. Other storage backends in Samba (cephfs, glusterfs, etc) do not fall under this restriction, so the restriction doesn't appear in the design doc. There's been little to no demand for alternative backends to PVC storage so far so I didn't want to put a lot of work into that. (And there are other issues with managing pods that might these vfs modules)

Unless Kubernetes gains the ability to add PVCs to an existing pod (and if I've overlooked this possibility please let me know) then the only way to support grouped shares with different PVCs would be to stop and then regenerate a previously running pod. We will need this ability when we start to support upgrades properly, so after we add support for upgrades, it might be possible to come back to this issue and revisit this restriction.

Just to satisfy my curiosity, what is your use case for having multiple PVCs on a grouped SmbShare? Is it mainly about resource consumption, or something else?

2. Is there already a plan, when the grouped share feature will be released? Currently the only way I found to get the CRDs with the additional properties into the cluster is by cloning the repo and doing a kustomize build in the `config/default` directory.

Thanks for the reminder that we ought to do another release. We should at least try do a release that corresponds roughly with the sambaXP conference which is when we created our previous release. I will also propose that afterward we start doing time-based releases, to keep in the habit of releasing versions of the software.

koenigsreiter commented 1 year ago

Hi! :)

Thank you very much for the reply.

I'm not entirely sure what you mean with the different storage Backends. In my opinion the only issue with the Backends would be whether all of the PVCs support to be mounted by multiple pods (e.g.: Being ReadWriteMany). If that is supported then there should be no issue regardless whether it's backed by rook, gluster etc.

I'm also not sure what the issue is with the Pods. If I saw it correctly, the exporter Pod is already managed by a Deployment. If this deployment is the actual Samba server then you can just add the PVCs to the Deployment and it will take care to update the underlying Pods of the Deployment (I actually tested this today in a playground Deployment and it is possible to just add mounts/volumeMounts to an existing Deployment). But I'm not sure whether there is an actual "samba server" pod that is the backend for the Kubernetes Service or whether the -exporter Pods run the samba server.

I'm currently quiet busy but if I get to it I will try to replicate this today manually without the operator :) Maybe it works out of the box.

Just to satisfy my curiosity, what is your use case for having multiple PVCs on a grouped SmbShare? Is it mainly about resource consumption, or something else?

Although resource consumption is one point it's also mainly about having just one IP for all shares since I currently have multiple PVCs in the cluster and migrating to one big PVC would be cumbersome, so I would like to export all my PVCs under one IP with multiple Shares outside of the cluster.

Thanks for the reminder that we ought to do another release. We should at least try do a release that corresponds roughly with the sambaXP conference which is when we created our previous release. I will also propose that afterward we start doing time-based releases, to keep in the habit of releasing versions of the software.

Wonderful! I'm looking forward to it! :) 👍

koenigsreiter commented 1 year ago

Hi!

Just in case it helps with further questions: I managed to replicate the Grouped Share manually without the Deployment.

You should have all the files you need in the following Gist.

Of course what you need is a ReadWriteMany Storage Class and a cluster, that can provision LoadBalancer IPs (I use MetalLB).

Basically after I applied these Files I could successfully create one Service with multiple Shares from different PVCs inside of it.

I'm quiet busy right now, but if I have some time I will try to create a PR, but unfortunately I'm not really a Go Expert, so what I can do might not be the best solution....

Furthermore, I also haven't tested this with an HA setup, because I saw the operator is capable of that.

Best regards and happy easter!

phlogistonjohn commented 1 year ago

Thanks very much for sharing that, I will look over it and see what I can learn!