knative / serving-operator

DEPRECATED: Development continues in https://github.com/knative/operator/
Apache License 2.0
39 stars 45 forks source link

Discussion on handling of configuration settings #278

Open k4leung4 opened 4 years ago

k4leung4 commented 4 years ago

At the moment, the serving operator will reconcile config maps. As such, user changes to config maps directly will be reconciled away if those settings conflicts with the ones that the operator is aware of.

If the desire is to allow users to configure serving or eventing via the operator custom resource, should we be a simple pass through for all supported config maps? Complications around being a simple pass through is that it may be overwhelming for users, as there may be numerous config maps for various parts of the serving system. There is also the concern around exposing development settings are not really meant for end-user consumption.

Or do we desire a higher level abstraction for configuration, and present a simplified set of settings for users to configure? Config map settings are not as rigorously reviewed as APIs, which means that some settings are temporary, renamed multiple times. This makes supporting them in the sense of cleaning up unused ones, migrating to different names, verifying the config map keys and values difficult.

What are people's thoughts on this?

duglin commented 4 years ago

I think it should allow for all options. If we have an abstraction layer then that will probably mean a reduced set of options and that's just bound to annoy people, especially if the operator's reconciliation/upgrade overrides any edits to the config that the use makes. Normally users of this will be admins and so consistency between what the operator exposes and what core Knative exposes will only make life easier on them.

One twist to think about as we discuss config reconciliation... in some environments (like a managed one) while the user might have access to the normal serving and eventing resources, they may not have access to the operator. That might be under the platform's control. Which means, if the "source of truth" for config changes is within the operator, but the user doesn't have access to the operator's yaml/config, then the user might be stuck. The only option available to them might be to modify the Kn config maps directly and if the operator reconciles away their changes then that would be less than ideal.

garron commented 4 years ago

I agree with the assessment that we don't have a good story for supporting both the managed platform operator and the user operator configuring aspects of the system. Right now, regardless of whether you're using the serving operator or not, it's challenging to have multiple sources of configuration.

Many unix tools use a hierarchy of configuration to deal with this, git for example:

If you're both the "platform operator" and the "user", you tend to just pick one of those places to do your configuration, but it doesn't really work very well for the platform and user to be trying to keep their changes up-to-date in the same place.

Assuming an operator-based approach, maybe we need to decide if the KnativeServing CR is intended to be the platform operator's configuration or the user operator's configuration (when those aren't the same entity) and depending on what is decided there, maybe we need another means for the other? (I'm not really sure how it could be the original ConfigMaps, but adding more ConfigMaps also seems undesireable)

jcrossley3 commented 4 years ago

IMO, until knative supports multi-tenant installations, i.e. namespace-scoped instead of a single cluster-scoped knative, then distinguishing between platform and user operators is meaningless. Right now, they're one in the same.

Maybe a good trade-off is to relax the strict overwrite logic between the KS CR and the CM's. We can simply perform the 3-way strategic merge for CM's like we do for all other resources. So if the key is in the CM but not the KS CR, it won't be overwritten. The KS CR will only take precedence for keys in both. That way, we leave the decision about which keys may be overridden to the admin.

evankanderson commented 4 years ago

See also #301, which suggests a way to perform diffs between old and new manifest rather than doing an "apply" of the new config over the user's kubectl apply settings.

jcrossley3 commented 4 years ago

Have we answered this fundamental question yet: how should knative be configured?

  1. a single custom resource
  2. multiple configmaps
  3. all of the above

I think both the operations and serving-api WG's need to agree on that answer before we talk implementation details.

And if 3 is the answer, we need to agree on a conflict resolution strategy.