Skip to content

Commit

Permalink
Include cluster ID tag on cloudscale servers (#13)
Browse files Browse the repository at this point in the history
  • Loading branch information
bastjan authored Nov 11, 2024
1 parent a201479 commit 808c663
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 24 deletions.
42 changes: 28 additions & 14 deletions pkg/machine/actuator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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 {
Expand All @@ -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
Expand Down Expand Up @@ -392,16 +399,22 @@ 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) {
const tokenKey = "token"

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)
Expand All @@ -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
}

Expand Down
62 changes: 52 additions & 10 deletions pkg/machine/actuator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand Down Expand Up @@ -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},
Expand All @@ -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{
Expand Down Expand Up @@ -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)
Expand All @@ -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{
Expand All @@ -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)
Expand All @@ -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{
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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",
Expand All @@ -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)
},
},
Expand All @@ -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{
Expand Down

0 comments on commit 808c663

Please sign in to comment.