kubernetes / kubeadm

Aggregator for issues filed against kubeadm
Apache License 2.0
3.76k stars 716 forks source link

Develop a unified policy for command like flags vs config file options #1040

Closed liztio closed 3 years ago

liztio commented 6 years ago

Assumptions:

We are looking for a happy medium between "everything is in the CLI flags" and "everything is configuration." This issue should be a formal policy that sorts new options into the "flag and configuration option" and "configuration option only" buckets. The latter is likely for more obscure flags, but where exactly the line between "common" and "obscure" should be firmly delineated.

KEP WIP doc: https://docs.google.com/document/d/1LSb2Ieb4XrxQ3cG6AEkwpfN1hb1L6pJdb0vMo10F41U/edit?usp=sharing

neolit123 commented 6 years ago

for reference the original proposal that the kublet went for: https://docs.google.com/document/d/1FdaEJUEh091qf5B98HM6_8MS764iXrxxigNIdwHYW9c/edit#heading=h.nlhhig66a0v6

PR about CLI flags overriding config (kubeadm init): https://github.com/kubernetes/kubernetes/pull/66968

viper as consideration: https://github.com/spf13/viper

rosti commented 6 years ago

I'll have a more in-depth look at viper.

The thing is that having everything either way (config file or cmd line flags) is awkward to support. Having fewer command line flags is a good thing. My bet is to support only those, that are likely to be overridden on a per-machine and/or per-execution basis (node names, certificate files, bootstrap tokens, etc.).

My proposed implementation in kubernetes/kubernetes#66968 for kubeadm init follows to a large extent the approach undertaken by kubelet. It's essentially a huge workaround over some of the cobra limitations. Also, no flags are deprecated, they are all simply imported as overrides of the config.

fabriziopandini commented 6 years ago

I think that before addressing the issue at technical level (viper, kubelet like or kubectl like) it would be great to figure out a list of use cases when flags and config files should be used at the same time in kubeadm. This will help in understanding when and why we really need this

fabriziopandini commented 6 years ago

.. from slack "More and more I think about it, more I'convinced that a suitable way forward for the current kubeadm UX issues (flags -> cfg, version defaults, etc) is to agree on a common way to implement cobra command in kubeadm Currently there is no consistent approach and this makes the problem more complex than it should be"

rosti commented 6 years ago

I have spent some time today looking into viper. At first glance, I am not very impressed with it. Seems to me, that if we try to stick with our current config model, we'll have to do some workarounds.

For example, implementing --apiserver-advertise-address will require it to be setup as an alias to api.advertiseAddress in order to read it from the config. Unfortunately, this would also mean, that we have yet another command line parameter (--api.advertiseAddress).

This may just be me, looking at a library which I haven't used at all, but it may just be better to stick to Cobra and provide manual overloading of only selected flags (as opposed to all config flags).

neolit123 commented 6 years ago

as much as i looked at it i can confirm the --api.advertiseAddress observation. if kubeadm started with viper from the beginning i think it could have worked nicely in the long run, but i'm not sure if it was even available back then.

there is also the option to YOLO the viper approach and deprecate the alias flags (i.e. those that we have right now).

timothysc commented 6 years ago

cc @sysrich

sysrich commented 6 years ago

Currently there is no consistent approach and this makes the problem more complex than it should be"

Indeed, and I fear I'm about to add another layer of complexity to the problem, but I hope in doing so we end up with a comprehensive solution

it would be great to figure out a list of use cases when flags and config files should be used at the same time in kubeadm

I think there are actually 3 classes or tiers of "configuration", and we need to deal with them in a structured and consistent way.

I'd describe the two being discussed so far are "user supplied config files" and "user supplied commandline flags"

My strong personal feeling is that all of the options we want the user to be able to define must be able to be set in a configuration file. This configuration file should live in /etc and be primarily configured by the user. I think distributions should be discouraged from packaging, bundling, or editing anything in /etc, leaving that existence and content of any such configuration purely for the user. (More on that later).

I think commandline flags should be reserved for options which only make sense for "one-time" custom execution parameters. For example the existing kubeadm init --dry-run and --feature-gates would be a great example of flags that make sense as flags. The --apiserver-* flags are flags which I think would be best kept to a configuration file - I don't see the benefit of having them as flags.

Regardless, I think all (or almost all) commandline flags should also be definable in a configuration file.

This creates a need to set precedence. I think commandline flags should always have a precedence above that of a config file.

I feel my above proposal might serve as a starting point of a policy for this issue, but now I introduce my new layer of complexity.

Distributions, or any other solution providers wanting to bundle k8s/kubeadm out of the box, need to have a way of providing their default configuration also. And yet, this configuration needs to be alterable by users. This is made even more complex when you consider many container OS platforms (CoreOS, Kubic, etc) all have read-only filesystems, encompassing either /usr or a significant portion of the root filesystem.

Luckily, systemd provides a model which I think we should consider here (wow I never thought I'd say that ;))

I think we should have "distro provided" configuration files, stored in /usr. These distro defaults should be automatically overridden by any user provided configuration file.

Whether or not this solution should include a systemd-like drop-in feature where selective parameters from the /usr config can be overridden by a narrowly defined override provided in /etc is an open question - I think it only makes sense if the configuration files are structured accordingly. If not I see no problem with a file provided in /etc totally overriding it's companion file in /usr

All applications should therefore consume configuration in the following order of precedence.

Does this sound like a viable way forward?

neolit123 commented 6 years ago

i would like to bring Windows in the mix, because kubeadm has plans to support the OS.

and i would leave it others to comment more on the default configs in /usr and /etc, but something to note here is that while Linux has a very unclear definition of where applications should store their data, Windows often lacks corresponding folders.

the only thing that maps well is:

rosti commented 6 years ago

I think, that there are a few more things to consider here on those grounds:

Therefore, I am a little bit hesitant to introduce pre-defined config file locations in kubeadm. If we do this, we'll end up with more code complexity, since we'll also have to introduce workarounds for Windows and macOS some day. Given this and the fact, that most cluster management systems will actually fetch a config file from somewhere and change a few machine specific parameters via the command line, then I don't think it's worth the complexity to introduce additional complexity by defining standard locations and multiple config files overriding each other.

The whole idea behind flags overriding the config file options is to avoid changing the config file if one needs to set the node name or the API server advertise address.

sysrich commented 6 years ago

The so called "config" files are actually a "recipe" on how to do a deployment

That might be true for some aspects for some aspects of the configuration files, but I'd describe things like deciding which CRI runtime is being used by kubelet as far more foundational. kubeadm reset won't even work right unless it knows which runtime to reset.

I might be able to agree with your points for some aspects of the configuration, but at the very least for those related to the choice of runtime, I strongly hold that we need to have tiering of the sort I describe in my earlier post

The model is one that is proven to work well in conjunction with other configuration management systems (eg. salt, ansibile, etc), and my experience with such tools strongly suggests they too benefit from knowing where to expect to draw their configuration from. I don't think the argument that "these other tools exist, so we don't need to worry about how to do things consistently ourselves" is a valid train of thought.

rosti commented 6 years ago

I can agree on the CRI part, but even that can be viewed as part of the deployment recipe - one can wish to deploy a machine as part of one cluster with CRI-O and then decide to reset it and join to another cluster with Docker.

The model is one that is proven to work well in conjunction with other configuration management systems (eg. salt, ansibile, etc), and my experience with such tools strongly suggests they too benefit from knowing where to expect to draw their configuration from. I don't think the argument that "these other tools exist, so we don't need to worry about how to do things consistently ourselves" is a valid train of thought.

The thing that bothers me in that model is that it may now be /etc and /usr. Next, some other distro folks will ask, say "we don't like /etc, we want /var", then some third party comes and asks "I need both /etc and /var". This is a Linux specific code, which we'll have to support in the future. It will also make kubeadm UX specific to different distributions. We are already struggling with supporting debs, rpms and the quirks of different Linux distros (systemd-resolved enabled or not, different cgroups drivers, etc). And I see here the potential of expanding on that. Another thing that is doubtful to me is, that I don't think that many of the authors of third party tools (that use kubeadm) will actually change their code and store the config files in the standard locations. They'll be happy to use --config and not bother doing anything at all.

sysrich commented 6 years ago

The thing that bothers me in that model is that it may now be /etc and /usr. Next, some other distro folks will ask, say "we don't like /etc, we want /var", then some third party comes and asks "I need both /etc and /var".

But the /usr and /etc split mirrors exactly what is done by systemd, which is used as the default init system by the vast majority of distributions out there.

I'm not saying my proposed way is the only way forward, but we need forward and I strongly disagree with your implications that the status quo is acceptable.

If Kubernetes doesn't have sensible, standardised, consistent models that allow distributions to provide sane config defaults to then be overridden by users, then the viability of shipping kubernetes as an integrated part of any end-to-end stack will be questionable. I think we need to consider these needs as a natural part of kubernetes growing up to a solution which seeks to be easily integrated across multiple platforms by multiple stakeholders.

We are already struggling with supporting debs, rpms and the quirks of different Linux distros (systemd-resolved enabled or not, different cgroups drivers, etc). And I see here the potential of expanding on that.

And I see the opposite - because so far every single one of the issues I've had getting Kubernetes and kubeadm integrated into openSUSE has been frustrated by the lack of consistency. Which led me to proposing further inconsistent solutions, which increased the complexity for the distro, and by proxy, Kubernetes when my inconsistent solutions led to bugs and integration issues.

Which also then prevents me from contributing to the messy incomplete starting points like the upstream rpm specfiles, because real distributions have had to hack so many nasty things to workaround the madness there is no longer any relationship to the rpm defs you see in the k8s git.

All of which conspires together to make things painful when we want to keep up with k8s versions, which change all of the above and suddenly we have weeks or months or work to do before we can deploy the new version to our users, just to have the next k8s version out before we've completed that work.. and the cycle continues ad infinitum....

This issue seeks to establish a unified policy precisely because Kubernetes needs to 'plant a flag' in the ground so other stakeholders, like Distributions, have a standardised, documented, and well thought out framework for living with.

Expecting everything to keep working together nicely with the current status quo of inconsistent flags, config files appearing and disappearing from different locations and being consumed and unconsumed by tools like kubeadm seemingly at random between versions really is not something which can continue indefinitely if we expect to keep on being able to play well with others.

The solution doesn't need to be perfect, I'm more than happy to compromise on my proposal, but we need a solution.

rosti commented 6 years ago

@sysrich thanks for the clarification. I agree with the expressed opinion. As long as all (or at least most of the) distros agree on the selected locations for default config files, I am perfectly fine with this.

timothysc commented 6 years ago

/assign @neolit123

neolit123 commented 6 years ago

as discussed in today's kubeadm office hours i have created a KEP doc for this but it's empty ATM: https://docs.google.com/document/d/1LSb2Ieb4XrxQ3cG6AEkwpfN1hb1L6pJdb0vMo10F41U/edit?usp=sharing

this feature is outside of the 1.12 scope, but we can try to finish the KEP soon.

one big preliminary ? here is Viper. if we want to fork Viper in k8s, at that point we can have it do what we want and possibly fix some bugs. also use it in other areas that are facing the same problem, e2e framework, kube-proxy etc.

xref: https://github.com/kubernetes/kubernetes/issues/66649 the e2e framework refactor is facing the same flags vs config issue.

fabriziopandini commented 6 years ago

@sysrich thanks for keeping this discussion moving I think that one missing piece of the discussion is how configuration from different layers should merge in case of conflicts (tricky case are maps, lists or nested/exclusive structures like the ones for ETCD configurations)

rosti commented 6 years ago

@neolit123 I don't think, that forking Viper is the best approach here. We should probably try contributing patches upstream to Viper first.

@fabriziopandini @sysrich merging configs from different locations seems like a bad idea to me. There are so many things that could go wrong and hard to find bugs just waiting to happen. If we have multiple configuration files, we should probably just search locations in a particular order and use the first one found.

neolit123 commented 6 years ago

viper is largely unmaintained, the idea of homebrewing an alternative received some +1 at sig-architecture.

sysrich commented 6 years ago

(sorry for the delay replying, was on vacation) @rosti @fabianofranz - I agree, which is why my proposal is one of precedence, not merging - merging would require a systemd-like drop-in feature (which would allow the selective replacement of specific parameters within an existing configuration file) This would require configuration files to be far more structured than I believe them to be right now, so lets ignore it for now.

So to spell out what I'm proposing more succinctly

I'm suggesting for kubeadm and all k8s related configuration distros should be able to distribute a default config in /usr

Users should be able to replace them with configuration files in /etc

And at runtime specific parameters should be settable as command-line switches

Any file in /usr should be ignored if their is a replacement file in /etc

And specific parameters from /usr or /etc should be ignored if their is replacement command-line switches provided at runtime

(And as @fabianofranz points out, some things are very tricky to handle in merge situations - these tricky things should NEVER be runtime switches)

mgoodness commented 6 years ago

As a user, I need a way to override certain values in my kubeadm config file. Node name is the most critical today, but I also look forward to setting cluster-dns on each node once it's part of kubelet componentconfig.

FWIW, I really like the way viper handles precedence: flag > env var > config file.

timothysc commented 6 years ago

/assign @rdodev @timothysc /unassign @liztio

timothysc commented 5 years ago

/assign @luxas

b/c this overlaps with grand-unified component config.

neolit123 commented 5 years ago

xref https://docs.google.com/document/d/1Dvct469xfjkgy3tjWMAKvRAJo4CmGH4cgSVGTDpay6A/edit

neolit123 commented 4 years ago

xref https://docs.google.com/document/d/1Dvct469xfjkgy3tjWMAKvRAJo4CmGH4cgSVGTDpay6A/edit

the proposal in this doc mostly staled at this point. i can still see some benefit of allowing patches from the CLI vs having a 100 flags + config or only config without granular overrides.

neolit123 commented 3 years ago

the policy for kubeadm is tribal knowledge at this point:

this is the case for kubeadm at least.

kubelet and other components are sparse:

other notes: