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 to Go 1.19 #4877

Merged
merged 1 commit into from Nov 17, 2022
Merged

Update to Go 1.19 #4877

merged 1 commit into from Nov 17, 2022

Conversation

KnVerey
Copy link
Contributor

@KnVerey KnVerey commented Nov 17, 2022

/cc @koba1t @annasong20

mod files updated via ./hack/for-each-module.sh 'sed -i "" "s/go 1.18/go 1.19/g" go.mod'

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 17, 2022
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: KnVerey

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 Nov 17, 2022
@@ -19,7 +19,7 @@ jobs:
- name: Set up Go 1.x
uses: actions/setup-go@v3
with:
go-version: ^1.18
go-version: '^1.19.0'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .0 addition should allow patch version updates but prevent CI from bumping itself up a minor version without us explicitly changing this file.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like the latest version is go1.19.3.
If you don't have any reason, Could you consider using the latest minor version?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ^ should make that happen automatically, and also automatically pick up new patch-level versions as they are available. Currently it is picking up 1.19.2:

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, Sorry.
I didn't know that. Thank you for showing the details!

@@ -3,6 +3,7 @@

run:
deadline: 5m
go: '1.19'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pinning as suggested by Anna

@k8s-ci-robot
Copy link
Contributor

@KnVerey: This PR has multiple commits, and the default merge method is: merge.
You can request commits to be squashed using the label: tide/merge-method-squash

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.

@@ -85,7 +85,7 @@ End-of-message
`Dockerfile` installs `kustomize fn` and copies the script into the container image.

```
FROM golang:1.18-stretch
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stretch is rather old and has stopped receiving security updates. bullseye is the current stable distribution

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the comments. Good to know as well (like the ^ thing)

@KnVerey
Copy link
Contributor Author

KnVerey commented Nov 17, 2022

Fix for presubmit failure: kubernetes/test-infra#28043

@KnVerey
Copy link
Contributor Author

KnVerey commented Nov 17, 2022

/test kustomize-presubmit-master

@koba1t
Copy link
Member

koba1t commented Nov 17, 2022

I think the below 2 files are using go1.18. So, Could you update that?

@yuwenma
Copy link
Contributor

yuwenma commented Nov 17, 2022

I think the below 2 files are using go1.18. So, Could you update that?

Oh, nice call. Besides, @KnVerey shall we consider using the distroless ("gcr.io/distroless/static") image to replace ln 132 FROM alpine:latest in AddGenerateDockerfile ? It is smaller and should be more secure. Not quite relevant to this PR though, defer to you.

@annasong20
Copy link
Contributor

annasong20 commented Nov 17, 2022

I compared this PR with @koba1t 's go1.18 PR #4699. That PR touches the following Dockerfiles:

that this PR doesn't. Do we want to change golang:1.18-stretch to golang:1.19-bullseye in these files?

Also, I'm assuming, since all checks passed, that we didn't change any go.sum files here because the go version upgrade didn't result in any package dependency upgrades?

My understanding of the touched files is rather basic, so feel free to ignore any uninformed comments. Really appreciate this PR!

@KnVerey
Copy link
Contributor Author

KnVerey commented Nov 17, 2022

Thanks for the sanity checks! I had indeed missed a bunch of places.

shall we consider using the distroless ("gcr.io/distroless/static") image to replace ln 132 FROM alpine:latest in AddGenerateDockerfile ? It is smaller and should be more secure. Not quite relevant to this PR though, defer to you.

I'm not opposed to reconsidering the images we use in various places (often in examples where distroless would be just fine, or where pinning a specific version of Go really isn't necessary), but it is out of scope for this PR.

@annasong20
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Nov 17, 2022
@k8s-ci-robot k8s-ci-robot merged commit 8ea5eb2 into kubernetes-sigs:master Nov 17, 2022
@KnVerey KnVerey deleted the go_1.19 branch November 17, 2022 18:27
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. 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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants