Switch to Go 1.12 & replace dep by go mod

[sylr]

Follow up of #602.

CC @domgreen

[GiedriusS]

How much effort would it be to switch to 1.12? It brings some new improvements in memory management which may be (very) useful for us: https://golang.org/doc/go1.12#runtime.

[sylr]

@GiedriusS I'll take a look 👍

[sylr]

So I've just downloaded 1.12 and it builds as it with my branch. So I guess we just need to change the golang version in circle-ci.

[bwplotka]

Thanks for this! It's generally LGTM, just:

• I am not entirely sure if everyone is fine with dropping deps, especially if someone imports Thanos (e.g prom-operator): https://kubernetes.slack.com/archives/CFFDS2Z7F/p1551230319073800
• Can we ensure we import exactly the same tags/commit versions? Just to make sure this change will not introduce any changed in logic in our deps. I know it might be tedious.. but worth it.

E.g recent change in minio broke one user's workflow... We might want to slowly upgrade deps using our dependabot integration instead. What do you think?

[sylr]

You mean dropping dep ?

FWIU go mod tries to honour versions specified in the Gopkg.toml (it did so for minio-go). However when it reconciles projects also using go mod it can(does ?) chose to download the version required by the dependency.

FYI information I tried hard to compile Thanos with a soon to be merged prometheus patch (prometheus/prometheus#5009) which will enlarge the dependency tree of projects importing prometheus and I couldn't make it work with dep. Moving to go mod and vendoring made it compile almost at first attempt (just needed to make this patch 5880dbb).

[bwplotka]

@domgreen as per discussion on K8s slack:

[bwplotka]

We need rebase as well

[sylr]

OK I will, should we jump directly to go 1.12 ?

[bwplotka]

We can, I don't think that matters much.

Can we add something to docs? Like this? https://github.com/prometheus/prometheus/blob/master/CONTRIBUTING.md#dependency-management (we can literally copy all)

[domgreen]

How does running make go for you? I get an error on gocheck:

go: launchpad.net/gocheck@v0.0.0-20140225173054-000000000087: bzr branch --use-existing-dir https://launchpad.net/~niemeyer/gocheck/trunk . in /home/domgreen/go1.12/pkg/mod/cache/vcs/f46ce2ae80d31f9b0a29099baa203e3b6d269dace4e5357a2cf74bd109e13339: exec: "bzr": executable file not found in $PATH

[domgreen]

As above ... need to think more about making people to install bzr

This might be needed ... in discussions.

[domgreen]

After talking with the amazing @myitcv it turns out that with mod the requirements graph that is generated means that the transitive dependencies need to be pulled from the VCS of the dependency so for our case we are seeing that there is a dependency that needs to be pulled via bzr ... normally the case is that we need git installed to pull most dependencies.

It turns out that the dependency in question is actually a dep of Prometheus and therefore doing the same go mod vendor in Prometheus also breaks.

Lets add this to our documentation that if you see this error you will need to install bzr.

[sylr]

@domgreen

sylvain@ubuntu-1604-dev:~/gopath/src/github.com/improbable-eng/thanos[go-1.11]$ GO111MODULE=on go mod why launchpad.net/gocheck
# launchpad.net/gocheck
github.com/improbable-eng/thanos/pkg/cluster
github.com/hashicorp/memberlist
github.com/hashicorp/go-msgpack/codec
github.com/hashicorp/go-msgpack/codec.test
labix.org/v2/mgo/bson
labix.org/v2/mgo/bson.test
launchpad.net/gocheck

sylvain@ubuntu-1604-dev:~/gopath/src/github.com/prometheus/prometheus[master]$ GO111MODULE=on go mod why launchpad.net/gocheck
go: finding google.golang.org/grpc/transport latest
# launchpad.net/gocheck
github.com/prometheus/prometheus/discovery/consul
github.com/hashicorp/consul/api
github.com/hashicorp/consul/api.test
github.com/hashicorp/serf/serf
github.com/hashicorp/go-msgpack/codec
github.com/hashicorp/go-msgpack/codec.test
labix.org/v2/mgo/bson
labix.org/v2/mgo/bson.test
launchpad.net/gocheck

[domgreen]

Can we add a check in make that the user has bzr installed and tell them to install it for deps if not as well as in the contributing docs.

[domgreen]

👍
Only changes that I would like:

• run make to ensure all formatting and deps are up to date (locally I had a file with formatting changes)
• check for bzr install prior to doing mods
• add info to CONTRIBUTING about need of bzr
• remove make dep and make vendor
• go mod tidy and fail build in CI if git diff is not as expected.

[domgreen]

Another pro tip from @myitcv is that we should do go mod tidy before we do go mod vendor this will ensure that we have a minimal go.{mod|sum} when we vendor the results. This needs to be done in the main Thanos package and the benchmark package 👍

Will update the request for change above to reflect this.

[myitcv]

Whether or not you choose to commit a vendor directory, in your CI run you should go mod tidy for all the modules in the repo to ensure the respective go.{mod,sum} files are complete and minimal. The final step in your CI run should then be something like test -z "$(git status --porcelain)" || (git status; git diff; false) to fail the build if any files in the working tree differ from the commit being CI-ed (this also catches untracked files). Else you would have no guarantee that the committed go.{mod,sum} (or any other files for that matter) are identical to the CI's run of go mod tidy.

[domgreen]

Thanks, @myitcv good to have an experienced go modder 🤘 take a look over this.

@sylr have just checked locally we can replace the need to use go mod vendor and rm -rf vendor we are not checking it in so let's not use go mod vendor.

So can we refactor the dep & vendor functions into a mod function that checks what Paul mentions above in CI.

Locally I think make just needs to ensure it calls go mod tidy before we commit.

[sylr]

@myitcv I've just one question about go mod vendor, if all the libs are already in the $GOPATH it will still ensure that the checked out version in $GOPATH are the ones defined in go.mod before vendoring them ?

[sylr]

@domgreen I prefer to keep go mod vendor as it does not mess with what you already have in your $GOPATH.

[myitcv]

@sylr

I've just one question about go mod vendor, if all the libs are already in the $GOPATH it will still ensure that the checked out version in $GOPATH are the ones defined in go.mod before vendoring them ?

Modules neither touches nor uses GOPATH/src at all (unless you specifically add a directory-based replace directive at a path that happens to be within GOPATH/src; that is not the case here).

Modules use the read-only modules cache (which happens to be located at GOPATH/pkg/mod, but that's an implementation detail).

go mod vendor is documented as follows:

Vendor resets the main module's vendor directory to include all packages needed to build and test all the main module's packages. It does not include test code for vendored packages.

So go mod vendor also does not touch your GOPATH/src; it simply updates the vendor directory of your main module accordingly.

I prefer to keep go mod vendor as it does not mess with what you already have in your $GOPATH.

Can you explain what you think go mod vendor is doing? Because as I explained above, it will not touch your GOPATH/src at all.

[domgreen]

After removing the go mod vendor and experimenting locally it does not have any impact to our dev workflow ... all I think we need from a go mod point of view is go mod tidy as @myitcv points out earlier in the thread.
We do not check in /vendor so should not need to put anything into that folder, if for a better dev experience down the line we wish to iterate on this idea lets wait till we have a concrete use case.

[sylr]

@myitcv All right, it seems I've been really misunderstanding the internals of go mod. I'll clear all go mod vendor then.

[domgreen]

Few nits but nearly there... doesnt matter if it takes a while longer we need to ensure we keep the quality of the project high :)

Also, did you run make locally? I'm still getting modifications to main_test.go after running make.

[domgreen]

Amazing thanks for the hard work.

I rm -rf $GOPATH and ran make and all worked perfectly. 👍

[sevagh]

The latest release of Thanos is still built with go 1.11 I think:

$ ./thanos --version
thanos, version 0.4.0 (branch: HEAD, revision: a676095877e7a87576b0f7776fccd4b3f8b65188)
  build user:       root@3e7111579ad1
  build date:       20190504-09:31:11
  go version:       go1.11.9

Does the CI system need to change?