knative / serving

Kubernetes-based, scale-to-zero, request-driven compute
https://knative.dev/docs/serving/
Apache License 2.0
5.53k stars 1.15k forks source link

Allow some controller configurations to be label-specific #462

Closed tcnghia closed 6 years ago

tcnghia commented 6 years ago

Expected Behavior

Elafros applications deployed in different namespaces can have their own configurations. An example is the domain suffix, a route controller configuration: each namespace may need choose their own domain suffix and not having to share a cluster-wide domain suffix. A large organization that owns multiple domains should be able to serve all of them while sharing the same cluster.

Actual Behavior

Controller configuration is current a single ConfigMap in the elafros controller namespace. There is no namespace-specific configuration.

Additional Info

Related issue #363

tcnghia commented 6 years ago

@vaikas-google @mattmoor WDYT

vaikas commented 6 years ago

I think we should definitely allow for a single cluster to be able to serve multiple domains. I do wonder however if namespace specific configuration is the best way. I'd think this might be label based to be more in the spirit of k8s style. I could certainly see where routes labeled certain way would use a specific domain and so forth.

tcnghia commented 6 years ago

I like the idea of using label selectors. Elafros users are K8s users, and will probably be more familiar with that style.

tcnghia commented 6 years ago

@vaikas-google @mattmoor ConfigMaps doesn't have label selectors. I think we will need to create a new resource type for this.

mattmoor commented 6 years ago

I was just thinking something like:

data:
  # Keys prefixed with our domain signal configuration options for us?
  elafros.dev/foo: bar
  elafros.dev/baz: blah

  # Keys containing another domain (and just that domain?) signal the selector.
  # Only support matchLabel selectors, which are pretty easy to read when serialized.
  foo.mattmoor.io: |
    app: foo
  bar.mattmoor.io: |
    app: bar

However, this is kind of an awkward convention for operators to learn, and it only covers domains. I think the idea of a Cluster-scoped resource that contains the selector would be a nicer interface for Operators to configure these domains, but a resource feels like overkill for just domains.

Thinking about this raised a few seemingly foundational questions about the approaches we're circling:

vaikas commented 6 years ago

I was thinking along the lines what @mattmoor is doing, but instead of having just a single value, to have a selector field that's a map[string]string, akin to what service takes in to specify the pods. So, maybe have a list of domains? Each domain then would have a selector that's map[string]string that selects over the routes labels? Most specific wins? domains:

 - foo.mattmoor.io:
    selector:
       app: foo
 - bar.mattmoor.io:
    selector:
      app: bar
vaikas commented 6 years ago

indent is funny, I guess I need to know how to actually escape things.

data:

domains:

mattmoor commented 6 years ago

I updated your example with triple-`

tcnghia commented 6 years ago

Are we comfortable with embedding YAML in string values (of ConfigMaps) though?

May be for just Domain we can start out with embedded YAML (since new type of resource is overkill), but if we start having more than that we should move to a separate resource. @vaikas-google @mattmoor WDYT

vaikas commented 6 years ago

I'd be fine with that to validate the approach at least.

mdemirhan commented 6 years ago

Let's put monitoring as an example into this mix. An operator can chose to have dedicated instances of Prometheus or ElasticSearch for different teams for security and isolation and each of those instances will have different configuration. Any ideas on how we can do this, other than exposing our full blown YAML files? Approach above seems to be feasible only for a select few properties that we support natively in Elafros, but doesn't extend to configure our dependencies (Istio, Prometheus, ElasticSearch, etc).

evankanderson commented 6 years ago

Added @mdemirhan for other types of operator-configuration scenarios that we may want to support.

I'm wondering if we're ordering the map the correct way, though. Would it make more sense to have a set of labels which apply to a grouping? I'm thinking of "application profiles" which would be created and assigned by the operator. These might include both domain names and e.g. logs destinations or additional sidecars to be loaded.

I'm also suspicious of the ordering process suggested by the syntax in https://github.com/elafros/elafros/issues/462#issuecomment-376936162. Whether we use "most labels wins" or some other syntax, it seems like there's an opportunity for conflict.

My preference would be to control this on a namespace basis, with a base-level default. (e.g. ela-system and ela-system-$NAMESPACE). Keys would be read from the namespace override if present there, and then from the ela-system default (and a default must be provided for each config key).

  1. namespace is guaranteed unique per-resource
  2. An operator can guarantee that a certain team/workload cannot escape their configuration by changing labels.
mdemirhan commented 6 years ago

I like the idea of controlling on a namespace basis as well. It also makes RBAC easier to deal with. For example, if I want to have two instances of Prometheus but restrict access to these instances on a per team basis as part of my operator API/config, doing this with selectors would be hard to do (or at least according to my limited understanding of k8s RBAC). With namespaces, I can restrict which namespaces can access to that service easily.

tcnghia commented 6 years ago

@mdemirhan I am usassigning myself here. Looking forward to seeing monitoring configuration, also seeing how namespace-based configurations would look.

josephburnett commented 6 years ago

I've consolidated the autoscaling parameters into elaconfig.yaml. The layer which binds the config map to Golang variables is highly reusable, so I put it in a separate repo: k8sflag.

https://github.com/elafros/elafros/pull/688

This doesn't yet solve the namespace problem. But it makes configuration easier to use, more consistent, and optionally dynamic (experiment/feature flags).

mdemirhan commented 6 years ago

I will close this work item as we have others tracking the operator configuration for monitoring.