From 7e0a67c348ce30d330b388eaf06508fd0faf8e38 Mon Sep 17 00:00:00 2001 From: Maximilian Geberl Date: Thu, 22 Jan 2026 10:54:51 +0100 Subject: [PATCH 1/5] WIP # Conflicts: # pkg/client/sdk.go # pkg/client/stackit.go # pkg/provider/core.go --- pkg/client/sdk.go | 48 ++++++++++++++++++++++++++++++++++++++++--- pkg/client/stackit.go | 5 +++++ 2 files changed, 50 insertions(+), 3 deletions(-) diff --git a/pkg/client/sdk.go b/pkg/client/sdk.go index dea59f27..05582a60 100644 --- a/pkg/client/sdk.go +++ b/pkg/client/sdk.go @@ -1,4 +1,8 @@ -package client +// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors +// +// SPDX-License-Identifier: Apache-2.0 + +package provider import ( "context" @@ -7,10 +11,9 @@ import ( "os" "strings" - "github.com/stackitcloud/machine-controller-manager-provider-stackit/pkg/metrics" "github.com/stackitcloud/stackit-sdk-go/core/config" "github.com/stackitcloud/stackit-sdk-go/core/oapierror" - iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" + "github.com/stackitcloud/stackit-sdk-go/services/iaas" ) // SdkStackitClient is an SDK implementation of StackitClient @@ -38,6 +41,7 @@ func NewStackitClient(serviceAccountKey string) (*SdkStackitClient, error) { var ( // ErrServerNotFound indicates the server was not found (404) ErrServerNotFound = errors.New("server not found") + ErrNicNotFound = errors.New("nic not found") ) // createIAASClient creates a new STACKIT SDK IAAS API client @@ -296,6 +300,36 @@ func (c *SdkStackitClient) GetNICsForServer(ctx context.Context, projectID, regi return nics, nil } +func (c *SdkStackitClient) ListNICs(ctx context.Context, projectID, region, networkID string) ([]*NIC, error) { + res, err := c.iaasClient.ListNics(ctx, projectID, region, networkID).Execute() + if err != nil { + return nil, fmt.Errorf("SDK ListServerNICs failed: %w", err) + } + + if res.Items == nil { + return []*NIC{}, nil + } + + nics := make([]*NIC, 0) + for _, nic := range *res.Items { + nics = append(nics, convertSDKNICtoNIC(&nic)) + } + + return nics, nil +} + +func (c *SdkStackitClient) DeleteNIC(ctx context.Context, projectID, region, networkID, nicID string) error { + err := c.iaasClient.DeleteNic(ctx, projectID, region, networkID, nicID).Execute() + if err != nil { + // Check if error is 404 Not Found - this is OK (idempotent) + if isNotFoundError(err) { + return fmt.Errorf("%w: %v", ErrNicNotFound, err) + } + return fmt.Errorf("SDK DeleteNic failed: %w", err) + } + return nil +} + func (c *SdkStackitClient) UpdateNIC(ctx context.Context, projectID, region, networkID, nicID string, allowedAddresses []string) (*NIC, error) { addresses := make([]iaas.AllowedAddressesInner, len(allowedAddresses)) @@ -337,6 +371,7 @@ func convertSDKNICtoNIC(nic *iaas.NIC) *NIC { AllowedAddresses: addresses, IPv4: nic.GetIpv4(), IPv6: nic.GetIpv6(), + Name: getStringValue(nic.Name), } } @@ -361,3 +396,10 @@ func isNotFoundError(err error) bool { } return false } + +func getStringValue(s *string) string { + if s == nil { + return "" + } + return *s +} \ No newline at end of file diff --git a/pkg/client/stackit.go b/pkg/client/stackit.go index 41b4810a..7201f355 100644 --- a/pkg/client/stackit.go +++ b/pkg/client/stackit.go @@ -25,6 +25,10 @@ type StackitClient interface { ListServers(ctx context.Context, projectID, region string, labelSelector map[string]string) ([]*Server, error) // GetNICsForServer retrieves a network interfaces for a given server GetNICsForServer(ctx context.Context, projectID, region, serverID string) ([]*NIC, error) + // ListNics list all nics for a network + ListNICs(ctx context.Context, projectID, region, networkID string) ([]*NIC, error) + // DeleteNIC delete a given nic by ID + DeleteNIC(ctx context.Context, projectID, region, networkID, nicID string) error // UpdateNIC updates a network interface UpdateNIC(ctx context.Context, projectID, region, networkID, nicID string, allowedAddresses []string) (*NIC, error) } @@ -95,4 +99,5 @@ type NIC struct { AllowedAddresses []string `json:"allowedAddresses,omitempty"` IPv4 string `json:"ipv4,omitempty"` IPv6 string `json:"ipv6,omitempty"` + Name string `json:"name"` } From 6be6d0a953245f8eaf0cedc5e8db3a13257daf5c Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Thu, 27 Aug 2026 13:01:41 +0200 Subject: [PATCH 2/5] fix core.go rebase isssues --- pkg/client/sdk.go | 17 ++++------ pkg/provider/core.go | 2 ++ pkg/provider/create.go | 40 +++++++++++++--------- pkg/provider/delete.go | 76 ++++++++++++++++++++++++++++++------------ 4 files changed, 88 insertions(+), 47 deletions(-) diff --git a/pkg/client/sdk.go b/pkg/client/sdk.go index 05582a60..239e73f8 100644 --- a/pkg/client/sdk.go +++ b/pkg/client/sdk.go @@ -1,8 +1,4 @@ -// SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Gardener contributors -// -// SPDX-License-Identifier: Apache-2.0 - -package provider +package client import ( "context" @@ -11,9 +7,10 @@ import ( "os" "strings" + "github.com/stackitcloud/machine-controller-manager-provider-stackit/pkg/metrics" "github.com/stackitcloud/stackit-sdk-go/core/config" "github.com/stackitcloud/stackit-sdk-go/core/oapierror" - "github.com/stackitcloud/stackit-sdk-go/services/iaas" + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) // SdkStackitClient is an SDK implementation of StackitClient @@ -301,7 +298,7 @@ func (c *SdkStackitClient) GetNICsForServer(ctx context.Context, projectID, regi } func (c *SdkStackitClient) ListNICs(ctx context.Context, projectID, region, networkID string) ([]*NIC, error) { - res, err := c.iaasClient.ListNics(ctx, projectID, region, networkID).Execute() + res, err := c.iaasClient.DefaultAPI.ListNics(ctx, projectID, region, networkID).Execute() if err != nil { return nil, fmt.Errorf("SDK ListServerNICs failed: %w", err) } @@ -311,7 +308,7 @@ func (c *SdkStackitClient) ListNICs(ctx context.Context, projectID, region, netw } nics := make([]*NIC, 0) - for _, nic := range *res.Items { + for _, nic := range res.Items { nics = append(nics, convertSDKNICtoNIC(&nic)) } @@ -319,7 +316,7 @@ func (c *SdkStackitClient) ListNICs(ctx context.Context, projectID, region, netw } func (c *SdkStackitClient) DeleteNIC(ctx context.Context, projectID, region, networkID, nicID string) error { - err := c.iaasClient.DeleteNic(ctx, projectID, region, networkID, nicID).Execute() + err := c.iaasClient.DefaultAPI.DeleteNic(ctx, projectID, region, networkID, nicID).Execute() if err != nil { // Check if error is 404 Not Found - this is OK (idempotent) if isNotFoundError(err) { @@ -402,4 +399,4 @@ func getStringValue(s *string) string { return "" } return *s -} \ No newline at end of file +} diff --git a/pkg/provider/core.go b/pkg/provider/core.go index 1cfacc0c..237047ae 100644 --- a/pkg/provider/core.go +++ b/pkg/provider/core.go @@ -13,6 +13,8 @@ const ( StackitProviderName = "stackit" StackitMachineLabel = "kubernetes.io/machine" StackitMachineClassLabel = "kubernetes.io/machineclass" + + migratedMachineAnnotation = "stackit.cloud/migrated-machine" ) // GetVolumeIDs extracts volume IDs from PersistentVolume specs diff --git a/pkg/provider/create.go b/pkg/provider/create.go index 930600b5..a6eaad4c 100644 --- a/pkg/provider/create.go +++ b/pkg/provider/create.go @@ -6,6 +6,7 @@ import ( "fmt" "maps" "slices" + "strconv" "github.com/gardener/machine-controller-manager/pkg/util/provider/driver" "github.com/gardener/machine-controller-manager/pkg/util/provider/machinecodes/codes" @@ -47,6 +48,10 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR return nil, status.Error(codes.InvalidArgument, err.Error()) } + if m, _ := strconv.ParseBool(req.Machine.Annotations[migratedMachineAnnotation]); m { + return nil, status.Error(codes.AlreadyExists, fmt.Errorf("create for migrated machine %s will not work", req.Machine.Name).Error()) + } + // Decode ProviderSpec from MachineClass providerSpec, err := decodeProviderSpec(req.MachineClass) if err != nil { @@ -68,12 +73,25 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR } // check if server already exists - server, err := p.getServerByName(ctx, projectID, providerSpec.Region, req.Machine.Name) + servers, err := p.getServersByName(ctx, projectID, providerSpec.Region, map[string]string{ + StackitMachineLabel: req.Machine.Name, + }) if err != nil { klog.Errorf("Failed to fetch server for machine %q: %v", req.Machine.Name, err) return nil, status.Error(codes.Unavailable, fmt.Sprintf("failed to fetch server: %v", err)) } + if len(servers) > 1 { + klog.Errorf("Multiple servers already exists for this machine %q: %v", req.Machine.Name, err) + return nil, status.Error(codes.AlreadyExists, fmt.Sprintf("failed to fetch server: %v", err)) + } + + var server *client.Server + + if len(servers) == 1 { + server = servers[0] + } + if server == nil { // Call STACKIT API to create server server, err = p.client.CreateServer(ctx, projectID, providerSpec.Region, p.createServerRequest(req, providerSpec)) @@ -233,26 +251,18 @@ func nicAddresses(nics []*client.NIC) []corev1.NodeAddress { return addresses } -func (p *Provider) getServerByName(ctx context.Context, projectID, region, serverName string) (*client.Server, error) { +func (p *Provider) getServersByName(ctx context.Context, projectID, region string, selector map[string]string) ([]*client.Server, error) { // Check if the server got already created - labelSelector := map[string]string{ - StackitMachineLabel: serverName, - } - servers, err := p.client.ListServers(ctx, projectID, region, labelSelector) + servers, err := p.client.ListServers(ctx, projectID, region, selector) if err != nil { - return nil, fmt.Errorf("SDK ListServers with labelSelector: %v failed: %w", labelSelector, err) + return nil, fmt.Errorf("SDK ListServers with labelSelector: %v failed: %w", selector, err) } - if len(servers) > 1 { - return nil, fmt.Errorf("%v servers found for server name %v", len(servers), serverName) - } - - if len(servers) == 1 { - return servers[0], nil + if len(servers) == 0 { + return nil, nil } - // no servers found len == 0 - return nil, nil + return servers, nil } func (p *Provider) patchNetworkInterfaces(ctx context.Context, projectID, serverID string, providerSpec *api.ProviderSpec) ([]*client.NIC, error) { diff --git a/pkg/provider/delete.go b/pkg/provider/delete.go index ff84e93c..15888d50 100644 --- a/pkg/provider/delete.go +++ b/pkg/provider/delete.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "strconv" "strings" "github.com/gardener/machine-controller-manager/pkg/util/provider/driver" @@ -35,9 +36,11 @@ func (p *Provider) DeleteMachine(ctx context.Context, req *driver.DeleteMachineR return nil, status.Error(codes.Unauthenticated, fmt.Sprintf("failed to initialize STACKIT client: %v", err)) } - var projectID, serverID string + var projectID string + var serverIDs []string var err error if req.Machine.Spec.ProviderID != "" { + var serverID string if !strings.HasPrefix(req.Machine.Spec.ProviderID, StackitProviderName) { return nil, status.Error(codes.InvalidArgument, "providerID is not empty and does not start with stackit://") } @@ -47,6 +50,7 @@ func (p *Provider) DeleteMachine(ctx context.Context, req *driver.DeleteMachineR if err != nil { klog.V(2).Infof("invalid ProviderID format: %v", err) } + serverIDs = append(serverIDs, serverID) } if projectID == "" { @@ -59,39 +63,67 @@ func (p *Provider) DeleteMachine(ctx context.Context, req *driver.DeleteMachineR return nil, status.Error(codes.Internal, err.Error()) } - if serverID == "" { - server, err := p.getServerByName(ctx, projectID, providerSpec.Region, req.Machine.Name) + if len(serverIDs) == 0 { + selector := map[string]string{ + StackitMachineLabel: req.Machine.Name, + } + + if m, _ := strconv.ParseBool(req.Machine.Annotations[migratedMachineAnnotation]); m { + selector = nil + } + + servers, err := p.getServersByName(ctx, projectID, providerSpec.Region, selector) if err != nil { return nil, status.Error(codes.Internal, fmt.Sprintf("failed to find server by name: %v", err)) } - if server != nil { - serverID = server.ID + for _, server := range servers { + if server.Name != req.Machine.Name { + continue + } + serverIDs = append(serverIDs, server.ID) } } - if serverID == "" { - klog.V(2).Infof("Server is already deleted for machine %q", req.Machine.Name) - return &driver.DeleteMachineResponse{}, nil + for _, id := range serverIDs { + // Call STACKIT API to delete server + err = p.client.DeleteServer(ctx, projectID, providerSpec.Region, id) + if err != nil { + // Check if server was not found (404) - this is OK for idempotency + if errors.Is(err, client.ErrServerNotFound) { + klog.V(2).Infof("Server %q already deleted for machine %q (idempotent)", id, req.Machine.Name) + return &driver.DeleteMachineResponse{}, nil + } + // All other errors are internal errors + klog.Errorf("Failed to delete server for machine %q: %v", req.Machine.Name, err) + return nil, status.Error(codes.Internal, fmt.Sprintf("failed to delete server: %v", err)) + } } - // Call STACKIT API to delete server - err = p.client.DeleteServer(ctx, projectID, providerSpec.Region, serverID) - if err != nil { - // Check if server was not found (404) - this is OK for idempotency - if errors.Is(err, client.ErrServerNotFound) { - klog.V(2).Infof("Server %q already deleted for machine %q (idempotent)", serverID, req.Machine.Name) - return &driver.DeleteMachineResponse{}, nil + if m, _ := strconv.ParseBool(req.Machine.Annotations[migratedMachineAnnotation]); m { + nics, err := p.client.ListNICs(ctx, projectID, providerSpec.Region, providerSpec.Networking.NetworkID) + if err != nil { + return nil, err + } + for _, nic := range nics { + if nic.Name != req.Machine.Name { + continue + } + err = p.client.DeleteNIC(ctx, projectID, providerSpec.Region, nic.NetworkID, nic.ID) + if err != nil { + // Check if server was not found (404) - this is OK for idempotency + if errors.Is(err, client.ErrNicNotFound) { + klog.V(2).Infof("Nic %q already deleted for machine %q (idempotent)", nic.ID, req.Machine.Name) + return &driver.DeleteMachineResponse{}, nil + } + // All other errors are internal errors + klog.Errorf("Failed to delete nic for machine %q: %v", req.Machine.Name, err) + return nil, status.Error(codes.Internal, fmt.Sprintf("failed to delete nic: %v", err)) + } } - // All other errors are internal errors - klog.Errorf("Failed to delete server for machine %q: %v", req.Machine.Name, err) - return nil, status.Error(codes.Internal, fmt.Sprintf("failed to delete server: %v", err)) - } - if err := p.WaitUntilServerDeleted(ctx, projectID, providerSpec.Region, serverID); err != nil { - klog.Errorf("Failed waiting for server %q to be deleted for machine %q: %v", serverID, req.Machine.Name, err) - return nil, status.Error(codes.DeadlineExceeded, fmt.Sprintf("failed waiting for server to be deleted: %v", err)) } + klog.V(2).Infof("Successfully deleted server for machine %q", req.Machine.Name) return &driver.DeleteMachineResponse{}, nil } From 6999aa0cd99fdea345cbfbd9195e7c78a14fd248 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Mon, 7 Sep 2026 12:09:07 +0200 Subject: [PATCH 3/5] add tests for the create and delete machine --- pkg/client/mock/client.go | 16 +++++++ pkg/client/stackit.go | 2 +- pkg/provider/create_basic_test.go | 57 +++++++++++++++++++++++ pkg/provider/delete_test.go | 75 +++++++++++++++++++------------ 4 files changed, 121 insertions(+), 29 deletions(-) diff --git a/pkg/client/mock/client.go b/pkg/client/mock/client.go index 43a7fbf2..32ebe3e9 100644 --- a/pkg/client/mock/client.go +++ b/pkg/client/mock/client.go @@ -16,6 +16,8 @@ type StackitClient struct { DeleteServerFunc func(ctx context.Context, projectID, region, serverID string) error ListServersFunc func(ctx context.Context, projectID, region string, labelSelector map[string]string) ([]*client.Server, error) GetNICsFunc func(ctx context.Context, projectID, region, serverID string) ([]*client.NIC, error) + ListNICsFunc func(ctx context.Context, projectID, region, networkID string) ([]*client.NIC, error) + DeleteNICFunc func(ctx context.Context, projectID, region, networkID, nicID string) error UpdateNICFunc func(ctx context.Context, projectID, region, networkID, nicID string, allowedAddresses []string) (*client.NIC, error) } @@ -64,6 +66,20 @@ func (m *StackitClient) GetNICsForServer(ctx context.Context, projectID, region, }, nil } +func (m *StackitClient) ListNICs(ctx context.Context, projectID, region, networkID string) ([]*client.NIC, error) { + if m.ListNICsFunc != nil { + return m.ListNICsFunc(ctx, projectID, region, networkID) + } + return []*client.NIC{}, nil +} + +func (m *StackitClient) DeleteNIC(ctx context.Context, projectID, region, networkID, nicID string) error { + if m.DeleteNICFunc != nil { + return m.DeleteNICFunc(ctx, projectID, region, networkID, nicID) + } + return nil +} + func (m *StackitClient) UpdateNIC(ctx context.Context, projectID, region, networkID, nicID string, allowedAddresses []string) (*client.NIC, error) { if m.UpdateNICFunc != nil { return m.UpdateNICFunc(ctx, projectID, region, networkID, nicID, allowedAddresses) diff --git a/pkg/client/stackit.go b/pkg/client/stackit.go index 7201f355..09c2adf5 100644 --- a/pkg/client/stackit.go +++ b/pkg/client/stackit.go @@ -25,7 +25,7 @@ type StackitClient interface { ListServers(ctx context.Context, projectID, region string, labelSelector map[string]string) ([]*Server, error) // GetNICsForServer retrieves a network interfaces for a given server GetNICsForServer(ctx context.Context, projectID, region, serverID string) ([]*NIC, error) - // ListNics list all nics for a network + // ListNICs list all nics for a network ListNICs(ctx context.Context, projectID, region, networkID string) ([]*NIC, error) // DeleteNIC delete a given nic by ID DeleteNIC(ctx context.Context, projectID, region, networkID, nicID string) error diff --git a/pkg/provider/create_basic_test.go b/pkg/provider/create_basic_test.go index c6a47d14..8cbdbf24 100644 --- a/pkg/provider/create_basic_test.go +++ b/pkg/provider/create_basic_test.go @@ -85,6 +85,63 @@ var _ = Describe("CreateMachine", func() { }) Context("with valid inputs", func() { + It("rejects creation of a migrated machine", func() { + machine.Annotations = map[string]string{migratedMachineAnnotation: "true"} + listServersCalled := false + mockClient.ListServersFunc = func(_ context.Context, _, _ string, _ map[string]string) ([]*client.Server, error) { + listServersCalled = true + return nil, nil + } + + _, err := provider.CreateMachine(ctx, req) + + Expect(err).To(HaveOccurred()) + statusErr, ok := status.FromError(err) + Expect(ok).To(BeTrue()) + Expect(statusErr.Code()).To(Equal(codes.AlreadyExists)) + Expect(listServersCalled).To(BeFalse()) + }) + + It("returns AlreadyExists when more than one server has the machine label", func() { + createServerCalled := false + mockClient.ListServersFunc = func(_ context.Context, projectID, region string, selector map[string]string) ([]*client.Server, error) { + Expect(projectID).To(Equal("11111111-2222-3333-4444-555555555555")) + Expect(region).To(Equal("eu01")) + Expect(selector).To(Equal(map[string]string{StackitMachineLabel: "test-machine"})) + return []*client.Server{{ID: "server-1"}, {ID: "server-2"}}, nil + } + mockClient.CreateServerFunc = func(_ context.Context, _, _ string, _ *client.CreateServerRequest) (*client.Server, error) { + createServerCalled = true + return nil, nil + } + + _, err := provider.CreateMachine(ctx, req) + + Expect(err).To(HaveOccurred()) + statusErr, ok := status.FromError(err) + Expect(ok).To(BeTrue()) + Expect(statusErr.Code()).To(Equal(codes.AlreadyExists)) + Expect(createServerCalled).To(BeFalse()) + }) + + It("reuses the sole server returned for the machine label", func() { + createServerCalled := false + mockClient.ListServersFunc = func(_ context.Context, _, _ string, selector map[string]string) ([]*client.Server, error) { + Expect(selector).To(Equal(map[string]string{StackitMachineLabel: "test-machine"})) + return []*client.Server{{ID: "existing-server", Name: "test-machine", Status: "ACTIVE"}}, nil + } + mockClient.CreateServerFunc = func(_ context.Context, _, _ string, _ *client.CreateServerRequest) (*client.Server, error) { + createServerCalled = true + return nil, nil + } + + resp, err := provider.CreateMachine(ctx, req) + + Expect(err).NotTo(HaveOccurred()) + Expect(resp.ProviderID).To(Equal("stackit://11111111-2222-3333-4444-555555555555/existing-server")) + Expect(createServerCalled).To(BeFalse()) + }) + It("should successfully create a machine", func() { resp, err := provider.CreateMachine(ctx, req) diff --git a/pkg/provider/delete_test.go b/pkg/provider/delete_test.go index a30dae1a..7cc063f9 100644 --- a/pkg/provider/delete_test.go +++ b/pkg/provider/delete_test.go @@ -53,6 +53,9 @@ var _ = Describe("DeleteMachine", func() { MachineType: "c2i.2", ImageID: "image-uuid-123", Region: "eu01", + Networking: &api.NetworkingSpec{ + NetworkID: "770e8400-e29b-41d4-a716-446655440000", + }, } providerSpecRaw, _ := mock.EncodeProviderSpec(providerSpec) @@ -120,42 +123,18 @@ var _ = Describe("DeleteMachine", func() { Expect(capturedServerID).To(Equal("550e8400-e29b-41d4-a716-446655440000")) }) - It("should poll GetServer until server is deleted", func() { - getServerCallCount := 0 - - mockClient.DeleteServerFunc = func(_ context.Context, _, _, _ string) error { - return nil - } - mockClient.GetServerFunc = func(_ context.Context, _, _, _ string) (*client.Server, error) { - getServerCallCount++ - // First call returns server still exists, second call returns not found - if getServerCallCount == 1 { - return &client.Server{ - ID: "550e8400-e29b-41d4-a716-446655440000", - Name: "test-machine", - Status: "SHUTTING_DOWN", - }, nil - } - return nil, fmt.Errorf("%w: status 404", client.ErrServerNotFound) - } - - resp, err := provider.DeleteMachine(ctx, req) - - Expect(err).NotTo(HaveOccurred()) - Expect(resp).NotTo(BeNil()) - Expect(getServerCallCount).To(BeNumerically(">=", 2)) - }) }) Context("with missing or invalid ProviderID", func() { It("should still delete the machine when ProviderID is missing", func() { machine.Spec.ProviderID = "" - mockClient.GetServerFunc = func(_ context.Context, _, _, _ string) (*client.Server, error) { - return &client.Server{ + mockClient.ListServersFunc = func(_ context.Context, _, _ string, selector map[string]string) ([]*client.Server, error) { + Expect(selector).To(Equal(map[string]string{StackitMachineLabel: "test-machine"})) + return []*client.Server{{ ID: "550e8400-e29b-41d4-a716-446655440000", Name: "test-machine", - }, nil + }}, nil } mockClient.DeleteServerFunc = func(_ context.Context, _, _, _ string) error { return nil @@ -178,6 +157,46 @@ var _ = Describe("DeleteMachine", func() { }) }) + Context("when deleting a migrated machine", func() { + It("deletes all matching servers and NICs after an unfiltered lookup", func() { + machine.Spec.ProviderID = "" + machine.Annotations = map[string]string{migratedMachineAnnotation: "true"} + var deletedServerIDs, deletedNICIDs []string + mockClient.ListServersFunc = func(_ context.Context, _, _ string, selector map[string]string) ([]*client.Server, error) { + Expect(selector).To(BeNil()) + return []*client.Server{ + {ID: "server-1", Name: "test-machine"}, + {ID: "other-server", Name: "another-machine"}, + {ID: "server-2", Name: "test-machine"}, + }, nil + } + mockClient.DeleteServerFunc = func(_ context.Context, _, _, serverID string) error { + deletedServerIDs = append(deletedServerIDs, serverID) + return nil + } + mockClient.ListNICsFunc = func(_ context.Context, _, _, networkID string) ([]*client.NIC, error) { + Expect(networkID).To(Equal("770e8400-e29b-41d4-a716-446655440000")) + return []*client.NIC{ + {ID: "nic-1", NetworkID: networkID, Name: "test-machine"}, + {ID: "other-nic", NetworkID: networkID, Name: "another-machine"}, + {ID: "nic-2", NetworkID: networkID, Name: "test-machine"}, + }, nil + } + mockClient.DeleteNICFunc = func(_ context.Context, _, _, networkID, nicID string) error { + Expect(networkID).To(Equal("770e8400-e29b-41d4-a716-446655440000")) + deletedNICIDs = append(deletedNICIDs, nicID) + return nil + } + + resp, err := provider.DeleteMachine(ctx, req) + + Expect(err).NotTo(HaveOccurred()) + Expect(resp).NotTo(BeNil()) + Expect(deletedServerIDs).To(ConsistOf("server-1", "server-2")) + Expect(deletedNICIDs).To(ConsistOf("nic-1", "nic-2")) + }) + }) + Context("when machine not found", func() { It("should return success if machine does not exist (idempotent)", func() { mockClient.DeleteServerFunc = func(_ context.Context, _, _, _ string) error { From fe038e052ab6da6545b4a7fea9f5ec8f0229131a Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Mon, 7 Sep 2026 14:01:47 +0200 Subject: [PATCH 4/5] fix make verify --- pkg/provider/delete.go | 1 - 1 file changed, 1 deletion(-) diff --git a/pkg/provider/delete.go b/pkg/provider/delete.go index 15888d50..0bcbaf76 100644 --- a/pkg/provider/delete.go +++ b/pkg/provider/delete.go @@ -121,7 +121,6 @@ func (p *Provider) DeleteMachine(ctx context.Context, req *driver.DeleteMachineR return nil, status.Error(codes.Internal, fmt.Sprintf("failed to delete nic: %v", err)) } } - } klog.V(2).Infof("Successfully deleted server for machine %q", req.Machine.Name) From 17def4f5970cc068e63e94d9f37176d992d0d3f2 Mon Sep 17 00:00:00 2001 From: Aniruddha Basak Date: Mon, 7 Sep 2026 14:15:54 +0200 Subject: [PATCH 5/5] fix make verify --- pkg/client/sdk.go | 6 +- pkg/provider/delete.go | 140 ++++++++++++++++++++++------------------- 2 files changed, 79 insertions(+), 67 deletions(-) diff --git a/pkg/client/sdk.go b/pkg/client/sdk.go index 239e73f8..8c294dc7 100644 --- a/pkg/client/sdk.go +++ b/pkg/client/sdk.go @@ -307,9 +307,9 @@ func (c *SdkStackitClient) ListNICs(ctx context.Context, projectID, region, netw return []*NIC{}, nil } - nics := make([]*NIC, 0) - for _, nic := range res.Items { - nics = append(nics, convertSDKNICtoNIC(&nic)) + nics := make([]*NIC, 0, len(res.Items)) + for i := range res.Items { + nics = append(nics, convertSDKNICtoNIC(&res.Items[i])) } return nics, nil diff --git a/pkg/provider/delete.go b/pkg/provider/delete.go index 0bcbaf76..be7e50b6 100644 --- a/pkg/provider/delete.go +++ b/pkg/provider/delete.go @@ -36,95 +36,107 @@ func (p *Provider) DeleteMachine(ctx context.Context, req *driver.DeleteMachineR return nil, status.Error(codes.Unauthenticated, fmt.Sprintf("failed to initialize STACKIT client: %v", err)) } - var projectID string - var serverIDs []string - var err error - if req.Machine.Spec.ProviderID != "" { - var serverID string - if !strings.HasPrefix(req.Machine.Spec.ProviderID, StackitProviderName) { - return nil, status.Error(codes.InvalidArgument, "providerID is not empty and does not start with stackit://") - } - - // Parse ProviderID to extract projectID and serverID - projectID, serverID, err = parseProviderID(req.Machine.Spec.ProviderID) - if err != nil { - klog.V(2).Infof("invalid ProviderID format: %v", err) - } - serverIDs = append(serverIDs, serverID) - } - - if projectID == "" { - // use the secret as a fallback - projectID = projectIDFromSecret - } - providerSpec, err := decodeProviderSpec(req.MachineClass) if err != nil { return nil, status.Error(codes.Internal, err.Error()) } - if len(serverIDs) == 0 { - selector := map[string]string{ - StackitMachineLabel: req.Machine.Name, + migrated, _ := strconv.ParseBool(req.Machine.Annotations[migratedMachineAnnotation]) + projectID, serverIDs, err := p.serverIDsForMachine(ctx, req, projectIDFromSecret, providerSpec.Region, migrated) + if err != nil { + return nil, err + } + serverAlreadyDeleted, err := p.deleteServers(ctx, projectID, providerSpec.Region, req.Machine.Name, serverIDs) + if err != nil { + return nil, err + } + if serverAlreadyDeleted { + return &driver.DeleteMachineResponse{}, nil + } + if migrated { + nicAlreadyDeleted, err := p.deleteMachineNICs(ctx, projectID, providerSpec.Region, providerSpec.Networking.NetworkID, req.Machine.Name) + if err != nil { + return nil, err } + if nicAlreadyDeleted { + return &driver.DeleteMachineResponse{}, nil + } + } + klog.V(2).Infof("Successfully deleted server for machine %q", req.Machine.Name) + + return &driver.DeleteMachineResponse{}, nil +} - if m, _ := strconv.ParseBool(req.Machine.Annotations[migratedMachineAnnotation]); m { - selector = nil +func (p *Provider) serverIDsForMachine(ctx context.Context, req *driver.DeleteMachineRequest, secretProjectID, region string, migrated bool) (projectID string, serverIDs []string, err error) { + projectID, serverIDs = "", nil + if providerID := req.Machine.Spec.ProviderID; providerID != "" { + if !strings.HasPrefix(providerID, StackitProviderName) { + return "", nil, status.Error(codes.InvalidArgument, "providerID is not empty and does not start with stackit://") } - servers, err := p.getServersByName(ctx, projectID, providerSpec.Region, selector) + var serverID string + projectID, serverID, err = parseProviderID(providerID) if err != nil { - return nil, status.Error(codes.Internal, fmt.Sprintf("failed to find server by name: %v", err)) + klog.V(2).Infof("invalid ProviderID format: %v", err) } + serverIDs = append(serverIDs, serverID) + } + if projectID == "" { + projectID = secretProjectID + } + if len(serverIDs) != 0 { + return projectID, serverIDs, nil + } - for _, server := range servers { - if server.Name != req.Machine.Name { - continue - } + selector := map[string]string{StackitMachineLabel: req.Machine.Name} + if migrated { + selector = nil + } + servers, err := p.getServersByName(ctx, projectID, region, selector) + if err != nil { + return "", nil, status.Error(codes.Internal, fmt.Sprintf("failed to find server by name: %v", err)) + } + for _, server := range servers { + if server.Name == req.Machine.Name { serverIDs = append(serverIDs, server.ID) } } + return projectID, serverIDs, nil +} - for _, id := range serverIDs { - // Call STACKIT API to delete server - err = p.client.DeleteServer(ctx, projectID, providerSpec.Region, id) - if err != nil { - // Check if server was not found (404) - this is OK for idempotency +func (p *Provider) deleteServers(ctx context.Context, projectID, region, machineName string, serverIDs []string) (bool, error) { + for _, serverID := range serverIDs { + if err := p.client.DeleteServer(ctx, projectID, region, serverID); err != nil { if errors.Is(err, client.ErrServerNotFound) { - klog.V(2).Infof("Server %q already deleted for machine %q (idempotent)", id, req.Machine.Name) - return &driver.DeleteMachineResponse{}, nil + klog.V(2).Infof("Server %q already deleted for machine %q (idempotent)", serverID, machineName) + return true, nil } - // All other errors are internal errors - klog.Errorf("Failed to delete server for machine %q: %v", req.Machine.Name, err) - return nil, status.Error(codes.Internal, fmt.Sprintf("failed to delete server: %v", err)) + klog.Errorf("Failed to delete server for machine %q: %v", machineName, err) + return false, status.Error(codes.Internal, fmt.Sprintf("failed to delete server: %v", err)) } } + return false, nil +} - if m, _ := strconv.ParseBool(req.Machine.Annotations[migratedMachineAnnotation]); m { - nics, err := p.client.ListNICs(ctx, projectID, providerSpec.Region, providerSpec.Networking.NetworkID) - if err != nil { - return nil, err +func (p *Provider) deleteMachineNICs(ctx context.Context, projectID, region, networkID, machineName string) (bool, error) { + nics, err := p.client.ListNICs(ctx, projectID, region, networkID) + if err != nil { + return false, err + } + for _, nic := range nics { + if nic.Name != machineName { + continue } - for _, nic := range nics { - if nic.Name != req.Machine.Name { - continue - } - err = p.client.DeleteNIC(ctx, projectID, providerSpec.Region, nic.NetworkID, nic.ID) - if err != nil { - // Check if server was not found (404) - this is OK for idempotency - if errors.Is(err, client.ErrNicNotFound) { - klog.V(2).Infof("Nic %q already deleted for machine %q (idempotent)", nic.ID, req.Machine.Name) - return &driver.DeleteMachineResponse{}, nil - } - // All other errors are internal errors - klog.Errorf("Failed to delete nic for machine %q: %v", req.Machine.Name, err) - return nil, status.Error(codes.Internal, fmt.Sprintf("failed to delete nic: %v", err)) + if err := p.client.DeleteNIC(ctx, projectID, region, nic.NetworkID, nic.ID); err != nil { + if errors.Is(err, client.ErrNicNotFound) { + klog.V(2).Infof("Nic %q already deleted for machine %q (idempotent)", nic.ID, machineName) + return true, nil } + klog.Errorf("Failed to delete nic for machine %q: %v", machineName, err) + return false, status.Error(codes.Internal, fmt.Sprintf("failed to delete nic: %v", err)) } } - klog.V(2).Infof("Successfully deleted server for machine %q", req.Machine.Name) - - return &driver.DeleteMachineResponse{}, nil + return false, nil } func (p *Provider) WaitUntilServerDeleted(ctx context.Context, projectID, region, serverID string) error {