⚠️ Bump to Go 1.13

[joelanford]

This PR builds on #598 by completing more of the tasks listed in #590:

• Prune any uses of xerrors (8932099)
• Use %w instead of %v in Errorf (fc42a0a)
• Prune uses of GO111MODULE=on from our scripts (e0a1fbb)

(note: we already did a soft-bump to 1.13 the previous release, this just starts using the features)

[k8s-ci-robot]

Hi @joelanford. Thanks for your PR.

I'm waiting for a kubernetes-sigs or kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

[alvaroaleman]

/ok-to-test

In order for the tests to use go 1.13, the image needs to be bumped here: https://github.com/kubernetes/test-infra/blob/e0a1e8e0d0f947087e7b8295f49b21d772f7a070/config/jobs/kubernetes-sigs/controller-runtime/controller-runtime-presubmits.yaml#L13

[DirectXMan12]

(removed my at-mention from the PR description -- that causes all sorts of issues when it merges)

[rajathagasthya]

I know this isn't changing current behavior, but maybe this tool should be installed with GO111MODULE=off? The current version will add this as a dependency to go.mod cluttering the file. We don't really want to carry a lint tool as a dependency and someone might accidentally commit it.

Related golang/go#30515

[joelanford]

I agree that it isn't great to clutter and inadvertantly change go.mod. The GO111MODULE=off approach doesn't work though:

$ ./hack/check-everything.sh
using tools
Checking for golangci-lint
Installing golangci-lint
go: cannot use path@version syntax in GOPATH mode

Instead, I updated ./hack/check-everything.sh to use the curl/goreleaser installation method that's used elsewhere. Was there some reason we weren't using that here?

[rajathagasthya]

The GO111MODULE=off approach doesn't work though

Ah, yes, because of the version. Got it.

Was there some reason we weren't using that here?

If I have to guess, it was done before go modules were introduced and it never changed.

[joelanford]

No huge rush here, but this is still waiting on kubernetes/test-infra#14373.

I think that should pass CI and be able to be merged without breaking master on controller-runtime, but I'm not positive.

@DirectXMan12 can you add ok-to-test to kubernetes/test-infra#14373 when you get a chance?

[alvaroaleman]

@joelanford Does it make sense to have two presubmits, one with go 1.12 for the release-0.2 branch and one with go 1.13 for the master branch or can we safely assume that the presubmit with go 1.13 will work on both?

[joelanford]

@alvaroaleman I see no reason it would not work on both. But the question in my mind is whether this PR will necessitate moving to v0.3.0 (I think not, but I'm not sure).

If we jump to v0.3.0 after this merges, it would probably make sense to continue using go1.12 for the release-0.2 branch.

If we just continue to v0.2.3 after this merges, then the next commit on release-0.2 will need go1.13, so I think there would be no reason to have a separate presubmit, unless there's a chance that we backport a bug fix to release-0.2 without releasing everything else on master.

[DirectXMan12]

We probably want to make sure we don't break go1.12 on release-0.2 -- IMO, we'll want to go to v0.3.0 for go1.13, but I'm not certain exactly how we want to deal with language version bumps.

[DirectXMan12]

What do other people think, especially end consumers of CR? Is go-1.13 in all the major distros at this point yet?

[joelanford]

Just updated kubernetes/test-infra#14373 to have separate builds for each branch and to bump master to use go 1.13.

I also updated this PR to move from xerrors (which we introduced in the DynamicRESTMapper PR) to errors.

Anything else we should handle here?

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

[k8s-ci-robot]

@joelanford: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-controller-runtime-test	070701a	link	/test pull-controller-runtime-test

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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. I understand the commands that are listed here.

[DirectXMan12]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: DirectXMan12, joelanford

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 [DirectXMan12]

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