Skip to content
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

Conversation

jsafrane
Copy link
Member

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.

    • With Selectors, user might request a PV with certain labels and a PV would be provisioned only if no existing PV matched.
    • With ConfigMap, a PV must be always dynamically provisioned as we don't have any way how to check if an existing PV conforms to a ConfigMap.
  • 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

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 18, 2017
Required bool

// Optional list of values that are allowed for this key in the ConfigMap.
AllowedValues []string

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

Copy link

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 {

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.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some alternatives are:

  1. (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.
  2. Have the parameters as a map in the PVC. This breaks the PVC "portability".
  3. 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?

Copy link
Member Author

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.

Copy link

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.

@davidopp
Copy link
Member

ref/ kubernetes/kubernetes#41442

@jsafrane
Copy link
Member Author

@thockin, can you please take a look and compare with #247

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

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 ?

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 ?

Copy link
Member Author

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
Copy link

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
Copy link

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.
Copy link

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.

@jsafrane
Copy link
Member Author

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 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants