Feature-2: Made chart more configurable

[onceinarow]

I was interested in trying this Helm Chart out today. Along the way, I wanted to be able to configure it a little better. As a result, I made a number of values more configurable, most notably the Ingress.yaml TLS configuration and the config.yaml content.

[jamesread]

Hey @onceinarow , thanks very much for taking the time to write this up, and on the whole I'm generally very happy with the changes, and this should be fine.

Your suggestion to make the service port configurable, as well as being able to specify the config, totally make sense.

I'm curious about specifying the replica count though - I don't mind it, but what benefit is there for multiple instances of OliveTin with the same config? Each instance would only be able to store its own logs for example.

I'm on holiday at the moment, so would probably finish this review in a few days of that's okay.

[onceinarow]

I'm curious about specifying the replica count though - I don't mind it, but what benefit is there for multiple instances of OliveTin with the same config? Each instance would only be able to store its own logs for example.

If each pod is treated as a separate instance of OliveTin, then it will not make sense to have more than one. My assumption was that this being a configurable variable provides the ability to horizontally scale the deployment in cases where one instance is not sufficient to support all incoming requests. It’s generally a standard to make replicas configurable so I added it while going through. The main thing I needed was the TLS config and simply added some configuration while I was in there. The beauty of configuration like this is that you can default it to 1 and maybe nobody will ever have to change it.

[jamesread]

Gotcha, but I don't want people to think that OliveTin can "magically" scale like that, having two instances unaware of each other, using the same config is likely to cause unexpected behaviour.

I do have a potential design in mind for "scaling out" OliveTin, but that isn't even written down as a design yet, and I'm not sure if it makes sense to take the project in this direction.

If we want to keep replicas, and have meaning, changing the deployment to a StatefulSet would at least make logical sense.

However I think that leaving replicas out of values.yml for now makes the most sense, and let advanced users who know what they're doing change it as a post-deployment operation.

Happy to keep the discussion going, but the main thing I want to avoid is people assuming that changing replicas to 2 makes OliveTin do something intelligent at the moment - it doesn't... yet.

[onceinarow]

Gotcha, but I don't want people to think that OliveTin can "magically" scale like that, having two instances unaware of each other, using the same config is likely to cause unexpected behaviour.

Sounds good, removed the configurable setting in the latest commit