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

migrate from dep to go module #2890

Merged
merged 3 commits into from
Jun 4, 2021
Merged

migrate from dep to go module #2890

merged 3 commits into from
Jun 4, 2021

Conversation

fenxiong
Copy link
Contributor

@fenxiong fenxiong commented Jun 4, 2021

Summary

Migrate dependency management from dep to go module.

Implementation details

This is based on #2647 but with the following differences, mainly to reduce the scope of the change:

  • Dependency version is pinned to the one before the migration and no dependency version is changed as part of the change (we should still upgrade them later, just not in this change);
  • The go build tools used in our ci such as staticcheck, gocyclo, goimports, etc. are not migrated to go mod as part of the change. They are not managed by dep before, hence not included in the dep to go mod migration. An exception is mockgen (used to generate mock) which is managed by dep before and as part of this change it is migrated to go mod.

The change consists of three parts. Below describes them in details.

Commit 1. Generate and vendor dependencies with go mod based on the one specified from dep (i.e. Gopkg.toml and Gopkg.lock)

This change is made via doing the following steps on top of dev branch:

Env: go 1.12.17 on a mac (using go 1.12 because agent built with go>=1.13 has issue on windows, the investigation of which is still tbd)

Steps:

  • initialize go.mod (go generates it based on Gopkg.toml and Gopkg.lock):
cd agent
export GO111MODULE=on
go mod init
  • override several dependencies version explicitly in go.mod to avoid changing dependency version, because not all versions auto generated by go match the one specified by dep (the list is constructed based on a comparison between a go.sum generated without version override and the existing Gopkg.lock file for each of the dependency we have. an override is added for each mismatch found between the two files):
replace (
	github.com/Microsoft/go-winio => github.com/Microsoft/go-winio v0.4.7
	github.com/containernetworking/plugins => github.com/containernetworking/plugins v0.8.6
	github.com/coreos/go-systemd => github.com/coreos/go-systemd v0.0.0-20170731111925-d21964639418
	github.com/davecgh/go-spew => github.com/davecgh/go-spew v1.1.0
	github.com/godbus/dbus => github.com/godbus/dbus v4.1.0+incompatible
	github.com/golang/mock => github.com/golang/mock v1.3.1-0.20190508161146-9fa652df1129
	github.com/golang/protobuf => github.com/golang/protobuf v1.2.0
	github.com/jmespath/go-jmespath => github.com/jmespath/go-jmespath v0.0.0-20180206201540-c2b33e8439af
	github.com/konsorten/go-windows-terminal-sequences => github.com/konsorten/go-windows-terminal-sequences v1.0.1
	github.com/pkg/errors => github.com/pkg/errors v0.8.1-0.20170505043639-c605e284fe17
	github.com/prometheus/client_model => github.com/prometheus/client_model v0.0.0-20180712105110-5c3871d89910
	github.com/sirupsen/logrus => github.com/sirupsen/logrus v1.1.1
	github.com/stretchr/testify => github.com/stretchr/testify v1.2.2
	github.com/vishvananda/netlink => github.com/vishvananda/netlink v0.0.0-20170220200719-fe3b5664d23a
	github.com/vishvananda/netns => github.com/vishvananda/netns v0.0.0-20171111001504-be1fbeda1936
	golang.org/x/crypto => golang.org/x/crypto v0.0.0-20171113213409-9f005a07e0d3
	golang.org/x/net => golang.org/x/net v0.0.0-20171107184841-a337091b0525
	golang.org/x/sys => golang.org/x/sys v0.0.0-20171114162044-bf42f188b9bc
	golang.org/x/tools => golang.org/x/tools v0.0.0-20171114152239-bd4635fd2559
)
  • generate vendor folder now that dependency versions are corrected:
go mod tidy
go mod vendor
  • verify no dependency version is changed: compare agent/vendor/modules.txt (generated by go mod vendor, also added in this pr) and the Gopkg.lock (dependency version used by dep) file, go through each dependency, make sure it has same version/commit between the two files.

There are some changes in the vendor folder despite the dependency version did not change. Changes consist of three types: (1) non go source file (e.g. .md file, license file, binary), for some reason dep pulls more of them than go mod vendor; (2) go source file with build tag +ignore, which go mod vendor does not pull; (3) file permission change, go mod vendor ignores execute permission of file being pulled, according to golang/go#34965 (comment) this is expected. My understanding is these changes should not affect the build (otherwise it would be a bug in go).

Commit 2. remove dep files and related make target after migrating to go mod

This part should be obvious.

Commit 3. fix mockgen and version-gen after migrating to go mod.

Two things need to be fixed to get our build process working after migrating to go mod:

  • make mockgen works with go mod: in order to do that we need to vendor mockgen/model package explicitly. this package is required in order to generate code with mockgen, but it's not automatically vendored with the golang/mock dependency. Vendering is done by creating a dummy tools.go file to import it but exclude it in build (with a build tag +build tools) and then rerun go mod tidy and go mod vendor. Using tools.go is the recommended way of managing tool dependency version with go mod cmd/go: clarify best practice for tool dependencies golang/go#25922 (comment).
  • turn off go mod when running version-gen.go. otherwise the build script can fail in container due to no network access (also added the explanation as comments)

Testing

Rely on existing tests.

Description for the changelog

N/A (not changing any agent functionality)

Licensing

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

fenxiong added 3 commits June 4, 2021 11:25
notes:
* dependencies autogenerated by go mod from dep do not always match the previous, so this change explicitly pinned some packages to the specific version that they were at before the migration, and no depedency version is changed as a result;
* there are some changes in the vendor folder despite the dependency version did not change, including: (1) non go source file, for some reason dep pulls more of them than `go mod vendor`; (2) go source file with build tag +ignore, which `go mod vendor` does not pull; (3) file permission change, `go mod vendor` ignores execute permission of file being pulled;
* the go build tools used in our ci such as staticcheck, gocyclo, goimports etc are not migrated to go mod as part of the change, because they are not managed by dep before hence not included in the dep to go mod migration.
* vendor mockgen/model package explicitly. this package is required in order to generate code with mockgen, but it's not automatically vendored with the golang/mock dependency. this commit makes sure the model package is vendored.
* turn off go mod when running version-gen.go. otherwise the build script can fail in container due to no network access.
@fenxiong fenxiong requested a review from a team June 4, 2021 20:44
@fenxiong fenxiong marked this pull request as ready for review June 4, 2021 20:44
@fenxiong fenxiong requested a review from angelcar June 4, 2021 20:44
Copy link
Contributor

@angelcar angelcar left a comment

Choose a reason for hiding this comment

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

\o/

Copy link
Contributor

@sharanyad sharanyad left a comment

Choose a reason for hiding this comment

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

wonder what is causing some of the file perm changes


DEP_VERSION=v0.5.0
.PHONY: get-dep
get-dep: bin/dep
Copy link
Contributor

Choose a reason for hiding this comment

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

👋

@@ -0,0 +1,80 @@
module github.com/aws/amazon-ecs-agent/agent
Copy link
Contributor

Choose a reason for hiding this comment

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

Welcome go mod!!

@fenxiong
Copy link
Contributor Author

fenxiong commented Jun 4, 2021

wonder what is causing some of the file perm changes

golang/go#34965 and the comments in it should explain

@fenxiong fenxiong merged commit 6e34829 into aws:dev Jun 4, 2021
@fenxiong fenxiong deleted the s2r-mod branch June 4, 2021 22:57
@fenxiong fenxiong added this to the 1.52.3 milestone Jun 8, 2021
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.

5 participants