🏃 ⚠ Support for Go modules

[andreykaipov]

This PR adds support for Go modules, both from a development and consumer point of view.

Development still works from within the GOPATH, but GO111MODULE=on must be set when running tests.

In addition, the CRD generator can now be run from outside of the GOPATH, so long as the project it's running against specifies the repo/module name in a PROJECT file.

I've also replaced gometalinter with golangci-lint in the test script as the former is deprecated and doesn't work with Go modules, while the latter does and has feature parity.

To make the review less of a headache, I've left the re-vendoring in a separate commit.

Please let me know if I missed anything!

[andreykaipov]

/assign @DirectXMan12

[droot]

/assign @Mengqi

[k8s-ci-robot]

@droot: GitHub didn't allow me to assign the following users: mengqi.

Note that only kubernetes-sigs members and repo collaborators can be assigned and that issues/PRs can only have 10 assignees at the same time.
For more information please see the contributor guide

In response to this:

/assign @Mengqi

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.

[droot]

@andreykaipov I am not much familiar with GO modules, will the existing kubebuilder projects (which uses dep and Gopkg.toml etc.) will continue to work after this change ?

[andreykaipov]

Yup! The only commit in this PR that changes the logic of a generator is 626472c.

Instead of validating the project to be under the GOPATH, it validates a PROJECT file exists. Then, instead of using ./pkg/apis as the import path, it uses <repo-name>/pkg/apis. This works both outside and inside of the GOPATH. The repo name is read from the PROJECT file, and Kubebuilder guarantees that field exists when it trims the GOPATH from the working directory (see https://github.com/kubernetes-sigs/kubebuilder/blob/master/pkg/scaffold/project/project.go#L41).

The goal after this is to get Kubebuilder to use Go modules too (kubernetes-sigs/kubebuilder#506) 😄

[mengqiy]

@andreykaipov IIUC it require GO111MODULE=on to be set to make controller-gen to work for an existing project, right? If so, how do you make it backward-compatible?

[andreykaipov]

Setting GO111MODULE=on is only required for testing this project from within the GOPATH since the build scripts now use -mod=vendor (02e1d02). It's not required for just running controller-gen though.

The change is backwards compatible as long as users of Kubebuilder do not delete the generated PROJECT file. That's where controller-gen is now discovering the package name instead of using the DirToGoPkg function.

[ghost]

Go modules plans on replacing dep entirely, is there need to be backwards compatible there? Most kubernetes projects now have tags to map to kubernetes versions, would it be possible to integrate these mappings, eg. kubernetes-1.13 to allow us to chose target versions where required, or would this throw a spanner in the works?

[mengqiy]

Go modules plans on replacing dep entirely, is there need to be backwards compatible there?

controller-tools follows semver and is at v0.1 ATM. If the changes is not backwards compatible, we need to bump the version to v0.2.

[andreykaipov]

So here's how I'm confirming the changes are backwards compatible. We'll create a new project with Kubebuilder, and then manually apply the changes from 626472c to the respective files in our vendor folder.

Create a new project, run dep ensure, and then create a new API to simulate an existing project.

$ mkdir -p $GOPATH/src/hello
$ cd $GOPATH/src/hello
$ kubebuilder init --dep
$ kubebuilder create api --group mygroup --version v1 --kind Abc < <(printf 'y\ny\n')
$ ls config/crds/
mygroup_v1_abc.yaml

Now manually apply the changes from 626472c to vendor/sigs.k8s.io/controller-tools/pkg/crd/generator/generator.go and vendor/sigs.k8s.io/controller-tools/pkg/crd/util/ulti.go.

Confirm controller-gen still works by creating a new API.

$ kubebuilder create api --group mygroup --version v1 --kind Xyz < <(printf 'y\ny\n')
$ ls config/crds/
mygroup_v1_abc.yaml     mygroup_v1_xyz.yaml

Alternatively, run controller-gen directly:

$ make manifests
go run vendor/sigs.k8s.io/controller-tools/cmd/controller-gen/main.go all
CRD manifests generated under '$GOPATH/src/hello/config/crds'
RBAC manifests generated under '$GOPATH/src/hello/config/rbac'

[mengqiy]

@andreykaipov It's not a breaking change from user perspective. That's great.

My only concern now is: if I'm a contributor to controller-tools, I'm in GOPATH and haven't yet set GO111MODULE=on, is it easy to discover that GO111MODULE=on needs to be set? If not, we may want to document it somewhere.

[andreykaipov]

@mengqiy That's a good idea. I can add a Development section to the README explaining that.

Edit: okiedoke - added it in 89d33b3. I think Go modules will become the default in Go 1.13 (see golang/go#27227), so we could probably remove the note once that happens. 😄

[mengqiy]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

Approval requirements bypassed by manually added approval.

This pull-request has been approved by: andreykaipov

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:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[DirectXMan12]

Hold the phone. People vendoring this with dep won't get dependency info out of this any more. I think we have to maintain a Gopkg.toml for a bit. At least until Go 1.13.

[DirectXMan12]

We also have to make sure they stay up to date.

[andreykaipov]

Sorry, what do you mean by not being able to get the dependency info if vendoring with dep? Taking the Kubebuilder project as an example, I'm able to run a dep ensure -update sigs.k8s.io/controller-tools and have it vendor the latest master successfully, so any consumers of the library using dep should still be able to do so. Maybe I'm overlooking something? 😬

[mengqiy]

This issue may happen if the users don't use kubebuilder, but use controller-runtime and controller-tools directly.