-
Notifications
You must be signed in to change notification settings - Fork 382
Define default provisioning params on plans #2282
Define default provisioning params on plans #2282
Conversation
c9cf6ee
to
fc1a0f3
Compare
* Vendor mergemap which helps merging parameters from multiple sources
11e4fcb
to
52e9628
Compare
06116fc
to
131ef41
Compare
After a service instance resolves a class and plan, reconcile the default parameters defined on the plan with those from the instance. Persist the original default parameters on the instance status so that they are not applied more than once, and also help with debugging.
We need to be able to update the spec on both instances and bindings eventually so that default parameters can be applied for both provisioning and binding.
21c589a
to
a24fd45
Compare
Plan: custom-mysql | ||
|
||
Parameters: | ||
No parameters defined |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was confusing a bit at first glance. I guess this is just the stuff provided from the svcat command the user entered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct, it is whatever they first entered in. I agree it's confusing and would like to do a follow-up PR for the svcat UX. This is a pretty big PR as-is, so I tried to limit it to just the server side changes and documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks solid to me.
/approve
@@ -1184,6 +1189,7 @@ | |||
"pkg/util" | |||
] | |||
revision = "6702109cc68eb6fe6350b83e14407c8d7309fd1a" | |||
version = "kubernetes-1.11.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some changes in here I wasn't expecting, but I guess that is just due to a newer point release of Dep you are using? ie there are some differences in version and branch elements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has to do with the information available in git at the time we last edited the lock file. When someone updated us to 1.11, they pointed at the branch, not the tag I'm assuming.
This won't change the commit used in our vendor directory, just tweaks the breadcrumb trail on how to find that SHA (i.e. prefer the tag over a moving branch).
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jboyd01 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
I gave this a spin on my AKS cluster and it looks good! /lgtm |
This allows an operator to define a plan and provide a default set of provisioning parameters. I am doing this incrementally so that it's not a giant PR. There will be follow-up PRs to add:
svcat create plan --from
, defaults on classes, etc. The set of milestones is laid out in the proposal.TODO:
Doc preview available at https://deploy-preview-2282--svc-cat.netlify.com/docs/service-plan-defaults/