Cleanup NICs when migrate from openstack MCM

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)

pkg/client/sdk.go

@@ -38,6 +38,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 +297,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.DefaultAPI.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, len(res.Items))
+	for i := range res.Items {
+		nics[i] = convertSDKNICtoNIC(&res.Items[i])
+	}
+
+	return nics, nil
+}
+
+func (c *SdkStackitClient) DeleteNIC(ctx context.Context, projectID, region, networkID, nicID string) error {
+	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) {
+			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 +368,7 @@ func convertSDKNICtoNIC(nic *iaas.NIC) *NIC {
 		AllowedAddresses: addresses,
 		IPv4:             nic.GetIpv4(),
 		IPv6:             nic.GetIpv6(),
+		Name:             nic.GetName(),
 	}
 }
 

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"`
 }

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

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"
@@ -41,74 +42,116 @@ 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
+	providerSpec, projectID, err := p.prepareMachineCreation(req)
+	if err != nil {
+		return nil, err
+	}
+
+	server, err := p.getOrCreateServer(ctx, req, projectID, providerSpec)
+	if err != nil {
+		return nil, err
+	}
+
+	if err := p.waitForServer(ctx, req.Machine.Name, projectID, providerSpec.Region, server.ID); err != nil {
+		return nil, err
+	}
+
+	nics, err := p.patchNetworkInterfaces(ctx, projectID, server.ID, providerSpec)
+	if err != nil {
+		klog.Errorf("Failed to patch NICs for server %q: %v", req.Machine.Name, err)
+		return nil, status.Error(codes.Unavailable, fmt.Sprintf("failed to patch NICs for server: %v", err))
+	}
+
+	// Generate ProviderID in format: stackit://<projectId>/<serverId>
+	providerID := fmt.Sprintf("%s://%s/%s", StackitProviderName, projectID, server.ID)
+	klog.V(2).Infof("Successfully created server %q with ID %q for machine %q", server.Name, server.ID, req.Machine.Name)
+
+	return &driver.CreateMachineResponse{
+		ProviderID: providerID,
+		NodeName:   req.Machine.Name,
+		Addresses:  nicAddresses(nics),
+	}, nil
+}
+
+func (p *Provider) prepareMachineCreation(req *driver.CreateMachineRequest) (*api.ProviderSpec, string, error) {
 	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())
+		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 {
-		return nil, status.Error(codes.Internal, err.Error())
+		return nil, "", status.Error(codes.Internal, err.Error())
 	}
 
 	// Validate ProviderSpec and Secret
 	validationErrs := validation.ValidateProviderSpecNSecret(providerSpec, req.Secret)
 	if len(validationErrs) > 0 {
-		return nil, status.Error(codes.InvalidArgument, validationErrs[0].Error())
+		return nil, "", status.Error(codes.InvalidArgument, validationErrs[0].Error())
 	}
 
 	// Extract credentials from Secret
 	projectID, serviceAccountKey := extractSecretCredentials(req.Secret.Data)
 
 	// Initialize client on first use (lazy initialization)
 	if err := p.ensureClient(serviceAccountKey); err != nil {
-		return nil, status.Error(codes.Internal, fmt.Sprintf("failed to initialize STACKIT client: %v", err))
+		return nil, "", status.Error(codes.Internal, fmt.Sprintf("failed to initialize STACKIT client: %v", err))
 	}
+	return providerSpec, projectID, nil
+}
 
-	// check if server already exists
-	server, err := p.getServerByName(ctx, projectID, providerSpec.Region, req.Machine.Name)
+func (p *Provider) getOrCreateServer(ctx context.Context, req *driver.CreateMachineRequest, projectID string, providerSpec *api.ProviderSpec) (*client.Server, error) {
+	servers, err := p.getServersByLabelSelector(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 server == nil {
-		// Call STACKIT API to create server
-		server, err = p.client.CreateServer(ctx, projectID, providerSpec.Region, p.createServerRequest(req, providerSpec))
-		if err != nil {
-			klog.Errorf("Failed to create server for machine %q: %v", req.Machine.Name, err)
-			if isResourceExhaustedError(err) {
-				return nil, status.Error(codes.ResourceExhausted, fmt.Sprintf("failed to create server: %v", err))
-			}
-			return nil, status.Error(codes.Unavailable, fmt.Sprintf("failed to create server: %v", err))
+	if len(servers) > 1 {
+		serverNames := make([]string, len(servers))
+		for i, server := range servers {
+			serverNames[i] = server.Name
 		}
+
+		klog.Errorf(
+			"Multiple servers already exist for this machine %q: servers=%v",
+			req.Machine.Name,
+			serverNames,
+		)
+		return nil, status.Error(codes.AlreadyExists, fmt.Sprintf("Multiple servers: %v already exists for the machine: %v", serverNames, req.Machine.Name))
 	}
 
-	if err := p.WaitUntilServerRunning(ctx, projectID, providerSpec.Region, server.ID); err != nil {
-		klog.Errorf("Failed waiting for server %q to reach ACTIVE state: %v", req.Machine.Name, err)
-		if isResourceExhaustedError(err) {
-			return nil, status.Error(codes.ResourceExhausted, fmt.Sprintf("failed waiting for server to be ACTIVE: %v", err))
-		}
-		return nil, status.Error(codes.DeadlineExceeded, fmt.Sprintf("failed waiting for server to be ACTIVE: %v", err))
+	if len(servers) == 1 {
+		return servers[0], nil
 	}
 
-	nics, err := p.patchNetworkInterfaces(ctx, projectID, server.ID, providerSpec)
+	server, err := p.client.CreateServer(ctx, projectID, providerSpec.Region, p.createServerRequest(req, providerSpec))
 	if err != nil {
-		klog.Errorf("Failed to patch NICs for server %q: %v", req.Machine.Name, err)
-		return nil, status.Error(codes.Unavailable, fmt.Sprintf("failed to patch NICs for server: %v", err))
+		klog.Errorf("Failed to create server for machine %q: %v", req.Machine.Name, err)
+		if isResourceExhaustedError(err) {
+			return nil, status.Error(codes.ResourceExhausted, fmt.Sprintf("failed to create server: %v", err))
+		}
+		return nil, status.Error(codes.Unavailable, fmt.Sprintf("failed to create server: %v", err))
 	}
+	return server, nil
+}
 
-	// Generate ProviderID in format: stackit://<projectId>/<serverId>
-	providerID := fmt.Sprintf("%s://%s/%s", StackitProviderName, projectID, server.ID)
-	klog.V(2).Infof("Successfully created server %q with ID %q for machine %q", server.Name, server.ID, req.Machine.Name)
-
-	return &driver.CreateMachineResponse{
-		ProviderID: providerID,
-		NodeName:   req.Machine.Name,
-		Addresses:  nicAddresses(nics),
-	}, nil
+func (p *Provider) waitForServer(ctx context.Context, machineName, projectID, region, serverID string) error {
+	if err := p.WaitUntilServerRunning(ctx, projectID, region, serverID); err != nil {
+		klog.Errorf("Failed waiting for server %q to reach ACTIVE state: %v", machineName, err)
+		if isResourceExhaustedError(err) {
+			return status.Error(codes.ResourceExhausted, fmt.Sprintf("failed waiting for server to be ACTIVE: %v", err))
+		}
+		return status.Error(codes.DeadlineExceeded, fmt.Sprintf("failed waiting for server to be ACTIVE: %v", err))
+	}
+	return nil
 }
 
 // nolint: gocyclo // this function is already pretty simple
@@ -233,26 +276,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) getServersByLabelSelector(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) {

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)