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 diff --git a/.golangci.yml b/.golangci.yml index ae540213..7871cfc3 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -42,8 +42,8 @@ linters: - stylecheck - tagalign - testifylint - - tenv - typecheck + - usetesting - unconvert - unparam - unused 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 dc616068..153b4885 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/ionos-cloud/cluster-api-provider-ionoscloud -go 1.24.13 +go 1.25 require ( github.com/go-logr/logr v1.4.3 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/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/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) diff --git a/internal/util/locker/locker_test.go b/internal/util/locker/locker_test.go index f1bd84ee..bc970520 100644 --- a/internal/util/locker/locker_test.go +++ b/internal/util/locker/locker_test.go @@ -18,28 +18,14 @@ package locker import ( "context" - "sync" "testing" + "testing/synctest" "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -func withTimeout(t *testing.T, f func()) { - t.Helper() - done := make(chan struct{}) - go func() { - f() - close(done) - }() - select { - case <-time.After(1 * time.Second): - t.Fatal("timed out") - case <-done: - } -} - func TestNew(t *testing.T) { locker := New() require.NotNil(t, locker) @@ -58,105 +44,149 @@ 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()) - - chDone := make(chan struct{}) - go func(t *testing.T) { - assert.NoError(t, l.Lock(context.Background(), "test")) - close(chDone) - }(t) - - chWaiting := make(chan struct{}) - go func() { - for range time.Tick(1 * time.Millisecond) { - if lwc.count() == 1 { - close(chWaiting) - break - } - } - }() - - withTimeout(t, func() { - <-chWaiting - }) - - select { - case <-chDone: - t.Fatal("lock should not have returned while it was still held") - default: - } - - l.Unlock("test") + synctest.Test(t, func(t *testing.T) { + l := New() + require.NoError(t, l.Lock(t.Context(), "test")) + lockState := l.locks["test"] + + require.EqualValues(t, 0, lockState.count()) + + // Start a goroutine that will wait for the lock + lockAcquired := false + go func() { + assert.NoError(t, l.Lock(t.Context(), "test")) + lockAcquired = true + }() + + // Wait for goroutine to enter waiting state + synctest.Wait() + require.EqualValues(t, 1, lockState.count(), "should have one waiter") + 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") + synctest.Wait() - withTimeout(t, func() { - <-chDone + require.True(t, lockAcquired) + require.EqualValues(t, 0, lockState.count()) }) - - require.EqualValues(t, 0, lwc.count()) } func TestLockerUnlock(t *testing.T) { - l := New() + synctest.Test(t, func(t *testing.T) { + l := New() - require.NoError(t, l.Lock(context.Background(), "test")) - l.Unlock("test") - - require.PanicsWithValue(t, "no such lock: test", func() { + require.NoError(t, l.Lock(t.Context(), "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") + }) + + require.NoError(t, l.Lock(t.Context(), "test")) }) } func TestLockerConcurrency(t *testing.T) { - l := New() - - var wg sync.WaitGroup - for range 10_000 { - wg.Add(1) - go func(t *testing.T) { - assert.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) - } + synctest.Test(t, func(t *testing.T) { + l := New() + + 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") + }() + } + + synctest.Wait() - withTimeout(t, wg.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) + // 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(t.Context()) + cancel() - l := New() + l := New() - withTimeout(t, func() { err := l.Lock(ctx, "test") require.ErrorIs(t, context.Canceled, err) }) } 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(t.Context(), 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) + + require.NotPanics(t, func() { l.Unlock("test") }) + }) +} + +func TestLockerMultipleKeysIsolation(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + l := New() + + 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) + require.NoError(t, l.Lock(t.Context(), "key2")) + require.EqualValues(t, 0, l.locks["key2"].count()) + + l.Unlock("key1") + l.Unlock("key2") + + require.Empty(t, l.locks) }) +} + +func TestLockerContextCanceledWhileWaiting(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + l := New() + require.NoError(t, l.Lock(t.Context(), "test")) + + ctx, cancel := context.WithCancel(t.Context()) + + lockCanceled := false + go func() { + err := l.Lock(ctx, "test") + assert.ErrorIs(t, err, context.Canceled) + lockCanceled = true + }() + + // Wait for goroutine to enter waiting state + synctest.Wait() + require.EqualValues(t, 1, l.locks["test"].count(), "should have one waiter") - require.NotPanics(t, func() { l.Unlock("test") }) + // Cancel while waiting + cancel() + synctest.Wait() + + require.True(t, lockCanceled) + + // Lock should still exist (held by main goroutine) + require.EqualValues(t, 0, l.locks["test"].count()) + + // Unlock and verify cleanup + l.Unlock("test") + require.Empty(t, l.locks) + }) } diff --git a/scope/cluster_test.go b/scope/cluster_test.go index 0235fd9d..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)) @@ -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() @@ -394,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) }