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

spec: Refactor NodeID a la VolumeHandle #106

Closed
akutz opened this issue Sep 18, 2017 · 5 comments
Closed

spec: Refactor NodeID a la VolumeHandle #106

akutz opened this issue Sep 18, 2017 · 5 comments

Comments

@akutz
Copy link
Contributor

akutz commented Sep 18, 2017

This issue is a proposal to refactor the NodeID so that it is inline with a VolumeHandle. For example:

Old

message NodeID {
  // Information about a node in the form of key-value pairs. This
  // information is opaque to the CO. Given this information will be
  // passed around by the CO, it is RECOMMENDED that each Plugin keeps
  // this information as small as possible. This field is REQUIRED.
  map<string, string> values = 1;
}

New

message NodeID {
  string id = 1;
  map<string, string> metadata = 2;
}

This would make it much easier to refer to a Node by some meaningful piece of information, such as an IP address or hostname, without embedding that information in an opaque map.

I would not have suggested such a change as I believe the opaqueness of the data maps is a virtue, but with the change to a volume's ID, this seems like a consistent shift towards that direction.

@jdef
Copy link
Member

jdef commented Sep 19, 2017

I like the idea of consistency. Along such lines, id would not be sensitive but metadata could contain sensitive information?

Not sure how crazy I am about stuffing IPs into metadata if there's a possibility that they're going to change. NodeID is expected to NOT CHANGE once it's generated by the plugin.

@akutz
Copy link
Contributor Author

akutz commented Sep 19, 2017

Hi @jdef,

It doesn't need to be IP addresses. It could be, for example, an EC2 Instance Identity document. The NodeID's ID field could be some unique piece of information that could be easily hashed or searched in a map and the metadata could be the rest of the document marshalled as JSON and then unmarshalled into a map.

@jieyu
Copy link
Member

jieyu commented Oct 2, 2017

I am +1 on this change. @saad-ali is working on adding topology information to NodeID. You may want to sync with him on that.

@saad-ali
Copy link
Member

saad-ali commented Oct 3, 2017

I think we should simplify NodeId and VolumeHandle to a simple string. See #116. Topology can be addressed separately.

@saad-ali
Copy link
Member

saad-ali commented Nov 1, 2018

So far a simple string has worked fine for existing CO and driver implementations. Therefore before we go to 1.0 we are closing this out.

@saad-ali saad-ali closed this as completed Nov 1, 2018
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