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

The organization of clustered/grouped SmbShares #341

Open cmu-rgrempel opened 5 months ago

cmu-rgrempel commented 5 months ago

I am a newbie checking out clustered and grouped shares with version 0.5. I thought I might jot down some first impressions as to how the CRDs are organized for this use case.

I note this passage from the documentation:

There are restrictions on what shares can be grouped together. The operator will perform a compatibility check for shares with explicit groupMode and the same group name. Most importantly, the Shares must share the same securityConfig, the same commonConfig, and same PVC name.

It occurs to me that, when clustered, it must also be the case that the scaling attribute of the grouped SmbShares would need to be identical. It would hardly make sense for one SmbShare in a group to have more nodes than another SmbShare in the same group, for instance.

This need to repeat configuration, and validate the repetition, in each grouped SmbShare strikes me as being a code smell. What it suggests is that there is really a higher-level entity actually being configured here, and that the SmbShare should (at least if grouped) simply refer to the higher-level entity.

Consider, for instance, the idea of an SmbGroup entity. This would basically correspond (ultimately) to a single StatefulSet. This statefulset would either have a single pod (in the simple case), or multiple pods (in the experimental clustered case). And, it would either have one SmbShare (in the simple case) or multiple (in the grouped case).

The settings in SmbCommonConfig and SmbSecurityConfig would then naturally either be settings of the SmbGroup itself, or the SmbGroup could refer to separate SmbConmonConfig and SmbSecurityConfig. In any event, the SmbShare would then indicate which SmbGroup it belongs to. And, of course the scaling properties would move to SmbGroup -- they are really properties of the group, not the individual share.

This would, I think, be clearer and reduce repeititon.

Now, perhaps it would make the very simplest case more complex. That is, in the non-clustered case where the statefulset will server just one share (so, non-clustered and non-grouped), instead of just having an SmbShare you'd also have to have an SmbGroup. However, perhaps the CRD could have a variant for that simple case? Or, it might actually be clearer anyway to explicitly create a group?

I do understand that you've put some thought into this, and that the focus on shares is deliberate. However, I think that it becomes quite conceptually awkward in the case of clustered, grouped shares -- it would be simpler to explicitly have a entity representing the group itself.

I will flesh out the idea better if I have time -- I thought it might be interesting to record a newbie's perspective in any event, for whatever it is worth.