canonical / operator

Pure Python framework for writing Juju charms
Apache License 2.0
245 stars 119 forks source link

Use controller storage if no local storage exists #371

Closed stub42 closed 4 years ago

stub42 commented 4 years ago

Charms should automatically use controller storage for state if the files that hold local state do not exist. This will fix new charms that forget to set the 'use_juju_for_storage=True' argument to main(), making it the default except for legacy deployments (which likely do not exist!)

jameinel commented 4 years ago

I think the goal has always been to transition to using it by default, we just wanted to have a controlled rollout.

I definitely think the operator framework can look for "min-juju-version=2.8" and use that as a signal that we must use state-get.

The question is more about what should be done if state-get is available but not obviously required (especially if someone was deploying with Juju 2.7 and upgraded to 2.8 then we have to worry about maintaining the charm's state.)

Is 'min-juju-version' enough for your use case? The one major caveat is that there is always a local disk that we can use to write things down. Just that sometimes that disk is not a stable storage that would persist the operator pod getting rescheduled.

stub42 commented 4 years ago

All charms I deal with are setting min-juju-version=2.8 and setting the use_juju_for_storage argument to True. It is quite likely we will forget to set min-juju-version at some point though, and we will not notice because we won't be testing or deploying to any 2.7 environments.

I'm mostly interested in getting the default changed now, before we accidentally deploy something with incorrect settings and actually have to care about a controlled change of the default in the future. Currently our production environment requires us to change k8s charms to 'use_juju_for_storage=True', but I expect it won't be too long before we have more deployments without the limitation and things could slip through the cracks.

jameinel commented 4 years ago

If charms aren't setting min-juju-version then Juju 2.8 will be creating stateful sets for the operator pods. Presumably that means that the charms won't deploy on Prodstack, which means that they will ultimately have to set min-juju-version.

That said, @chipaca is going to look at changing the default logic so that if we are in a situation where we can reasonably expect the default position to use state-get then we will do so.

chipaca commented 4 years ago

comments on the linked pull request are welcome :-)