Fix build and use go modules for build

[gnufied]

Use go modules to pin dependency versions.

Fixes https://github.com/container-storage-interface/spec/issues/434

[saad-ali]

I'll let @jdef review and approve this.

[gnufied]

@jdef @jieyu To fix the build and keep import paths same - I had to move go.mod file to root of the project. But I have verified by importing it in bunch of project and it works same as before. I do not see a way around this if we want to use go modules.

[jieyu]

@gnufied have you tried placing go.mod in both the root dir and in lib/go/csi (like many multiple module go project)?

[gnufied]

@jieyu I have and it looks like exporting/publishing a module from a subdirectory is treated different from how local multiple modules refer each other from same repo.

Take https://github.com/gnufied/sandbox2/tree/v1.8.0 for example. If you go get

~> go get github.com/gnufied/sandbox2@v1.8.0
# you get an empty clone in pkg/mod/github.com/gnufied/sandbox2@1.8.0 with just go.mod file

# get nested module
~> go get github.com/gnufied/sandbox2/lib/spec/sandbox2@v1.8.0
go: finding github.com/gnufied/sandbox2/lib/spec v1.8.0
go: finding github.com v1.8.0
go: finding github.com/gnufied v1.8.0
go: finding github.com/gnufied/sandbox2/lib/spec/sandbox2 v1.8.0
go: finding github.com/gnufied/sandbox2/lib v1.8.0
go get github.com/gnufied/sandbox2/lib/spec/sandbox2@v1.8.0: module github.com/gnufied/sandbox2@v1.8.0 found, but does not contain package github.com/gnufied/sandbox2/lib/spec/sandbox2

[jdef]

maybe we should not yet upgrade to go.mod and instead patch the makefile to pin the grpc-go version like it already does for protobufs

On Thu, May 28, 2020 at 5:29 PM Hemant Kumar notifications@github.com wrote:

@jieyu https://github.com/jieyu I have and it looks like exporting/publishing a repsitory from a subdirectory is treated different from how local multiple modules refer each other from same repo.

Take https://github.com/gnufied/sandbox2/tree/v1.8.0 for example. If you go get

~> go get github.com/gnufied/sandbox2@v1.8.0

you get an empty clone in pkg/mod/github.com/gnufied/sandbox2@1.8.0 with just go.mod file

get nested module

~> go get github.com/gnufied/sandbox2/lib/spec/sandbox@v1.8.0 go: finding github.com/gnufied/sandbox2/lib/spec/sandbox v1.8.0 go: finding github.com v1.8.0 go: finding github.com/gnufied/sandbox2/lib/spec v1.8.0 go: finding github.com/gnufied/sandbox2/lib v1.8.0 go: finding github.com/gnufied v1.8.0 go: extracting github.com/gnufied/sandbox2 v1.8.0 go get github.com/gnufied/sandbox2/lib/spec/sandbox@v1.8.0: module github.com/gnufied/sandbox2@v1.8.0 found, but does not contain package github.com/gnufied/sandbox2/lib/spec/sandbox

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/container-storage-interface/spec/pull/435#issuecomment-635618396, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR5KLCPXDT6GW57OYPAUVDRT3JT3ANCNFSM4NIABGNQ .

-- James DeFelice 585.241.9488 (voice) 650.649.6071 (fax)

[gnufied]

@jdef the trick is - we will have to probably pin grpc-go and https://github.com/googleapis/go-genproto - so it gets bit uglier as time flies.

This PR however works at the cost of having go.mod file at root level. For example:

# This runs k8s unit tests by replacing github.com/container-storage-interface
# with github.com/gnufied/1.6.0 which is built from this PR.
go: downloading github.com/gnufied/spec v1.6.0
go: extracting github.com/gnufied/spec v1.6.0
go: finding github.com/gnufied/spec v1.6.0
=== RUN   TestNodeExpandVolume
--- PASS: TestNodeExpandVolume (0.00s)

Alternately we can start thinking about moving language bindings in their own repo, which will be more cleaner (and may be take the bitter pill of pinning version via Makefile for now)

[xing-yang]

LGTM

[saad-ali]

Discussed this offline. Given the concerns that @jdef raised, and since we are so close to cutting a CSI 1.3, we agreed to go with the less risky (but more hacky) approach (patch the makefile to pin the grpc-go version), and move to go mod approach afterwards for the CSI 1.4 release. This is just to mitigate unforeseen issues with switching to go modules and rushing the decision before we can think it though.

Thank you @gnufied for your patience and for driving this!

[saad-ali]

maybe we should not yet upgrade to go.mod and instead patch the makefile to pin the grpc-go version like it already does for protobufs

Doesn't look like there is a cleaner way to do this. https://github.com/golang/go/issues/34055 asks for the ability to serve a module from a subdirectory but the proposed solution won't work for us because it depends on being able to return a <meta name="go-import" ...> tag which we can't do on github repos (see https://github.com/golang/go/issues/34055#issuecomment-780762909).

maybe we should not yet upgrade to go.mod and instead patch the makefile to pin the grpc-go version like it already does for protobufs

Pinning dependencies in this way is a game of whack-a-mole. For example, the build is broken again right now.

Alternately we can start thinking about moving language bindings in their own repo, which will be more cleaner (and may be take the bitter pill of pinning version via Makefile for now)

That seems like a lot of unnecessary overhead.

This PR however works at the cost of having go.mod file at root level. For example:

I am in favor of this approach. go.mod at root is a little ugly but not the end of the world. Overall the benefit of go modules to fix dependencies vs existing solution outweighs the drawback, IMO.

[gnufied]

@saad-ali @jdef I have updated the branch and tested and verified that using via go get or go mod works.

[saad-ali]

Thanks @gnufied

/lgtm

Anyone have any objections to this change?

[humblec]

Thanks @gnufied

/lgtm

Anyone have any objections to this change?

lgtm from my side as well :+1:

[saad-ali]

Thanks everyone, merging!