They are supported by kube-apiserver, but not snap set kube-apiserver. It takes them as boolean values instead, e.g. contention-profiling=true and contention-profiling=false.
My specific concern here is that this case will break on the following line:
I don't think tuple accepts 2 arguments. That and the None probably needs to be 'true' since it's eventually gonna go through FlagManager to a snap set call.
More generally, I get the feeling this case shouldn't exist at all. The description in config.yaml makes me think api-extra-args follows the same conventions as snap set, with the profiling=true example, but the case here says otherwise.
Setting api-extra-args will allow us to override it with storage-backend=etcd3, which seems reasonable.
Once we clear api-extra-args, it will set it to storage-backend=null, instead of reverting to etcd2.
I think it'd be worthwhile to fix the logic here so it reverts back properly. That or prevent overrides from happening if that's something we don't want.
Similarly, the positioning of this would not allow us to override admission-control even though it lets us override other options. Let's make the behavior consistent one way or the other.
This problem is basically unsolvable. FlagManger supports options like this: {'foo': ['bar', 'baz']}
Which will render to: --foo=bar --foo=baz
Let's assume an environment has the following state: {'foo': ['bar']}
And we run: juju config kubernetes-master api-extra-args="foo=baz"
There is no way to tell if the operator wants end result to be: {'foo': ['bar', 'baz']}
or {'foo': ['baz']}
My feeling is that we should make it {'foo': ['baz']} and trust that the operator knows what they are doing.
Similarly if an operator sets and then unsets certain option it's not obvious that they want it reverted to a value harcoded in the charm. An example might be "--min-request-timeout" which the charm sets to 300s.
The operator might set it to 600s, realize it's too short and "unset" to revert expecting the flag to be removed (and in consequence having k8s default value of 1800s).
ACK your point about positioning above admission-control, I'll get it fixed
My feeling is that we should make it {'foo': ['baz']} and trust that the operator knows what they are doing.
👍 to this
An example might be "--min-request-timeout" which the charm sets to 300s. The operator might set it to 600s, realize it's too short and "unset" to revert expecting the flag to be removed (and in consequence having k8s default value of 1800s).
Intuition tells me that juju config kubernetes-master api-extra-args="" should give the same result no matter what it was set to prior. I think that behavior is important.
If you take that stance, then I think it becomes clear: removing a value from api-extra-args has to revert to charm default (300s in your example). If the operator really wants to bypass the charm default, they can do it by overriding with an explicit value.
That said, I don't see a quick fix for this with how the charm uses FlagManager today. Seems like there might be a lot of refactoring involved to get it right. :\
@Cynerva I pushed new (rebased) commit with that fixes it. Config options are not revert back to charm defaults when they are unset.
Please note that we are still affected by https://bugs.launchpad.net/snapd/+bug/1699768 so unsetting some options will break hooks. There is unfortunately nothing I can do about that.
This is a great refactor! Thanks for cleaning things up.