Skip to content

Commit f69b95a

Browse files
breuerfelixnschad
breuerfelix
and
nschad
authored
try fetching server before creating (#78)
* try fetching server before creating Signed-off-by: Felix Breuer <f.breuer94@gmail.com> * update * fix test Signed-off-by: Felix Breuer <f.breuer94@gmail.com> * make labels a const Signed-off-by: Felix Breuer <f.breuer94@gmail.com> * Apply suggestions from code review Co-authored-by: Niclas Schad <niclas.schad@gmail.com> --------- Signed-off-by: Felix Breuer <f.breuer94@gmail.com> Co-authored-by: Niclas Schad <niclas.schad@gmail.com>
1 parent 83087ba commit f69b95a

10 files changed

+81
-78
lines changed

‎pkg/provider/core.go‎

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,13 @@ import (
1818
"k8s.io/klog/v2"
1919
)
2020

21+
const (
22+
StackitProviderName = "stackit"
23+
StackitMachineLabel = "mcm.gardener.cloud/machine"
24+
StackitMachineClassLabel = "mcm.gardener.cloud/machineclass"
25+
StackitRoleLabel = "mcm.gardener.cloud/role"
26+
)
27+
2128
// CreateMachine handles a machine creation request by creating a STACKIT server
2229
//
2330
// This method creates a new server in STACKIT infrastructure based on the ProviderSpec
@@ -38,6 +45,12 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR
3845
klog.V(2).Infof("Machine creation request has been received for %q", req.Machine.Name)
3946
defer klog.V(2).Infof("Machine creation request has been processed for %q", req.Machine.Name)
4047

48+
// Check if incoming provider in the MachineClass is a provider we support
49+
if req.MachineClass.Provider != StackitProviderName {
50+
err := fmt.Errorf("requested for Provider '%s', we only support '%s'", req.MachineClass.Provider, StackitProviderName)
51+
return nil, status.Error(codes.InvalidArgument, err.Error())
52+
}
53+
4154
// Decode ProviderSpec from MachineClass
4255
providerSpec, err := decodeProviderSpec(req.MachineClass)
4356
if err != nil {
@@ -68,9 +81,9 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR
6881
}
6982
}
7083
// Add MCM-specific labels for server identification and orphan VM detection
71-
labels["mcm.gardener.cloud/machine"] = req.Machine.Name
72-
labels["mcm.gardener.cloud/machineclass"] = req.MachineClass.Name
73-
labels["mcm.gardener.cloud/role"] = "node"
84+
labels[StackitMachineLabel] = req.Machine.Name
85+
labels[StackitMachineClassLabel] = req.MachineClass.Name
86+
labels[StackitRoleLabel] = "node"
7487

7588
// Create server request
7689
createReq := &CreateServerRequest{
@@ -177,7 +190,7 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR
177190
}
178191

179192
// Generate ProviderID in format: stackit://<projectId>/<serverId>
180-
providerID := fmt.Sprintf("stackit://%s/%s", projectID, server.ID)
193+
providerID := fmt.Sprintf("%s://%s/%s", StackitProviderName, projectID, server.ID)
181194

182195
// NodeName is the machine name (will register with this name in Kubernetes)
183196
nodeName := req.Machine.Name
@@ -351,7 +364,8 @@ func (p *Provider) ListMachines(ctx context.Context, req *driver.ListMachinesReq
351364
}
352365

353366
// Call STACKIT API to list all servers
354-
servers, err := p.client.ListServers(ctx, projectID, providerSpec.Region)
367+
labelSelector := fmt.Sprintf("%s=%s", StackitMachineClassLabel, req.MachineClass.Name)
368+
servers, err := p.client.ListServers(ctx, projectID, providerSpec.Region, labelSelector)
355369
if err != nil {
356370
klog.Errorf("Failed to list servers for MachineClass %q: %v", req.MachineClass.Name, err)
357371
return nil, status.Error(codes.Internal, fmt.Sprintf("failed to list servers: %v", err))
@@ -361,26 +375,16 @@ func (p *Provider) ListMachines(ctx context.Context, req *driver.ListMachinesReq
361375
// We use the "mcm.gardener.cloud/machineclass" label to identify which servers belong to this MachineClass
362376
machineList := make(map[string]string)
363377
for _, server := range servers {
364-
// Skip servers without labels
365-
if server.Labels == nil {
366-
continue
367-
}
378+
// Generate ProviderID in format: stackit://<projectId>/<serverId>
379+
providerID := fmt.Sprintf("stackit://%s/%s", projectID, server.ID)
368380

369-
// Check if server has the matching MachineClass label
370-
if machineClassLabel, ok := server.Labels["mcm.gardener.cloud/machineclass"]; ok {
371-
if machineClassLabel == req.MachineClass.Name {
372-
// Generate ProviderID in format: stackit://<projectId>/<serverId>
373-
providerID := fmt.Sprintf("stackit://%s/%s", projectID, server.ID)
374-
375-
// Get machine name from labels (fallback to server name if not found)
376-
machineName := server.Name
377-
if machineLabel, ok := server.Labels["mcm.gardener.cloud/machine"]; ok {
378-
machineName = machineLabel
379-
}
380-
381-
machineList[providerID] = machineName
382-
}
381+
// Get machine name from labels (fallback to server name if not found)
382+
machineName := server.Name
383+
if machineLabel, ok := server.Labels[StackitMachineLabel]; ok {
384+
machineName = machineLabel
383385
}
386+
387+
machineList[providerID] = machineName
384388
}
385389

386390
klog.V(2).Infof("Found %d machines for MachineClass %q", len(machineList), req.MachineClass.Name)

‎pkg/provider/core_create_machine_basic_test.go‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ var _ = Describe("CreateMachine", func() {
6060
ObjectMeta: metav1.ObjectMeta{
6161
Name: "test-machine-class",
6262
},
63+
Provider: "stackit",
6364
ProviderSpec: runtime.RawExtension{
6465
Raw: providerSpecRaw,
6566
},
@@ -148,6 +149,15 @@ var _ = Describe("CreateMachine", func() {
148149
Expect(ok).To(BeTrue())
149150
Expect(statusErr.Code()).To(Equal(codes.InvalidArgument))
150151
})
152+
153+
It("should fail when Provider is wrong", func() {
154+
req.MachineClass.Provider = "openstack"
155+
156+
_, err := provider.CreateMachine(ctx, req)
157+
158+
Expect(err).To(HaveOccurred())
159+
Expect(err.Error()).To(ContainSubstring("requested for Provider 'openstack', we only support 'stackit'"))
160+
})
151161
})
152162

153163
Context("with invalid Secret", func() {

‎pkg/provider/core_create_machine_config_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ var _ = Describe("CreateMachine", func() {
5757
ObjectMeta: metav1.ObjectMeta{
5858
Name: "test-machine-class",
5959
},
60+
Provider: "stackit",
6061
ProviderSpec: runtime.RawExtension{
6162
Raw: providerSpecRaw,
6263
},

‎pkg/provider/core_create_machine_networking_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ var _ = Describe("CreateMachine - Networking", func() {
6868
ObjectMeta: metav1.ObjectMeta{
6969
Name: "test-machine-class",
7070
},
71+
Provider: "stackit",
7172
ProviderSpec: runtime.RawExtension{
7273
Raw: providerSpecRaw,
7374
},
@@ -115,6 +116,7 @@ var _ = Describe("CreateMachine - Networking", func() {
115116
ObjectMeta: metav1.ObjectMeta{
116117
Name: "test-machine-class",
117118
},
119+
Provider: "stackit",
118120
ProviderSpec: runtime.RawExtension{
119121
Raw: providerSpecRaw,
120122
},
@@ -164,6 +166,7 @@ var _ = Describe("CreateMachine - Networking", func() {
164166
ObjectMeta: metav1.ObjectMeta{
165167
Name: "test-machine-class",
166168
},
169+
Provider: "stackit",
167170
ProviderSpec: runtime.RawExtension{
168171
Raw: providerSpecRaw,
169172
},
@@ -206,6 +209,7 @@ var _ = Describe("CreateMachine - Networking", func() {
206209
ObjectMeta: metav1.ObjectMeta{
207210
Name: "test-machine-class",
208211
},
212+
Provider: "stackit",
209213
ProviderSpec: runtime.RawExtension{
210214
Raw: providerSpecRaw,
211215
},
@@ -255,6 +259,7 @@ var _ = Describe("CreateMachine - Networking", func() {
255259
ObjectMeta: metav1.ObjectMeta{
256260
Name: "test-machine-class",
257261
},
262+
Provider: "stackit",
258263
ProviderSpec: runtime.RawExtension{
259264
Raw: providerSpecRaw,
260265
},
@@ -302,6 +307,7 @@ var _ = Describe("CreateMachine - Networking", func() {
302307
ObjectMeta: metav1.ObjectMeta{
303308
Name: "test-machine-class",
304309
},
310+
Provider: "stackit",
305311
ProviderSpec: runtime.RawExtension{
306312
Raw: providerSpecRaw,
307313
},

‎pkg/provider/core_create_machine_storage_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ var _ = Describe("CreateMachine", func() {
5757
ObjectMeta: metav1.ObjectMeta{
5858
Name: "test-machine-class",
5959
},
60+
Provider: "stackit",
6061
ProviderSpec: runtime.RawExtension{
6162
Raw: providerSpecRaw,
6263
},

‎pkg/provider/core_create_machine_userdata_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ var _ = Describe("CreateMachine", func() {
5858
ObjectMeta: metav1.ObjectMeta{
5959
Name: "test-machine-class",
6060
},
61+
Provider: "stackit",
6162
ProviderSpec: runtime.RawExtension{
6263
Raw: providerSpecRaw,
6364
},

‎pkg/provider/core_list_machines_test.go‎

Lines changed: 7 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ var _ = Describe("ListMachines", func() {
7272

7373
Context("with valid inputs", func() {
7474
It("should list machines filtered by MachineClass label", func() {
75-
mockClient.listServersFunc = func(_ context.Context, _, _ string) ([]*Server, error) {
75+
mockClient.listServersFunc = func(_ context.Context, _, _, selector string) ([]*Server, error) {
76+
Expect(selector).To(ContainSubstring("mcm.gardener.cloud/machineclass=test-machine-class"))
77+
7678
return []*Server{
7779
{
7880
ID: "server-1",
@@ -90,14 +92,6 @@ var _ = Describe("ListMachines", func() {
9092
"mcm.gardener.cloud/machine": "machine-2",
9193
},
9294
},
93-
{
94-
ID: "server-3",
95-
Name: "machine-3",
96-
Labels: map[string]string{
97-
"mcm.gardener.cloud/machineclass": "other-machine-class",
98-
"mcm.gardener.cloud/machine": "machine-3",
99-
},
100-
},
10195
}, nil
10296
}
10397

@@ -112,16 +106,8 @@ var _ = Describe("ListMachines", func() {
112106
})
113107

114108
It("should return empty list when no servers match", func() {
115-
mockClient.listServersFunc = func(_ context.Context, _, _ string) ([]*Server, error) {
116-
return []*Server{
117-
{
118-
ID: "server-1",
119-
Name: "machine-1",
120-
Labels: map[string]string{
121-
"mcm.gardener.cloud/machineclass": "other-machine-class",
122-
},
123-
},
124-
}, nil
109+
mockClient.listServersFunc = func(_ context.Context, _, _, _ string) ([]*Server, error) {
110+
return []*Server{}, nil
125111
}
126112

127113
resp, err := provider.ListMachines(ctx, req)
@@ -132,7 +118,7 @@ var _ = Describe("ListMachines", func() {
132118
})
133119

134120
It("should return empty list when no servers exist", func() {
135-
mockClient.listServersFunc = func(_ context.Context, _, _ string) ([]*Server, error) {
121+
mockClient.listServersFunc = func(_ context.Context, _, _, _ string) ([]*Server, error) {
136122
return []*Server{}, nil
137123
}
138124

@@ -142,38 +128,11 @@ var _ = Describe("ListMachines", func() {
142128
Expect(resp).NotTo(BeNil())
143129
Expect(resp.MachineList).To(BeEmpty())
144130
})
145-
146-
It("should handle servers without labels gracefully", func() {
147-
mockClient.listServersFunc = func(_ context.Context, _, _ string) ([]*Server, error) {
148-
return []*Server{
149-
{
150-
ID: "server-1",
151-
Name: "machine-1",
152-
Labels: nil, // No labels
153-
},
154-
{
155-
ID: "server-2",
156-
Name: "machine-2",
157-
Labels: map[string]string{
158-
"mcm.gardener.cloud/machineclass": "test-machine-class",
159-
"mcm.gardener.cloud/machine": "machine-2",
160-
},
161-
},
162-
}, nil
163-
}
164-
165-
resp, err := provider.ListMachines(ctx, req)
166-
167-
Expect(err).NotTo(HaveOccurred())
168-
Expect(resp).NotTo(BeNil())
169-
Expect(resp.MachineList).To(HaveLen(1))
170-
Expect(resp.MachineList).To(HaveKeyWithValue("stackit://11111111-2222-3333-4444-555555555555/server-2", "machine-2"))
171-
})
172131
})
173132

174133
Context("when STACKIT API fails", func() {
175134
It("should return Internal error on API failure", func() {
176-
mockClient.listServersFunc = func(_ context.Context, _, _ string) ([]*Server, error) {
135+
mockClient.listServersFunc = func(_ context.Context, _, _, _ string) ([]*Server, error) {
177136
return nil, fmt.Errorf("API connection failed")
178137
}
179138

‎pkg/provider/core_mocks_test.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ type mockStackitClient struct {
1616
createServerFunc func(ctx context.Context, projectID, region string, req *CreateServerRequest) (*Server, error)
1717
getServerFunc func(ctx context.Context, projectID, region, serverID string) (*Server, error)
1818
deleteServerFunc func(ctx context.Context, projectID, region, serverID string) error
19-
listServersFunc func(ctx context.Context, projectID, region string) ([]*Server, error)
19+
listServersFunc func(ctx context.Context, projectID, region, labelSelector string) ([]*Server, error)
2020
}
2121

2222
func (m *mockStackitClient) CreateServer(ctx context.Context, projectID, region string, req *CreateServerRequest) (*Server, error) {
@@ -48,9 +48,9 @@ func (m *mockStackitClient) DeleteServer(ctx context.Context, projectID, region,
4848
return nil
4949
}
5050

51-
func (m *mockStackitClient) ListServers(ctx context.Context, projectID, region string) ([]*Server, error) {
51+
func (m *mockStackitClient) ListServers(ctx context.Context, projectID, region, labelSelector string) ([]*Server, error) {
5252
if m.listServersFunc != nil {
53-
return m.listServersFunc(ctx, projectID, region)
53+
return m.listServersFunc(ctx, projectID, region, labelSelector)
5454
}
5555
return []*Server{}, nil
5656
}

‎pkg/provider/sdk_client.go‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,23 @@ func createIAASClient(serviceAccountKey string) (*iaas.APIClient, error) {
8787

8888
// CreateServer creates a new server via STACKIT SDK
8989
//
90-
//nolint:gocyclo//TODO:refactor
90+
//nolint:gocyclo,funlen//TODO:refactor
9191
func (c *SdkStackitClient) CreateServer(ctx context.Context, projectID, region string, req *CreateServerRequest) (*Server, error) {
92+
// Check if the server got already created
93+
labelSelector := fmt.Sprintf("mcm.gardener.cloud/machine=%s", req.Name)
94+
servers, err := c.ListServers(ctx, projectID, region, labelSelector)
95+
if err != nil {
96+
return nil, fmt.Errorf("SDK ListServers with labelSelector: %v failed: %w", labelSelector, err)
97+
}
98+
99+
if len(servers) > 1 {
100+
return nil, fmt.Errorf("%v servers found for server name %v", len(servers), req.Name)
101+
}
102+
103+
if len(servers) == 1 {
104+
return servers[0], nil
105+
}
106+
92107
// Convert our request to SDK payload
93108
payload := &iaas.CreateServerPayload{
94109
Name: ptr(req.Name),
@@ -258,8 +273,14 @@ func (c *SdkStackitClient) DeleteServer(ctx context.Context, projectID, region,
258273
}
259274

260275
// ListServers lists all servers in a project via STACKIT SDK
261-
func (c *SdkStackitClient) ListServers(ctx context.Context, projectID, region string) ([]*Server, error) {
262-
sdkResponse, err := c.iaasClient.ListServers(ctx, projectID, region).Execute()
276+
func (c *SdkStackitClient) ListServers(ctx context.Context, projectID, region, labelSelector string) ([]*Server, error) {
277+
serverRequest := c.iaasClient.ListServers(ctx, projectID, region)
278+
279+
if labelSelector != "" {
280+
serverRequest = serverRequest.LabelSelector(labelSelector)
281+
}
282+
283+
sdkResponse, err := serverRequest.Execute()
263284
if err != nil {
264285
return nil, fmt.Errorf("SDK ListServers failed: %w", err)
265286
}

‎pkg/provider/stackit_client.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ type StackitClient interface {
2626
// DeleteServer deletes a server by ID from STACKIT
2727
DeleteServer(ctx context.Context, projectID, region, serverID string) error
2828
// ListServers lists all servers in a project
29-
ListServers(ctx context.Context, projectID, region string) ([]*Server, error)
29+
ListServers(ctx context.Context, projectID, region, labelSelector string) ([]*Server, error)
3030
}
3131

3232
// CreateServerRequest represents the request to create a server

0 commit comments

Comments
 (0)