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.

Sign up for GitHub

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

Jump to bottom

Move from dep to go modules #1303

Merged
merged 17 commits into from
May 9, 2019
Merged

Move from dep to go modules #1303

merged 17 commits into from
May 9, 2019

Conversation

daixiang0
Copy link
Contributor

@daixiang0 daixiang0 commented Mar 27, 2019 •
edited
Loading

  • add go module config
  • update Makefile
  • update README
  • update CI config
  • update test tool

@daixiang0
update vendor
9086dc9 
Signed-off-by: Xiang Dai <764524258@qq.com>
@daixiang0
move to module
f06843a 
Signed-off-by: Xiang Dai <764524258@qq.com>
@daixiang0
update Makefile
b60d2e9 
Signed-off-by: Xiang Dai <764524258@qq.com>
@bboreham
Copy link
Contributor

Same comment as I made on #1232: please note the libraries that are updated in this PR, and any interesting changes in behaviour.

@daixiang0
Copy link
Contributor Author

daixiang0 commented Mar 27, 2019 •
edited
Loading

@bboreham hi, take an example that why go-grpc-middleware is mentioned and grpc-gateway is not:

# GO111MODULE=on go mod why github.com/grpc-ecosystem/grpc-gateway
# github.com/grpc-ecosystem/grpc-gateway
(main module does not need package github.com/grpc-ecosystem/grpc-gateway)
# GO111MODULE=on go mod why github.com/grpc-ecosystem/go-grpc-middleware
# github.com/grpc-ecosystem/go-grpc-middleware
github.com/cortexproject/cortex/pkg/util/grpcclient
github.com/grpc-ecosystem/go-grpc-middleware

use go mod why easily to explain.

Also, grpc-gateway is in go.sum indeed:

# git grep grpc-gateway
go.sum:142:github.com/grpc-ecosystem/grpc-gateway v0.0.0-20190311121628-6523154b0128/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY=
go.sum:143:github.com/grpc-ecosystem/grpc-gateway v1.5.0/go.mod h1:RSKVYQBd5MCa4OVpNdGskqpgL2+G+NZTnrVHpWWfpdw=
go.sum:144:github.com/grpc-ecosystem/grpc-gateway v1.6.3 h1:oQ+8y59SMDn8Ita1Sh4f94XCUVp8AB84sppXP8Qgiow=
go.sum:145:github.com/grpc-ecosystem/grpc-gateway v1.6.3/go.mod h1:RSKVYQBd5MCa4OVpNdGskqpgL2+G+NZTnrVHpWWfpdw=
......

If we depend on one project A, but A does not have go.mod, the dependence of A would be auto found by go mod tidy refer to doc:

If a dependency of your module does not itself have a go.mod (e.g., because the dependency has not yet opted in to modules itself), or if its go.mod file is missing one or more of its dependencies (e.g., because the module author did not run go mod tidy), then the missing transitive dependencies will be added to your module's requirements, along with an // indirect comment to indicate that the dependency is not from a direct import within your module.

@daixiang0
Copy link
Contributor Author

BTW, for go mod, no need vendor. Keep vendor and dep config to support dep and in future, they would be removed if not support dep.

@bboreham
Copy link
Contributor

Thank you for answering the subordinate question.

For clarity, my main request is for a list of the changes to dependency versions, and for some analysis of what is different. This is to guide what we should watch out for when updating.

@daixiang0
Copy link
Contributor Author

daixiang0 commented Mar 28, 2019 •
edited
Loading

i compare Gopkg.lock and go.mod to get following list:
since different rule between dep and go mod, for old version, if no version in Gopkg.lock, i use revision
@bboreham you can double check it.

|name | old | new | | --- | --- | --- | |cloud.google.com/go|ed41f43dafb848a06a7136373760f3c09326fb44|v0.28.0| |contrib.go.opencensus.io/exporter/ocagent|v0.2.0|-| |github.com/Azure/azure-sdk-for-go|a1a2da0aba294fe51ba47119e652ff2f08a78afb|-| |github.com/Azure/go-autorest|v11.5.1|-| |github.com/Masterminds/squirrel|20f192218cf52a73397fa2df45bdda720f3e47c8|v0.0.0-20161115235646-20f192218cf5| |github.com/NYTimes/gziphandler|v1.1.1|v0.0.0-20190221231647-dd0439581c76| |github.com/alecthomas/units|2efee857e7cfd4f3d0138cc3cbb1b4966962b93a|-| |github.com/armon/go-socks5|e75332964ef517daa070d7c38a9466a0d687e0a5|v0.0.0-20160902184237-e75332964ef5| |github.com/aws/aws-sdk-go|v1.15.90|v0.0.0-20181204211515-ddc06f9fad88| |github.com/beorn7/perks|4c0e84591b9aa9e6dcfdf3e020114cd81f89d5f9|-| |github.com/blang/semver|v3.5.0|v0.0.0-20170130170546-b38d23b8782a| |github.com/bradfitz/gomemcache|1952afaa557dc08e8e0d89eafab110fb501c1a2b|v0.0.0-20170208213004-1952afaa557d| |github.com/cenkalti/backoff|v1.0.0|v0.0.0-20160321131554-32cd0c5b3aef // indirect| |github.com/census-instrumentation/opencensus-proto|v0.1.0|-| |github.com/cespare/xxhash|v1.0.0|-| |github.com/codahale/hdrhistogram|3a0bb77429bd3a61596f5e8a3172445844342120|-| |github.com/davecgh/go-spew|v1.1.0|-| |github.com/dgrijalva/jwt-go|v3.2.0|v0.0.0-20180308231308-06ea1031745c // indirect| |github.com/etcd-io/bbolt|v1.3.1-etcd.8|v0.0.0-20180912205654-7ee3ded59d48| |github.com/fluent/fluent-logger-golang|v1.2.1|v0.0.0-20161018015219-28bdb662295c // indirect| |github.com/fsouza/fake-gcs-server|v1.3.0|v0.0.0-20180925145035-9f9efdc50d6a| |github.com/go-kit/kit|v0.5.0|v0.8.0| |github.com/go-logfmt/logfmt|v0.3.0|-| |github.com/go-stack/stack|v1.6.0|-| |github.com/gocql/gocql|697e7c57f99bb127606c4d4f975c2ff4f34eca1c|v0.0.0-20180113133114-697e7c57f99b| |github.com/gogo/googleapis|v1.1.0|v0.0.0-20180813112041-8558fb44d2f1 // indirect| |github.com/gogo/protobuf|v1.1.1|v1.2.0| |github.com/gogo/status|v1.0.3|v0.0.0-20180521094753-d60b5acac426| |github.com/golang/protobuf|v1.2.0|v1.2.0| |github.com/golang/snappy|553a641470496b2327abcac10b36396bd98e45c9|v0.0.0-20170215233205-553a64147049| |github.com/google/btree|e89373fe6b4a7413d7acd6da1725b83ef713e6e4|-| |github.com/google/gofuzz|44d81051d367757e1c7c6a5a86423ece9afcf63c|v0.0.0-20161122191042-44d81051d367 // indirect| |github.com/googleapis/gax-go|v2.0.3|-| |github.com/googleapis/gnostic|v0.2.0|-| |github.com/gophercloud/gophercloud|fe1ba5ce12dda9d3056d303dfe080d5eccdd1780|v0.0.0-20190307220656-fe1ba5ce12dd // indirect| |github.com/gorilla/context|v1.1|v0.0.0-20160226214623-1ea25387ff6f // indirect| |github.com/gorilla/mux|v1.6.2|v1.6.2| |github.com/gregjones/httpcache|9cad4c3443a7200dd6400aef47183728de563a38|-| |github.com/grpc-ecosystem/go-grpc-middleware|v1.0.0|v0.0.0-20180502091642-c250d6563d4d| |github.com/grpc-ecosystem/grpc-gateway|v1.8.3|-| |github.com/hailocab/go-hostpool|e80d13ce29ede4452c43dea11e79b9bc8a15b478|v0.0.0-20160125115350-e80d13ce29ed // indirect| |github.com/hashicorp/consul|v1.1.0|v0.0.0-20180615161029-bed22a81e9fd| |github.com/hashicorp/go-cleanhttp|3573b8b52aa7b37b9358d966a898feb387f62437|v0.0.0-20170211013415-3573b8b52aa7| |github.com/hashicorp/go-rootcerts|6bb64b370b90e7ef1fa532be9e591a81c3493e00|-| |github.com/hashicorp/golang-lru|0fb14efe8c47ae851c0034ed7a448854d3d34cf3|-| |github.com/hashicorp/serf|v0.8.1|v0.0.0-20170206200529-d6574a5bb122 // indirect| |github.com/jmespath/go-jmespath|0b12d6b5|-| |github.com/jonboulle/clockwork|v0.1.0|v0.0.0-20160615175015-2eee05ed7941| |github.com/json-iterator/go|1.1.5|v0.0.0-20180806060727-1624edc4454b| |github.com/julienschmidt/httprouter|v1.1|-| |github.com/kr/logfmt|b84e30acd515aadc4b783ad4ff83aff3299bdfe0|-| |github.com/lann/builder|f22ce00fd9394014049dad11c244859432bd6820|v0.0.0-20150808151131-f22ce00fd939 // indirect| |github.com/lann/ps|62de8c46ede02a7675c4c79c84883eb164cb71e3|v0.0.0-20150810152359-62de8c46ede0 // indirect| |github.com/lib/pq|2704adc878c21e1329f46f6e56a1c387d788ff94|v1.0.0| |github.com/mattes/migrate|v1.3.1|v0.0.0-20170420185049-7dde43471463| |github.com/mattn/go-colorable|v0.0.7|v0.0.0-20161103160040-d22884950486 // indirect| |github.com/mattn/go-isatty|v0.0.2|v0.0.0-20170322234413-fc9e8d8ef484 // indirect| |github.com/mgutz/ansi|9520e82c474b0a04dd04f8a40959027271bab992|v0.0.0-20170206155736-9520e82c474b| |github.com/miekg/dns|6ebcb714d36901126ee2807031543b38c56de963|-| |github.com/mitchellh/go-homedir|b8bc1bf767474819792c23f32d8286a45736f1c6|-| |github.com/modern-go/concurrent|1.0.3|-| |github.com/modern-go/reflect2|1.0.1|-| |github.com/mwitkow/go-conntrack|cc309e4a22231782e8893f3c35ced0967807a33e|-| |github.com/oklog/oklog|v0.2.2|-| |github.com/oklog/ulid|v0.3.0|-| |github.com/opentracing-contrib/go-grpc|4b5a12d3ff02ba61ae861b7797e17a0c4f0ecea9|v0.0.0-20180928155321-4b5a12d3ff02| |github.com/opentracing-contrib/go-stdlib|1de4cc2120e71f745a5810488bf64b29b6d7d9f6|v0.0.0-20170113013457-1de4cc2120e7| |github.com/opentracing/opentracing-go|v1.0.2|v1.0.1| |github.com/petar/GoLLRB|53be0d36a84c2a886ca057d34b6aa4468df9ccb4|v0.0.0-20130427215148-53be0d36a84c // indirect| |github.com/peterbourgon/diskv|v2.0.1|-| |github.com/philhofer/fwd|98c11a7a6ec829d672b03833c3d69a7fae1ca972|v0.0.0-20160129035939-98c11a7a6ec8 // indirect| |github.com/pkg/errors|v0.8.0|v0.8.0| |github.com/pmezard/go-difflib|v1.0.0|-| |github.com/prometheus/alertmanager|fb713f6d8239b57c646cae30f78e8b4b8861a1aa|v0.0.0-20180112102915-fb713f6d8239| |github.com/prometheus/client_golang|bcbbc08eb2ddff3af83bbf11e7ec13b4fd730b6e|v0.9.1| |github.com/prometheus/client_model|6f3806018612930941127f2a7c6c453ba2c527d2|-| |github.com/prometheus/common|7600349dcfe1abd18d72d3a1770870d9800a7801|v0.0.0-20181119215939-b36ad289a3ea| |github.com/prometheus/procfs|6ac8c5d890d415025dd5aae7595bcb2a6e7e2fad|-| |github.com/prometheus/prometheus|59369491cfdfe8dcb325723d6d28a837887a07b9|v0.0.0-20190312040920-59369491cfdf| |github.com/prometheus/tsdb|v0.6.1|v0.6.1| |github.com/samuel/go-zookeeper|1d7be4effb13d2d908342d349d71a284a7542693|-| |github.com/satori/go.uuid|v1.1.0|-| |github.com/segmentio/fasthash|a72b379d632eab4b49e4f4b2c765cfebf0a74796|v0.0.0-20180216231524-a72b379d632e| |github.com/sercand/kuberesolver|v1.0.0|v0.0.0-20171128105722-aa801ca26294 // indirect| |github.com/sirupsen/logrus|v1.0.6|v0.0.0-20180721070001-3e01752db018 // indirect| |github.com/stretchr/testify|v1.1.4|v1.3.0| |github.com/tinylib/msgp|v1.0|v0.0.0-20161221055906-38a6f61a768d // indirect| |github.com/uber/jaeger-client-go|v2.14.0|v0.0.0-20180430180415-b043381d9447| |github.com/uber/jaeger-lib|v1.5.0|v0.0.0-20180511153330-ed3a127ec5fe // indirect| |github.com/weaveworks/billing-client|be0d55e547b147ea1817f037cab9458bf7fc7850|v0.0.0-20171006123215-be0d55e547b1| |github.com/weaveworks/common|c1808abf9c462ba088ef5c764053a316a58cde24|v0.0.0-20181109173936-c1808abf9c46| |github.com/weaveworks/mesh|5015f896ab62d3e9fe757456c757521ce0c3faff|v0.0.0-20170131170447-5015f896ab62| |github.com/weaveworks/promrus|v1.2.0|v0.0.0-20171010152908-0599d764e054 // indirect| |go.opencensus.io|v0.18.0|-| |golang.org/x/crypto|c7af5bf2638a1164f2eb5467c39c6cffbd13a02e|-| |golang.org/x/net|adae6a3d119ae4890b46832a2e88a95adc62b8e7|v0.0.0-20181220203305-927f97764cc3| |golang.org/x/oauth2|d668ce993890a79bda886613ee587a69dd5da7a6|v0.0.0-20181203162652-d668ce993890 // indirect| |golang.org/x/sync|f52d1811a62927559de87708c8913c1650ce4f26|-| |golang.org/x/sys|ec83556a53fe16b65c452a104ea9d1e86a671852|v0.0.0-20190209173611-3b5209105503| |golang.org/x/text|a9a820217f98f7c8a207ec1e45a874e1fe12c478|-| |golang.org/x/time|8be79e1e0910c292df4e79c241bb7e8f7e725959|v0.0.0-20170424234030-8be79e1e0910| |golang.org/x/tools|4bb9a6d30bdc727aebd0747694f31de632a5282e|v0.0.0-20190114222345-bf090417da8b| |google.golang.org/api|6142e720c068c6cd71f2258e007ff1991572e1d5|v0.0.0-20181203233308-6142e720c068| |google.golang.org/appengine|v1.0.0|-| |google.golang.org/genproto|2b5a72b8730b0b16380010cfe5286c42108d88e7|| |google.golang.org/grpc|v1.16.0|v1.19.0| |gopkg.in/check.v1|-|v1.0.0-20180628173108-788fd7840127 // indirect| |gopkg.in/fsnotify/fsnotify.v1|v1.4.7|-| |gopkg.in/inf.v0|v0.9.0|-| |gopkg.in/yaml.v2|v2.2.1|v2.2.2| |k8s.io/api|05914d821849570fba9eacfb29466f2d8d3cd229|-| |k8s.io/apimachinery|2b1284ed4c93a43499e781493253e2ac5959c4fd|-| |k8s.io/client-go|a47917edff34c2c3f7be36398e3ebad6011ce05c|-| |k8s.io/klog|71442cd4037d612096940ceb0f3fec3f7fff66e0|-| |k8s.io/kube-openapi| - | v0.0.0-20180629012420-d83b052f768a| |labix.org/v2/mgo |-| v0.0.0-20140701140051-000000000287| |launchpad.net/gocheck |-|v0.0.0-20140225173054-000000000087| |sigs.k8s.io/yaml |-| v1.1.0|

@bboreham
Copy link
Contributor

Having a list is a good step, but before I deploy in production I like to know what behaviour has changed.

I.e., scan the release notes of each dependency and flag anything that may be relevant.

You’re right it’s a big list. Maybe we can divide and conquer?

@daixiang0
Copy link
Contributor Author

daixiang0 commented Apr 1, 2019 •
edited
Loading

Having a list is a good step, but before I deploy in production I like to know what behaviour has changed.

It is hard to cover all cases, while now all tests pass.
Too many projects base on this, from my side, get to know what behaviour has changed is very tough.

You’re right it’s a big list. Maybe we can divide and conquer?

Agree, which part does we begin?

@bboreham
Copy link
Contributor

bboreham commented Apr 1, 2019

Start at the top of the list?
Maybe call out “I will do the first 5” to avoid duplication?

@bboreham
Copy link
Contributor

bboreham commented Apr 1, 2019

I'll start:

First line - cloud.google.com/go was ed41f43dafb8 now v0.28.0 - I can't find where Google publishes release notes for this library. Punt and come back.

Second line - contrib.go.opencensus.io/exporter/ocagent was v0.2.0 now "-". But the files are not deleted from vendor/contrib.go.opencensus.io by this PR, they are updated. For instance https://github.com/cortexproject/cortex/pull/1303/files#diff-1151b378d60e59d0ef038ae0f47bcf9dR4
Is this an error in your method for listing changes?
Same for the next two lines.

github.com/Masterminds/squirrel was 20f192218cf5 now v0.0.0-20161115235646-20f192218cf5 - this is actually the same commit. Could make this clearer in the list.

@daixiang0
Copy link
Contributor Author

daixiang0 commented Apr 3, 2019 •
edited
Loading

Is this an error in your method for listing changes?

refer this comment, i only compare between Gopkg.lock and go.mod, not go.sum or vender, as i said in the beginning of comment.

github.com/Masterminds/squirrel was 20f192218cf5 now v0.0.0-20161115235646-20f192218cf5 - this is actually the same commit. Could make this clearer in the list.

It is difference rule between go dep and go mod, let us update list during check :)

@daixiang0
Copy link
Contributor Author

daixiang0 commented Apr 3, 2019 •
edited
Loading

@bboreham hi, edit list here is too bad, i create a google docs, you can edit it.

For vendor change, i will remove commit about vendor so that all commits only relate to go mod not go dep. WDYT?

@daixiang0
Copy link
Contributor Author

@bboreham hi, any update? The list i has finished.

@bboreham
Copy link
Contributor

bboreham commented Apr 8, 2019

My objective is to know what changed, before I have to support a change in production.

The size of diff in this PR has come down from +900,000 to +100,000, which is good, but still I do not see a change in your table that matches this change in the code:
https://github.com/cortexproject/cortex/pull/1303/files#diff-d2af11b35109d03a552583a9464bdf56R107

I appreciate you don't know either, I'm not saying this is bad. I am saying I can't approve something I don't understand.

@daixiang0
Copy link
Contributor Author

daixiang0 commented Apr 9, 2019 •
edited
Loading

@bboreham thanks for response, i have remove changesd about vendor.

@daixiang0
Copy link
Contributor Author

I get more knowledge in this process, and i find that my env not clean.
I run in a container to ensure clean env then update mod, the list also has been updated.

@daixiang0
update CI config
7863579 
Signed-off-by: Xiang Dai <764524258@qq.com>
@daixiang0
update test tool
ac682ba 
Signed-off-by: Xiang Dai <764524258@qq.com>
@daixiang0
Update go.mod and go.sum with go mod tidy
14ecf87 
Signed-off-by: Xiang Dai <764524258@qq.com>
@daixiang0
Revert "update vendor"
2935f3d 
This reverts commit 9086dc9.

Signed-off-by: Xiang Dai <764524258@qq.com>
@daixiang0
update module file
93ef338 
Signed-off-by: Xiang Dai <764524258@qq.com>
@daixiang0
Update README
41c6063 
Signed-off-by: Xiang Dai <764524258@qq.com>
@gouthamve gouthamve force-pushed the mod branch 3 times, most recently from fb8b0bf to 4b85eda Compare May 6, 2019 13:48
@gouthamve
Merge branch 'master' into mod
88db4e8 
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@daixiang0 daixiang0 force-pushed the mod branch 3 times, most recently from 09bdfa0 to a39ec5e Compare May 7, 2019 02:37
@gouthamve
fix go.sum
9103d01 
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
@gouthamve
Nuke the final dep bits
ed4ec98 
Signed-off-by: Goutham Veeramachaneni <gouthamve@gmail.com>
tomwilkie
tomwilkie reviewed May 7, 2019
View reviewed changes
README.md Outdated Show resolved Hide resolved
@tomwilkie
Copy link
Contributor

Thanks for the PR @daixiang0! @gouthamve is going to do a build of this and stick it in our dev env to test things out. If all is well, we should be able to get this merged.

@tomwilkie tomwilkie mentioned this pull request May 7, 2019
Closed
@daixiang0
fix language issue
d42a235 
Signed-off-by: Xiang Dai <764524258@qq.com>
@tomwilkie
Copy link
Contributor

FYI I tried deleing vendor and running go mod vendor, and there were no git changes. Good work!

gouthamve
gouthamve reviewed May 7, 2019
View reviewed changes
Makefile
@@ -137,8 +137,12 @@ shell:
configs-integration-test:
/bin/bash -c "go test -tags 'netgo integration' -timeout 30s ./pkg/configs/... ./pkg/ruler/..."

dep-check:
dep check
mod-check:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tomwilkie We make sure there are no changes in CI too :)

bboreham
bboreham reviewed May 7, 2019
View reviewed changes
Copy link
Contributor

@bboreham bboreham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you try it without changing the tools subtree ?

tools/test Outdated Show resolved Hide resolved
tools/test Outdated Show resolved Hide resolved
@bboreham
Copy link
Contributor

bboreham commented May 7, 2019

Has anyone done a forwards/backwards compatibility check on the new protobuf library?
See also #1370

@daixiang0
Revert "update test tool"
408e409 
This reverts commit ac682ba.

Signed-off-by: Xiang Dai <764524258@qq.com>
@tomwilkie
Copy link
Contributor

Has anyone done a forwards/backwards compatibility check on the new protobuf library?

We've done 3 rolling upgrades (two dev envs and one prod env) and didn't hit any problems.

I can't see anything that might indicate a compatibility change - and thats kinda the whole point of protobufs.

@tomwilkie
Copy link
Contributor

LGTM

@tomwilkie tomwilkie merged commit e2154c4 into cortexproject:master May 9, 2019
@bboreham
Copy link
Contributor

bboreham commented May 9, 2019

thats kinda the whole point of protobufs

strangely this did not save us in #938

@vtolstov vtolstov mentioned this pull request May 9, 2019
Closed
@daixiang0 daixiang0 deleted the mod branch May 11, 2019 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomwilkie tomwilkie tomwilkie left review comments

@gouthamve gouthamve gouthamve approved these changes

@bboreham bboreham bboreham left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@daixiang0 @bboreham @weeco @gouthamve @tomwilkie