Define version in protobuf

[paulcwarren]

Is there a way to define the csi version in the protobuf and therefore in the generated code?

As a CO implementer we are needing to write code to validate that plugins support the version of CSI that the CO currently supports. With just the VERSION file in this repo this forces us to duplicate this version in our code with all of the obvious management headaches and potential pitfalls of that. If possible, in our opinion it would be a lot nicer to have that the version defined and maintained in the protobuf.

[akutz]

Hi @paulcwarren,

I believe this is the purpose of the RPC GetSupportedVersions. It's protobuf definition should never change (as simple as it is), and it returns a list of the API versions supported by the SP.

[julian-hj]

Right, but as a CO, it would be very nice to be able to test that the version of CSI we have submoduled into our CSI client is actually supported by the plugin. Since we don't have any version included in the generated .proto.go code, that means that we have to hard code the expected version, or parse it at runtime.

Similarly, the CSI client has to include a version in each CSI request. Today, we have to hard code that as well. Again, it would be nice to just grab it from generated code in the csi package.

[akutz]

Hi @julian-hj,

I believe parsing it at runtime is the expected behavior. As a CO you may encounter many different SPs, and you'll need to query their GetSupportedVersions RPC to understand whether or not the SPs are compatible with your CO's CSI implementation.

Regarding importing the CSI language bindings, if you're using Go then I suggest you don't use sub-modules but instead use a tool like dep to pin a specific version of CSI. It's what GoCSI does as, you're correct, we need to know against which version of CSI we're writing code.

If both of the above answers are completely off base, then it's likely I simply do not understand the question.

[julian-hj]

To me it seems like using run-time parsing to handle a compile-time dependency seems unnecessary and error prone, so I still contend that it would be better to have a constant in our generated code.

[akutz]

Hi @julian-hj,

I guess I'm confused. Are you talking about:

• Knowing when a remote SP supports a specific version of the CSI API?
• Knowing the version of the API supported by a CSI library you're importing into the codebase of a CO or SP?

If it's the former then the GetSupportedVersions RPC is the documented answer. If it's the latter, then, while I am fine using a dependency management tool like dep, I'd have nothing against generating the version (the last known annotated tag) into the CSI proto file.

[jdef]

Is this at all related? protocolbuffers/protobuf#3520 (comment)

…

On Fri, Jan 19, 2018 at 2:33 PM, Schley Andrew Kutz < ***@***.***> wrote: Hi @julian-hj <https://github.com/julian-hj>, I guess I'm confused. Are you talking about: - Knowing when a remote SP supports a specific version of the CSI API? - Knowing the version of the API supported by a CSI library you're importing into the codebase of a CO or SP? If it's the former then the GetSupportedVersions RPC is the documented answer. If it's the latter, then, while I am fine using a dependency management tool like dep, I'd have nothing against generating the version (the last known annotated tag) into the CSI proto file. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#170 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACPVLHeF42Z3lvtEeAuAKsnMADEQzp3wks5tMO3_gaJpZM4Rkymj> .

[julian-hj]

@jdef, yeah, that looks like the best available workaround for the limitation that protobuf lacks support for defining constants.

[akutz]

Hi Guys,

I'm sorry for belaboring the same point, but I'm still trying to figure out the issue @julian-hj is discussing. Is it point 2 from above?

Knowing the version of the API supported by a CSI library you're importing into the codebase of a CO or SP?

Again, I just want to make sure I understand the problem.

[julian-hj]

@akutz yes it is point #2

Cheers,
-Julian

[saad-ali]

To clarify the ask here is to have a const in the generated proto buf for version. Would be nice to have if it's not super ugly. Will leave it as post-v1