-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
*: Use go modules for SDK dependency management #1566
Conversation
@joelanford Guessing this is on hold until the above PR is merged? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after CI turns green.
/test sanity |
I'm pretty worried that this will result in more frequent flakes. Since we'll be relying on several hundred dependencies (totaling multiple GB of data) being downloaded successfully for every individual test run. Right now, there are 5 tests that each fetch the entire set of modules (unit, sanity, go e2e, helm e2e, and ansible e2e), so the problem is also 5x exacerbated. Any thoughts on alternatives? I'm more worried about a solution that will work for OpenShift CI than Travis since we're migrating away from Travis. Does OpenShift CI offer a better caching solution (e.g. mounting a volume, using a cached local base image, etc.). Would it help to rebuild a base image every once in awhile (e.g. every time we bump our kubernetes dependencies) that has the vast majority of the modules pre-fetched. |
@joelanford We could set up Openshift-CI to use a Dockerfile as a root image and then do a |
@AlexNPavel Ah okay that's what we were talking about the other day. That definitely sounds like an improvement. I've got a bunch of comments & questions on that:
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/test e2e-aws-ansible |
1 similar comment
/test e2e-aws-ansible |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
Description of the change:
go.mod
andgo.sum
Gopkg.toml
andGopkg.lock
/vendor
dep
.Makefile
,.travis.yml
, andhack/image/
to use go modules (and relatedgo mod
) commands in CI.Motivation for the change:
Go modules is the official dependency manager of the Go community and has been incorporated into the go toolchain. By default, new operator-sdk projects now use go modules, and our users will expect operator-sdk to use them as well.