From f6bcfa42d475ab09b5c712a27faed33414052e2f Mon Sep 17 00:00:00 2001
From: Sebastian Widmer <sebastian.widmer@vshn.net>
Date: Wed, 13 Nov 2024 13:26:39 +0100
Subject: [PATCH] Fix filtering servers by cluster id; cloudscale only supports
 single tag filter

Follow up of #13
---
 pkg/machine/actuator.go      | 13 +++++++--
 pkg/machine/actuator_test.go | 55 ++++++++++++++++++++++++++++--------
 2 files changed, 54 insertions(+), 14 deletions(-)

diff --git a/pkg/machine/actuator.go b/pkg/machine/actuator.go
index a23ded9..a9e699d 100644
--- a/pkg/machine/actuator.go
+++ b/pkg/machine/actuator.go
@@ -209,14 +209,21 @@ func (a *Actuator) Delete(ctx context.Context, machine *machinev1beta1.Machine)
 
 func (a *Actuator) getServer(ctx context.Context, sc cloudscale.ServerService, machineCtx machineContext) (*cloudscale.Server, error) {
 	lookupKey := cloudscale.TagMap{
-		machineNameTag:      machineCtx.machine.Name,
-		machineClusterIDTag: machineCtx.clusterId,
+		machineNameTag: machineCtx.machine.Name,
 	}
 
-	ss, err := sc.List(ctx, cloudscale.WithTagFilter(lookupKey))
+	ssa, err := sc.List(ctx, cloudscale.WithTagFilter(lookupKey))
 	if err != nil {
 		return nil, fmt.Errorf("failed to list servers: %w", err)
 	}
+	// The cloudscale API does not support filtering by multiple tags, so we have to filter manually
+	ss := make([]cloudscale.Server, 0, len(ssa))
+	for _, s := range ssa {
+		if tk := s.TaggedResource.Tags[machineClusterIDTag]; tk != "" && tk == machineCtx.clusterId {
+			ss = append(ss, s)
+		}
+	}
+
 	if len(ss) == 0 {
 		return nil, nil
 	}
diff --git a/pkg/machine/actuator_test.go b/pkg/machine/actuator_test.go
index 2a43e39..21a9de7 100644
--- a/pkg/machine/actuator_test.go
+++ b/pkg/machine/actuator_test.go
@@ -128,7 +128,6 @@ func Test_Actuator_Create_ComplexMachineE2E(t *testing.T) {
 			ZonalResourceRequest: cloudscale.ZonalResourceRequest{
 				Zone: providerSpec.Zone,
 			},
-
 			Flavor:       providerSpec.Flavor,
 			Image:        providerSpec.Image,
 			VolumeSizeGB: providerSpec.RootVolumeSizeGB,
@@ -150,6 +149,12 @@ func Test_Actuator_Create_ComplexMachineE2E(t *testing.T) {
 		}),
 	).DoAndReturn(cloudscaleServerFromServerRequest(func(s *cloudscale.Server) {
 		s.UUID = "created-server-uuid"
+		s.TaggedResource = cloudscale.TaggedResource{
+			Tags: cloudscale.TagMap{
+				machineNameTag:      machine.Name,
+				machineClusterIDTag: clusterID,
+			},
+		}
 	}))
 
 	require.NoError(t, actuator.Create(ctx, machine))
@@ -444,6 +449,7 @@ func Test_Actuator_Create_AntiAffinityPools(t *testing.T) {
 
 func Test_Actuator_Exists(t *testing.T) {
 	t.Parallel()
+	const clusterID = "cluster-id"
 
 	tcs := []struct {
 		name    string
@@ -455,6 +461,12 @@ func Test_Actuator_Exists(t *testing.T) {
 			servers: []cloudscale.Server{
 				{
 					Name: "app-test",
+					TaggedResource: cloudscale.TaggedResource{
+						Tags: cloudscale.TagMap{
+							machineNameTag:      "app-test",
+							machineClusterIDTag: clusterID,
+						},
+					},
 				},
 			},
 			exists: true,
@@ -464,14 +476,27 @@ func Test_Actuator_Exists(t *testing.T) {
 			servers: []cloudscale.Server{},
 			exists:  false,
 		},
+		{
+			name: "machine has wrong cluster ID",
+			servers: []cloudscale.Server{
+				{
+					Name: "app-test",
+					TaggedResource: cloudscale.TaggedResource{
+						Tags: cloudscale.TagMap{
+							machineNameTag:      "app-test",
+							machineClusterIDTag: "wrong-cluster-id",
+						},
+					},
+				},
+			},
+			exists: false,
+		},
 	}
 
 	for _, tc := range tcs {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 
-			const clusterID = "cluster-id"
-
 			ctx := context.Background()
 
 			ctrl := gomock.NewController(t)
@@ -504,8 +529,7 @@ func Test_Actuator_Exists(t *testing.T) {
 			actuator := newActuator(c, ss, sgs)
 
 			ss.EXPECT().List(ctx, csTagMatcher{t: t, tags: map[string]string{
-				machineNameTag:      machine.Name,
-				machineClusterIDTag: clusterID,
+				machineNameTag: machine.Name,
 			}}).Return(tc.servers, nil)
 
 			exists, err := actuator.Exists(ctx, machine)
@@ -554,11 +578,16 @@ func Test_Actuator_Update(t *testing.T) {
 	ss.EXPECT().List(ctx, csTagMatcher{
 		t: t,
 		tags: map[string]string{
-			machineNameTag:      machine.Name,
-			machineClusterIDTag: clusterID,
+			machineNameTag: machine.Name,
 		},
 	}).Return([]cloudscale.Server{{
 		UUID: "machine-uuid",
+		TaggedResource: cloudscale.TaggedResource{
+			Tags: cloudscale.TagMap{
+				machineNameTag:      machine.Name,
+				machineClusterIDTag: clusterID,
+			},
+		},
 	}}, nil)
 
 	require.NoError(t, actuator.Update(ctx, machine))
@@ -586,12 +615,17 @@ func Test_Actuator_Delete(t *testing.T) {
 				ss.EXPECT().List(
 					gomock.Any(),
 					csTagMatcher{t: t, tags: map[string]string{
-						machineNameTag:      machine.Name,
-						machineClusterIDTag: clusterID,
+						machineNameTag: machine.Name,
 					}},
 				).Return([]cloudscale.Server{
 					{
 						UUID: "machine-uuid",
+						TaggedResource: cloudscale.TaggedResource{
+							Tags: cloudscale.TagMap{
+								machineNameTag:      machine.Name,
+								machineClusterIDTag: clusterID,
+							},
+						},
 					},
 				}, nil)
 				ss.EXPECT().Delete(
@@ -605,8 +639,7 @@ func Test_Actuator_Delete(t *testing.T) {
 				ss.EXPECT().List(
 					gomock.Any(),
 					csTagMatcher{t: t, tags: map[string]string{
-						machineNameTag:      machine.Name,
-						machineClusterIDTag: clusterID,
+						machineNameTag: machine.Name,
 					}},
 				).Return([]cloudscale.Server{}, nil)
 			},