Bug 1801357: Migrate from glide to go modules #102
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
/hold This is missing the glide.diff fix, I want to see if it passess the tests first and I'll deal with that later. |
openshift-ci-robot
added
do-not-merge/hold
juanluisvaladas
force-pushed
the
migrate-go-mod
branch
3 times, most recently
from
0b493e2
to
2d52f90
Compare
|
/test verify-deps Doesn't look like a legit failure |
|
/retest |
juanluisvaladas
force-pushed
the
migrate-go-mod
branch
from
2d52f90
to
ffff885
Compare
juanluisvaladas
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
/hold cancel |
openshift-ci-robot
removed
the
do-not-merge/hold
juanluisvaladas
force-pushed
the
migrate-go-mod
branch
from
ffff885
to
3783ca7
Compare
|
I'm particularly concerned about the patch, directory, I'm quite unhappy about this solution... |
|
/retest |
juanluisvaladas
force-pushed
the
migrate-go-mod
branch
4 times, most recently
from
2f1d266
to
c6d5616
Compare
|
/hold |
openshift-ci-robot
added
the
do-not-merge/hold
juanluisvaladas
force-pushed
the
migrate-go-mod
branch
from
c6d5616
to
150eed4
Compare
|
/retest |
juanluisvaladas
force-pushed
the
migrate-go-mod
branch
3 times, most recently
from
9cf2090
to
02538b7
Compare
|
/retest |
1 similar comment
|
/retest |
It was mandatory to migrate to go modules due to syntax incompatibility
on the sigs.k8s.io-structured-merge-diff module definition. Besides
this incompatibility, it was a matter of time until we found more
problems and glide is abandoned at this stage.
There are a few non obvious things going on:
* go.mod: Unnecessary large, however, to avoid changing dependencies
some stuff it was necessary to add some stuff that normally
wouldn't make sense. After the 1.17 rebase it'll be cleaner.
There is a require and replace discrepancy on
github.com/miekg/dns. This is because go mod tidy modifies
the require because one dependency asks for a newer version.
The k8s.io/ packages are replaced beacuse this issue:
golang/go#26366
* pkg/dependencymagnet: go mod vendor won't actually pull a module
unless it's required by some go code. We need to explictly
add dependencies on a fake moduleso that we can actually
build.
* Removed files: go mod vendor removes unnecessary files, lots of
files have been removed, but those shouldn't be required at
all.
* Updated github.com/openshift/library-go: The version we were using
was incompatible with go mod. So I updated to the latest.
* Makefile: export GO111MODULE=on library go runs some stuff with
-mod=vendor. Prior to go 1.13 this environment variable
is mandatory for it to work.
Additionally we need to explictly define
GO_BUILD_PACKAGES_EXPANDED for the same reason.
we need to add unxeport GOPATH and export GOPROXY are
necessary due to some odd behavior with go get modernc.org/*.
The queries to this domain ultimately get converted to
https://gitlab.com/cznic/* and go tries to run
git clone https://gitlab.com/cznic/*
instead of
git clone https://gitlab.com/cznic/*.git.
For some reason adding these two variables fixes it.
* patch directory: We needed to patch a single file in the netlink
library. This is the most similar solution to glide.diff I
could find. It's not elegant and doesn't scale well, but
since we are only patching one file in one library it's
not so terrible.
* HACKING.md: This document is now outdated, it wil be updated after
the 1.17 rebase.
juanluisvaladas
force-pushed
the
migrate-go-mod
branch
from
02538b7
to
aff2a46
Compare
|
I guess we can remove that, I removed all non go files and tests. Needs to pass tests again, particularly the make verify-deps, but it should be fine. |
You can just update the vishvananda/netlink dep to Other than that, I think this seems OK? Ugh, vendoring. Sigh. |
|
Regarding upgrading the netlink library, kube-proxy has github.com/docker/libnetwork/ipvs as a dependency and this has this lines: I could try to upgrade github.com/docker/libnetwork upstream to v1.1.0 but even if they accepted it, that would take some time. |
|
/retest |
|
blah. ok btw, as seen in the github status, this needs |
|
/retitle Bug 1801357: Migrate from glide to go modules |
openshift-ci-robot
removed
the
do-not-merge/hold
openshift-ci-robot
changed the title
|
@juanluisvaladas: This pull request references Bugzilla bug 1801357, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
openshift-ci-robot
added
the
bugzilla/invalid-bug
|
/bugzilla refresh |
openshift-ci-robot
added
bugzilla/valid-bug
|
@juanluisvaladas: This pull request references Bugzilla bug 1801357, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@danwinship can you approve it? |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship, juanluisvaladas 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 |
openshift-ci-robot
added
the
approved
|
/retest |
|
@juanluisvaladas: All pull requests linked via external trackers have merged. Bugzilla bug 1801357 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Migrate from glide to go modules
It was mandatory to migrate to go modules due to syntax incompatibility
on the sigs.k8s.io-structured-merge-diff module definition. Besides
this incompatibility, it was a matter of time until we found more
problems and glide is abandoned at this stage.
There are a few non obvious things going on:
go.mod: Unnecessary large, however, to avoid changing dependencies
some stuff it was necessary to add some stuff that normally
wouldn't make sense. After the 1.17 rebase it'll be cleaner.
There is a require and replace discrepancy on
github.com/miekg/dns. This is because go mod tidy modifies
the require because one dependency asks for a newer version.
The k8s.io/ packages are replaced beacuse this issue:
cmd/go: go mod -vendor prunes non-package directories golang/go#26366
pkg/dependencymagnet: go mod vendor won't actually pull a module
unless it's required by some go code. We need to explictly
add dependencies on a fake moduleso that we can actually
build.
Removed files: go mod vendor removes unnecessary files, lots of
files have been removed, but those shouldn't be required at
all.
Updated github.com/openshift/library-go: The version we were using
was incompatible with go mod. So I updated to the latest.
Makefile: export GO111MODULE=on library go runs some stuff with
-mod=vendor. Prior to go 1.13 this environment variable
is mandatory for it to work.
Additionally we need to explictly define
GO_BUILD_PACKAGES_EXPANDED for the same reason.
we need to add unxeport GOPATH and export GOPROXY are
necessary due to some odd behavior with go get modernc.org/.
The queries to this domain ultimately get converted to
https://gitlab.com/cznic/ and go tries to run
git clone https://gitlab.com/cznic/*
instead of
git clone https://gitlab.com/cznic/*.git.
For some reason adding these two variables fixes it.
patch directory: We needed to patch a single file in the netlink
library. This is the most similar solution to glide.diff I
could find. It's not elegant and doesn't scale well, but
since we are only patching one file in one library it's
not so terrible.
HACKING.md: This document is now outdated, it wil be updated after
the 1.17 rebase.