node_id should be required for plugins that implement ControllerPublishVolume

[saad-ali]

node_id in the GetNodeIDResponse is currently defined as optional:

  message Result {
    // The ID of the node which SHALL be used by CO in
    // `ControllerPublishVolume`. This is an OPTIONAL field. If unset,
    // the CO SHALL leave the `node_id` field unset in
    // `ControllerPublishVolume`.
    NodeID node_id = 1;
  }

If it is not returned, the CO is expected to not pass it on subsequent ControllerPublishVolume calls:

  // The ID of the node. This field is OPTIONAL. The CO SHALL set (or
  // clear) this field to match the `NodeID` returned by `GetNodeID`.
  // `GetNodeID` is allowed to omit `NodeID` from a successful `Result`;
  // in such cases the CO SHALL NOT specify this field.
  NodeID node_id = 3;

I don't think that controller node ID should be optional. If a plugin has PUBLISH_UNPUBLISH_VOLUME controller capability, then it should implement GetNodeId() and return a non-empty node ID. If a plugin doesn't require a call to connect storage to a node then it should not implement a PUBLISH_UNPUBLISH_VOLUME controller capability.

[julian-hj]

Makes sense to me.

[jieyu]

SGTM. @saad-ali do you want to send a PR for this?

[akutz]

Hi @jieyu,

I'd propose we hold off on this as #117 changes NodeID to a simple string. I could make the doc change there?

[jieyu]

xref #141