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

[META] Go 1.16 #2067

Closed
2 tasks
Adirio opened this issue Mar 8, 2021 · 10 comments
Closed
2 tasks

[META] Go 1.16 #2067

Adirio opened this issue Mar 8, 2021 · 10 comments
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. triage/accepted Indicates an issue or PR is ready to be actively worked on. triage/blocked
Milestone

Comments

@Adirio
Copy link
Contributor

Adirio commented Mar 8, 2021

This issue aims to tracks all the changes that need to/can be done once moving to go 1.16. This is obviously gated by dependencies.

  • [cleanup] Go 1.16 sets GO111MODULES to on by default instead of auto. There are several remanents of this env variable that we should remove throughout the codebase.
  • [enhancement] Go 1.16 includes a new embed package that allows to embed files into the binary using //go:...-style comments. This allows to split the files template from their logic offering way better support by IDEs (templates will no longer be a string, so they will be styled, auto-completed, and all the goodness of IDEs).

If there is any other feature enabled/required by go 1.16 that you will like to see in this list, please comment and I will update the list.

@Adirio Adirio added the kind/feature Categorizes issue or PR as related to a new feature. label Mar 8, 2021
@k8s-ci-robot k8s-ci-robot removed the kind/feature Categorizes issue or PR as related to a new feature. label Mar 8, 2021
@camilamacedo86
Copy link
Member

We also need before that just to clarify, have the same bump in controller-runtime, declarative-kb and k8s-api before we are able to address that here.

@camilamacedo86 camilamacedo86 added the triage/accepted Indicates an issue or PR is ready to be actively worked on. label Mar 9, 2021
@camilamacedo86 camilamacedo86 added this to the next milestone Mar 9, 2021
@kuritka
Copy link

kuritka commented Mar 18, 2021

The go install command can install go programs without affecting mod file dependencies. For example, executing go install github.com/dummy/myapp@v0.1.2 is enough. Will you plan to update Makefile ?
something like this :

# would work in GO1.16
generate: 
	## Download controller-gen locally if necessary.
	go install sigs.k8s.io/controller-tools/cmd/controller-gen@v0.4.1
	## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
	controller-gen object:headerFile="hack/boilerplate.go.txt" paths="./..."

or you plan to support older GO versions (stay with go get... / go-get-tool),
like this:

# without GO1.16
generate: controller-gen ## Generate code containing DeepCopy, DeepCopyInto, and DeepCopyObject method implementations.
	$(CONTROLLER_GEN) object:headerFile="hack/boilerplate.go.txt" paths="./..."

CONTROLLER_GEN = $(shell pwd)/bin/controller-gen
controller-gen: ## Download controller-gen locally if necessary.
	$(call go-get-tool,$(CONTROLLER_GEN),sigs.k8s.io/controller-tools/cmd/controller-gen@v0.4.1)

# go-get-tool will 'go get' any package $2 and install it to $1.
PROJECT_DIR := $(shell dirname $(abspath $(lastword $(MAKEFILE_LIST))))
define go-get-tool
@[ -f $(1) ] || { \
set -e ;\
TMP_DIR=$$(mktemp -d) ;\
cd $$TMP_DIR ;\
go mod init tmp ;\
echo "Downloading $(2)" ;\
GOBIN=$(PROJECT_DIR)/bin go get $(2) ;\
rm -rf $$TMP_DIR ;\
}
endef

@DirectXMan12
Copy link
Contributor

@kuritka AFAIK, go install-ing something is still global, so while we could potentially switch to using go install instead of go mod init; go get, we'd still have to have the GOBIN trick to avoid clobbering the user's existing configuration, unfortunately

@estroz
Copy link
Contributor

estroz commented May 11, 2021

1.16 is backwards compatible, so are there really any blockers here? Removing GO111MODULES should be done prior to switching, embed is just a nice-to-have so not necessary, and the current way bins are installed is fine and imo a feature request.

@estroz
Copy link
Contributor

estroz commented May 18, 2021

I believe I found a blocker: our test matrix. I think some bug fixes (features?) in go 1.16 made module installation more strict (see https://github.com/kubernetes-sigs/kubebuilder/pull/2182/checks?check_run_id=2614771863#step:6:520), which results in errors in go/v2 tests. For full backwards-compatibility, we'll likely want to start running all tests in golang:{x}.{y} containers like in prow.

Edit: adding go mod tidy to the testdata test script effectively fixes the module-related command changes in go 1.16 and should unblock update PRs for now. The test matrix upgrade suggestion still stands.

@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle stale
  • Mark this issue or PR as rotten with /lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Aug 16, 2021
@k8s-triage-robot
Copy link

The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs.

This bot triages issues and PRs according to the following rules:

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue or PR as fresh with /remove-lifecycle rotten
  • Close this issue or PR with /close
  • Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Sep 16, 2021
@sonnysideup
Copy link

I'm slightly confused. Is this still an issue now that 3.1.0 is out? It seems like #2182 updates kubebuilder to Go 1.16 but perhaps that's related to building the CLI vs what the CLI supports?

@estroz
Copy link
Contributor

estroz commented Sep 21, 2021

Both the kubebuilder binary and projects created by the binary use go 1.16, so I'm going to close this.

/close

@k8s-ci-robot
Copy link
Contributor

@estroz: Closing this issue.

In response to this:

Both the kubebuilder binary and projects created by the binary use go 1.16, so I'm going to close this.

/close

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. triage/accepted Indicates an issue or PR is ready to be actively worked on. triage/blocked
Projects
None yet
Development

No branches or pull requests

8 participants