From 9ada6c628cd18caf131bc66a525b99a5429f87b3 Mon Sep 17 00:00:00 2001 From: Felix Breuer Date: Thu, 8 Jan 2026 16:07:59 +0100 Subject: [PATCH 1/5] try fetching server before creating Signed-off-by: Felix Breuer --- pkg/provider/core.go | 39 +++++++------ .../core_create_machine_basic_test.go | 10 ++++ .../core_create_machine_config_test.go | 1 + .../core_create_machine_networking_test.go | 6 ++ .../core_create_machine_storage_test.go | 1 + .../core_create_machine_userdata_test.go | 1 + pkg/provider/core_list_machines_test.go | 55 +++---------------- pkg/provider/core_mocks_test.go | 6 +- pkg/provider/sdk_client.go | 25 ++++++++- pkg/provider/stackit_client.go | 2 +- 10 files changed, 72 insertions(+), 74 deletions(-) diff --git a/pkg/provider/core.go b/pkg/provider/core.go index d1b70bf6..eab11df3 100644 --- a/pkg/provider/core.go +++ b/pkg/provider/core.go @@ -18,6 +18,8 @@ import ( "k8s.io/klog/v2" ) +const STACKIT_PROVIDER = "stackit" + // CreateMachine handles a machine creation request by creating a STACKIT server // // This method creates a new server in STACKIT infrastructure based on the ProviderSpec @@ -38,6 +40,12 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR klog.V(2).Infof("Machine creation request has been received for %q", req.Machine.Name) defer klog.V(2).Infof("Machine creation request has been processed for %q", req.Machine.Name) + // Check if incoming provider in the MachineClass is a provider we support + if req.MachineClass.Provider != STACKIT_PROVIDER { + err := fmt.Errorf("requested for Provider '%s', we only support '%s'", req.MachineClass.Provider, STACKIT_PROVIDER) + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + // Decode ProviderSpec from MachineClass providerSpec, err := decodeProviderSpec(req.MachineClass) if err != nil { @@ -177,7 +185,7 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR } // Generate ProviderID in format: stackit:/// - providerID := fmt.Sprintf("stackit://%s/%s", projectID, server.ID) + providerID := fmt.Sprintf("%s://%s/%s", STACKIT_PROVIDER, projectID, server.ID) // NodeName is the machine name (will register with this name in Kubernetes) nodeName := req.Machine.Name @@ -351,7 +359,8 @@ func (p *Provider) ListMachines(ctx context.Context, req *driver.ListMachinesReq } // Call STACKIT API to list all servers - servers, err := p.client.ListServers(ctx, projectID, providerSpec.Region) + labelSelector := "mcm.gardener.cloud/machineclass=" + req.MachineClass.Name + servers, err := p.client.ListServers(ctx, projectID, providerSpec.Region, labelSelector) if err != nil { klog.Errorf("Failed to list servers for MachineClass %q: %v", req.MachineClass.Name, err) return nil, status.Error(codes.Internal, fmt.Sprintf("failed to list servers: %v", err)) @@ -361,26 +370,16 @@ func (p *Provider) ListMachines(ctx context.Context, req *driver.ListMachinesReq // We use the "mcm.gardener.cloud/machineclass" label to identify which servers belong to this MachineClass machineList := make(map[string]string) for _, server := range servers { - // Skip servers without labels - if server.Labels == nil { - continue - } + // Generate ProviderID in format: stackit:/// + providerID := fmt.Sprintf("stackit://%s/%s", projectID, server.ID) - // Check if server has the matching MachineClass label - if machineClassLabel, ok := server.Labels["mcm.gardener.cloud/machineclass"]; ok { - if machineClassLabel == req.MachineClass.Name { - // Generate ProviderID in format: stackit:/// - providerID := fmt.Sprintf("stackit://%s/%s", projectID, server.ID) - - // Get machine name from labels (fallback to server name if not found) - machineName := server.Name - if machineLabel, ok := server.Labels["mcm.gardener.cloud/machine"]; ok { - machineName = machineLabel - } - - machineList[providerID] = machineName - } + // Get machine name from labels (fallback to server name if not found) + machineName := server.Name + if machineLabel, ok := server.Labels["mcm.gardener.cloud/machine"]; ok { + machineName = machineLabel } + + machineList[providerID] = machineName } klog.V(2).Infof("Found %d machines for MachineClass %q", len(machineList), req.MachineClass.Name) diff --git a/pkg/provider/core_create_machine_basic_test.go b/pkg/provider/core_create_machine_basic_test.go index 95e40eda..30ef4a8b 100644 --- a/pkg/provider/core_create_machine_basic_test.go +++ b/pkg/provider/core_create_machine_basic_test.go @@ -60,6 +60,7 @@ var _ = Describe("CreateMachine", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-machine-class", }, + Provider: "stackit", ProviderSpec: runtime.RawExtension{ Raw: providerSpecRaw, }, @@ -148,6 +149,15 @@ var _ = Describe("CreateMachine", func() { Expect(ok).To(BeTrue()) Expect(statusErr.Code()).To(Equal(codes.InvalidArgument)) }) + + It("should fail when Provider is wrong", func() { + req.MachineClass.Provider = "openstack" + + _, err := provider.CreateMachine(ctx, req) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("requested for Provider 'openstack', we only support 'stackit'")) + }) }) Context("with invalid Secret", func() { diff --git a/pkg/provider/core_create_machine_config_test.go b/pkg/provider/core_create_machine_config_test.go index 1b57bb03..7a9c7469 100644 --- a/pkg/provider/core_create_machine_config_test.go +++ b/pkg/provider/core_create_machine_config_test.go @@ -57,6 +57,7 @@ var _ = Describe("CreateMachine", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-machine-class", }, + Provider: "stackit", ProviderSpec: runtime.RawExtension{ Raw: providerSpecRaw, }, diff --git a/pkg/provider/core_create_machine_networking_test.go b/pkg/provider/core_create_machine_networking_test.go index 6d5952cd..94122098 100644 --- a/pkg/provider/core_create_machine_networking_test.go +++ b/pkg/provider/core_create_machine_networking_test.go @@ -68,6 +68,7 @@ var _ = Describe("CreateMachine - Networking", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-machine-class", }, + Provider: "stackit", ProviderSpec: runtime.RawExtension{ Raw: providerSpecRaw, }, @@ -115,6 +116,7 @@ var _ = Describe("CreateMachine - Networking", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-machine-class", }, + Provider: "stackit", ProviderSpec: runtime.RawExtension{ Raw: providerSpecRaw, }, @@ -164,6 +166,7 @@ var _ = Describe("CreateMachine - Networking", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-machine-class", }, + Provider: "stackit", ProviderSpec: runtime.RawExtension{ Raw: providerSpecRaw, }, @@ -206,6 +209,7 @@ var _ = Describe("CreateMachine - Networking", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-machine-class", }, + Provider: "stackit", ProviderSpec: runtime.RawExtension{ Raw: providerSpecRaw, }, @@ -255,6 +259,7 @@ var _ = Describe("CreateMachine - Networking", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-machine-class", }, + Provider: "stackit", ProviderSpec: runtime.RawExtension{ Raw: providerSpecRaw, }, @@ -302,6 +307,7 @@ var _ = Describe("CreateMachine - Networking", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-machine-class", }, + Provider: "stackit", ProviderSpec: runtime.RawExtension{ Raw: providerSpecRaw, }, diff --git a/pkg/provider/core_create_machine_storage_test.go b/pkg/provider/core_create_machine_storage_test.go index aaaee5ac..1f81dd0b 100644 --- a/pkg/provider/core_create_machine_storage_test.go +++ b/pkg/provider/core_create_machine_storage_test.go @@ -57,6 +57,7 @@ var _ = Describe("CreateMachine", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-machine-class", }, + Provider: "stackit", ProviderSpec: runtime.RawExtension{ Raw: providerSpecRaw, }, diff --git a/pkg/provider/core_create_machine_userdata_test.go b/pkg/provider/core_create_machine_userdata_test.go index 049ada46..c0d90c7a 100644 --- a/pkg/provider/core_create_machine_userdata_test.go +++ b/pkg/provider/core_create_machine_userdata_test.go @@ -58,6 +58,7 @@ var _ = Describe("CreateMachine", func() { ObjectMeta: metav1.ObjectMeta{ Name: "test-machine-class", }, + Provider: "stackit", ProviderSpec: runtime.RawExtension{ Raw: providerSpecRaw, }, diff --git a/pkg/provider/core_list_machines_test.go b/pkg/provider/core_list_machines_test.go index 5732ea31..817ca691 100644 --- a/pkg/provider/core_list_machines_test.go +++ b/pkg/provider/core_list_machines_test.go @@ -72,7 +72,9 @@ var _ = Describe("ListMachines", func() { Context("with valid inputs", func() { It("should list machines filtered by MachineClass label", func() { - mockClient.listServersFunc = func(_ context.Context, _, _ string) ([]*Server, error) { + mockClient.listServersFunc = func(_ context.Context, _, _, selector string) ([]*Server, error) { + Expect(selector).To(ContainSubstring("mcm.gardener.cloud/machineclass=test-machine-class")) + return []*Server{ { ID: "server-1", @@ -90,14 +92,6 @@ var _ = Describe("ListMachines", func() { "mcm.gardener.cloud/machine": "machine-2", }, }, - { - ID: "server-3", - Name: "machine-3", - Labels: map[string]string{ - "mcm.gardener.cloud/machineclass": "other-machine-class", - "mcm.gardener.cloud/machine": "machine-3", - }, - }, }, nil } @@ -112,16 +106,8 @@ var _ = Describe("ListMachines", func() { }) It("should return empty list when no servers match", func() { - mockClient.listServersFunc = func(_ context.Context, _, _ string) ([]*Server, error) { - return []*Server{ - { - ID: "server-1", - Name: "machine-1", - Labels: map[string]string{ - "mcm.gardener.cloud/machineclass": "other-machine-class", - }, - }, - }, nil + mockClient.listServersFunc = func(_ context.Context, _, _, _ string) ([]*Server, error) { + return []*Server{}, nil } resp, err := provider.ListMachines(ctx, req) @@ -132,7 +118,7 @@ var _ = Describe("ListMachines", func() { }) It("should return empty list when no servers exist", func() { - mockClient.listServersFunc = func(_ context.Context, _, _ string) ([]*Server, error) { + mockClient.listServersFunc = func(_ context.Context, _, _, _ string) ([]*Server, error) { return []*Server{}, nil } @@ -142,38 +128,11 @@ var _ = Describe("ListMachines", func() { Expect(resp).NotTo(BeNil()) Expect(resp.MachineList).To(BeEmpty()) }) - - It("should handle servers without labels gracefully", func() { - mockClient.listServersFunc = func(_ context.Context, _, _ string) ([]*Server, error) { - return []*Server{ - { - ID: "server-1", - Name: "machine-1", - Labels: nil, // No labels - }, - { - ID: "server-2", - Name: "machine-2", - Labels: map[string]string{ - "mcm.gardener.cloud/machineclass": "test-machine-class", - "mcm.gardener.cloud/machine": "machine-2", - }, - }, - }, nil - } - - resp, err := provider.ListMachines(ctx, req) - - Expect(err).NotTo(HaveOccurred()) - Expect(resp).NotTo(BeNil()) - Expect(resp.MachineList).To(HaveLen(1)) - Expect(resp.MachineList).To(HaveKeyWithValue("stackit://11111111-2222-3333-4444-555555555555/server-2", "machine-2")) - }) }) Context("when STACKIT API fails", func() { It("should return Internal error on API failure", func() { - mockClient.listServersFunc = func(_ context.Context, _, _ string) ([]*Server, error) { + mockClient.listServersFunc = func(_ context.Context, _, _, _ string) ([]*Server, error) { return nil, fmt.Errorf("API connection failed") } diff --git a/pkg/provider/core_mocks_test.go b/pkg/provider/core_mocks_test.go index cd9aafef..0de56d0e 100644 --- a/pkg/provider/core_mocks_test.go +++ b/pkg/provider/core_mocks_test.go @@ -16,7 +16,7 @@ type mockStackitClient struct { createServerFunc func(ctx context.Context, projectID, region string, req *CreateServerRequest) (*Server, error) getServerFunc func(ctx context.Context, projectID, region, serverID string) (*Server, error) deleteServerFunc func(ctx context.Context, projectID, region, serverID string) error - listServersFunc func(ctx context.Context, projectID, region string) ([]*Server, error) + listServersFunc func(ctx context.Context, projectID, region, labelSelector string) ([]*Server, error) } 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, return nil } -func (m *mockStackitClient) ListServers(ctx context.Context, projectID, region string) ([]*Server, error) { +func (m *mockStackitClient) ListServers(ctx context.Context, projectID, region, labelSelector string) ([]*Server, error) { if m.listServersFunc != nil { - return m.listServersFunc(ctx, projectID, region) + return m.listServersFunc(ctx, projectID, region, labelSelector) } return []*Server{}, nil } diff --git a/pkg/provider/sdk_client.go b/pkg/provider/sdk_client.go index 3adcb560..9b6435f5 100644 --- a/pkg/provider/sdk_client.go +++ b/pkg/provider/sdk_client.go @@ -89,6 +89,21 @@ func createIAASClient(serviceAccountKey string) (*iaas.APIClient, error) { // //nolint:gocyclo//TODO:refactor func (c *SdkStackitClient) CreateServer(ctx context.Context, projectID, region string, req *CreateServerRequest) (*Server, error) { + // Check if the server got already created + labelSelector := "mcm.gardener.cloud/machine=" + req.Name + servers, err := c.ListServers(ctx, projectID, region, labelSelector) + if err != nil { + return nil, fmt.Errorf("SDK ListServers with labelSelector: %v failed: %w", labelSelector, err) + } + + if len(servers) > 1 { + return nil, fmt.Errorf("%v servers found for server name %v", len(servers), req.Name) + } + + if len(servers) == 1 { + return servers[0], nil + } + // Convert our request to SDK payload payload := &iaas.CreateServerPayload{ Name: ptr(req.Name), @@ -258,8 +273,14 @@ func (c *SdkStackitClient) DeleteServer(ctx context.Context, projectID, region, } // ListServers lists all servers in a project via STACKIT SDK -func (c *SdkStackitClient) ListServers(ctx context.Context, projectID, region string) ([]*Server, error) { - sdkResponse, err := c.iaasClient.ListServers(ctx, projectID, region).Execute() +func (c *SdkStackitClient) ListServers(ctx context.Context, projectID, region, labelSelector string) ([]*Server, error) { + serverRequest := c.iaasClient.ListServers(ctx, projectID, region) + + if labelSelector != "" { + serverRequest.LabelSelector(labelSelector) + } + + sdkResponse, err := serverRequest.Execute() if err != nil { return nil, fmt.Errorf("SDK ListServers failed: %w", err) } diff --git a/pkg/provider/stackit_client.go b/pkg/provider/stackit_client.go index dc339c74..442ebb3c 100644 --- a/pkg/provider/stackit_client.go +++ b/pkg/provider/stackit_client.go @@ -26,7 +26,7 @@ type StackitClient interface { // DeleteServer deletes a server by ID from STACKIT DeleteServer(ctx context.Context, projectID, region, serverID string) error // ListServers lists all servers in a project - ListServers(ctx context.Context, projectID, region string) ([]*Server, error) + ListServers(ctx context.Context, projectID, region, labelSelector string) ([]*Server, error) } // CreateServerRequest represents the request to create a server From 2828eaa0e26d5888de670ca5195760d08d5c1dd7 Mon Sep 17 00:00:00 2001 From: Felix Breuer Date: Fri, 9 Jan 2026 13:48:46 +0100 Subject: [PATCH 2/5] update --- pkg/provider/sdk_client.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/provider/sdk_client.go b/pkg/provider/sdk_client.go index 9b6435f5..b0f45b85 100644 --- a/pkg/provider/sdk_client.go +++ b/pkg/provider/sdk_client.go @@ -277,7 +277,7 @@ func (c *SdkStackitClient) ListServers(ctx context.Context, projectID, region, l serverRequest := c.iaasClient.ListServers(ctx, projectID, region) if labelSelector != "" { - serverRequest.LabelSelector(labelSelector) + serverRequest = serverRequest.LabelSelector(labelSelector) } sdkResponse, err := serverRequest.Execute() From 37ef886f9453daac750c8ace723c3ce18620cbf7 Mon Sep 17 00:00:00 2001 From: Felix Breuer Date: Mon, 12 Jan 2026 11:23:06 +0100 Subject: [PATCH 3/5] fix test Signed-off-by: Felix Breuer --- pkg/provider/core.go | 8 ++++---- pkg/provider/sdk_client.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/provider/core.go b/pkg/provider/core.go index eab11df3..0864ad2d 100644 --- a/pkg/provider/core.go +++ b/pkg/provider/core.go @@ -18,7 +18,7 @@ import ( "k8s.io/klog/v2" ) -const STACKIT_PROVIDER = "stackit" +const StackitProviderName = "stackit" // CreateMachine handles a machine creation request by creating a STACKIT server // @@ -41,8 +41,8 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR defer klog.V(2).Infof("Machine creation request has been processed for %q", req.Machine.Name) // Check if incoming provider in the MachineClass is a provider we support - if req.MachineClass.Provider != STACKIT_PROVIDER { - err := fmt.Errorf("requested for Provider '%s', we only support '%s'", req.MachineClass.Provider, STACKIT_PROVIDER) + if req.MachineClass.Provider != StackitProviderName { + err := fmt.Errorf("requested for Provider '%s', we only support '%s'", req.MachineClass.Provider, StackitProviderName) return nil, status.Error(codes.InvalidArgument, err.Error()) } @@ -185,7 +185,7 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR } // Generate ProviderID in format: stackit:/// - providerID := fmt.Sprintf("%s://%s/%s", STACKIT_PROVIDER, projectID, server.ID) + providerID := fmt.Sprintf("%s://%s/%s", StackitProviderName, projectID, server.ID) // NodeName is the machine name (will register with this name in Kubernetes) nodeName := req.Machine.Name diff --git a/pkg/provider/sdk_client.go b/pkg/provider/sdk_client.go index b0f45b85..a6e0c5cc 100644 --- a/pkg/provider/sdk_client.go +++ b/pkg/provider/sdk_client.go @@ -87,7 +87,7 @@ func createIAASClient(serviceAccountKey string) (*iaas.APIClient, error) { // CreateServer creates a new server via STACKIT SDK // -//nolint:gocyclo//TODO:refactor +//nolint:gocyclo,funlen//TODO:refactor func (c *SdkStackitClient) CreateServer(ctx context.Context, projectID, region string, req *CreateServerRequest) (*Server, error) { // Check if the server got already created labelSelector := "mcm.gardener.cloud/machine=" + req.Name From 3447ea7a35f750cadb0dd5e62579a8dfbb707490 Mon Sep 17 00:00:00 2001 From: Felix Breuer Date: Wed, 14 Jan 2026 13:48:00 +0100 Subject: [PATCH 4/5] make labels a const Signed-off-by: Felix Breuer --- pkg/provider/core.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/pkg/provider/core.go b/pkg/provider/core.go index 0864ad2d..d4e4fb43 100644 --- a/pkg/provider/core.go +++ b/pkg/provider/core.go @@ -18,7 +18,12 @@ import ( "k8s.io/klog/v2" ) -const StackitProviderName = "stackit" +const ( + StackitProviderName = "stackit" + StackitMachineLabel = "mcm.gardener.cloud/machine" + StackitMachineClassLabel = "mcm.gardener.cloud/machineclass" + StackitRoleLabel = "mcm.gardener.cloud/role" +) // CreateMachine handles a machine creation request by creating a STACKIT server // @@ -76,9 +81,9 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR } } // Add MCM-specific labels for server identification and orphan VM detection - labels["mcm.gardener.cloud/machine"] = req.Machine.Name - labels["mcm.gardener.cloud/machineclass"] = req.MachineClass.Name - labels["mcm.gardener.cloud/role"] = "node" + labels[StackitMachineLabel] = req.Machine.Name + labels[StackitMachineClassLabel] = req.MachineClass.Name + labels[StackitRoleLabel] = "node" // Create server request createReq := &CreateServerRequest{ @@ -359,7 +364,7 @@ func (p *Provider) ListMachines(ctx context.Context, req *driver.ListMachinesReq } // Call STACKIT API to list all servers - labelSelector := "mcm.gardener.cloud/machineclass=" + req.MachineClass.Name + labelSelector := StackitMachineClassLabel + "=" + req.MachineClass.Name servers, err := p.client.ListServers(ctx, projectID, providerSpec.Region, labelSelector) if err != nil { klog.Errorf("Failed to list servers for MachineClass %q: %v", req.MachineClass.Name, err) @@ -375,7 +380,7 @@ func (p *Provider) ListMachines(ctx context.Context, req *driver.ListMachinesReq // Get machine name from labels (fallback to server name if not found) machineName := server.Name - if machineLabel, ok := server.Labels["mcm.gardener.cloud/machine"]; ok { + if machineLabel, ok := server.Labels[StackitMachineLabel]; ok { machineName = machineLabel } From f6bfe9b6aefe7cbf77b66595400f82cc1741d97b Mon Sep 17 00:00:00 2001 From: Felix Breuer Date: Wed, 14 Jan 2026 14:00:56 +0100 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Niclas Schad --- pkg/provider/core.go | 2 +- pkg/provider/sdk_client.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/provider/core.go b/pkg/provider/core.go index d4e4fb43..f36e799d 100644 --- a/pkg/provider/core.go +++ b/pkg/provider/core.go @@ -364,7 +364,7 @@ func (p *Provider) ListMachines(ctx context.Context, req *driver.ListMachinesReq } // Call STACKIT API to list all servers - labelSelector := StackitMachineClassLabel + "=" + req.MachineClass.Name + labelSelector := fmt.Sprintf("%s=%s", StackitMachineClassLabel, req.MachineClass.Name) servers, err := p.client.ListServers(ctx, projectID, providerSpec.Region, labelSelector) if err != nil { klog.Errorf("Failed to list servers for MachineClass %q: %v", req.MachineClass.Name, err) diff --git a/pkg/provider/sdk_client.go b/pkg/provider/sdk_client.go index a6e0c5cc..72079648 100644 --- a/pkg/provider/sdk_client.go +++ b/pkg/provider/sdk_client.go @@ -90,7 +90,7 @@ func createIAASClient(serviceAccountKey string) (*iaas.APIClient, error) { //nolint:gocyclo,funlen//TODO:refactor func (c *SdkStackitClient) CreateServer(ctx context.Context, projectID, region string, req *CreateServerRequest) (*Server, error) { // Check if the server got already created - labelSelector := "mcm.gardener.cloud/machine=" + req.Name + labelSelector := fmt.Sprintf("mcm.gardener.cloud/machine=%s", req.Name) servers, err := c.ListServers(ctx, projectID, region, labelSelector) if err != nil { return nil, fmt.Errorf("SDK ListServers with labelSelector: %v failed: %w", labelSelector, err)