-
Notifications
You must be signed in to change notification settings - Fork 5.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposal for user configuration of dynamic provisioning #543
Proposal for user configuration of dynamic provisioning #543
Conversation
Required bool | ||
|
||
// Optional list of values that are allowed for this key in the ConfigMap. | ||
AllowedValues []string |
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.
from the provisioning side I expect these need to be machine-readable values, but from the end-user side you are going to want human-readable values, so an array of strings isn't sufficient
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.
also, what if the necessary value is a number value? Like [ Key:"CrazyNumberOfDisks", Value: "34" ]
? Should one of the AllowedValues strings be something like "0-100"? Not necessary for the first version.
|
||
// ProvisioningOption describes one option that can be used in PVC provisioningOptions | ||
// when requesting a volume of a particular StorageClass. | ||
type ProvisioningOptionSpec struct { |
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 is a similar approach to how we handled Parameters in OpenShift templates, but we've found it generally ended up being insufficient / we kept needing to add fields to it to improve the experience. In the open service broker API we decided instead to define parameters via JSON schema v4 since that will provide more flexibility and there are a plethora of JSON schema validators (and GUI form builders) out there.
Although from the perspective of the storage admin that is setting this up, it is more complex to define a JSON schema than a limited set of fields like is proposed here.
name: my-provisioning-config | ||
``` | ||
|
||
* Note that this ConfigMap is used only during provisioning. Changes in the ConfigMap are not reflected in already provisioned PVs. |
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 is actually why I don't like this approach. From a GUI perspective a user is probably not going to be picking an existing ConfigMap, I see us instead providing a form to fill out the parameter values and then generating a new ConfigMap to go along with the PVC they are creating. But this extra ConfigMap will sit around forever, and probably never be used again. This just adds clutter to the GUI for ConfigMaps the users actually DO care about, like their application configs. I also don't like that this means two requests to set up the PVC correctly, 1 for the PVC and 1 to create a config map. It's increasing the surface area for failure.
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.
Some alternatives are:
- (Current method) Create a different storageclass for each permutation of parameters. This causes clutter to the StorageClasses, but it eliminates the two-step PVC process.
- Have the parameters as a map in the PVC. This breaks the PVC "portability".
- Have each storage feature exposed as a first class field in the PVC. It could be difficult to represent all the features generically, and still has the question of portability. What happens if you have zone in the PVC, but then you take the spec on a platform that doesn't support zones?
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.
The idea is that most people won't care about configuration of storage at all. Admin prepares a handful of storage classes for generic usage and user pick one of them and that's it. ConfigMap is for those who need specific options for their data. This proposal assumes there will be a limited amount of those, otherwise it gets indeed complicated.
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.
I'm not really sure this would cause an issue that wouldn't be solved by other methods in a GUI, but I understand what you mean. It could get cluttered.
I really like this idea. There is a connection from ConfigMap--PVC--StorageClass--PV. I think the ConfigMap (which a UI could create for the user) would be an easy way for that user to adjust the PVC request.
provisioner: kubernetes.io/aws-ebs | ||
parameters: | ||
type: gp2 | ||
zones: us-east-1a, us-east-1b, us-east-1c, us-east-2a, us-east-2b, us-east-2c |
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.
why do you repeat all the allowed values for zones here ? If the PVC doesnt specify a zone , then what is the default value you we get or the user would always have to specify a zone in the PVC using config map ?
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.
i see below you mean the admin restricts to these zones. if this doesnt repeated everything in the allowed, it will look more realistic example. My question about is there a default still stands ?
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.
if PVC does not specify a parameter value, e.g. a zone, then the provisioner is free to choose one. With zone, we do semi-random distribution of PVCs across all zones, for other parameter the default may be different.
Required bool | ||
|
||
// Optional list of values that are allowed for this key in the ConfigMap. | ||
AllowedValues []string |
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.
also, what if the necessary value is a number value? Like [ Key:"CrazyNumberOfDisks", Value: "34" ]
? Should one of the AllowedValues strings be something like "0-100"? Not necessary for the first version.
requests: | ||
storage: 1500Mi | ||
provisioningOptions: | ||
name: my-provisioning-config |
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.
👍
name: my-provisioning-config | ||
``` | ||
|
||
* Note that this ConfigMap is used only during provisioning. Changes in the ConfigMap are not reflected in already provisioned PVs. |
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.
I'm not really sure this would cause an issue that wouldn't be solved by other methods in a GUI, but I understand what you mean. It could get cluttered.
I really like this idea. There is a connection from ConfigMap--PVC--StorageClass--PV. I think the ConfigMap (which a UI could create for the user) would be an easy way for that user to adjust the PVC request.
I am closing this proposal, we as sig-storage have other priorities and we did not find people to lead this effort. Volunteers are welcome at kubernetes/enhancements#244 ! |
This is counter-proposal for #247. I've been talking to @msau42 and @saad-ali and with upcoming scheduler changes it would be much better not to allow generic selector as proposed in #247, which is hard to evaluate and allow only exact configuration of dynamic provisioning using ConfigMaps in user namespace(s).
I leave both PRs open and close one when we decide which way to go.
Pros of ConfigMaps:
PVCs are platform independent, as long as user supplies updated
ConfigMap(s)
when deploying to another platform.It is strict and thus easier to handle in the provisioner and in scheduler. No selectors like "zone NOT IN [east, west]"
Cons:
It works only with dynamic provisioning.
ConfigMap
, a PV must be always dynamically provisioned as we don't have any way how to check if an existing PV conforms to aConfigMap
.One extra object to care about.
External dynamic provisioners must be able to read all ConfigMaps on the system. This should be OK as long as users don't use ConfigMaps for their mysql passwords.
Again, "zone" is used here just for an example, it may be phased out by upcoming scheduler changes, but we still need a way how to configure encryption or nr. of Gluster replicas.
cc: @thockin @vishh @jwforres @derekwaynecarr @eparis
@kubernetes/sig-storage-api-reviews