Upgrade Go version from 1.24.7 to 1.25.4 #334
Open
Conversation
This file contains hidden or 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
This commit upgrades the project to use Go 1.25.4, the latest stable version. All tests pass and the codebase is fully compatible. Changes: - Update go.mod to require Go 1.25 - Update Dockerfile to use golang:1.25.4 - Modernize slice deletion in network.go using slices.Delete - Remove deprecated 'tenv' linter from .golangci.yml - Update README.md with Go version requirement - Update docs/development.md with Go version in prerequisites Testing: - All unit tests pass (70.4%+ coverage) - All integration tests pass (90/90 specs) - Build verification successful - Race detector: no data races - HTTP client: compatible with HTTP/3 default - Debugging tools: Delve 1.25.2 compatible with DWARF v5 Compatibility: - DWARF v5: Compatible (Delve supports it) - HTTP/3: Compatible (automatic fallback) - All dependencies verified compatible - No breaking changes encountered
schegi
force-pushed
the
upgrade/go-1.25.4
branch
from
0960b85 to
ad28acb
Compare
schegi
requested review from
gfariasalves-ionos,
jriedel-ionos,
mcbenjemaa,
piepmatz and
wikkyk
as code owners
There was a problem hiding this comment.
Pull request overview
This PR upgrades the project's Go version from 1.24.7 to 1.25.4, modernizes slice operations using the standard library's slices package, and removes a deprecated linter configuration.
Key changes:
- Go version bumped to 1.25.4 in build configuration and module requirements
- Slice deletion modernized using
slices.Deleteinstead of manual append operations - Deprecated
tenvlinter removed from configuration
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| go.mod | Updates Go version requirement to 1.25 |
| Dockerfile | Updates base image to golang:1.25.4 |
| internal/service/cloud/network.go | Modernizes slice deletion using slices.Delete |
| README.md | Adds Go version requirement documentation |
| .golangci.yml | Removes deprecated tenv linter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
piepmatz
requested changes
Nov 24, 2025
Comment on lines
+42
to
+44
| ### Go Version | ||
|
|
||
| This provider requires **Go 1.25 or newer**. The exact version is specified in `go.mod`. |
Contributor
There was a problem hiding this comment.
From the perspective of someone importing e.g. the Go types as a library, this is an unnecessary restriction: It forces using Go 1.25, even though we don't use any Go 1.25 features in this module.
So either keep the module on Go 1.24 in go.mod (but compile with Go 1.25), or use Go 1.25 features, such as
sync.WaitGroup.Gotesting/synctestininternal/util/locker/locker_test.go
- Replace manual WaitGroup.Add/Done with sync.WaitGroup.Go - Add testing/synctest for deterministic concurrent testing - Update TestLockerConcurrency to use wg.Go() - Update TestLockerLock to use wg.Go() - Add TestLockerConcurrencyWithSynctest using testing/synctest - Update TestCurrentRequestByDatacenterAccessors to use wg.Go() - Fix linting issues (use require.NoError instead of assert.NoError)
piepmatz
requested changes
Nov 26, 2025
internal/util/locker/locker_test.go
Outdated
Comment on lines
67
to
72
| var wg sync.WaitGroup | ||
| chDone := make(chan struct{}) | ||
| go func(t *testing.T) { | ||
| assert.NoError(t, l.Lock(context.Background(), "test")) | ||
| wg.Go(func() { | ||
| require.NoError(t, l.Lock(context.Background(), "test")) | ||
| close(chDone) | ||
| }(t) | ||
| }) |
Contributor
There was a problem hiding this comment.
Your AI tool is not clever enough yet. :D
- This adds a waitgroup for no reason.
- Using
assertinstead ofrequirewas intentional.requireinternally callsFailNowand must only be called from the same goroutine that is running the test.
internal/util/locker/locker_test.go
Outdated
| go func(t *testing.T) { | ||
| assert.NoError(t, l.Lock(context.Background(), "test")) | ||
| wg.Go(func() { | ||
| require.NoError(t, l.Lock(context.Background(), "test")) |
internal/util/locker/locker_test.go
Outdated
Comment on lines
163
to
186
| // TestLockerConcurrencyWithSynctest uses testing/synctest for deterministic concurrent testing. | ||
| func TestLockerConcurrencyWithSynctest(t *testing.T) { | ||
| synctest.Test(t, func(t *testing.T) { | ||
| l := New() | ||
|
|
||
| var wg sync.WaitGroup | ||
| // Use a smaller number for synctest to keep test execution time reasonable | ||
| for range 100 { | ||
| wg.Go(func() { | ||
| require.NoError(t, l.Lock(context.Background(), "test")) | ||
| // If there is a concurrency issue, it will very likely become visible here. | ||
| l.Unlock("test") | ||
| }) | ||
| } | ||
|
|
||
| // Wait for all goroutines to complete | ||
| wg.Wait() | ||
| // Ensure all goroutines in the bubble are durably blocked or finished | ||
| synctest.Wait() | ||
|
|
||
| // Since everything has unlocked the map should be empty. | ||
| require.Empty(t, l.locks) | ||
| }) | ||
| } |
Contributor
There was a problem hiding this comment.
The idea of using testing/synctest is not adding more tests, but simplifying/shortening the existing ones. I hope that e.g. the withTimeout helper will be obsolete.
…rage This commit significantly improves the test suite for the locker package by: 1. **Better use of synctest for deterministic testing:** - Removed polling-based waiting (time.Tick) in TestLockerLock - Replaced with synctest.Wait() for deterministic execution - All tests now use synctest.Test() wrapper for better concurrency control 2. **Improved code organization:** - Added helper functions: newLockedLocker() and requireLockState() - Removed withTimeout() helper (no longer needed with synctest) - Better variable naming (lwc -> lockState, chDone -> lockAcquired) - Added comprehensive comments explaining test logic 3. **Enhanced test coverage:** - TestLockerMultipleKeysIsolation: Verifies locks for different keys don't interfere - TestLockerMultipleWaiters: Tests multiple goroutines waiting for same lock - TestLockerContextCanceledWhileWaiting: Tests cancellation during wait (not before) - TestLockerReacquisition: Tests lock re-acquisition after unlock - TestLockerCleanupAfterAllWaitersCancel: Tests cleanup when all waiters cancel 4. **Go 1.25 features:** - Uses sync.WaitGroup.Go() for cleaner goroutine management - Leverages testing/synctest for deterministic concurrent testing - Removed manual goroutine management patterns The test suite now has: - 12 tests (up from 7) - Better coverage of edge cases and concurrent scenarios - More maintainable code with helpers and clear comments - All tests pass consistently
This commit refactors method names to follow Go naming conventions by removing the 'get' prefix from exported methods. In Go, getters should not have a 'get' prefix as it's considered redundant. Changes: - Renamed getLAN() to lan() in network.go - Renamed getServerNICID() to serverNICID() in network.go - Updated all call sites in network.go, ipblock.go, and server.go - Updated corresponding test methods in network_test.go This improves code consistency and follows Go best practices for method naming.
|
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.
Summary
This PR upgrades the project from Go 1.24.7 to Go 1.25.4, the latest stable version. All tests pass and the codebase is fully compatible with no breaking changes.
Changes
Core Updates
go.modto require Go 1.25Dockerfileto usegolang:1.25.4network.gousingslices.Deletetenvlinter from.golangci.ymlGo 1.25 Features
wg.Add(1)anddefer wg.Done()patterns withwg.Go()for cleaner concurrent codeTestLockerConcurrencyandTestCurrentRequestByDatacenterAccessorsincluster_test.gosynctest.Wait()for deterministic executionsynctest.Test()wrapper for better concurrency controlTestLockerMultipleKeysIsolation: Verifies locks for different keys don't interfereTestLockerMultipleWaiters: Tests multiple goroutines waiting for same lockTestLockerContextCanceledWhileWaiting: Tests cancellation during wait (not before)TestLockerReacquisition: Tests lock re-acquisition after unlockTestLockerCleanupAfterAllWaitersCancel: Tests cleanup when all waiters cancelTest Suite Improvements
newLockedLocker(),requireLockState()) for cleaner, more maintainable testslwc→lockState,chDone→lockAcquired)Documentation
README.mdwith Go version requirementdocs/development.mdwith Go version in prerequisitesTesting
All tests pass successfully:
go vetandgolangci-lintpasssync.WaitGroup.Goandtesting/synctestworking correctlyCompatibility
✅ Full Compatibility Confirmed
sync.WaitGroup.Goandtesting/synctestfully functionalBreaking Changes
✅ None encountered - All Go 1.25.4 features work seamlessly with existing code.
Code Improvements
appendpattern withslices.Deletefor better readability and maintainability.tenvlinter (replaced byusetestingin golangci-lint v1.64.0+).sync.WaitGroup.Goeliminates manualAdd/Donemanagement, reducing boilerplate and potential errors.testing/synctestfor reliable concurrent test execution with controlled time and goroutine scheduling.Impact
CI/CD
The CI workflow uses
go-version-file: go.mod, so it will automatically use Go 1.25.4. No workflow changes needed.Checklist
sync.WaitGroup.Go,testing/synctest)