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

bump(*): Vendor dependencies #177

Merged

Conversation

tnozicka
Copy link
Contributor

This ensures offline builds and ensures the project requirements are met.

Also the project level golang builder is about to be switched to force vendoring mode.

/priority critical-urgent
/assign clcollins jeefy jeremyeder meowfaceman

This ensures offline builds and ensures the project requirements are met.
@openshift-ci-robot openshift-ci-robot added the priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. label Dec 10, 2019
@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Dec 10, 2019
@mdwn
Copy link
Contributor

mdwn commented Dec 10, 2019

Left a message in Slack, but do you have any context about this? I'm not averse to it but I'm not sure of what's driving this from a policy/business sense.

@tnozicka
Copy link
Contributor Author

tnozicka commented Dec 11, 2019

The "policy/business sense" requires our product to be built offline. Being dependent on golang cache breaks the requirement.

I've sent and org wide mail, but to cite a bit here:

There is also an unfortunate change that the Golang has forgotten to
address when enabling modules by default - vendor dir is ignored in
golang 1.13 unless using -mod=vendor explicitly. In other words, with
switch to golang-1.13 every component will ignore its vendor folder on
builds. As in some repos like origin we patch vendor content, this has
severe impact on ignoring patches / backports at scale, not only the
issue with offline mode. Golang is also tracking the fix, I think it was this one:
golang/go#33848
As for now Golang team is recommending the following:
"Some people will want to routinely opt-in to vendoring by setting a
GOFLAGS=-mod=vendor environment variable." [1]
As the workflow for OpenShift org mandates using vendoring we have
decided to fix the golang-1.13 image by adding the environment variable
GOFLAGS='-mod=vendor' there. With the scale of the org (new repos are
growing exponentially with the split) fixing and hoping to maintain
individual repos is just too risky. And the whole org requires
vendoring anyways

The origin golang-1.13 builder image you use will be enforcing vendoring mode in openshift/release#6337

If you are on a team owning these please take actions asap. The options
are:
a) add vendor dir to your repo with go mod vendor and commit
b) run go mod vendor in CI as a pre step before running build
c) explicitly use -mod=mod in your build system to retain non-
vendoring module mode
d) in CI config for your job, unset the global GOFLAGS variable
e) use different builder image
Again, as the org is required to use vendoring, a) is the preferred
option, unless there are exceptional circumstances.

@jeefy
Copy link
Contributor

jeefy commented Dec 11, 2019

/approve

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 11, 2019
@mdwn
Copy link
Contributor

mdwn commented Dec 11, 2019

/lgtm
Okay. The reasoning here sounds good, so I'll merge this.

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Dec 11, 2019
@openshift-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jeefy, meowfaceman, tnozicka

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot merged commit bbee5a4 into openshift:master Dec 11, 2019
@tnozicka tnozicka deleted the vendor-dependencies branch December 12, 2019 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants