From 808c66324696aeec1be176273f6fd03e3f044ac3 Mon Sep 17 00:00:00 2001 From: Sebastian Widmer Date: Mon, 11 Nov 2024 11:10:49 +0100 Subject: [PATCH] Include cluster ID tag on cloudscale servers (#13) --- pkg/machine/actuator.go | 42 ++++++++++++++++-------- pkg/machine/actuator_test.go | 62 ++++++++++++++++++++++++++++++------ 2 files changed, 80 insertions(+), 24 deletions(-) diff --git a/pkg/machine/actuator.go b/pkg/machine/actuator.go index 50dc9f9..a23ded9 100644 --- a/pkg/machine/actuator.go +++ b/pkg/machine/actuator.go @@ -21,8 +21,11 @@ import ( ) const ( - antiAffinityTag = "machine-api-provider-cloudscale_appuio_io_antiAffinityKey" - machineNameTag = "machine-api-provider-cloudscale_appuio_io_name" + antiAffinityTag = "machine-api-provider-cloudscale_appuio_io_antiAffinityKey" + machineNameTag = "machine-api-provider-cloudscale_appuio_io_name" + machineClusterIDTag = "machine-api-provider-cloudscale_appuio_io_cluster_id" + + machineClusterIDLabelName = "machine.openshift.io/cluster-api-cluster" ) // Actuator is responsible for performing machine reconciliation. @@ -82,6 +85,7 @@ func (a *Actuator) Create(ctx context.Context, machine *machinev1beta1.Machine) spec.Tags = make(map[string]string) } spec.Tags[machineNameTag] = machine.Name + spec.Tags[machineClusterIDTag] = mctx.clusterId // Null is not allowed for SSH keys in the cloudscale API if spec.SSHKeys == nil { @@ -149,7 +153,7 @@ func (a *Actuator) Exists(ctx context.Context, machine *machinev1beta1.Machine) } sc := a.serverClientFactory(mctx.token) - s, err := a.getServer(ctx, sc, machine) + s, err := a.getServer(ctx, sc, *mctx) return s != nil, err } @@ -161,7 +165,7 @@ func (a *Actuator) Update(ctx context.Context, machine *machinev1beta1.Machine) } sc := a.serverClientFactory(mctx.token) - s, err := a.getServer(ctx, sc, machine) + s, err := a.getServer(ctx, sc, *mctx) if err != nil { return fmt.Errorf("failed to get server %q: %w", machine.Name, err) } @@ -186,7 +190,7 @@ func (a *Actuator) Delete(ctx context.Context, machine *machinev1beta1.Machine) } sc := a.serverClientFactory(mctx.token) - s, err := a.getServer(ctx, sc, machine) + s, err := a.getServer(ctx, sc, *mctx) if err != nil { return fmt.Errorf("failed to get server %q: %w", machine.Name, err) } @@ -203,8 +207,11 @@ func (a *Actuator) Delete(ctx context.Context, machine *machinev1beta1.Machine) return nil } -func (a *Actuator) getServer(ctx context.Context, sc cloudscale.ServerService, machine *machinev1beta1.Machine) (*cloudscale.Server, error) { - lookupKey := cloudscale.TagMap{machineNameTag: machine.Name} +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, + } ss, err := sc.List(ctx, cloudscale.WithTagFilter(lookupKey)) if err != nil { @@ -214,7 +221,7 @@ func (a *Actuator) getServer(ctx context.Context, sc cloudscale.ServerService, m return nil, nil } if len(ss) > 1 { - return nil, fmt.Errorf("found multiple servers with name %q", machine.Name) + return nil, fmt.Errorf("found multiple servers with name %q", machineCtx.machine.Name) } return &ss[0], nil @@ -392,9 +399,10 @@ func cloudscaleServerInterfacesFromProviderSpecInterfaces(interfaces []csv1beta1 } type machineContext struct { - machine *machinev1beta1.Machine - spec csv1beta1.CloudscaleMachineProviderSpec - token string + machine *machinev1beta1.Machine + clusterId string + spec csv1beta1.CloudscaleMachineProviderSpec + token string } func (a *Actuator) getMachineContext(ctx context.Context, machine *machinev1beta1.Machine) (*machineContext, error) { @@ -402,6 +410,11 @@ func (a *Actuator) getMachineContext(ctx context.Context, machine *machinev1beta origMachine := machine.DeepCopy() + clusterId, ok := machine.Labels[machineClusterIDLabelName] + if !ok { + return nil, fmt.Errorf("cluster ID label %q not found on machine %q", machineClusterIDLabelName, machine.Name) + } + spec, err := csv1beta1.ProviderSpecFromRawExtension(machine.Spec.ProviderSpec.Value) if err != nil { return nil, fmt.Errorf("failed to get provider spec from machine %q: %w", machine.Name, err) @@ -423,9 +436,10 @@ func (a *Actuator) getMachineContext(ctx context.Context, machine *machinev1beta } return &machineContext{ - machine: origMachine, - spec: *spec, - token: token, + machine: origMachine, + clusterId: clusterId, + spec: *spec, + token: token, }, nil } diff --git a/pkg/machine/actuator_test.go b/pkg/machine/actuator_test.go index e0b5e5e..2a43e39 100644 --- a/pkg/machine/actuator_test.go +++ b/pkg/machine/actuator_test.go @@ -29,12 +29,17 @@ func Test_Actuator_Create_ComplexMachineE2E(t *testing.T) { ctx := context.Background() + const clusterID = "cluster-id" + ctrl := gomock.NewController(t) defer ctrl.Finish() machine := &machinev1beta1.Machine{ ObjectMeta: metav1.ObjectMeta{ Name: "app-test", + Labels: map[string]string{ + machineClusterIDLabelName: clusterID, + }, }, } providerSpec := csv1beta1.CloudscaleMachineProviderSpec{ @@ -115,7 +120,8 @@ func Test_Actuator_Create_ComplexMachineE2E(t *testing.T) { TaggedResourceRequest: cloudscale.TaggedResourceRequest{ Tags: ptr.To(cloudscale.TagMap{ - machineNameTag: machine.Name, + machineNameTag: machine.Name, + machineClusterIDTag: clusterID, }), }, Zone: providerSpec.Zone, @@ -190,6 +196,8 @@ func Test_Actuator_Create_ComplexMachineE2E(t *testing.T) { func Test_Actuator_Create_AntiAffinityPools(t *testing.T) { const zone = "rma1" + const clusterID = "cluster-id" + tcs := []struct { name string apiMock func(*testing.T, *machinev1beta1.Machine, csv1beta1.CloudscaleMachineProviderSpec, *csmock.MockServerService, *csmock.MockServerGroupService) @@ -229,7 +237,8 @@ func Test_Actuator_Create_AntiAffinityPools(t *testing.T) { }, TaggedResourceRequest: cloudscale.TaggedResourceRequest{ Tags: ptr.To(cloudscale.TagMap{ - machineNameTag: machine.Name, + machineNameTag: machine.Name, + machineClusterIDTag: clusterID, }), }, ServerGroups: []string{newSGUUID}, @@ -266,7 +275,8 @@ func Test_Actuator_Create_AntiAffinityPools(t *testing.T) { }, TaggedResourceRequest: cloudscale.TaggedResourceRequest{ Tags: ptr.To(cloudscale.TagMap{ - machineNameTag: machine.Name, + machineNameTag: machine.Name, + machineClusterIDTag: clusterID, }), }, ServerGroups: []string{existingUUID}, @@ -320,7 +330,8 @@ func Test_Actuator_Create_AntiAffinityPools(t *testing.T) { }, TaggedResourceRequest: cloudscale.TaggedResourceRequest{ Tags: ptr.To(cloudscale.TagMap{ - machineNameTag: machine.Name, + machineNameTag: machine.Name, + machineClusterIDTag: clusterID, }), }, ServerGroups: []string{newSGUUID}, @@ -374,7 +385,8 @@ func Test_Actuator_Create_AntiAffinityPools(t *testing.T) { }, TaggedResourceRequest: cloudscale.TaggedResourceRequest{ Tags: ptr.To(cloudscale.TagMap{ - machineNameTag: machine.Name, + machineNameTag: machine.Name, + machineClusterIDTag: clusterID, }), }, ServerGroups: []string{newSGUUID}, @@ -398,6 +410,9 @@ func Test_Actuator_Create_AntiAffinityPools(t *testing.T) { machine := &machinev1beta1.Machine{ ObjectMeta: metav1.ObjectMeta{ Name: "app-test", + Labels: map[string]string{ + machineClusterIDLabelName: "cluster-id", + }, }, } providerSpec := csv1beta1.CloudscaleMachineProviderSpec{ @@ -455,6 +470,8 @@ func Test_Actuator_Exists(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() + const clusterID = "cluster-id" + ctx := context.Background() ctrl := gomock.NewController(t) @@ -463,6 +480,9 @@ func Test_Actuator_Exists(t *testing.T) { machine := &machinev1beta1.Machine{ ObjectMeta: metav1.ObjectMeta{ Name: "app-test", + Labels: map[string]string{ + machineClusterIDLabelName: clusterID, + }, }, } providerSpec := csv1beta1.CloudscaleMachineProviderSpec{ @@ -483,7 +503,10 @@ func Test_Actuator_Exists(t *testing.T) { sgs := csmock.NewMockServerGroupService(ctrl) actuator := newActuator(c, ss, sgs) - ss.EXPECT().List(ctx, csTagMatcher{t: t, tags: map[string]string{machineNameTag: machine.Name}}).Return(tc.servers, nil) + ss.EXPECT().List(ctx, csTagMatcher{t: t, tags: map[string]string{ + machineNameTag: machine.Name, + machineClusterIDTag: clusterID, + }}).Return(tc.servers, nil) exists, err := actuator.Exists(ctx, machine) require.NoError(t, err) @@ -497,12 +520,17 @@ func Test_Actuator_Update(t *testing.T) { ctx := context.Background() + const clusterID = "cluster-id" + ctrl := gomock.NewController(t) defer ctrl.Finish() machine := &machinev1beta1.Machine{ ObjectMeta: metav1.ObjectMeta{ Name: "app-test", + Labels: map[string]string{ + machineClusterIDLabelName: clusterID, + }, }, } providerSpec := csv1beta1.CloudscaleMachineProviderSpec{ @@ -524,8 +552,11 @@ func Test_Actuator_Update(t *testing.T) { actuator := newActuator(c, ss, sgs) ss.EXPECT().List(ctx, csTagMatcher{ - t: t, - tags: map[string]string{machineNameTag: machine.Name}, + t: t, + tags: map[string]string{ + machineNameTag: machine.Name, + machineClusterIDTag: clusterID, + }, }).Return([]cloudscale.Server{{ UUID: "machine-uuid", }}, nil) @@ -542,6 +573,8 @@ func Test_Actuator_Update(t *testing.T) { func Test_Actuator_Delete(t *testing.T) { t.Parallel() + const clusterID = "cluster-id" + tcs := []struct { name string apiMock func(*testing.T, *machinev1beta1.Machine, *csmock.MockServerService, *csmock.MockServerGroupService) @@ -552,7 +585,10 @@ func Test_Actuator_Delete(t *testing.T) { apiMock: func(t *testing.T, machine *machinev1beta1.Machine, ss *csmock.MockServerService, sgs *csmock.MockServerGroupService) { ss.EXPECT().List( gomock.Any(), - csTagMatcher{t: t, tags: map[string]string{machineNameTag: machine.Name}}, + csTagMatcher{t: t, tags: map[string]string{ + machineNameTag: machine.Name, + machineClusterIDTag: clusterID, + }}, ).Return([]cloudscale.Server{ { UUID: "machine-uuid", @@ -568,7 +604,10 @@ func Test_Actuator_Delete(t *testing.T) { apiMock: func(t *testing.T, machine *machinev1beta1.Machine, ss *csmock.MockServerService, sgs *csmock.MockServerGroupService) { ss.EXPECT().List( gomock.Any(), - csTagMatcher{t: t, tags: map[string]string{machineNameTag: machine.Name}}, + csTagMatcher{t: t, tags: map[string]string{ + machineNameTag: machine.Name, + machineClusterIDTag: clusterID, + }}, ).Return([]cloudscale.Server{}, nil) }, }, @@ -586,6 +625,9 @@ func Test_Actuator_Delete(t *testing.T) { machine := &machinev1beta1.Machine{ ObjectMeta: metav1.ObjectMeta{ Name: "app-test", + Labels: map[string]string{ + machineClusterIDLabelName: clusterID, + }, }, } providerSpec := csv1beta1.CloudscaleMachineProviderSpec{