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.

Sign up for GitHub

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

Jump to bottom

Bump go toolchain to 1.24.1 #19440

Merged
merged 3 commits into from
Mar 6, 2025
Merged

Bump go toolchain to 1.24.1 #19440

merged 3 commits into from
Mar 6, 2025

Conversation

ivanvc
Copy link
Member

@ivanvc ivanvc commented Feb 18, 2025 •
edited
Loading

@k8s-ci-robot
Copy link

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@ivanvc ivanvc mentioned this pull request Feb 18, 2025
Closed
16 tasks
@ivanvc ivanvc force-pushed the bump-go-to-1.24.0 branch 2 times, most recently from 08ba228 to 39f7b2b Compare February 26, 2025 00:45
@ivanvc ivanvc mentioned this pull request Feb 26, 2025
Merged
@ivanvc
Copy link
Member Author

ivanvc commented Feb 26, 2025

/test all

@codecov Codecov
Copy link

codecov bot commented Feb 26, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 68.83%. Comparing base (3d38153) to head (7456d52).
Report is 22 commits behind head on main.

Additional details and impacted files

see 23 files with indirect coverage changes

@@            Coverage Diff             @@
##             main   #19440      +/-   ##
==========================================
- Coverage   68.88%   68.83%   -0.05%     
==========================================
  Files         421      421              
  Lines       35901    35901              
==========================================
- Hits        24730    24713      -17     
- Misses       9744     9757      +13     
- Partials     1427     1431       +4

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d38153...7456d52. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ivanvc
Copy link
Member Author

ivanvc commented Mar 1, 2025

/test pull-etcd-govulncheck

@ivanvc ivanvc changed the title [WIP] Bump go toolchain to 1.24.0 Bump go toolchain to 1.24.0 Mar 1, 2025
@ivanvc ivanvc marked this pull request as ready for review March 1, 2025 01:23
@ivanvc ivanvc force-pushed the bump-go-to-1.24.0 branch from 70e2357 to 74e2c21 Compare March 1, 2025 04:24
@ivanvc
Copy link
Member Author

ivanvc commented Mar 1, 2025

The usetesting linter shows a lot of violations. I addressed them, but this pull request would be huge with all the changes:

146 files changed, 1157 insertions(+), 1212 deletions(-)

The problem is that it's suggesting to use functions that are not in Go 1.23. Therefore, I will disable the linter so we can merge this. And I will open multiple pull requests targeting main. Once I finish with them, I'll reenable the linter.

@ivanvc ivanvc force-pushed the bump-go-to-1.24.0 branch from 3e38505 to 6e80928 Compare March 5, 2025 00:18
@ivanvc ivanvc mentioned this pull request Mar 5, 2025
Open
17 tasks
ahrtr
ahrtr reviewed Mar 5, 2025
View reviewed changes
server/embed/config_test.go
@@ -843,7 +843,7 @@ func TestLogRotation(t *testing.T) {
logOutputs: []string{"/tmp/path"},
logRotationConfig: `{"maxsize": true}`,
wantErr: true,
wantErrMsg: errors.New("invalid log rotation config: json: cannot unmarshal bool into Go struct field logRotationConfig.maxsize of type int"),
wantErrMsg: errors.New("invalid log rotation config: json: cannot unmarshal bool into Go struct field logRotationConfig.Logger.maxsize of type int"),
Copy link
Member

Choose a reason for hiding this comment

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

question: is this a side effect to bump the go version?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the error message is different now, it also shows the embedded structure.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added more context in 31650ab's commit message:

The embedded structure is now also part of the log message. The failure
from the tests is the following:

--- FAIL: TestLogRotation (0.00s)
    --- FAIL: TestLogRotation/invalid_logger_config (0.00s)
        config_test.go:868: test "invalid logger config", expected
        error: invalid log rotation config: json: cannot unmarshal
               bool into Go struct field logRotationConfig.maxsize
               of type int,
        got: invalid log rotation config: json: cannot unmarshal
             bool into Go struct field logRotationConfig.Logger.maxsize
             of type int
FAIL

@ivanvc ivanvc changed the title Bump go toolchain to 1.24.0 Bump go toolchain to 1.24.1 Mar 5, 2025
@ivanvc
Bump go toolchain to 1.24.1
0287731 
Signed-off-by: Ivan Valdes <ivan@vald.es>
@ivanvc ivanvc force-pushed the bump-go-to-1.24.0 branch 2 times, most recently from 9b42a59 to 6b280cf Compare March 5, 2025 17:05
ivanvc added 2 commits March 5, 2025 09:05
@ivanvc
server: Fix TestLogRotation error message in Go 1.24
31650ab 
The embedded structure is now also part of the log message. The failure
from the tests is the following:

--- FAIL: TestLogRotation (0.00s)
    --- FAIL: TestLogRotation/invalid_logger_config (0.00s)
        config_test.go:868: test "invalid logger config", expected
        error: invalid log rotation config: json: cannot unmarshal
               bool into Go struct field logRotationConfig.maxsize
               of type int,
        got: invalid log rotation config: json: cannot unmarshal
             bool into Go struct field logRotationConfig.Logger.maxsize
             of type int
FAIL

Signed-off-by: Ivan Valdes <ivan@vald.es>
@ivanvc
Disable usetesting linter
7456d52 
The linter fails with several warnings from new Go 1.24 functions. Due
to the amount of changed lines, it will be better to address them in
several follow-up pull requests.

> 146 files changed, 1157 insertions(+), 1212 deletions(-)

Signed-off-by: Ivan Valdes <ivan@vald.es>
@ivanvc ivanvc force-pushed the bump-go-to-1.24.0 branch from 6b280cf to 7456d52 Compare March 5, 2025 17:06
@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahrtr, ivanvc

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

@ivanvc
Copy link
Member Author

ivanvc commented Mar 6, 2025

/retest

@ahrtr ahrtr merged commit 8e4de78 into etcd-io:main Mar 6, 2025
33 checks passed
@ivanvc ivanvc deleted the bump-go-to-1.24.0 branch March 6, 2025 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ahrtr ahrtr

Assignees
No one assigned
Labels
approved area/clientv3 area/etcdctl area/etcdutl area/performance area/testing area/tooling size/M stage/merge-when-tests-green
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ivanvc @k8s-ci-robot @ahrtr