Proposal: Default Service Plans #1896
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
|
||
| ## Summary | ||
|
|
||
| 1. Add default provisioning and binding parameters to Service Classes. |
There was a problem hiding this comment.
binding parameters could be unique per plan, makes sense to tie them to plans; same comment about provisioning parameters, perhaps the class just has a default plan pointer (or use the existing feature for default plan) and then allow a plan to say how it wants to be defaulted
There was a problem hiding this comment.
Originally I had the defaulting parameters defined on both the class and the plan, and if a plan doesn't have defaults defined, it uses the defaults on the class. Since the only broker that I'm familiar with (azure) uses the same defaults across all plans in a class, I wasn't sure if that was necessary.
What do you think about going back to that?
| ## Summary | ||
|
|
||
| 1. Add default provisioning and binding parameters to Service Classes. | ||
| 1. Add a "Service Type" field to classes and plans, e.g. mysql or redis. |
There was a problem hiding this comment.
this would have to happen at the OSB level to make it inner-operable. or use a special formatting and use tags https://github.com/openservicebrokerapi/servicebroker/blob/master/spec.md#service-object
There was a problem hiding this comment.
That's something that my teammates will be discussing at the OSB F2F next week. 👍 Hopefully we can all agree upon how to format a tag, and standard names for the type (e.g. mysql vs MySQL vs Oracle's Spiffy DB of Doom) so that people can rely on the tag being consistent across brokers.
I'll add a section to the proposal outlining changes that I am proposing to the OSB API for v3, and ideas for interim solutions for OSB API v2.
|
|
||
| 1. Add default provisioning and binding parameters to Service Classes. | ||
| 1. Add a "Service Type" field to classes and plans, e.g. mysql or redis. | ||
| 1. Add a is-default-plan annotation to Service Plans to flag it as the default |
There was a problem hiding this comment.
this could be a tag as well
| 1. Add a "Service Type" field to classes and plans, e.g. mysql or redis. | ||
| 1. Add a is-default-plan annotation to Service Plans to flag it as the default | ||
| plan for a particular service type. | ||
| 1. Service Instances and Bindings fall back to the defaults set on the class |
There was a problem hiding this comment.
Something that is supported by OSB and is 100% up to the broker's impl is binding types. There could be n types of bindings that come from a result of different parameters being sent to a binding call all resulting in different types of binding results, all of which could be needed as the "default" case.
There was a problem hiding this comment.
So depending on the binding parameters sent to the same service, I could get back a set of credentials, the results of a log drain, the results of routing changes, etc?
Do you have an example of where a service does this, and how that would play out with defaulting? I am not quite sure I understand the pitfalls that we should be watching out for here and if they would require changes to defaulting or something else.
| plan for a particular service type. | ||
| 1. Service Instances and Bindings fall back to the defaults set on the class | ||
| when parameters are not provided. | ||
| 1. Services can be provisioned using only the Service Type, e.g. mysql, and the |
There was a problem hiding this comment.
This is the idea of service dependencies that has been floated (and has a doc, somewhere)
There was a problem hiding this comment.
Yup! Azure has already added support for dependencies. That's what lets us provision a database server and individual databases on top of an existing server separately which is pretty useful IMO.
The way azure handles that is by using different classes to distinguish between each type, e.g. mysql-database-only, mysql-server-only, mysql-server-and-database. I think that those will end up needing to be different service types and provide a suggestion for what that could look like later in the proposal.
I'll consolidate OSB API dependencies into a single section so it's easier to identify and discuss.
| | Field | Description | | ||
| |-------|-------------| | ||
| | Service Type | Examples: mysql, redis | | ||
| | annotation: is-default-plan=true | <p>Specifies whether or not a Service Plan is the default for its Service Type.</p> <p>When annotated as the default and a Service Type is set, this plan is used to dynamically provision an instance for that type.</p> | |
There was a problem hiding this comment.
This feels like it should be moved to the service class definition. Otherwise it is too easy to have two plans with this annotation set to true
There was a problem hiding this comment.
I was hoping that we could have the controller enforce that only 1 plan has the default set to true for a particular service type. Do you think that is feasible?
There was a problem hiding this comment.
If there was more than one default what would end users (app operators) experience?
There was a problem hiding this comment.
For example:
Let's say that there are two brokers installed on the cluster that both provide a MySQL database and the operator hasn't picked which one should be the default. When the developer installs a helm chart that needs a MySQL database, it will fail. The status message on the ServiceInstance will indicate that there is more than one default plan for MySQL, and that either the cluster operator should pick a default and then touch the service instance (to retrigger provisioning), or you edit the service instance to select specific class+plan.
Here's more information on how the default plan is resolved.
|
|
||
| Notes: | ||
|
|
||
| * The controller will enforce that only one default is set per service type. |
There was a problem hiding this comment.
What should the controller do when there are two?
There was a problem hiding this comment.
Good catch! I just realized that I didn't think through how this would work fully.
Here's how I am thinking default would be defined (below). What do you think?
- During a list, brokers provide classes/plans. A broker can suggest a plan as a good default by including a tag.
- Service catalog looks for the default tag during list and populates an annotation, "is-suggested-plan" or something along those lines. I'm not tied to annotations, just going along with what Storage Classes do until we have better ideas
- Cluster operators can flag a plan as the default using svcat. This will add a different annotation
is-default-plan, and Service catalog will handle unsetting theis-default-planannotation on any other plan of the same service type. It doesn't unset the annotationis-suggested-planfrom the broker, just if the cluster operator has previously picked a different plan as the default.
When service catalog searches for the default plan it will:
- List all plans that have that are flagged as suggested or default for that service type.
- When more than one plan has
is-default-planset, the operation should fail. - When there is one plan with
is-default-planset, that plan is used as the default. - Otherwise, when more than one plan has
is-suggested-planset, the operation should fail. This happens when multiple brokers exist on the cluster, provide a default plan for a particular service type, and the operator has not picked a default. - When there is on plan with
is-suggested-planset, that plan is used as the default.
In the simplest case, there is one broker that provides default plans, the operator does nothing and people can still get a default plan resolved without any extra work. Otherwise, the operator must step in and pick a default (or the user needs to be specific instead of relying on a default).
There was a problem hiding this comment.
I've updated the proposal with a how service plan selection works.
| Notes: | ||
|
|
||
| * The controller will enforce that only one default is set per service type. | ||
| * For simplification, defaults are set at the class level only, plans cannot override them. |
There was a problem hiding this comment.
I think this needs to be sorted out now. Perhaps both class and plan have defaults and there is a merge rule, but the defaults on class are applied and merged into the defaults for plan, and that result is the thing that is used for parameter defaults
There was a problem hiding this comment.
Agreed! 👍 I actually had that logic and merging in here originally so it's easy to convince me to add it back. 😀
| | Field | Description | | ||
| |-------|-------------| | ||
| | Service Type | Examples: mysql, redis | | ||
| | Provision Parameters | Overrides the default provision parameters defined on the class/plan. | |
There was a problem hiding this comment.
These are in addition to parameters? does sc have to look up and recalculate the defaults + provision parameters for each update? Does it have to setup a watcher to watch the defaults so that they can flow to the existing instances? Or does it have a copy of the defaults used.
There was a problem hiding this comment.
Sorry for the confusion, that was poor naming on my part. I'll fix the name of the field and the example yaml. This is the existing parameters field on the service instance spec.
When a service instance is provisioned, the controller:
- Selects the appropriate class/plan using either the values specified by the instance, or by the default service type if the instance doesn't specify a plan.
- The defaults defined on the class and plan are merged with the parameters defined on the instance. The precedence order is Class < Plan < Instance.
- The final set of defaults are persisted on the instance.
There is no reference or tie back to the objects that provided the defaults. They are only used to default the parameters, and then going forward the parameters must be managed on the instance directly, just as they are today.
| | Service Type | Examples: mysql, redis | | ||
| | Service Plan | When a ServicePlan is not specified, the default ServicePlan for the type is selected. | | ||
| | Bind Parameters|Overrides the default bind parameters defined on the class/plan. | | ||
| | Bind Response Transform|Allow overriding the default bind response transform set at the class level. | |
There was a problem hiding this comment.
I think this one is being worked on already or there is part of an impl in the repo already
There was a problem hiding this comment.
Thanks for the heads up! I'll update the proposal to use the same terms as what is being added right now.
Hurray! I am really excited to collaborate on this. 💖 |
Describe workarounds for v2 and what we would like for v3
Provision Parameters really was the existing parameters field
| |-------|-------------| | ||
| | Service Type | Examples: mysql, redis| | ||
| | <p>Default Provision Parameters</p><p>Default Bind Parameters</p> | <p>Default parameters to supply to the broker during provisioning and binding.</p> <p>OSB API V3 - Make the schema required, and defaults optional.| | ||
| | Default Secret Transform | Default json key mapping to transform a non-standard broker response into something that can be relied upon. See #1868 for implementation details. | |
There was a problem hiding this comment.
nit: [#1868](https://github.com/kubernetes-incubator/service-catalog/pull/1868)
| the operator. Service Catalog is very close to this model today, and I am | ||
| proposing that we "fill in the gaps" following the precedent set by storage. | ||
|
|
||
| ## Goals and Non-Goals |
There was a problem hiding this comment.
nit: can you make these separate lists? I'm guessing the last two bullets are your non-goals but it doesn't scan easily
| * Avoid making changes to Service Catalog that aren't _strictly_ required to | ||
| support this feature. For example, we aren't going to rename Service Bindings | ||
| to Service Claims. | ||
| * Dynamic provisioning concepts is out of scope. Concepts such as binding to |
There was a problem hiding this comment.
wording here confuses me a bit, dynamic provisioning is discussed below in Service Plan... is the idea that we're only discussing the creation of resources in this proposal?
| | Field | Description | | ||
| |-------|-------------| | ||
| | Service Type | Examples: mysql, redis| | ||
| | <p>Default Provision Parameters</p><p>Default Bind Parameters</p> | <p>Default parameters to supply to the broker during provisioning and binding.</p> <p>OSB API V3 - Make the schema required, and defaults optional.| |
There was a problem hiding this comment.
should bind/provision be separate rows in this table?
| | Service Type | Examples: mysql, redis | | ||
| | annotation: is-default-plan=true | <p>Specifies whether or not a Service Plan is the default for its Service Type.</p> <p>When annotated as the default and a Service Type is set, this plan is used to dynamically provision an instance for that type.</p> | | ||
| | annotation: is-suggested-plan=true | <p>Specifies whether or not a Service Plan is the broker suggested default for its Service Type.</p> <p>When annotated as the default and a Service Type is set, *and no plans with `is-default-plan` are defined, this plan is used to dynamically provision an instance for that type.</p> | | ||
| | <p>Default Provision Parameters</p><p>Default Bind Parameters</p> | <p>Default parameters to supply to the broker during provisioning and binding.</p><p>Overrides the defaults defined on the class.</p>| |
There was a problem hiding this comment.
should these be different rows?
More svcat examples * svcat set * svcat describe for service type, and scope * svcat get behavior with namespaced brokers Update toc
|
I've updated the proposal with milestones for breaking up the implementation into phases. |
|
F2F Feedback
|
* Add default plans by class introduced in F2F
carolynvs
added
do-not-merge/hold
|
/retest |
|
We've shifted priorities so I'm closing this issue. |
Let’s wholesale rip off how default storage classes for volumes work and apply it to Service Catalog!
See the proposal file in the PR for the details. I am using github now because it's just easier for me to manage than google docs. Apologies for the confusion! 😊