-
Notifications
You must be signed in to change notification settings - Fork 45
KnativeServing operator should enforce the configuration defined in the CR #163
Comments
This relies on the Transform function now being immutable, returning a new Manifest instead of mutating its source resources. This is related to knative#163 as well.
/assign garron |
This relies on the Transform function now being immutable, returning a new Manifest instead of mutating its source resources. This is related to knative#163 as well.
This relies on the Transform function now being immutable, returning a new Manifest instead of mutating its source resources. This is related to knative#163 as well.
…to be canonical (#165) * Add an e2e test for #138 After creating the KnativeServing instance, we then update it twice, once with an entry for the config-logging ConfigMap and then again without the key, verifying that the operator correctly syncs the ConfigMap each time. * Uses a pre-release of manifestival to fix #138 This relies on the Transform function now being immutable, returning a new Manifest instead of mutating its source resources. This is related to #163 as well. * Address golint warnings * Transform manifest once and pass to each reconcile stage * Let's confine the arbitrariness of the configmap to the fixture This also makes the WaitForConfigMap helper more helpful
I was going to open an issue but I think this one might be related, if not let me know and I'll open a new one. Overall: where should a user modify things if they want to change their Knative install? Today, w/o the operator, if someone wants to do something like modify the template of the URL generated for their ksvc they would modify the config-network configMap's domainTemplate value. However, with the operator any updates made directly to that config-map would eventually be overwritten by the operator's reconciliation loop. This means that we have two different sources of truth based on how Knative was installed. This also means that the user needs to know which install mechanism was used so they can modify the correct data to get the results they want. This is not a good user experience for people manually managing Knative, nor is it good for tool authors who will want their tools to work against all Knative installs easily (and with minimal conditional logic). It would seem to me that either: I think either way can work, we just need to pick a direction. But the current state doesn't seem ideal. Thoughts? /cc @houshengbo @mattmoor |
I tend to disagree with this being a black and white decision. If Knative Serving was installed without the operator, changes should definitely be made through ConfigMap edits. If Knative Serving was installed with the operator, changes should be made through the operator's API surface which is the KnativeServing CR currently. Edits to the configuration are in the operator's persona, not the developers. I feel like the operator should know how Knative Serving was installed. The rest, I feel, is documentation of the specific platform. |
In theory I hear ya.... but in practice I keep thinking about situations where someone is trying to create a tool, or even just a simple bash script, that they want to hand to someone else and in order to make sure it "just works" means they have to code up all config changes twice and wrapper them with an if-statement each time. That's why I'm leaning towards option 2 - it allows for people to only need to know about the configMaps. |
If the choice is between 1 and 2, I would choose 1. The policy that would enable 2a runs counter to the declarative (spec->status) nature of k8s. |
Can you elaborate on how it doesn't adhere to kube? With 2 the operator just sets the initial values for certain resources. I don't think kube has anything to say about whether it must then revert any changes on those resources. I think it's up to the semantics of the operator to decide that. |
Maybe "runs counter" and "doesn't adhere" are too strong. It's just that with 2a you still have two sources of truth, right? The spec for the initial defaults and whatever someone might manually change them to. Of course, we can make the semantics of the operator whatever we like, but I personally prefer the single source of truth in the first option you suggested. |
It might depend on our point of view, but for each resource there is only one source of truth. It is true that depending on the resource (and it's policy) its source of truth might be in a kn configMap or might be in the operator's CR/manifest. But, I'm hoping that for configMaps that we expect users to be able to edit, the source of truth would be in kn's configMaps not in the operator. But obviously 1 works too :-) I'm just not excited by today's model. |
If you're using an operator, the surface area of what you can do should be encoded by the operator. If we encourage users to reach around we imo have failed to provide a good API for the human-operator and need to ask ourselves why we want to use an operator in the first place. Allowing to reach-around also makes certain decisions around how to handle these config maps in an upgrade impossible. |
This issue was split out of #138 to separate the internal operator state management (handled in that issue) from discussion on how the operator should go about ensuring that it's configuration is in effect.
Consensus at the operator working group meeting was that the KnativeServing CR should be the source of truth and this issue is intended to cover work in making that so.
The text was updated successfully, but these errors were encountered: