From ad28acb1abaa4b2ead5cffc76cbb735a68d4ba14 Mon Sep 17 00:00:00 2001 From: Stefan Scheglmann Date: Fri, 21 Nov 2025 21:00:54 +0100 Subject: [PATCH 01/12] Upgrade Go version from 1.24.7 to 1.25.4 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 --- .golangci.yml | 1 - Dockerfile | 2 +- README.md | 4 ++++ go.mod | 2 +- internal/service/cloud/network.go | 2 +- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index ae540213..8e540926 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -42,7 +42,6 @@ linters: - stylecheck - tagalign - testifylint - - tenv - typecheck - unconvert - unparam diff --git a/Dockerfile b/Dockerfile index bd948659..ec8e7e4f 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build the manager binary -FROM golang:1.24.7 AS builder +FROM golang:1.25.4 AS builder ARG TARGETOS ARG TARGETARCH diff --git a/README.md b/README.md index 02ca81c8..79256076 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,10 @@ If you need help with CAPIC, please visit the [#cluster-api-ionoscloud][slack] c ## Compatibility +### Go Version + +This provider requires **Go 1.25 or newer**. The exact version is specified in `go.mod`. + ### Cluster API Versions This provider's versions are compatible with the following versions of Cluster API: diff --git a/go.mod b/go.mod index 51e5b272..eb9943f3 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/ionos-cloud/cluster-api-provider-ionoscloud -go 1.24.7 +go 1.25 require ( github.com/go-logr/logr v1.4.3 diff --git a/internal/service/cloud/network.go b/internal/service/cloud/network.go index 7fc1df31..a4af1100 100644 --- a/internal/service/cloud/network.go +++ b/internal/service/cloud/network.go @@ -599,7 +599,7 @@ func (s *Service) removeNICFromFailoverGroup( // Found the NIC, remove it from the failover group log.V(4).Info("Found NIC in failover group", "nicID", nicID) - ipFailoverConfig = append(ipFailoverConfig[:index], ipFailoverConfig[index+1:]...) + ipFailoverConfig = slices.Delete(ipFailoverConfig, index, index+1) props := sdk.LanProperties{IpFailover: &ipFailoverConfig} log.V(4).Info("Patching LAN failover group to remove NIC", "nicID", nicID) From d2ff8b504ecb4e3113b710022dcb2e30ff2aa7b9 Mon Sep 17 00:00:00 2001 From: Stefan Scheglmann Date: Tue, 25 Nov 2025 12:06:34 +0100 Subject: [PATCH 02/12] feat: use Go 1.25 features in tests - 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) --- internal/util/locker/locker_test.go | 42 ++++++++++++++++++++++------- scope/cluster_test.go | 8 +++--- 2 files changed, 36 insertions(+), 14 deletions(-) diff --git a/internal/util/locker/locker_test.go b/internal/util/locker/locker_test.go index f1bd84ee..32c2c566 100644 --- a/internal/util/locker/locker_test.go +++ b/internal/util/locker/locker_test.go @@ -20,9 +20,9 @@ import ( "context" "sync" "testing" + "testing/synctest" "time" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -64,11 +64,12 @@ func TestLockerLock(t *testing.T) { require.EqualValues(t, 0, lwc.count()) + 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) + }) chWaiting := make(chan struct{}) go func() { @@ -119,13 +120,11 @@ func TestLockerConcurrency(t *testing.T) { var wg sync.WaitGroup for range 10_000 { - wg.Add(1) - go func(t *testing.T) { - assert.NoError(t, l.Lock(context.Background(), "test")) + 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") - wg.Done() - }(t) + }) } withTimeout(t, wg.Wait) @@ -160,3 +159,28 @@ func TestLockerContextDeadlineExceeded(t *testing.T) { require.NotPanics(t, func() { l.Unlock("test") }) } + +// 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) + }) +} diff --git a/scope/cluster_test.go b/scope/cluster_test.go index 0235fd9d..707e344c 100644 --- a/scope/cluster_test.go +++ b/scope/cluster_test.go @@ -365,10 +365,8 @@ func TestCurrentRequestByDatacenterAccessors(t *testing.T) { // If there is a concurrency issue, it will very likely become visible here. var wg sync.WaitGroup for i := range 10_000 { - wg.Add(1) - go func(t *testing.T, id string) { - defer wg.Done() - + id := strconv.Itoa(i) + wg.Go(func() { req, exists := cluster.GetCurrentRequestByDatacenter(id) assert.False(t, exists) assert.Zero(t, req) @@ -386,7 +384,7 @@ func TestCurrentRequestByDatacenterAccessors(t *testing.T) { req, exists = cluster.GetCurrentRequestByDatacenter(id) assert.False(t, exists) assert.Zero(t, req) - }(t, strconv.Itoa(i)) + }) } wg.Wait() From 151a31a99f4a596647f669e0558e9bb4f24e7856 Mon Sep 17 00:00:00 2001 From: Stefan Scheglmann Date: Thu, 27 Nov 2025 21:34:01 +0100 Subject: [PATCH 03/12] test: improve locker test suite with Go 1.25 features and better coverage 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 --- internal/util/locker/locker_test.go | 313 ++++++++++++++++++++-------- 1 file changed, 227 insertions(+), 86 deletions(-) diff --git a/internal/util/locker/locker_test.go b/internal/util/locker/locker_test.go index 32c2c566..338afacd 100644 --- a/internal/util/locker/locker_test.go +++ b/internal/util/locker/locker_test.go @@ -23,21 +23,23 @@ import ( "testing/synctest" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func withTimeout(t *testing.T, f func()) { +// Helper to create a locked locker for testing. +func newLockedLocker(t *testing.T, key string) *Locker { t.Helper() - done := make(chan struct{}) - go func() { - f() - close(done) - }() - select { - case <-time.After(1 * time.Second): - t.Fatal("timed out") - case <-done: - } + l := New() + require.NoError(t, l.Lock(context.Background(), key)) + return l +} + +// Helper to verify lock exists with expected waiters. +func requireLockState(t *testing.T, l *Locker, key string, expectedWaiters int32) { + t.Helper() + require.Contains(t, l.locks, key, "lock %q should exist", key) + require.EqualValues(t, expectedWaiters, l.locks[key].count(), "lock %q should have %d waiters", key, expectedWaiters) } func TestNew(t *testing.T) { @@ -58,129 +60,268 @@ func TestLockWithCounter(t *testing.T) { } func TestLockerLock(t *testing.T) { - l := New() - require.NoError(t, l.Lock(context.Background(), "test")) - lwc := l.locks["test"] - - require.EqualValues(t, 0, lwc.count()) - - var wg sync.WaitGroup - chDone := make(chan struct{}) - wg.Go(func() { + synctest.Test(t, func(t *testing.T) { + l := New() require.NoError(t, l.Lock(context.Background(), "test")) - close(chDone) - }) + lockState := l.locks["test"] - chWaiting := make(chan struct{}) - go func() { - for range time.Tick(1 * time.Millisecond) { - if lwc.count() == 1 { - close(chWaiting) - break - } - } - }() + // Verify initial state: lock is held, no waiters + require.EqualValues(t, 0, lockState.count()) - withTimeout(t, func() { - <-chWaiting - }) + // Start a goroutine that will wait for the lock + lockAcquired := make(chan struct{}) + go func(t *testing.T) { + assert.NoError(t, l.Lock(context.Background(), "test")) + close(lockAcquired) + }(t) - select { - case <-chDone: - t.Fatal("lock should not have returned while it was still held") - default: - } + // Wait for goroutine to enter waiting state (deterministic with synctest) + synctest.Wait() + require.EqualValues(t, 1, lockState.count(), "should have one waiter") - l.Unlock("test") + // Verify the waiting goroutine hasn't acquired the lock yet + select { + case <-lockAcquired: + t.Fatal("lock should not have been acquired while still held") + default: + } - withTimeout(t, func() { - <-chDone - }) + // Release the lock - waiting goroutine should now acquire it + l.Unlock("test") + <-lockAcquired + synctest.Wait() - require.EqualValues(t, 0, lwc.count()) + // Verify final state: no waiters + require.EqualValues(t, 0, lockState.count()) + }) } func TestLockerUnlock(t *testing.T) { - l := New() - - require.NoError(t, l.Lock(context.Background(), "test")) - l.Unlock("test") + synctest.Test(t, func(t *testing.T) { + l := New() - require.PanicsWithValue(t, "no such lock: test", func() { + require.NoError(t, l.Lock(context.Background(), "test")) l.Unlock("test") - }) - withTimeout(t, func() { - require.NoError(t, l.Lock(context.Background(), "test")) + require.PanicsWithValue(t, "no such lock: test", func() { + l.Unlock("test") + }) + + go func() { + assert.NoError(t, l.Lock(context.Background(), "test")) + }() + synctest.Wait() }) } func TestLockerConcurrency(t *testing.T) { - l := New() + synctest.Test(t, func(t *testing.T) { + l := New() - var wg sync.WaitGroup - for range 10_000 { - 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") - }) - } + var wg sync.WaitGroup + for range 10_000 { + 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") + }) + } - withTimeout(t, wg.Wait) + wg.Wait() + synctest.Wait() - // Since everything has unlocked the map should be empty. - require.Empty(t, l.locks) + // Since everything has unlocked the map should be empty. + require.Empty(t, l.locks) + }) } func TestLockerContextCanceled(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) - cancel() + synctest.Test(t, func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() - l := New() + l := New() - withTimeout(t, func() { err := l.Lock(ctx, "test") require.ErrorIs(t, context.Canceled, err) + synctest.Wait() }) } func TestLockerContextDeadlineExceeded(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) - defer cancel() + synctest.Test(t, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) + defer cancel() - l := New() - require.NoError(t, l.Lock(ctx, "test")) + l := New() + require.NoError(t, l.Lock(ctx, "test")) - withTimeout(t, func() { - err := l.Lock(ctx, "test") - require.ErrorIs(t, context.DeadlineExceeded, err) - }) + lockFailed := make(chan struct{}) + go func() { + err := l.Lock(ctx, "test") + assert.ErrorIs(t, context.DeadlineExceeded, err) + close(lockFailed) + }() - require.NotPanics(t, func() { l.Unlock("test") }) + <-lockFailed + synctest.Wait() + + require.NotPanics(t, func() { l.Unlock("test") }) + }) } -// TestLockerConcurrencyWithSynctest uses testing/synctest for deterministic concurrent testing. -func TestLockerConcurrencyWithSynctest(t *testing.T) { +// TestLockerMultipleKeysIsolation verifies that locks for different keys don't interfere with each other. +func TestLockerMultipleKeysIsolation(t *testing.T) { synctest.Test(t, func(t *testing.T) { l := New() + // Lock key1 + require.NoError(t, l.Lock(context.Background(), "key1")) + requireLockState(t, l, "key1", 0) + + // Should be able to lock key2 immediately (different key) + key2Acquired := make(chan struct{}) + go func() { + assert.NoError(t, l.Lock(context.Background(), "key2")) + close(key2Acquired) + }() + + <-key2Acquired + synctest.Wait() + + // Both keys should be locked + requireLockState(t, l, "key1", 0) + requireLockState(t, l, "key2", 0) + + // Unlock both + l.Unlock("key1") + l.Unlock("key2") + synctest.Wait() + + // Both locks should be cleaned up + require.Empty(t, l.locks) + }) +} + +// TestLockerMultipleWaiters verifies that multiple goroutines can wait for the same lock. +func TestLockerMultipleWaiters(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + l := newLockedLocker(t, "test") + const numWaiters = 10 + var wg sync.WaitGroup - // Use a smaller number for synctest to keep test execution time reasonable - for range 100 { + results := make([]bool, numWaiters) + + // Start multiple waiters + for i := range numWaiters { wg.Go(func() { - require.NoError(t, l.Lock(context.Background(), "test")) - // If there is a concurrency issue, it will very likely become visible here. + //nolint:testifylint // Using assert in goroutine per go-require rule + assert.NoError(t, l.Lock(context.Background(), "test")) + results[i] = true l.Unlock("test") }) } - // Wait for all goroutines to complete + // Wait for all goroutines to enter waiting state + synctest.Wait() + require.EqualValues(t, numWaiters, l.locks["test"].count(), "should have %d waiters", numWaiters) + + // Release lock - all waiters should acquire it sequentially + l.Unlock("test") wg.Wait() - // Ensure all goroutines in the bubble are durably blocked or finished synctest.Wait() - // Since everything has unlocked the map should be empty. + // All should have succeeded + for i := range numWaiters { + require.True(t, results[i], "waiter %d should have acquired lock", i) + } + + // Lock should be cleaned up + require.Empty(t, l.locks) + }) +} + +// TestLockerContextCanceledWhileWaiting verifies context cancellation that happens while waiting (not before). +func TestLockerContextCanceledWhileWaiting(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + l := newLockedLocker(t, "test") + + ctx, cancel := context.WithCancel(context.Background()) + + lockCanceled := make(chan struct{}) + go func() { + err := l.Lock(ctx, "test") + assert.ErrorIs(t, err, context.Canceled) + close(lockCanceled) + }() + + // Wait for goroutine to enter waiting state + synctest.Wait() + require.EqualValues(t, 1, l.locks["test"].count(), "should have one waiter") + + // Cancel while waiting + cancel() + <-lockCanceled + synctest.Wait() + + // Lock should still exist (held by main goroutine) + requireLockState(t, l, "test", 0) + + // Unlock and verify cleanup + l.Unlock("test") + synctest.Wait() + require.Empty(t, l.locks) + }) +} + +// TestLockerReacquisition verifies that a lock can be re-acquired after unlock. +func TestLockerReacquisition(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + l := New() + + // Lock, unlock, lock again + require.NoError(t, l.Lock(context.Background(), "test")) + l.Unlock("test") + + require.NoError(t, l.Lock(context.Background(), "test")) + l.Unlock("test") + + // Lock should be cleaned up + require.Empty(t, l.locks) + }) +} + +// TestLockerCleanupAfterAllWaitersCancel verifies that lock is cleaned up when all waiters cancel. +func TestLockerCleanupAfterAllWaitersCancel(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + l := newLockedLocker(t, "test") + + ctx, cancel := context.WithCancel(context.Background()) + + const numWaiters = 5 + var wg sync.WaitGroup + for range numWaiters { + wg.Go(func() { + err := l.Lock(ctx, "test") + require.ErrorIs(t, err, context.Canceled) + }) + } + + // Wait for all goroutines to enter waiting state + synctest.Wait() + require.EqualValues(t, numWaiters, l.locks["test"].count(), "should have %d waiters", numWaiters) + + // Cancel all waiters + cancel() + wg.Wait() + synctest.Wait() + + // Lock should still exist (held by main goroutine) + requireLockState(t, l, "test", 0) + + // Unlock - now lock should be cleaned up + l.Unlock("test") + synctest.Wait() require.Empty(t, l.locks) }) } From f64d076f25ec7f2f5dbb22323cf9fefc275ea820 Mon Sep 17 00:00:00 2001 From: Stefan Scheglmann Date: Thu, 27 Nov 2025 22:45:47 +0100 Subject: [PATCH 04/12] refactor: remove 'get' prefix from method names to follow Go conventions 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. --- internal/service/cloud/ipblock.go | 2 +- internal/service/cloud/network.go | 18 +++++++++--------- internal/service/cloud/network_test.go | 10 +++++----- internal/service/cloud/server.go | 2 +- 4 files changed, 16 insertions(+), 16 deletions(-) diff --git a/internal/service/cloud/ipblock.go b/internal/service/cloud/ipblock.go index a8df3e7e..373814a1 100644 --- a/internal/service/cloud/ipblock.go +++ b/internal/service/cloud/ipblock.go @@ -199,7 +199,7 @@ func (s *Service) ReconcileFailoverIPBlockDeletion(ctx context.Context, ms *scop } // Check if IPBlock is still being used - lan, err := s.getLAN(ctx, ms) + lan, err := s.lan(ctx, ms) if err != nil { return false, err } diff --git a/internal/service/cloud/network.go b/internal/service/cloud/network.go index a4af1100..10c5fbbe 100644 --- a/internal/service/cloud/network.go +++ b/internal/service/cloud/network.go @@ -69,7 +69,7 @@ func (s *Service) ReconcileLAN(ctx context.Context, ms *scope.Machine) (requeue } defer ms.Locker.Unlock(lockKey) - lan, request, err := scopedFindResource(ctx, ms, s.getLAN, s.getLatestLANCreationRequest) + lan, request, err := scopedFindResource(ctx, ms, s.lan, s.getLatestLANCreationRequest) if err != nil { return false, err } @@ -109,7 +109,7 @@ func (s *Service) ReconcileLANDeletion(ctx context.Context, ms *scope.Machine) ( defer ms.Locker.Unlock(lockKey) // Try to retrieve the cluster LAN or even check if it's currently still being created. - lan, request, err := scopedFindResource(ctx, ms, s.getLAN, s.getLatestLANCreationRequest) + lan, request, err := scopedFindResource(ctx, ms, s.lan, s.getLatestLANCreationRequest) if err != nil { return false, err } @@ -146,8 +146,8 @@ func (s *Service) ReconcileLANDeletion(ctx context.Context, ms *scope.Machine) ( return err == nil, err } -// getLAN tries to retrieve the cluster-related LAN in the data center. -func (s *Service) getLAN(ctx context.Context, ms *scope.Machine) (*sdk.Lan, error) { +// lan tries to retrieve the cluster-related LAN in the data center. +func (s *Service) lan(ctx context.Context, ms *scope.Machine) (*sdk.Lan, error) { depth := int32(2) // for listing the LANs with their number of NICs // check if the LAN exists @@ -410,7 +410,7 @@ func (s *Service) swapNICInFailoverGroup( matchLabels client.MatchingLabels, ) (requeue bool, err error) { log := s.logger.WithName("swapNICInFailoverGroup") - nicID, err := s.getServerNICID(ctx, ms) + nicID, err := s.serverNICID(ctx, ms) if err != nil { return false, err } @@ -470,7 +470,7 @@ func (s *Service) swapNICInFailoverGroup( } func (s *Service) ensureFailoverDeletion(ctx context.Context, ms *scope.Machine) (requeue bool, err error) { - nicID, err := s.getServerNICID(ctx, ms) + nicID, err := s.serverNICID(ctx, ms) if err != nil || nicID == "" { return false, err } @@ -478,8 +478,8 @@ func (s *Service) ensureFailoverDeletion(ctx context.Context, ms *scope.Machine) return s.removeNICFromFailoverGroup(ctx, ms, nicID) } -func (s *Service) getServerNICID(ctx context.Context, ms *scope.Machine) (string, error) { - log := s.logger.WithName("getServerNICID") +func (s *Service) serverNICID(ctx context.Context, ms *scope.Machine) (string, error) { + log := s.logger.WithName("serverNICID") server, err := s.getServer(ctx, ms) if err != nil { if isNotFound(err) { @@ -611,7 +611,7 @@ func (s *Service) retrieveLANFailoverConfig( ) (requeue bool, err error) { log := s.logger.WithName("retrieveLANFailoverConfig") - gotLAN, err := s.getLAN(ctx, ms) + gotLAN, err := s.lan(ctx, ms) if err != nil { return true, err } diff --git a/internal/service/cloud/network_test.go b/internal/service/cloud/network_test.go index 4425e7c8..8154a3e8 100644 --- a/internal/service/cloud/network_test.go +++ b/internal/service/cloud/network_test.go @@ -86,7 +86,7 @@ func (s *lanSuite) TestNetworkDeleteLANSuccessful() { func (s *lanSuite) TestNetworkGetLANSuccessful() { lan := s.exampleLAN() s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{lan}}, nil).Once() - foundLAN, err := s.service.getLAN(s.ctx, s.machineScope) + foundLAN, err := s.service.lan(s.ctx, s.machineScope) s.NoError(err) s.NotNil(foundLAN) s.Equal(lan, *foundLAN) @@ -94,7 +94,7 @@ func (s *lanSuite) TestNetworkGetLANSuccessful() { func (s *lanSuite) TestNetworkGetLANNotFound() { s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{}}, nil).Once() - lan, err := s.service.getLAN(s.ctx, s.machineScope) + lan, err := s.service.lan(s.ctx, s.machineScope) s.NoError(err) s.Nil(lan) } @@ -104,7 +104,7 @@ func (s *lanSuite) TestNetworkGetLAN_ExistingLAN() { s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{lan}}, nil).Once() s.machineScope.IonosMachine.Spec.NetworkID = ptr.To("42") - foundLAN, err := s.service.getLAN(s.ctx, s.machineScope) + foundLAN, err := s.service.lan(s.ctx, s.machineScope) s.NoError(err) s.NotNil(foundLAN) s.Equal(lan, *foundLAN) @@ -115,14 +115,14 @@ func (s *lanSuite) TestNetworkGetLAN_LANIDNotFound() { s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{lan}}, nil).Once() s.machineScope.IonosMachine.Spec.NetworkID = ptr.To("2") - foundLAN, err := s.service.getLAN(s.ctx, s.machineScope) + foundLAN, err := s.service.lan(s.ctx, s.machineScope) s.EqualError(err, "LAN with ID 2 not found") s.Nil(foundLAN) } func (s *lanSuite) TestNetworkGetLANErrorNotUnique() { s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{s.exampleLAN(), s.exampleLAN()}}, nil).Once() - lan, err := s.service.getLAN(s.ctx, s.machineScope) + lan, err := s.service.lan(s.ctx, s.machineScope) s.Error(err) s.Nil(lan) } diff --git a/internal/service/cloud/server.go b/internal/service/cloud/server.go index 7bbda6c5..ae310cd5 100644 --- a/internal/service/cloud/server.go +++ b/internal/service/cloud/server.go @@ -318,7 +318,7 @@ func (s *Service) createServer(ctx context.Context, secret *corev1.Secret, ms *s return errors.New("unable to obtain bootstrap data from secret") } - lan, err := s.getLAN(ctx, ms) + lan, err := s.lan(ctx, ms) if err != nil { return err } From 2850f91ea91b97a526accc613aa366bef6a940cb Mon Sep 17 00:00:00 2001 From: Stefan Scheglmann Date: Mon, 15 Dec 2025 16:02:45 +0100 Subject: [PATCH 05/12] Bump to go 1.25.5 in Dockerfile --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index ec8e7e4f..74dd326e 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build the manager binary -FROM golang:1.25.4 AS builder +FROM golang:1.25.5 AS builder ARG TARGETOS ARG TARGETARCH From f45dcea0b0ad8bddde2e29aca32a036995f7e9a3 Mon Sep 17 00:00:00 2001 From: Stefan Scheglmann Date: Mon, 15 Dec 2025 16:28:06 +0100 Subject: [PATCH 06/12] fix: capture loop variable by value in TestLockerMultipleWaiters The loop variable `i` was captured by reference in the closure, causing all goroutines to write to `results[9]` instead of their respective indices. This fix captures the loop variable by value to ensure each goroutine uses the correct index. --- internal/util/locker/locker_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/util/locker/locker_test.go b/internal/util/locker/locker_test.go index 338afacd..0b3c9b4e 100644 --- a/internal/util/locker/locker_test.go +++ b/internal/util/locker/locker_test.go @@ -214,6 +214,8 @@ func TestLockerMultipleWaiters(t *testing.T) { // Start multiple waiters for i := range numWaiters { + //nolint:copyloopvar // We want to use the loop variable in the goroutine + i := i wg.Go(func() { //nolint:testifylint // Using assert in goroutine per go-require rule assert.NoError(t, l.Lock(context.Background(), "test")) From 0a748eef70d81318afe2105de88e2ed971fdaa27 Mon Sep 17 00:00:00 2001 From: Stefan Scheglmann Date: Fri, 27 Feb 2026 12:54:54 +0100 Subject: [PATCH 07/12] Bump to go 1.25.7 in Dockerfile Co-Authored-By: Claude Sonnet 4.6 (1M context) --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index 74dd326e..ec40d0b6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build the manager binary -FROM golang:1.25.5 AS builder +FROM golang:1.25.7 AS builder ARG TARGETOS ARG TARGETARCH From 1a79a3cc5d8e401d62fe011a8653f24593ab7ab4 Mon Sep 17 00:00:00 2001 From: Gaurav Gahlot Date: Fri, 6 Mar 2026 12:43:58 +0100 Subject: [PATCH 08/12] Bump to Go v1.25.8 in Dockerfile Signed-off-by: Gaurav Gahlot --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index ec40d0b6..6e621b8d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build the manager binary -FROM golang:1.25.7 AS builder +FROM golang:1.25.8 AS builder ARG TARGETOS ARG TARGETARCH From c59fc2f9ac0497a46e7cf84e0215ed8e785206ff Mon Sep 17 00:00:00 2001 From: Gaurav Gahlot Date: Fri, 6 Mar 2026 12:48:35 +0100 Subject: [PATCH 09/12] bump opentelemetry sdk to v1.40.0 Signed-off-by: Gaurav Gahlot --- go.mod | 15 +++++++-------- go.sum | 32 ++++++++++++++++---------------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/go.mod b/go.mod index eb9943f3..c144a909 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/onsi/ginkgo/v2 v2.23.4 github.com/onsi/gomega v1.38.0 github.com/spf13/pflag v1.0.7 - github.com/stretchr/testify v1.10.0 + github.com/stretchr/testify v1.11.1 k8s.io/api v0.30.14 k8s.io/apimachinery v0.30.14 k8s.io/client-go v0.30.14 @@ -93,7 +93,6 @@ require ( github.com/prometheus/client_model v0.6.0 // indirect github.com/prometheus/common v0.45.0 // indirect github.com/prometheus/procfs v0.12.0 // indirect - github.com/rogpeppe/go-internal v1.14.1 // indirect github.com/sagikazarmark/locafero v0.7.0 // indirect github.com/shopspring/decimal v1.3.1 // indirect github.com/sirupsen/logrus v1.9.3 // indirect @@ -106,15 +105,15 @@ require ( github.com/stretchr/objx v0.5.2 // indirect github.com/subosito/gotenv v1.6.0 // indirect github.com/valyala/fastjson v1.6.4 // indirect - go.opentelemetry.io/auto/sdk v1.1.0 // indirect + go.opentelemetry.io/auto/sdk v1.2.1 // indirect go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.59.0 // indirect go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.59.0 // indirect - go.opentelemetry.io/otel v1.34.0 // indirect + go.opentelemetry.io/otel v1.40.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.22.0 // indirect go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.20.0 // indirect - go.opentelemetry.io/otel/metric v1.34.0 // indirect - go.opentelemetry.io/otel/sdk v1.32.0 // indirect - go.opentelemetry.io/otel/trace v1.34.0 // indirect + go.opentelemetry.io/otel/metric v1.40.0 // indirect + go.opentelemetry.io/otel/sdk v1.40.0 // indirect + go.opentelemetry.io/otel/trace v1.40.0 // indirect go.opentelemetry.io/proto/otlp v1.0.0 // indirect go.uber.org/automaxprocs v1.6.0 // indirect go.uber.org/multierr v1.11.0 // indirect @@ -124,7 +123,7 @@ require ( golang.org/x/net v0.41.0 // indirect golang.org/x/oauth2 v0.27.0 // indirect golang.org/x/sync v0.15.0 // indirect - golang.org/x/sys v0.33.0 // indirect + golang.org/x/sys v0.40.0 // indirect golang.org/x/term v0.32.0 // indirect golang.org/x/text v0.26.0 // indirect golang.org/x/time v0.10.0 // indirect diff --git a/go.sum b/go.sum index d783292d..dfa29eb8 100644 --- a/go.sum +++ b/go.sum @@ -264,8 +264,8 @@ github.com/stretchr/testify v1.7.0/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/ github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= github.com/stretchr/testify v1.8.1/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= -github.com/stretchr/testify v1.10.0 h1:Xv5erBjTwe/5IxqUQTdXv5kgmIvbHo3QQyRwhJsOfJA= -github.com/stretchr/testify v1.10.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= +github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= +github.com/stretchr/testify v1.11.1/go.mod h1:wZwfW3scLgRK+23gO65QZefKpKQRnfz6sD981Nm4B6U= github.com/subosito/gotenv v1.6.0 h1:9NlTDc1FTs4qu0DDq7AEtTPNw6SVm7uBMsUCUjABIf8= github.com/subosito/gotenv v1.6.0/go.mod h1:Dk4QP5c2W3ibzajGcXpNraDfq2IrhjMIvMSWPKKo0FU= github.com/tmc/grpc-websocket-proxy v0.0.0-20220101234140-673ab2c3ae75 h1:6fotK7otjonDflCTK0BCfls4SPy3NcCVb5dqqmbRknE= @@ -293,28 +293,28 @@ go.etcd.io/etcd/raft/v3 v3.5.10 h1:cgNAYe7xrsrn/5kXMSaH8kM/Ky8mAdMqGOxyYwpP0LA= go.etcd.io/etcd/raft/v3 v3.5.10/go.mod h1:odD6kr8XQXTy9oQnyMPBOr0TVe+gT0neQhElQ6jbGRc= go.etcd.io/etcd/server/v3 v3.5.10 h1:4NOGyOwD5sUZ22PiWYKmfxqoeh72z6EhYjNosKGLmZg= go.etcd.io/etcd/server/v3 v3.5.10/go.mod h1:gBplPHfs6YI0L+RpGkTQO7buDbHv5HJGG/Bst0/zIPo= -go.opentelemetry.io/auto/sdk v1.1.0 h1:cH53jehLUN6UFLY71z+NDOiNJqDdPRaXzTel0sJySYA= -go.opentelemetry.io/auto/sdk v1.1.0/go.mod h1:3wSPjt5PWp2RhlCcmmOial7AvC4DQqZb7a7wCow3W8A= +go.opentelemetry.io/auto/sdk v1.2.1 h1:jXsnJ4Lmnqd11kwkBV2LgLoFMZKizbCi5fNZ/ipaZ64= +go.opentelemetry.io/auto/sdk v1.2.1/go.mod h1:KRTj+aOaElaLi+wW1kO/DZRXwkF4C5xPbEe3ZiIhN7Y= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.59.0 h1:rgMkmiGfix9vFJDcDi1PK8WEQP4FLQwLDfhp5ZLpFeE= go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.59.0/go.mod h1:ijPqXp5P6IRRByFVVg9DY8P5HkxkHE5ARIa+86aXPf4= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.59.0 h1:CV7UdSGJt/Ao6Gp4CXckLxVRRsRgDHoI8XjbL3PDl8s= go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.59.0/go.mod h1:FRmFuRJfag1IZ2dPkHnEoSFVgTVPUd2qf5Vi69hLb8I= -go.opentelemetry.io/otel v1.34.0 h1:zRLXxLCgL1WyKsPVrgbSdMN4c0FMkDAskSTQP+0hdUY= -go.opentelemetry.io/otel v1.34.0/go.mod h1:OWFPOQ+h4G8xpyjgqo4SxJYdDQ/qmRH+wivy7zzx9oI= +go.opentelemetry.io/otel v1.40.0 h1:oA5YeOcpRTXq6NN7frwmwFR0Cn3RhTVZvXsP4duvCms= +go.opentelemetry.io/otel v1.40.0/go.mod h1:IMb+uXZUKkMXdPddhwAHm6UfOwJyh4ct1ybIlV14J0g= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.22.0 h1:9M3+rhx7kZCIQQhQRYaZCdNu1V73tm4TvXs2ntl98C4= go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.22.0/go.mod h1:noq80iT8rrHP1SfybmPiRGc9dc5M8RPmGvtwo7Oo7tc= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.20.0 h1:gvmNvqrPYovvyRmCSygkUDyL8lC5Tl845MLEwqpxhEU= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc v1.20.0/go.mod h1:vNUq47TGFioo+ffTSnKNdob241vePmtNZnAODKapKd0= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.22.0 h1:FyjCyI9jVEfqhUh2MoSkmolPjfh5fp2hnV0b0irxH4Q= go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp v1.22.0/go.mod h1:hYwym2nDEeZfG/motx0p7L7J1N1vyzIThemQsb4g2qY= -go.opentelemetry.io/otel/metric v1.34.0 h1:+eTR3U0MyfWjRDhmFMxe2SsW64QrZ84AOhvqS7Y+PoQ= -go.opentelemetry.io/otel/metric v1.34.0/go.mod h1:CEDrp0fy2D0MvkXE+dPV7cMi8tWZwX3dmaIhwPOaqHE= -go.opentelemetry.io/otel/sdk v1.32.0 h1:RNxepc9vK59A8XsgZQouW8ue8Gkb4jpWtJm9ge5lEG4= -go.opentelemetry.io/otel/sdk v1.32.0/go.mod h1:LqgegDBjKMmb2GC6/PrTnteJG39I8/vJCAP9LlJXEjU= -go.opentelemetry.io/otel/sdk/metric v1.32.0 h1:rZvFnvmvawYb0alrYkjraqJq0Z4ZUJAiyYCU9snn1CU= -go.opentelemetry.io/otel/sdk/metric v1.32.0/go.mod h1:PWeZlq0zt9YkYAp3gjKZ0eicRYvOh1Gd+X99x6GHpCQ= -go.opentelemetry.io/otel/trace v1.34.0 h1:+ouXS2V8Rd4hp4580a8q23bg0azF2nI8cqLYnC8mh/k= -go.opentelemetry.io/otel/trace v1.34.0/go.mod h1:Svm7lSjQD7kG7KJ/MUHPVXSDGz2OX4h0M2jHBhmSfRE= +go.opentelemetry.io/otel/metric v1.40.0 h1:rcZe317KPftE2rstWIBitCdVp89A2HqjkxR3c11+p9g= +go.opentelemetry.io/otel/metric v1.40.0/go.mod h1:ib/crwQH7N3r5kfiBZQbwrTge743UDc7DTFVZrrXnqc= +go.opentelemetry.io/otel/sdk v1.40.0 h1:KHW/jUzgo6wsPh9At46+h4upjtccTmuZCFAc9OJ71f8= +go.opentelemetry.io/otel/sdk v1.40.0/go.mod h1:Ph7EFdYvxq72Y8Li9q8KebuYUr2KoeyHx0DRMKrYBUE= +go.opentelemetry.io/otel/sdk/metric v1.40.0 h1:mtmdVqgQkeRxHgRv4qhyJduP3fYJRMX4AtAlbuWdCYw= +go.opentelemetry.io/otel/sdk/metric v1.40.0/go.mod h1:4Z2bGMf0KSK3uRjlczMOeMhKU2rhUqdWNoKcYrtcBPg= +go.opentelemetry.io/otel/trace v1.40.0 h1:WA4etStDttCSYuhwvEa8OP8I5EWu24lkOzp+ZYblVjw= +go.opentelemetry.io/otel/trace v1.40.0/go.mod h1:zeAhriXecNGP/s2SEG3+Y8X9ujcJOTqQ5RgdEJcawiA= go.opentelemetry.io/proto/otlp v1.0.0 h1:T0TX0tmXU8a3CbNXzEKGeU5mIVOdf0oykP+u2lIVU/I= go.opentelemetry.io/proto/otlp v1.0.0/go.mod h1:Sy6pihPLfYHkr3NkUbEhGHFhINUSI/v80hjKIs5JXpM= go.uber.org/automaxprocs v1.6.0 h1:O3y2/QNTOdbF+e/dpXNNW7Rx2hZ4sTIPyybbxyNqTUs= @@ -367,8 +367,8 @@ golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.33.0 h1:q3i8TbbEz+JRD9ywIRlyRAQbM0qF7hu24q3teo2hbuw= -golang.org/x/sys v0.33.0/go.mod h1:BJP2sWEmIv4KK5OTEluFJCKSidICx8ciO85XgH3Ak8k= +golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ= +golang.org/x/sys v0.40.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.2.0/go.mod h1:TVmDHMZPmdnySmBfhjOoOdhjzdE1h4u1VwSiw2l1Nuc= From a8c9ed9eb78935596f550719b687d4fcceb1104a Mon Sep 17 00:00:00 2001 From: Stefan Scheglmann Date: Fri, 6 Mar 2026 21:44:32 +0000 Subject: [PATCH 10/12] fix: address PR review comments - Revert function renames (getLAN, getServerNICID) as they were unrelated scope creep - Rewrite locker tests per reviewer feedback: replace channels with bools, remove unnecessary goroutines, use single sync mechanism (synctest.Wait), remove redundant/duplicate tests - Enable usetesting linter to replace removed tenv - Replace context.Background() with t.Context() in all test files Co-Authored-By: Claude Opus 4.6 --- .golangci.yml | 1 + internal/service/cloud/image_test.go | 3 +- internal/service/cloud/ipblock.go | 2 +- internal/service/cloud/network.go | 18 +-- internal/service/cloud/network_test.go | 10 +- internal/service/cloud/server.go | 2 +- internal/util/locker/locker_test.go | 213 +++++-------------------- scope/cluster_test.go | 6 +- scope/machine_test.go | 25 ++- 9 files changed, 72 insertions(+), 208 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 8e540926..7871cfc3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -43,6 +43,7 @@ linters: - tagalign - testifylint - typecheck + - usetesting - unconvert - unparam - unused diff --git a/internal/service/cloud/image_test.go b/internal/service/cloud/image_test.go index 4a353c05..267e5aeb 100644 --- a/internal/service/cloud/image_test.go +++ b/internal/service/cloud/image_test.go @@ -17,7 +17,6 @@ limitations under the License. package cloud import ( - "context" "fmt" "slices" "testing" @@ -184,7 +183,7 @@ func TestFilterImagesByName(t *testing.T) { } func TestLookupImagesBySelector(t *testing.T) { - ctx := context.Background() + ctx := t.Context() ionosClient := clienttest.NewMockClient(t) ionosClient.EXPECT().ListLabels(ctx).Return([]sdk.Label{ // wrong resource type diff --git a/internal/service/cloud/ipblock.go b/internal/service/cloud/ipblock.go index 373814a1..a8df3e7e 100644 --- a/internal/service/cloud/ipblock.go +++ b/internal/service/cloud/ipblock.go @@ -199,7 +199,7 @@ func (s *Service) ReconcileFailoverIPBlockDeletion(ctx context.Context, ms *scop } // Check if IPBlock is still being used - lan, err := s.lan(ctx, ms) + lan, err := s.getLAN(ctx, ms) if err != nil { return false, err } diff --git a/internal/service/cloud/network.go b/internal/service/cloud/network.go index 10c5fbbe..a4af1100 100644 --- a/internal/service/cloud/network.go +++ b/internal/service/cloud/network.go @@ -69,7 +69,7 @@ func (s *Service) ReconcileLAN(ctx context.Context, ms *scope.Machine) (requeue } defer ms.Locker.Unlock(lockKey) - lan, request, err := scopedFindResource(ctx, ms, s.lan, s.getLatestLANCreationRequest) + lan, request, err := scopedFindResource(ctx, ms, s.getLAN, s.getLatestLANCreationRequest) if err != nil { return false, err } @@ -109,7 +109,7 @@ func (s *Service) ReconcileLANDeletion(ctx context.Context, ms *scope.Machine) ( defer ms.Locker.Unlock(lockKey) // Try to retrieve the cluster LAN or even check if it's currently still being created. - lan, request, err := scopedFindResource(ctx, ms, s.lan, s.getLatestLANCreationRequest) + lan, request, err := scopedFindResource(ctx, ms, s.getLAN, s.getLatestLANCreationRequest) if err != nil { return false, err } @@ -146,8 +146,8 @@ func (s *Service) ReconcileLANDeletion(ctx context.Context, ms *scope.Machine) ( return err == nil, err } -// lan tries to retrieve the cluster-related LAN in the data center. -func (s *Service) lan(ctx context.Context, ms *scope.Machine) (*sdk.Lan, error) { +// getLAN tries to retrieve the cluster-related LAN in the data center. +func (s *Service) getLAN(ctx context.Context, ms *scope.Machine) (*sdk.Lan, error) { depth := int32(2) // for listing the LANs with their number of NICs // check if the LAN exists @@ -410,7 +410,7 @@ func (s *Service) swapNICInFailoverGroup( matchLabels client.MatchingLabels, ) (requeue bool, err error) { log := s.logger.WithName("swapNICInFailoverGroup") - nicID, err := s.serverNICID(ctx, ms) + nicID, err := s.getServerNICID(ctx, ms) if err != nil { return false, err } @@ -470,7 +470,7 @@ func (s *Service) swapNICInFailoverGroup( } func (s *Service) ensureFailoverDeletion(ctx context.Context, ms *scope.Machine) (requeue bool, err error) { - nicID, err := s.serverNICID(ctx, ms) + nicID, err := s.getServerNICID(ctx, ms) if err != nil || nicID == "" { return false, err } @@ -478,8 +478,8 @@ func (s *Service) ensureFailoverDeletion(ctx context.Context, ms *scope.Machine) return s.removeNICFromFailoverGroup(ctx, ms, nicID) } -func (s *Service) serverNICID(ctx context.Context, ms *scope.Machine) (string, error) { - log := s.logger.WithName("serverNICID") +func (s *Service) getServerNICID(ctx context.Context, ms *scope.Machine) (string, error) { + log := s.logger.WithName("getServerNICID") server, err := s.getServer(ctx, ms) if err != nil { if isNotFound(err) { @@ -611,7 +611,7 @@ func (s *Service) retrieveLANFailoverConfig( ) (requeue bool, err error) { log := s.logger.WithName("retrieveLANFailoverConfig") - gotLAN, err := s.lan(ctx, ms) + gotLAN, err := s.getLAN(ctx, ms) if err != nil { return true, err } diff --git a/internal/service/cloud/network_test.go b/internal/service/cloud/network_test.go index 8154a3e8..4425e7c8 100644 --- a/internal/service/cloud/network_test.go +++ b/internal/service/cloud/network_test.go @@ -86,7 +86,7 @@ func (s *lanSuite) TestNetworkDeleteLANSuccessful() { func (s *lanSuite) TestNetworkGetLANSuccessful() { lan := s.exampleLAN() s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{lan}}, nil).Once() - foundLAN, err := s.service.lan(s.ctx, s.machineScope) + foundLAN, err := s.service.getLAN(s.ctx, s.machineScope) s.NoError(err) s.NotNil(foundLAN) s.Equal(lan, *foundLAN) @@ -94,7 +94,7 @@ func (s *lanSuite) TestNetworkGetLANSuccessful() { func (s *lanSuite) TestNetworkGetLANNotFound() { s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{}}, nil).Once() - lan, err := s.service.lan(s.ctx, s.machineScope) + lan, err := s.service.getLAN(s.ctx, s.machineScope) s.NoError(err) s.Nil(lan) } @@ -104,7 +104,7 @@ func (s *lanSuite) TestNetworkGetLAN_ExistingLAN() { s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{lan}}, nil).Once() s.machineScope.IonosMachine.Spec.NetworkID = ptr.To("42") - foundLAN, err := s.service.lan(s.ctx, s.machineScope) + foundLAN, err := s.service.getLAN(s.ctx, s.machineScope) s.NoError(err) s.NotNil(foundLAN) s.Equal(lan, *foundLAN) @@ -115,14 +115,14 @@ func (s *lanSuite) TestNetworkGetLAN_LANIDNotFound() { s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{lan}}, nil).Once() s.machineScope.IonosMachine.Spec.NetworkID = ptr.To("2") - foundLAN, err := s.service.lan(s.ctx, s.machineScope) + foundLAN, err := s.service.getLAN(s.ctx, s.machineScope) s.EqualError(err, "LAN with ID 2 not found") s.Nil(foundLAN) } func (s *lanSuite) TestNetworkGetLANErrorNotUnique() { s.mockListLANsCall().Return(&sdk.Lans{Items: &[]sdk.Lan{s.exampleLAN(), s.exampleLAN()}}, nil).Once() - lan, err := s.service.lan(s.ctx, s.machineScope) + lan, err := s.service.getLAN(s.ctx, s.machineScope) s.Error(err) s.Nil(lan) } diff --git a/internal/service/cloud/server.go b/internal/service/cloud/server.go index ae310cd5..7bbda6c5 100644 --- a/internal/service/cloud/server.go +++ b/internal/service/cloud/server.go @@ -318,7 +318,7 @@ func (s *Service) createServer(ctx context.Context, secret *corev1.Secret, ms *s return errors.New("unable to obtain bootstrap data from secret") } - lan, err := s.lan(ctx, ms) + lan, err := s.getLAN(ctx, ms) if err != nil { return err } diff --git a/internal/util/locker/locker_test.go b/internal/util/locker/locker_test.go index 0b3c9b4e..687f72bf 100644 --- a/internal/util/locker/locker_test.go +++ b/internal/util/locker/locker_test.go @@ -18,7 +18,6 @@ package locker import ( "context" - "sync" "testing" "testing/synctest" "time" @@ -27,21 +26,6 @@ import ( "github.com/stretchr/testify/require" ) -// Helper to create a locked locker for testing. -func newLockedLocker(t *testing.T, key string) *Locker { - t.Helper() - l := New() - require.NoError(t, l.Lock(context.Background(), key)) - return l -} - -// Helper to verify lock exists with expected waiters. -func requireLockState(t *testing.T, l *Locker, key string, expectedWaiters int32) { - t.Helper() - require.Contains(t, l.locks, key, "lock %q should exist", key) - require.EqualValues(t, expectedWaiters, l.locks[key].count(), "lock %q should have %d waiters", key, expectedWaiters) -} - func TestNew(t *testing.T) { locker := New() require.NotNil(t, locker) @@ -62,36 +46,28 @@ func TestLockWithCounter(t *testing.T) { func TestLockerLock(t *testing.T) { synctest.Test(t, func(t *testing.T) { l := New() - require.NoError(t, l.Lock(context.Background(), "test")) + require.NoError(t, l.Lock(t.Context(), "test")) lockState := l.locks["test"] - // Verify initial state: lock is held, no waiters require.EqualValues(t, 0, lockState.count()) // Start a goroutine that will wait for the lock - lockAcquired := make(chan struct{}) - go func(t *testing.T) { - assert.NoError(t, l.Lock(context.Background(), "test")) - close(lockAcquired) - }(t) + lockAcquired := false + go func() { + assert.NoError(t, l.Lock(t.Context(), "test")) + lockAcquired = true + }() - // Wait for goroutine to enter waiting state (deterministic with synctest) + // Wait for goroutine to enter waiting state synctest.Wait() require.EqualValues(t, 1, lockState.count(), "should have one waiter") - - // Verify the waiting goroutine hasn't acquired the lock yet - select { - case <-lockAcquired: - t.Fatal("lock should not have been acquired while still held") - default: - } + require.False(t, lockAcquired, "lock should not have been acquired while still held") // Release the lock - waiting goroutine should now acquire it l.Unlock("test") - <-lockAcquired synctest.Wait() - // Verify final state: no waiters + require.True(t, lockAcquired) require.EqualValues(t, 0, lockState.count()) }) } @@ -100,17 +76,14 @@ func TestLockerUnlock(t *testing.T) { synctest.Test(t, func(t *testing.T) { l := New() - require.NoError(t, l.Lock(context.Background(), "test")) + require.NoError(t, l.Lock(t.Context(), "test")) l.Unlock("test") require.PanicsWithValue(t, "no such lock: test", func() { l.Unlock("test") }) - go func() { - assert.NoError(t, l.Lock(context.Background(), "test")) - }() - synctest.Wait() + require.NoError(t, l.Lock(t.Context(), "test")) }) } @@ -118,18 +91,23 @@ func TestLockerConcurrency(t *testing.T) { synctest.Test(t, func(t *testing.T) { l := New() - var wg sync.WaitGroup - for range 10_000 { - wg.Go(func() { - require.NoError(t, l.Lock(context.Background(), "test")) - // If there is a concurrency issue, it will very likely become visible here. + const numWorkers = 10_000 + results := make([]bool, numWorkers) + + for i := range numWorkers { + go func() { + assert.NoError(t, l.Lock(t.Context(), "test")) + results[i] = true l.Unlock("test") - }) + }() } - wg.Wait() synctest.Wait() + for i := range numWorkers { + require.True(t, results[i], "worker %d should have acquired lock", i) + } + // Since everything has unlocked the map should be empty. require.Empty(t, l.locks) }) @@ -137,124 +115,62 @@ func TestLockerConcurrency(t *testing.T) { func TestLockerContextCanceled(t *testing.T) { synctest.Test(t, func(t *testing.T) { - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(t.Context()) cancel() l := New() err := l.Lock(ctx, "test") require.ErrorIs(t, context.Canceled, err) - synctest.Wait() }) } func TestLockerContextDeadlineExceeded(t *testing.T) { synctest.Test(t, func(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Millisecond) + ctx, cancel := context.WithTimeout(t.Context(), 1*time.Millisecond) defer cancel() l := New() require.NoError(t, l.Lock(ctx, "test")) - lockFailed := make(chan struct{}) - go func() { - err := l.Lock(ctx, "test") - assert.ErrorIs(t, context.DeadlineExceeded, err) - close(lockFailed) - }() - - <-lockFailed - synctest.Wait() + err := l.Lock(ctx, "test") + require.ErrorIs(t, context.DeadlineExceeded, err) require.NotPanics(t, func() { l.Unlock("test") }) }) } -// TestLockerMultipleKeysIsolation verifies that locks for different keys don't interfere with each other. func TestLockerMultipleKeysIsolation(t *testing.T) { synctest.Test(t, func(t *testing.T) { l := New() - // Lock key1 - require.NoError(t, l.Lock(context.Background(), "key1")) - requireLockState(t, l, "key1", 0) + require.NoError(t, l.Lock(t.Context(), "key1")) + require.EqualValues(t, 0, l.locks["key1"].count()) // Should be able to lock key2 immediately (different key) - key2Acquired := make(chan struct{}) - go func() { - assert.NoError(t, l.Lock(context.Background(), "key2")) - close(key2Acquired) - }() - - <-key2Acquired - synctest.Wait() + require.NoError(t, l.Lock(t.Context(), "key2")) + require.EqualValues(t, 0, l.locks["key2"].count()) - // Both keys should be locked - requireLockState(t, l, "key1", 0) - requireLockState(t, l, "key2", 0) - - // Unlock both l.Unlock("key1") l.Unlock("key2") synctest.Wait() - // Both locks should be cleaned up require.Empty(t, l.locks) }) } -// TestLockerMultipleWaiters verifies that multiple goroutines can wait for the same lock. -func TestLockerMultipleWaiters(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - l := newLockedLocker(t, "test") - const numWaiters = 10 - - var wg sync.WaitGroup - results := make([]bool, numWaiters) - - // Start multiple waiters - for i := range numWaiters { - //nolint:copyloopvar // We want to use the loop variable in the goroutine - i := i - wg.Go(func() { - //nolint:testifylint // Using assert in goroutine per go-require rule - assert.NoError(t, l.Lock(context.Background(), "test")) - results[i] = true - l.Unlock("test") - }) - } - - // Wait for all goroutines to enter waiting state - synctest.Wait() - require.EqualValues(t, numWaiters, l.locks["test"].count(), "should have %d waiters", numWaiters) - - // Release lock - all waiters should acquire it sequentially - l.Unlock("test") - wg.Wait() - synctest.Wait() - - // All should have succeeded - for i := range numWaiters { - require.True(t, results[i], "waiter %d should have acquired lock", i) - } - - // Lock should be cleaned up - require.Empty(t, l.locks) - }) -} - -// TestLockerContextCanceledWhileWaiting verifies context cancellation that happens while waiting (not before). func TestLockerContextCanceledWhileWaiting(t *testing.T) { synctest.Test(t, func(t *testing.T) { - l := newLockedLocker(t, "test") + l := New() + require.NoError(t, l.Lock(t.Context(), "test")) - ctx, cancel := context.WithCancel(context.Background()) + ctx, cancel := context.WithCancel(t.Context()) - lockCanceled := make(chan struct{}) + lockCanceled := false go func() { err := l.Lock(ctx, "test") assert.ErrorIs(t, err, context.Canceled) - close(lockCanceled) + lockCanceled = true }() // Wait for goroutine to enter waiting state @@ -263,65 +179,14 @@ func TestLockerContextCanceledWhileWaiting(t *testing.T) { // Cancel while waiting cancel() - <-lockCanceled synctest.Wait() - // Lock should still exist (held by main goroutine) - requireLockState(t, l, "test", 0) - - // Unlock and verify cleanup - l.Unlock("test") - synctest.Wait() - require.Empty(t, l.locks) - }) -} - -// TestLockerReacquisition verifies that a lock can be re-acquired after unlock. -func TestLockerReacquisition(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - l := New() - - // Lock, unlock, lock again - require.NoError(t, l.Lock(context.Background(), "test")) - l.Unlock("test") - - require.NoError(t, l.Lock(context.Background(), "test")) - l.Unlock("test") - - // Lock should be cleaned up - require.Empty(t, l.locks) - }) -} - -// TestLockerCleanupAfterAllWaitersCancel verifies that lock is cleaned up when all waiters cancel. -func TestLockerCleanupAfterAllWaitersCancel(t *testing.T) { - synctest.Test(t, func(t *testing.T) { - l := newLockedLocker(t, "test") - - ctx, cancel := context.WithCancel(context.Background()) - - const numWaiters = 5 - var wg sync.WaitGroup - for range numWaiters { - wg.Go(func() { - err := l.Lock(ctx, "test") - require.ErrorIs(t, err, context.Canceled) - }) - } - - // Wait for all goroutines to enter waiting state - synctest.Wait() - require.EqualValues(t, numWaiters, l.locks["test"].count(), "should have %d waiters", numWaiters) - - // Cancel all waiters - cancel() - wg.Wait() - synctest.Wait() + require.True(t, lockCanceled) // Lock should still exist (held by main goroutine) - requireLockState(t, l, "test", 0) + require.EqualValues(t, 0, l.locks["test"].count()) - // Unlock - now lock should be cleaned up + // Unlock and verify cleanup l.Unlock("test") synctest.Wait() require.Empty(t, l.locks) diff --git a/scope/cluster_test.go b/scope/cluster_test.go index 707e344c..ad54485f 100644 --- a/scope/cluster_test.go +++ b/scope/cluster_test.go @@ -181,7 +181,7 @@ func TestCluster_GetControlPlaneEndpointIP(t *testing.T) { }, }, } - got, err := c.GetControlPlaneEndpointIP(context.Background()) + got, err := c.GetControlPlaneEndpointIP(t.Context()) require.NoError(t, err) require.Equal(t, tt.want, got) }) @@ -272,7 +272,7 @@ func TestClusterListMachines(t *testing.T) { require.NoError(t, err) require.NotNil(t, cs) - machines, err := cs.ListMachines(context.Background(), test.searchLabels) + machines, err := cs.ListMachines(t.Context(), test.searchLabels) require.NoError(t, err) require.Len(t, machines, len(test.expectedNames)) @@ -392,7 +392,7 @@ func TestCurrentRequestByDatacenterAccessors(t *testing.T) { lockKey := cluster.currentRequestByDatacenterLockKey() require.Equal(t, "uid/currentRequestByDatacenter", lockKey) - _ = cluster.Locker.Lock(context.Background(), lockKey) + _ = cluster.Locker.Lock(t.Context(), lockKey) require.Empty(t, cluster.IonosCluster.Status.CurrentRequestByDatacenter) cluster.Locker.Unlock(lockKey) } diff --git a/scope/machine_test.go b/scope/machine_test.go index c878bd2d..7287f2fa 100644 --- a/scope/machine_test.go +++ b/scope/machine_test.go @@ -17,7 +17,6 @@ limitations under the License. package scope import ( - "context" "testing" "time" @@ -119,7 +118,7 @@ func TestCountMachinesWithDifferentLabels(t *testing.T) { scope.ClusterScope.Cluster.SetName("test-cluster") count, err := scope.CountMachines( - context.Background(), + t.Context(), client.MatchingLabels{clusterv1.MachineControlPlaneLabel: ""}, ) @@ -133,7 +132,7 @@ func TestCountMachinesWithDifferentLabels(t *testing.T) { cpMachines := []string{"cp-test-1", "cp-test-2", "cp-test-3"} for _, machine := range cpMachines { - err = scope.client.Create(context.Background(), createMachineWithLabels(machine, cpLabels, 0)) + err = scope.client.Create(t.Context(), createMachineWithLabels(machine, cpLabels, 0)) require.NoError(t, err) } @@ -143,16 +142,16 @@ func TestCountMachinesWithDifferentLabels(t *testing.T) { workerMachines := []string{"worker-test-1", "worker-test-2", "worker-test-3"} for _, machine := range workerMachines { - err = scope.client.Create(context.Background(), createMachineWithLabels(machine, workerLabels, 0)) + err = scope.client.Create(t.Context(), createMachineWithLabels(machine, workerLabels, 0)) require.NoError(t, err) } - count, err = scope.CountMachines(context.Background(), nil) + count, err = scope.CountMachines(t.Context(), nil) require.NoError(t, err) require.Equal(t, 6, count) count, err = scope.CountMachines( - context.Background(), + t.Context(), client.MatchingLabels{clusterv1.MachineControlPlaneLabel: ""}, ) @@ -168,7 +167,7 @@ func TestMachineFindLatestMachineWithControlPlaneLabel(t *testing.T) { scope.IonosMachine.SetName("cp-test-1") controlPlaneLabel := client.MatchingLabels{clusterv1.MachineControlPlaneLabel: ""} - latestMachine, err := scope.FindLatestMachine(context.Background(), controlPlaneLabel) + latestMachine, err := scope.FindLatestMachine(t.Context(), controlPlaneLabel) require.NoError(t, err) require.Nil(t, latestMachine) @@ -179,19 +178,19 @@ func TestMachineFindLatestMachineWithControlPlaneLabel(t *testing.T) { cpMachines := []string{"cp-test-1", "cp-test-2", "cp-test-3"} for _, machine := range cpMachines { - err = scope.client.Create(context.Background(), createMachineWithLabels(machine, cpLabels, 0)) + err = scope.client.Create(t.Context(), createMachineWithLabels(machine, cpLabels, 0)) require.NoError(t, err) } - latestMachine, err = scope.FindLatestMachine(context.Background(), controlPlaneLabel) + latestMachine, err = scope.FindLatestMachine(t.Context(), controlPlaneLabel) require.NoError(t, err) require.NotNil(t, latestMachine) require.Equal(t, "cp-test-3", latestMachine.Name) - err = scope.client.Delete(context.Background(), latestMachine) + err = scope.client.Delete(t.Context(), latestMachine) require.NoError(t, err) - latestMachine, err = scope.FindLatestMachine(context.Background(), controlPlaneLabel) + latestMachine, err = scope.FindLatestMachine(t.Context(), controlPlaneLabel) require.NoError(t, err) require.NotNil(t, latestMachine) require.Equal(t, "cp-test-2", latestMachine.Name) @@ -213,13 +212,13 @@ func TestFindLatestMachineMachineIsReceiver(t *testing.T) { cpMachines := []string{"cp-test-2", "cp-test-1"} for index, machine := range cpMachines { err = scope.client.Create( - context.Background(), + t.Context(), createMachineWithLabels(machine, cpLabels, time.Duration(index)*time.Minute), ) require.NoError(t, err) } - latestMachine, err := scope.FindLatestMachine(context.Background(), controlPlaneLabel) + latestMachine, err := scope.FindLatestMachine(t.Context(), controlPlaneLabel) require.NoError(t, err) require.Nil(t, latestMachine) } From d8911c37b3acb7b15be111a4ed7c4a23e205e70d Mon Sep 17 00:00:00 2001 From: Stefan Scheglmann Date: Fri, 6 Mar 2026 21:44:36 +0000 Subject: [PATCH 11/12] chore: add dev tool directories to .gitignore Add .devcontainer, .claude, CLAUDE.md, and .cursor to .gitignore. Co-Authored-By: Claude Opus 4.6 --- .gitignore | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.gitignore b/.gitignore index 5a2ec1b1..2f8a9757 100644 --- a/.gitignore +++ b/.gitignore @@ -33,6 +33,7 @@ envfile .idea .run .vscode +.devcontainer *.swp *.swo *~ @@ -55,3 +56,8 @@ clusterctl-settings.json templates/clusterclass-template-replaced.yaml templates/cluster-template-topology-replaced.yaml output/ + +# ai stuff +.claude +CLAUDE.md +.cursor From 2e066a779ed5e96725c0be9be9ef8bd3a8fa0a25 Mon Sep 17 00:00:00 2001 From: Stefan Scheglmann Date: Mon, 9 Mar 2026 11:05:41 +0100 Subject: [PATCH 12/12] Matthias last comments --- internal/ionoscloud/clienttest/mock_client.go | 1 - internal/util/locker/locker_test.go | 2 -- 2 files changed, 3 deletions(-) diff --git a/internal/ionoscloud/clienttest/mock_client.go b/internal/ionoscloud/clienttest/mock_client.go index fce681fc..afe8b33a 100644 --- a/internal/ionoscloud/clienttest/mock_client.go +++ b/internal/ionoscloud/clienttest/mock_client.go @@ -21,7 +21,6 @@ import ( context "context" ionoscloud "github.com/ionos-cloud/sdk-go/v6" - mock "github.com/stretchr/testify/mock" ) diff --git a/internal/util/locker/locker_test.go b/internal/util/locker/locker_test.go index 687f72bf..bc970520 100644 --- a/internal/util/locker/locker_test.go +++ b/internal/util/locker/locker_test.go @@ -153,7 +153,6 @@ func TestLockerMultipleKeysIsolation(t *testing.T) { l.Unlock("key1") l.Unlock("key2") - synctest.Wait() require.Empty(t, l.locks) }) @@ -188,7 +187,6 @@ func TestLockerContextCanceledWhileWaiting(t *testing.T) { // Unlock and verify cleanup l.Unlock("test") - synctest.Wait() require.Empty(t, l.locks) }) }