diff --git a/pkg/provider/core.go b/pkg/provider/core.go index d1b70bf6..f36e799d 100644 --- a/pkg/provider/core.go +++ b/pkg/provider/core.go @@ -18,6 +18,13 @@ import ( "k8s.io/klog/v2" ) +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 // // 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 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 != StackitProviderName { + err := fmt.Errorf("requested for Provider '%s', we only support '%s'", req.MachineClass.Provider, StackitProviderName) + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + // Decode ProviderSpec from MachineClass providerSpec, err := decodeProviderSpec(req.MachineClass) if err != nil { @@ -68,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{ @@ -177,7 +190,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", StackitProviderName, projectID, server.ID) // NodeName is the machine name (will register with this name in Kubernetes) nodeName := req.Machine.Name @@ -351,7 +364,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 := 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) 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 // 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[StackitMachineLabel]; 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..72079648 100644 --- a/pkg/provider/sdk_client.go +++ b/pkg/provider/sdk_client.go @@ -87,8 +87,23 @@ 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 := 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) + } + + 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 = 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