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

Deprecate VolumeMetaData in controller and node plugin API? #29

Closed
govint opened this issue May 30, 2017 · 7 comments
Closed

Deprecate VolumeMetaData in controller and node plugin API? #29

govint opened this issue May 30, 2017 · 7 comments

Comments

@govint
Copy link

govint commented May 30, 2017

Given the CO never modifies any of the volume meta-data or plugin supplied volume-info is it necessary to keep passing the volume-meta-data on plugin API? The volume ID should be sufficient for the plugin to identify the meta-data for a volume on its own.

Most likely, the plugin may return a subset of meta-data its keeping per volume via API responses. Passing that to the plugin isn't needed frankly.

@jdef
Copy link
Member

jdef commented May 31, 2017

There were at least a few goal posts here:

  1. Non-identifying volume metadata (VolumeMetadata), useful for plugins when interacting w/ volumes, may contain sensitive bits (keys, certs, etc), and is separate from...
  2. Identifying volume metadata (VolumeID), useful to both CO and plugins, shouldn't contain sensitive bits (and may, for example, appear in log files).
  3. The CSI working group (and wider storage community) felt that CSI should clearly separate these two kinds of metadata: VolumeMetadata and VolumeID.
  4. It should be possible to write a relatively stateless CSI plugin; this is made easier if the CO can track both types of metadata and send it back to the plugin for subsequent RPCs.

As a result, the spec requires that CO track this additional per-volume metadata. This both increases the "data burden" of the CO and also adds a small amount of overhead to some RPCs.

Are there additional consequences that you're concerned with?

@julian-hj
Copy link
Contributor

julian-hj commented Jun 1, 2017

The primary purpose of this metadata block was to give the plugin a place to put volume information so that it does not need to maintain state (No. 4 above). Plugins can return a data blob that will get passed back to them in subsequent lifecycle calls.

For plugins that maintain their own backing store, I agree--the metadata block is totally unnecessary. But we did not want to force plugin authors to have a backing store as that makes plugin implementation and configuration quite a bit more complex.

@jdef
Copy link
Member

jdef commented Jun 2, 2017

Thanks for clarifying @julian-hj, next time I'll arrange my bullets in priority order :)

@oritnm
Copy link

oritnm commented Jun 4, 2017

VolumeMetadata should not be the place to store sensitive data such as secrets, certs etc. Those needs to be passed thru a different mechanism since the same volume may be used by different containers each with a different secret and different access permission such as in the case the volume is shared filesystem.

@govint
Copy link
Author

govint commented Jun 5, 2017

Agree on the CO keeping identifying meta-data (volume-ID), thats needed. But,

  1. Storage plugins can mostly be expected to be stateless, as they front-end a storage system that generates/owns the meta-data. The plugin most likely is ferrying meta-data from the storage system and doesn't need the CO to keep any on its behalf.

  2. Changes can happen on the storage system over time, invalidating the meta-data cached by the CO. Blocks allocated vs. free(for a volume) is one example. Do we need plugins to implement logic to handle this?

  3. If the CO inadvertently modifies or looses meta-data (CO crashes) then the plugin is hosed as well.
    a. Plugins would need to implement a scheme to verify that its getting back valid meta-data (which may still be stale (2)).
    b. Meta-data given to the CO would have to be something that the plugin can regenerate by itself in case the CO has lost the data.
    c. Clearly, the plugin can't keep sensitive (to loss) meta-data with the CO, which doesn't make it very useful in the first place.

There's nothing greatly wrong in a plugin using the CO to stash some meta-data on its behalf but its benefit is unclear. Or, the spec could detail how best the plugin can use this capability and avoid pitfalls.

@jdef
Copy link
Member

jdef commented Sep 20, 2017

#103

@jdef
Copy link
Member

jdef commented Dec 19, 2017

metadata was replaced by attributes.

please re-open if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants