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

Update Cluster Autoscaler to go 1.17 #4587

Merged
merged 5 commits into from
Jan 18, 2022
Merged

Conversation

MaciekPytel
Copy link
Contributor

This is a partial cherry-pick of a #4567.

Kubernetes has moved to go 1.17 and no longer compiles on go 1.16 (currently used by CA). This bumps CA to go1.17 and is a prerequisite to updating deps (which I'll do in a follow-up PR).
Some gotchas:

  • Go version tag in go.mod is purposefully left at go1.16. This is needed, because go1.17 changes how go.mod work and the upstream kubernetes haven't transitioned to the new go.mod yet (they misrepresent go version in go.mod exactly the same way). [go1.17] Update to go1.17.2 kubernetes#105563 (comment)
  • Added go:build tags matching existing +build tags for all cluster-autoscaler files across different providers. This should have no effect, but it is a part of golang migration to new syntax and both tags is expected by go fmt in go1.17.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jan 10, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MaciekPytel

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 10, 2022
@towca
Copy link
Collaborator

towca commented Jan 12, 2022

/lgtm
/hold

nit: It might be worth pointing out the difference in comments somewhere (e.g. add a comment to the Dockerfile saying that this should be the authoritative version and why it's different than go.mod + an opposite comment in go.mod). I know I've been using the version from go.mod as the authoritative one so far, others might be surprised as well.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 12, 2022
@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 12, 2022
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 17, 2022
@MaciekPytel
Copy link
Contributor Author

/hold cancel
@towca Added FAQ entry, can you review?

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 17, 2022
@towca
Copy link
Collaborator

towca commented Jan 17, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 17, 2022
Kubernetes 1.23 uses that version and the vendored code no
longer compiles using go1.16 (which we used previously).
Following the upstream Kubernetes I'm leaving the go version
in go.mod file at 1.16, as bumping it to 1.17 changes go mod
behavior in a way that breaks some kubernetes tooling
(kubernetes/kubernetes#105563 (comment)
for context).
CA no longer compiles on go1.16.
As of go1.17 both tags are expected to exist simultaneously.
Added tags in all cluster autoscaler files. Added verify-gomod.sh
exceptions for non-compliant autogenerated VPA files.
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 18, 2022
@towca
Copy link
Collaborator

towca commented Jan 18, 2022

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 18, 2022
@k8s-ci-robot k8s-ci-robot merged commit 2190f27 into kubernetes:master Jan 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/cluster-autoscaler cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants