Skip to content
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

Move to go modules #625

Closed
markmandel opened this issue Feb 26, 2019 · 21 comments
Closed

Move to go modules #625

markmandel opened this issue Feb 26, 2019 · 21 comments
Labels
area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. help wanted We would love help on these issues. Please come help us! kind/cleanup Refactoring code, fixing up documentation, etc
Milestone

Comments

@markmandel
Copy link
Collaborator

Apparently Go Modules are a thing now 😄 and where Go is headingin the near future (1.13 I think they will be default).

We should convert over from dep to the modules at some point in the future.

@markmandel markmandel added kind/cleanup Refactoring code, fixing up documentation, etc area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. help wanted We would love help on these issues. Please come help us! labels Feb 26, 2019
@heartrobotninja
Copy link
Contributor

I will be working on this shortly.

@aLekSer
Copy link
Collaborator

aLekSer commented Feb 27, 2019

@heartrobotninja Please don't forget about #414 which currently was manually fixed in /vendor
kubernetes/kubernetes#70277 is merged, but not sure about target Kubernetes version.

@aLekSer
Copy link
Collaborator

aLekSer commented Mar 4, 2019

@heartrobotninja also note that we can use vendors with modules at the same time to mitigate mentioned above issue:
https://github.com/golang/go/wiki/Modules#how-do-i-use-vendoring-with-modules-is-vendoring-going-away

@heartrobotninja
Copy link
Contributor

Here is what we have so far:

Moving straight to go modules is ideal, but not doable right now. We have code in vendor that contains a fix that will not be released until March 25th, and there is no guarantee when the platforms will update to 1.14 since they are still on 1.12.

Given this, it might be best that we move to go modules while also keeping the vendor directory. This would require re-implementing the fix from #414. If we are ok with doing that, I can do both parts fairly quickly.

@markmandel
Copy link
Collaborator Author

It sounds like a way forward, but what does "re-implementing the fix" entail? Just making sure it's pasted into the new vendor directory?

Question - with go modules,without a vendor directory, does that mean on each CI build, we would have to download all our dependencies every time?

@heartrobotninja
Copy link
Contributor

re-implementing would just be re-applying the fix. I know there is a cache for go modules, but I am currently trying to hunt down how that works with CI builds.

@markmandel
Copy link
Collaborator Author

Gotcha. We'd likely want to store that in GCS and move it back and forth.

@heartrobotninja
Copy link
Contributor

As for CI:
We could either cache all dependencies prior to building. This still makes us download still, but we do it in advance of building. There might be some magic we can do there where we keep the cache warmed.

The other option is doing module-aware vendoring. This is mostly what we do now, so will likely consist of the least changes. Both are supported and I would propose we just do module-aware vendoring for now.

@markmandel
Copy link
Collaborator Author

I would propose we just do module-aware vendoring for now.

SGTM 👍

@heartrobotninja
Copy link
Contributor

Currently have modules working for LOCAL_GO=true. Currently working out some issues with the non-local version failing. Been looking into why this would be a problem for docker run.

@heartrobotninja
Copy link
Contributor

What I am currently running into: The intermediate containers which get instantiated by RUN aren't inheriting environment variables.

@heartrobotninja
Copy link
Contributor

So, the go.mod file isn't visible in the intermediate container. That is the actual problem.

@heartrobotninja
Copy link
Contributor

I finally have gotten things to get past the missing mod file, but now I am trying to sort through the differences in local vs docker because while local building works fine, docker has some issues finding certain packages. The current one is:
build agones.dev/agones/cmd/controller: cannot load k8s.io/api/admissionregistration/v1alpha1: cannot find module providing package k8s.io/api/admissionregistration/v1alpha1

@heartrobotninja
Copy link
Contributor

Its apparently by design per kubernetes/client-go#551 they removed v1alpha1 from master, but we still have an indirect dependency on it via an import of an import.

@heartrobotninja
Copy link
Contributor

Fun fact, when running go build as we do for build-controller-binary, I can't get the build to recognize that a go.mod file exists unless I put a command before go build. For instance if I put pwd && go build -mod=vendor it works.

@heartrobotninja
Copy link
Contributor

Progress! I have make build and make test working. I need to do some e2e testing and documentation and I will have a pull ready.

@heartrobotninja
Copy link
Contributor

e2e testing worked. Fixing some issues with the gen-grpc files that have cropped up due to vendoring.

@heartrobotninja
Copy link
Contributor

Design Doc:
https://goo.gl/uNhtJ2

@markmandel
Copy link
Collaborator Author

Can this be closed now?

@heartrobotninja
Copy link
Contributor

Yep!

@markmandel markmandel added this to the 0.9.0 milestone Apr 1, 2019
@roberthbailey
Copy link
Member

roberthbailey commented Apr 17, 2019

Should the GOPATH section of the build/README be removed (or updated) in light of this change or is is still necessary to install inside of GOPATH for IDE integration?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build-tools Development tooling. I.e. pretty much everything in the `build` directory. help wanted We would love help on these issues. Please come help us! kind/cleanup Refactoring code, fixing up documentation, etc
Projects
None yet
Development

No branches or pull requests

4 participants