Upgrade Go to 1.23 #6647
Merged
Upgrade Go to 1.23 #6647
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
antoninbas
force-pushed
the
bump-go-to-1.23
branch
from
70aeff0
to
42bc9f8
Compare
* Update some for loops in response to the following change introduced
in Go 1.22: each iteration of the loop creates new variables, to avoid
accidental sharing bugs. As a result of this change, a few existing
loop variables had become heap-allocated, so we updated the loop to
avoid the extra allocation. Other loops that were affected by the
change (compiling differently), but for which the loop variable was
stack-allocated, were reviewed individually and when it made sense,
the loop was refactored to avoid the extra variable copy. The `go test
-gcflags=-d=loopvar=2` command was used to find affected loops.
* The antrea/codegen image was updated to use Go 1.23. No changes in
generated files.
* golangci-lint was updated to the latest available version (v1.60.3)
- govet reported some invalid usages of printf, which was addressed;
this lead to some refactoring of the `antctl check` code
- a new gosec rule (G115), which looks for potential integer
overflows when converting between integer types, caused a lot of
errors to be reported in the codebase. There were too many errors
to be addressed as part of this PR, so we only handled a few of
them for which the fix was straightforward. When the rule
implementation is improved (so that it reports fewer false
positives), we should review all errors and address them as needed.
Fixes antrea-io#6644
Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
antoninbas
force-pushed
the
bump-go-to-1.23
branch
from
42bc9f8
to
304b2c4
Compare
Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
tnqn
reviewed
Sep 2, 2024
pkg/antctl/raw/check/util.go
Outdated
| @@ -216,13 +216,14 @@ func GenerateRandomNamespace(baseName string) string { | |||
| } | |||
|
|
|||
| func Teardown(ctx context.Context, client kubernetes.Interface, clusterName string, namespace string) { | |||
| fmt.Fprintf(os.Stdout, fmt.Sprintf("[%s] ", clusterName)+"Deleting installation tests setup...\n") | |||
| logger := NewLogger(fmt.Sprintf("[%s] ", clusterName)) | |||
There was a problem hiding this comment.
could we add a Logger parameter to Teardown and remove clusterName?
pkg/antctl/raw/check/util.go
Outdated
| @@ -233,8 +234,8 @@ func Teardown(ctx context.Context, client kubernetes.Interface, clusterName stri | |||
| return false, nil | |||
| }) | |||
| if err != nil { | |||
| fmt.Fprintf(os.Stdout, fmt.Sprintf("[%s] ", clusterName)+"Setup deletion failed\n") | |||
| logger.Success("Setup deletion failed") | |||
There was a problem hiding this comment.
Suggested change
| logger.Success("Setup deletion failed") | |
| logger.Fail("Setup deletion failed") |
pkg/antctl/raw/check/util.go
Outdated
| } else { | ||
| fmt.Fprintf(os.Stdout, fmt.Sprintf("[%s] ", clusterName)+"Setup deletion successful\n") | ||
| logger.Fail("Setup deletion successful") |
There was a problem hiding this comment.
Suggested change
| logger.Fail("Setup deletion successful") | |
| logger.Success("Setup deletion successful") |
luolanzone
reviewed
Sep 3, 2024
luolanzone
reviewed
Sep 3, 2024
| @@ -22,7 +22,7 @@ function echoerr { | |||
| } | |||
|
|
|||
| ANTREA_ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}" )/../" && pwd )" | |||
| IMAGE_NAME="antrea/codegen:kubernetes-1.29.2-build.2" | |||
| IMAGE_NAME="antrea/codegen:kubernetes-1.29.2-build.3" | |||
There was a problem hiding this comment.
One question: I assume this image is already uploaded?
There was a problem hiding this comment.
Yes, I already pushed it
Signed-off-by: Antonin Bas <antonin.bas@broadcom.com>
|
/test-all |
|
/test-all |
2 similar comments
|
/test-all |
|
/test-all |
|
/test-e2e |
|
/test-e2e |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Update some for loops in response to the following change introduced in Go 1.22: each iteration of the loop creates new variables, to avoid accidental sharing bugs. As a result of this change, a few existing loop variables had become heap-allocated, so we updated the loop to avoid the extra allocation. Other loops that were affected by the change (compiling differently), but for which the loop variable was stack-allocated, were reviewed individually and when it made sense, the loop was refactored to avoid the extra variable copy. The
go test -gcflags=-d=loopvar=2command was used to find affected loops.The antrea/codegen image was updated to use Go 1.23. No changes in generated files.
golangci-lint was updated to the latest available version (v1.60.3)
antctl checkcodeFixes #6644