✨ Update go to 1.18 #1895
✨ Update go to 1.18 #1895
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Hi @Vaxuite. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the 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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Vaxuite The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
size/S
|
/hold |
k8s-ci-robot
added
the
do-not-merge/hold
|
@alvaroaleman apparently we needed to bump this to update kubebuilder? kubernetes-sigs/kubebuilder#2559 |
| @@ -185,7 +185,7 @@ func (c *multiNamespaceCache) WaitForCacheSync(ctx context.Context) bool { | |||
| func (c *multiNamespaceCache) IndexField(ctx context.Context, obj client.Object, field string, extractValue client.IndexerFunc) error { | |||
| isNamespaced, err := objectutil.IsAPINamespaced(obj, c.Scheme, c.RESTMapper) | |||
| if err != nil { | |||
| return nil //nolint:nilerr | |||
| return nil //nolint:ignore | |||
There was a problem hiding this comment.
Why are we changing the linters?
There was a problem hiding this comment.
@Vaxuite @alvaroaleman, it shows that what is not right here is we change the liners.
However, to make C+R begin to use go 1.18, why should we not bump the modules?
I could not follow that.
There was a problem hiding this comment.
I'm with @alvaroaleman here; I'm not sure if there's an immediate reason to update the go.mod's go version unless we do that on purpose to support go 1.18 features
usually, we bump the k8s version and go modules on deps before we upgrade Kubebuilder
Why?
- We want to ensure the same go version across the projects in the same ecosystem. Otherwise, it is hard for contributors to switch the projects.
- We faced problems in the past based on incompatibilities, i.e. with go 1.17:
For example, with some recent changes, we can't use k8s 1.23 with older versions of Go (For ex: operator-framework/operator-sdk#5558 (comment)) - this is a vice versa case.
Then, I do not understand how controller-runtime is using go 1.18 when its go mods are not bumped with 1.18?
There was a problem hiding this comment.
So when you specify the go version in the module file its a minimum version the module must run with. So with this module on go 1.17, if you run it with go 1.18 that will work fine and this does not really need changing.
There was a problem hiding this comment.
What made you think that this module needed to be bumped in the first place? @camilamacedo86
There was a problem hiding this comment.
The motivation here is not bump to allow:
By keeping it on 1.17 we allow consumers of CR to use Go 1.17+.
But we are using go 1.18 in the ci, see: https://kubernetes.slack.com/archives/C02MRBMN00Z/p1652178727902549?thread_ts=1652177106.531319&cid=C02MRBMN00Z
+1 this approach has my support either.
and we should do the same in Kubebuilder.
|
Thank you for your contribution @Vaxuite I'm with @alvaroaleman here, I'm not sure if there's an immediate reason to update the go.mod's go version unless we do that on purpose to support go 1.18 features |
Updates the go version to 1.18 and fixes all of the linter errors that result from doing so.