-
Notifications
You must be signed in to change notification settings - Fork 5
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
WiP - GoCSI #2
WiP - GoCSI #2
Conversation
fa0fff2
to
c20f6f6
Compare
How about separating the commit into two commits? one is real patch and the other is vendor code. |
Hi Lei,
Because it wouldn't compile that way. The decision is more straight-forward than that. Should it build with a "go ..." sans any "make" beforehand? If the answer is yes then the vendor code should be part of the commit.
IMO a PR should be a single, atomic commit in order to to wholly add a feature in its entirety or remove it completely later. I see no value on dividing it into multiple commits at the point of merging it. Especially not when one depends on the other, and in this case N would depend on Commit N+1, which just doesn't make sense.
…--
-a
On Jun 26, 2017, at 8:24 PM, Lei Xue ***@***.***> wrote:
How about separating the commit into two commits? one is real patch and the other is vendor code.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
To be clear, I'd opt for removing the vendored code entirely and just declaring that the Makefiles are required for building or installing anything. Which is what I do on my projects. However, for an example I wanted things to be cleaner and more aligned with go's normal tooling workflow. |
f34c66a
to
24161f9
Compare
Hi @carmark / @jdef / @edisonxiang, Please see the updated README for additional information. I've included the current version of it here for your convenience:
|
206d3e4
to
9c7ddcb
Compare
40db170
to
c247033
Compare
I'm not sure I understand the value of the plugins mechanism here. IMO, the value of an example is the fact that it is approachable and digestible, and I worry that this is much closer to a real implementation than an example. Inclusion of EBS, for example, is a red flag. I'd much prefer that, if we have examples, they are approximately toys with no cleverness beyond JUST what is needed to demonstrate fulfillment of the API. I don't think an omnibus plugin is a bad thing, per se, but it a) needs bounds (e.g. only open standards) and b) is probably not an "example". |
Hi @thockin, I'm inclined to agree with you. I originally targeted the Thank you for your feedback! |
There is a lot more code here than what should be in a "demonstration for an API". Also, why is there even a platform aspect here ? (I mean UNIX vs Windows). |
Re: platform, to what are you referring? If you mean the Makefiles it's because I am determining whether the artifact being built has an extension of ".exe". And Go plug-ins are only supported on Linux which is why I called that out. Thank you for your feedback. I'm concerned there is confusion because I tried to provide an overview of different concepts and ideas -- examples. Also, Go isn't the only language capable of gRPC, so I don't want to assume that Go is a default choice. I'm going to close this PR and submit things in smaller chunks since my attempt at providing value as discreet elements in a single PR has seemingly confused people -- unable to see the trees because of the forest in the way. |
This patch provides a Go-based CSI client and server capable of supporting additional storage platforms at runtime via Go plug-ins.
Note: For anyone reviewing this, please do not comment on the vendoring of
grpc
orprotobuf
under themod/moc
directory. I will be updating this PR to explain why vendoring of these packages is necessary. The tl;dr is vendoring+Go plug-ins+gRPC type registry.csc
The Container Storage Client (
csc
) is a CLI program written in Go that provides one-to-one mappings with the currently available CSI RPCs.csd
The Container Storage Daemon (
csd
) is a CLI program written in Go that provides gRPC endpoints for all of the currently defined CSI services: Controller, Identity, and Node. The program's storage support can be extended at runtime via Go plug-ins.$ CSI_PLUGINS=$(pwd)/mock.so csd mock 2017/06/26 02:06:34 loaded plug-in: mock.so 2017/06/26 02:06:34 registered endpoint: mock 2017/06/26 02:06:34 mock.Serve 2017/06/26 02:07:06 csd.ListVolumes 2017/06/26 02:07:06 ...Volume.ID=1 2017/06/26 02:07:06 ...Volume.ID=2 2017/06/26 02:07:06 ...Volume.ID=3
csp
This package contains packages that can be compiled as stand-alone CSI daemons (similar to
csd
) as well as Go plug-ins that can be loaded intocsd
in order to extend the latter's storage platform support at runtime.Travis-CI
The forked repo from which this PR was generated is configured to use Travis-CI. The current builds of this PR's branch show the automated testing using the produced artifacts: