Skip to content

Switch to go modules#5109

Merged
sougou merged 8 commits intovitessio:masterfrom
planetscale:morgo-new-go-modules
Sep 5, 2019
Merged

Switch to go modules#5109
sougou merged 8 commits intovitessio:masterfrom
planetscale:morgo-new-go-modules

Conversation

@morgo
Copy link
Contributor

@morgo morgo commented Aug 17, 2019

This switches to use go modules instead of govendor in bootstrap.sh. Because it modifies bootstrap.sh the results from CI are inconclusive. Here is the result of local test:

$ make docker_bootstrap && make docker_test flavor=mysql57
..
2019/08/17 15:49:22 mysql57.unit                                FLAKY (1/2 failed)
2019/08/17 15:49:22 mysql57.unit_race                           PASS
2019/08/17 15:49:22 mysql57.update_stream                       PASS
2019/08/17 15:49:22 mysql57.update_stream_rbr                   PASS
2019/08/17 15:49:22 mysql57.vertical_split                      PASS
2019/08/17 15:49:22 mysql57.vertical_split_rbr                  PASS
2019/08/17 15:49:22 mysql57.vschema                             PASS
2019/08/17 15:49:22 mysql57.vtctld                              PASS
2019/08/17 15:49:22 mysql57.vtctld_web                          PASS
2019/08/17 15:49:22 mysql57.vtgate_buffer                       PASS
2019/08/17 15:49:22 mysql57.vtgate_utils                        PASS
2019/08/17 15:49:22 mysql57.vtgatev2                            PASS
2019/08/17 15:49:22 mysql57.vtgatev3                            PASS
2019/08/17 15:49:22 mysql57.vttest_sample                       PASS
2019/08/17 15:49:22 mysql57.worker                              PASS
2019/08/17 15:49:22 mysql57.xtrabackup                          PASS
2019/08/17 15:49:22 mysql57.xtrabackup_xbstream                 PASS
2019/08/17 15:49:22 ============================================================
2019/08/17 15:49:22 53 PASSED, 1 FLAKY, 0 FAILED, 0 SKIPPED
2019/08/17 15:49:22 Total time: 2h0m1.1s

It requires a minimum of go 1.11, but we recently bumped the minimum to 1.12 in #5017

Another common issue, is that go modules may be disabled while in the GOPATH. It might be required to move the vitess directory to another location on the system.

There is another small change, in that the go get command in bootstrap.sh has been moved to be with the hooks installation. This helps make a new-style bootstrap very fast (I want to make bootstrap eventually obsolete, but as a stepping stone making it run inline in the CI process should get us there faster). Here is what a python-less java-less CI runner will execute:

morgo@morgox1:~/vitess/src/vitess.io/vitess$ time BUILD_GIT_HOOKS=0 BUILD_JAVA=0 BUILD_PYTHON=0 ./bootstrap.sh 
skipping protoc install. remove /home/morgo/vitess/dist/vt-protoc-3.6.1 to force re-install.
skipping etcd install. remove /home/morgo/vitess/dist/etcd to force re-install.
skipping Consul install. remove /home/morgo/vitess/dist/consul to force re-install.

bootstrap finished - run 'source dev.env' or 'source build.env' in your shell before building.

real	0m0.143s
user	0m0.106s
sys	0m0.061s

Note: We should push new versions of the Docker images after this PR merges.

morgo added 2 commits August 16, 2019 09:18
Signed-off-by: Morgan Tocker <tocker@gmail.com>
Signed-off-by: Morgan Tocker <tocker@gmail.com>
Copy link
Contributor

@dkhenry dkhenry left a comment

Choose a reason for hiding this comment

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

Ok lets do it 👍

@sougou
Copy link
Contributor

sougou commented Aug 30, 2019

Still need to confirm about gotools.

@morgo morgo force-pushed the morgo-new-go-modules branch from 7047042 to d0ad751 Compare August 30, 2019 15:42
@morgo
Copy link
Contributor Author

morgo commented Aug 30, 2019

I'm just running make docker_bootstrap && make docker_test flavor=mysql57 locally, since the CI system can not test bootstrap. I'll paste the results when done.

@morgo
Copy link
Contributor Author

morgo commented Aug 30, 2019

@dweitzman and I chatted on slack - and he had an alternative solution to removing go get: we add a package called tools in tools/tools.go:

// +build tools

package tools

// These imports ensure that "go mod tidy" won't remove deps
// for build-time dependencies like linters and code generators
import (
    _ "github.com/golang/lint/golint"
    _ "github.com/golang/mock/mockgen"
    _ "golang.org/x/tools/cmd/cover"
    _ "golang.org/x/tools/cmd/goimports"
    _ "golang.org/x/tools/cmd/goyacc"
)

The hooks can be modified to call go run github.com/golang/lint/golint instead of golint etc. I like this solution because it versions the tools (and can include new ones easier than making sure someone re-runs bootstrap.sh).

I am going to take a look into this - please don't merge yet :-)

@morgo morgo force-pushed the morgo-new-go-modules branch from d0ad751 to 59e546c Compare August 30, 2019 16:42
Signed-off-by: Morgan Tocker <tocker@gmail.com>
@morgo morgo force-pushed the morgo-new-go-modules branch from 59e546c to 4d8a6a9 Compare August 30, 2019 16:45
Signed-off-by: Morgan Tocker <tocker@gmail.com>
Signed-off-by: Morgan Tocker <tocker@gmail.com>
@dweitzman
Copy link
Collaborator

Another observation: CI/CD builds should have -mod=readonly so they fail if go.sum is out of date rather than silently trusting whatever the tags are in git repos at the moment

Signed-off-by: Morgan Tocker <tocker@gmail.com>
@morgo
Copy link
Contributor Author

morgo commented Sep 4, 2019

Since this PR changes bootstrap.sh, the results of CI are inconclusive. Here is the result of the test-suite run locally:

2019/09/04 08:08:34 ============================================================
2019/09/04 08:08:34 mysql57.backup                              PASS
2019/09/04 08:08:34 mysql57.backup_mysqlctld                    PASS
2019/09/04 08:08:34 mysql57.backup_only                         PASS
2019/09/04 08:08:34 mysql57.backup_transform                    PASS
2019/09/04 08:08:34 mysql57.backup_transform_mysqlctld          PASS
2019/09/04 08:08:34 mysql57.binlog                              PASS
2019/09/04 08:08:34 mysql57.cache_invalidation                  PASS
2019/09/04 08:08:34 mysql57.cell_aliases                        PASS
2019/09/04 08:08:34 mysql57.cell_no_aliases                     PASS
2019/09/04 08:08:34 mysql57.check_make_parser                   PASS
2019/09/04 08:08:34 mysql57.custom_sharding                     PASS
2019/09/04 08:08:34 mysql57.encrypted_replication               PASS
2019/09/04 08:08:34 mysql57.encrypted_transport                 PASS
2019/09/04 08:08:34 mysql57.initial_sharding                    PASS
2019/09/04 08:08:34 mysql57.initial_sharding_bytes              PASS
2019/09/04 08:08:34 mysql57.initial_sharding_multi              PASS
2019/09/04 08:08:34 mysql57.java                                PASS
2019/09/04 08:08:34 mysql57.keyrange                            PASS
2019/09/04 08:08:34 mysql57.keyspace                            PASS
2019/09/04 08:08:34 mysql57.legacy_resharding                   PASS
2019/09/04 08:08:34 mysql57.local_example                       PASS
2019/09/04 08:08:34 mysql57.merge_sharding                      PASS
2019/09/04 08:08:34 mysql57.merge_sharding_bytes                PASS
2019/09/04 08:08:34 mysql57.messaging                           PASS
2019/09/04 08:08:34 mysql57.mysql_server                        PASS
2019/09/04 08:08:34 mysql57.mysqlctl                            PASS
2019/09/04 08:08:34 mysql57.prepared_statement                  PASS
2019/09/04 08:08:34 mysql57.python_client                       PASS
2019/09/04 08:08:34 mysql57.reparent                            PASS
2019/09/04 08:08:34 mysql57.resharding                          PASS
2019/09/04 08:08:34 mysql57.resharding_bytes                    PASS
2019/09/04 08:08:34 mysql57.resharding_rbr                      PASS
2019/09/04 08:08:34 mysql57.schema                              PASS
2019/09/04 08:08:34 mysql57.sharded                             PASS
2019/09/04 08:08:34 mysql57.tabletmanager                       PASS
2019/09/04 08:08:34 mysql57.tabletmanager_consul                PASS
2019/09/04 08:08:34 mysql57.tabletmanager_etcd2                 PASS
2019/09/04 08:08:34 mysql57.unit                                PASS
2019/09/04 08:08:34 mysql57.unit_race                           FLAKY (1/2 failed)
2019/09/04 08:08:34 mysql57.update_stream                       PASS
2019/09/04 08:08:34 mysql57.update_stream_rbr                   PASS
2019/09/04 08:08:34 mysql57.vertical_split                      PASS
2019/09/04 08:08:34 mysql57.vertical_split_rbr                  PASS
2019/09/04 08:08:34 mysql57.vschema                             PASS
2019/09/04 08:08:34 mysql57.vtctld                              PASS
2019/09/04 08:08:34 mysql57.vtctld_web                          PASS
2019/09/04 08:08:34 mysql57.vtgate_buffer                       PASS
2019/09/04 08:08:34 mysql57.vtgate_utils                        PASS
2019/09/04 08:08:34 mysql57.vtgatev2                            PASS
2019/09/04 08:08:34 mysql57.vtgatev3                            PASS
2019/09/04 08:08:34 mysql57.vttest_sample                       PASS
2019/09/04 08:08:34 mysql57.worker                              PASS
2019/09/04 08:08:34 mysql57.xtrabackup                          PASS
2019/09/04 08:08:34 mysql57.xtrabackup_xbstream                 PASS
2019/09/04 08:08:34 ============================================================
2019/09/04 08:08:34 53 PASSED, 1 FLAKY, 0 FAILED, 0 SKIPPED
2019/09/04 08:08:34 Total time: 2h44m33.8s

Signed-off-by: Morgan Tocker <tocker@gmail.com>
@morgo morgo force-pushed the morgo-new-go-modules branch from 33f6291 to fb4eec3 Compare September 4, 2019 15:31
Signed-off-by: Morgan Tocker <tocker@gmail.com>
@morgo morgo force-pushed the morgo-new-go-modules branch from 6ccc75d to 5b111fe Compare September 4, 2019 17:56
@morgo
Copy link
Contributor Author

morgo commented Sep 4, 2019

This PR is now ready for review/merge. Just to clarify on a few suggestions from @dweitzman:

Another observation: CI/CD builds should have -mod=readonly so they fail if go.sum is out of date rather than silently trusting whatever the tags are in git repos at the moment

I will look at doing this in a future PR, as I revisit CI/CD. It makes sense to me.

Should we put go mod download in the Docker images?

I went with yes. This will lead to faster CI builds, at the cost of larger Docker images. The docker module cache on my system is ~1GB -- so it could be quite a bit. We have lite Docker images for now and there are other ways to slim down. I checked with @sougou and while image size increases are regrettable, better CI performance is the right tradeoff for now.

should we set GOPROXY=https://goproxy.io`?

I went for 'no' for now. My thinking process was that we can see how this develops in Go 1.13, and maybe our CI tool will suggest a proxy that we should use that they maintain. The minimum is only 1.12, which we meet. Happy to reconsider if anyone feel strongly on this one.

@sougou sougou merged commit 18e0e98 into vitessio:master Sep 5, 2019
@morgo morgo deleted the morgo-new-go-modules branch September 5, 2019 02:20
@deepthi
Copy link
Collaborator

deepthi commented Sep 5, 2019

"Since this PR changes bootstrap.sh, the results of CI are inconclusive. "
That isn't quite true because the test runner actually copies over the new bootstrap.sh and runs bootstrap if the file changed in a PR.
https://github.com/vitessio/vitess/blob/master/docker/test/run.sh#L181

Build is green so any CI issues with the PR were either transient or due to other changes.

@deepthi
Copy link
Collaborator

deepthi commented Sep 5, 2019

Just catching up to this PR, this is great!
What are the instructions for local builds? Do we delete the vendor dir and re-bootstrap?

@morgo
Copy link
Contributor Author

morgo commented Sep 5, 2019

Just catching up to this PR, this is great!
What are the instructions for local builds? Do we delete the vendor dir and re-bootstrap?

Yes to delete vendor dir. You should not need to re-bootstrap though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants