Best approach for enabling LoadBalancer Servcies

[phlogistonjohn]

PR #52 adds initial support for automatically creating Services for the server instances we create. However, those Services are configured for ClusterIP always.

I hacked in support for enabling LoadBalancer to the branch I created in preparation for my recent sambaXP demo by having the operator check an annotation. In the long run, doing this via an annotation seems too hacky. So I'm starting this discussion to get opinions. Currently my preferences are either one of the following:

1. Extend the SmbShare spec. Add something like publish: [cluster|external] to the spec so that each share can be published externally or not.
2. Add a new CR - SmbCommonConfig (or something along these lines). Much like the SmbSecurityConfig the SmbCommonConfig acts like a "template" for properties of the server instance but does not configure any single server instance. Shares can refer to a common config by name. The SmbCommonConfig would have a subsection like "network:" with `publish: [cluster|external]`` under it.

Originally, I preferred item 1 but I was recently reminded of other things that may need get passed to the deployments/pods in the future. Specifically, I was skimming my rook folder and was reminded of the fact that rook can pass settings onto the managed pods like scheduling paramaters, tolerations, etc. So it might be good to establish a "home" for that kind of stuff soon so I now lean slightly toward choice 2.

Note that if we do go with choice 2 I would like to keep things simple by allowing the admins to set a default, simlar to how a default storage class is selected. Then an SmbShare that doesn't specify a specific SmbCommonConfig will use the "default". If no SmbCommonConfig is created then the operator simply falls back to built-in defaults.

[raghavendra-talur]

I like the 2nd approach too. As it is a CR, I am assuming that we can have many of those and that approach can be used to choose one setting or another. That should work.

SmbCommonConfig, the name confuses me a little and I interpret the common in that as something that is common to all shares. If more people feel similarly, we can look for a different name for it.

[raghavendra-talur]

Something along the lines SmbBaseConfig

[obnoxxx]

I like approach number 2.

In general, the smb.conf file has two classes of parameters, global parameters and share parameters. so this approach would use three buckets: the security related global options, the other global options, and the share specific options. This does make sense!

Note however, that the share options can also be specified in the global section, in which case they act as defaults for any shares. In case we end up exposing more share tunings, we might want to also exposing them in a global template, as a default for potentially multiple expicit share definitions. This could be done as part of the global section described here, or else, I think better as a SmbShareType or SmbShareTemplate CR that can be referenced in a share definition. With that, we could use multiple templates in one "server" instance. Then again, the server is not in the focus - so not sure what we really gain?!...

[phlogistonjohn]

It seems like Option 2 has gotten good traction so far. A couple of things I want to say in reaction to both replies so far:

Content

Indeed, this config can be used for non-security, global, samba options. However, I also imagined it as initially containing things that are per-server or per-instance but are not necessarily part of Samba's config. As I originally wrote, the first thing I'd add is configuration for how we set up services (with LoadBalancer enabled or not). Later, I could also see us adding a way to customize parameters like tolerations, etc.

That said I'm certainly open to it having smb related options too, I just wanted to clarify that the initial use-cases I am thinking of don't directly set smb/samba configs.

Names

The following names have been proposed:

• SmbCommonConfig
• SmbBaseConfig
• SmbShareType
• SmbShareTemplate

I'll add to the list:

• SmbGeneralConfig

I think SmbSecurityConfig has created a precedent for SmbConfig as the name of our "template" configs as opposed to configs that map 1:1 with results (the shares). I think SmbSecurityTemplate would have been OK, but I don't want to go back and change things now. Plus I think Config is easier to type. :-)

I don't really get SmbShareType as a name. I don't see the item's contents really typifying the share. I also don't want the new name to begin with "SmbShare". As it is less easy to distingusish between a "SmbShare" and a plain "SmbShare" especially w.r.t. auto- & tab-completions. I even want to shy away from "SmbS..." names, because in my demo it was already getting annoying to autocomplete with the names we already have :-)

In my view that leaves "SmbCommonConfig", "SmbBaseConfig", and "SmbGeneralConfig". What I like about "base" is that it's short, but I don't really know what it is a "base" of. I don't want to imply that having one of these is mandatory, so I am unsure about base. I see SmbGeneralConfig and SmbCommonConfig as pretty equivalent. But I came up with SmbCommonConfig first, so that's still my preference. :-)

Over the next day or two I will work on a new PR to add a new CR to the design doc. It'll start small, with just the short-term requirement for network/service properties. I'll probably start off with SmbCommonConfig as the name unless someone comes up with a really brilliant argument in the meantime. But I won't discourage you from trying :-)

[phlogistonjohn]

PR #66 is up.