Skip to content

WIP: Cleanup NICs when migrate from openstack MCM #89

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dergeberl wants to merge 1 commit into
base: main
Choose a base branch
from
+118 −35
Open

WIP: Cleanup NICs when migrate from openstack MCM #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 81 additions & 35 deletions pkg/provider/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"errors"
"fmt"
"slices"
"strconv"
"strings"

"github.com/gardener/machine-controller-manager/pkg/util/provider/driver"
Expand All @@ -28,6 +29,8 @@ const (
StackitRoleLabel = "mcm.gardener.cloud/role"
)

const migratedMachineAnnotation = "stackit.cloud/migrated-machine"

// CreateMachine handles a machine creation request by creating a STACKIT server
//
// This method creates a new server in STACKIT infrastructure based on the ProviderSpec
Expand All @@ -54,6 +57,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 {
Expand Down Expand Up @@ -186,12 +193,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 *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, createReq)
Expand All @@ -216,26 +236,18 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR
}, nil
}

func (p *Provider) getServerByName(ctx context.Context, projectID, region, serverName string) (*Server, error) {
func (p *Provider) getServersByName(ctx context.Context, projectID, region string, selector map[string]string) ([]*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) == 0 {
return nil, nil
}

if len(servers) == 1 {
return servers[0], nil
}

// no servers found len == 0
return nil, nil
return servers, nil
}

func (p *Provider) patchNetworkInterface(ctx context.Context, projectID, serverID string, providerSpec *api.ProviderSpec) error {
Expand Down Expand Up @@ -305,9 +317,11 @@ func (p *Provider) DeleteMachine(ctx context.Context, req *driver.DeleteMachineR
return nil, status.Error(codes.Internal, 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://")
}
Expand All @@ -317,6 +331,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 == "" {
Expand All @@ -328,36 +343,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, 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, 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, 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))
}

klog.V(2).Infof("Successfully deleted server %q for machine %q", serverID, req.Machine.Name)
}
klog.V(2).Infof("Successfully deleted server for machine %q", req.Machine.Name)

return &driver.DeleteMachineResponse{}, nil
}
Expand Down
32 changes: 32 additions & 0 deletions pkg/provider/sdk_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,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
Expand Down Expand Up @@ -320,6 +321,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))

Expand Down Expand Up @@ -361,6 +392,7 @@ func convertSDKNICtoNIC(nic *iaas.NIC) *NIC {
ID: getStringValue(nic.Id),
NetworkID: getStringValue(nic.NetworkId),
AllowedAddresses: addresses,
Name: getStringValue(nic.Name),
}
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/provider/stackit_client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,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)
}
Expand Down Expand Up @@ -97,4 +101,5 @@ type NIC struct {
ID string
NetworkID string
AllowedAddresses []string
Name string
}