Vendor tool dependencies

[heartrobotninja]

With the conversion to go modules, our tool dependencies initially lost their vendored copies. It turns out there is an idiomatic way to fix this as described in golang/go#25922 (comment) .

Proposal 1a: Do exactly as the above link describes and create agones/tools/tools.go and put all tools in that file as imports.

The only thing we cannot do with that is vendor the annotations.proto and http.proto from grpc-ecosystem/grpc-gateway/third_party. If I copy it over, then anyone runs go mod vendor it gets deleted.

Proposal 1b: Create the path agones/proto/googleapis/google/api and copy them from googleapis/google/api to there (since that is where grpc-gateway gets theirs from anyway and since their third_party directory appears to be non-module vendorable).

[heartrobotninja]

The more I look at this, the more that the problem looks like other tools outside of agones.

For instance, one of the go module pulls that Mark was seeing is actually because we use generate-groups.sh in k8s.io/code-generator which has a go install line. That would be fine, but the deps in each directory aren't explicitly met so go mod tries to retrieve them.

One place where we could control it, but I am not sure we want to is the GRPC tooling and goimport install in build-image/Dockerfile. In order to do this, we would need to copy in agones to the image being built so we can use the vendor directory.

[markmandel]

I can't find the comment, but was there something with a tools.go that could be used to solve this issue?

[markmandel]

I see. I'm with you now. Sounds like copying the proto files from third_party should be the way to go?

Hopefully they don't update any time soon? Or we make sure we keep that in check.

[markmandel]

@heartrobotninja might also be worth looking at the splitting of images that @Kuqd is working on in #630 - that may also resolve some issues.

[markmandel]

I just installed @Kuqd 's updated - and I'm hitting:

+ protoc -I /go/src/agones.dev/agones/vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis -I . --grpc_out=/go/src/agones.dev/agones/sdks/cpp/.generated --plugin=protoc-gen-grpc=/usr/local/bin/grpc_cpp_plugin sdk.proto
/go/src/agones.dev/agones/vendor/github.com/grpc-ecosystem/grpc-gateway/third_party/googleapis: warning: directory does not exist.
google/api/annotations.proto: File not found.
sdk.proto: Import "google/api/annotations.proto" was not found or had errors.
includes/sdk.mk:71: recipe for target 'run-sdk-command' failed
make[1]: *** [run-sdk-command] Error 1

When trying to generate the gRPC code. If the easy solution is to import the proto files into an appropriate location - I'm all in. Maybe they should be baked into our build image?

[heartrobotninja]

Addressing the last three comments:

So there are two issues:

1. The protos were in the vendor directory which is not ok to do anymore. The grpc-ecosystem/grpc-gateway/third_party is not available via go modules. So whenever go mod vendor is run, anything that isn't imported / available via modules will get removed. Which is what you are running into with the build breaking. This should be remedied by moving the protos to their own directory. While investigating, I found that the protos are just copies from googleapis. I recommend we just pull those protos straight from there.

2. The module pull-in you were seeing while running tools post go modules conversion could be vendored in a tools.go directory. Previously this would had been problematic, but with @Kuqd 's work in Removes the sdk generation tooling from our main build image #630 this makes it much easier to accomplish since the new dockerfiles which contain the go install/go get commands appear to mount the agones directory.

[markmandel]

Small side note - is there any downside to adding common_mounts += -v $(build_path)/.gomod:/go/pkg/mod to the build/Makefile to cache the mod directory?

I think this would also be useful.

[riimi]

I am tring to build agones on windows with wsl.
When I first tried to build, the following error occurred.
docker: Error response from daemon: Mount denied: The source path "$(build_path)/.gomod" doesn't exist and is not known to Docker.
I think the line common_mounts += -v $(build_path)/.gomod:/go/pkg/mod makes the error.

[markmandel]

@riimi this sounds like a separate issue - can you file this as a separate bug, and we can track this as it's own issue? (I think I have an idea of why this might be happening, but we don't have many Windows devs atm)

[markmandel]

Is this complete? Can we close this issue now?

[heartrobotninja]

Not yet. I have a second round of vendoring I will be doing for this, but I don't have the time to get to it until next week. *From: *Mark Mandel <[email protected]> *Date: *Thu, May 16, 2019 at 4:54 AM *To: *GoogleCloudPlatform/agones *Cc: *heartrobotninja, Mention Is this complete? Can we close this issue now?

…

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#695>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABM5DZ5WOP4MBV264UPGMYDPVTEE3ANCNFSM4HEE65RQ> .

-- Joseph DeStefanis Google Inc.

[markmandel]

Doing another round of cleanup. I believe this issue can be closed now?

[markmandel]

Looks like no response on this issue. Closing.

Please reopen if required.