Skip to content

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 13 commits into
base: main
Choose a base branch
from
Open

Cleanup NICs when migrate from openstack MCM #89

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
7e0a67c
WIP
dergeberl Jan 22, 2026
6be6d0a
fix core.go rebase isssues
aniruddha2000 Aug 27, 2026
6999aa0
add tests for the create and delete machine
aniruddha2000 Sep 7, 2026
fe038e0
fix make verify
aniruddha2000 Sep 7, 2026
17def4f
fix make verify
aniruddha2000 Sep 7, 2026
95cc8ce
resolve partial review
aniruddha2000 Sep 8, 2026
297978b
fix make check
aniruddha2000 Sep 8, 2026
bd1f1ff
remove error check to have OS compatibilty
aniruddha2000 Sep 8, 2026
5e8df25
remove error shadow
aniruddha2000 Sep 10, 2026
b47d91d
add docstring
aniruddha2000 Sep 10, 2026
e776b6d
collect errors and return one time for servers and nics
aniruddha2000 Sep 11, 2026
54cba9a
fix test
aniruddha2000 Sep 11, 2026
b8fb2b0
remove deleted var
aniruddha2000 Sep 11, 2026
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
16 changes: 16 additions & 0 deletions pkg/client/mock/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
Expand Down
32 changes: 32 additions & 0 deletions pkg/client/sdk.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

@stackit-ske-bot stackit-ske-bot Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, fmt.Errorf("SDK ListServerNICs failed: %w", err)
return nil, fmt.Errorf("SDK ListNICs failed: %w", err)

Fix error message to reference ListNICs instead of ListServerNICs.

}

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))

Expand Down Expand Up @@ -337,6 +368,7 @@ func convertSDKNICtoNIC(nic *iaas.NIC) *NIC {
AllowedAddresses: addresses,
IPv4: nic.GetIpv4(),
IPv6: nic.GetIpv6(),
Name: nic.GetName(),
}
}

Expand Down
5 changes: 5 additions & 0 deletions pkg/client/stackit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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"`
}
2 changes: 2 additions & 0 deletions pkg/provider/core.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
131 changes: 83 additions & 48 deletions pkg/provider/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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())

@stackit-ske-bot stackit-ske-bot Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, "", status.Error(codes.AlreadyExists, fmt.Errorf("create for migrated machine %s will not work", req.Machine.Name).Error())
if m, _ := strconv.ParseBool(req.Machine.Annotations[migratedMachineAnnotation]); m {
return nil, "", status.Error(codes.AlreadyExists, fmt.Sprintf("creation of migrated machine %q is not supported", req.Machine.Name))
}

Avoid allocating an intermediate error object solely to call .Error(), and use clear, quoted phrasing.

}
Comment thread
aniruddha2000 marked this conversation as resolved.

// 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

@stackit-ske-bot stackit-ske-bot Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
serverNames[i] = server.Name
serverNames[i] = fmt.Sprintf("%s (id: %s)", server.Name, server.ID)

Include server.ID alongside server name so duplicate machine instances can be distinguished and debugged in STACKIT IaaS.

}

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))

@stackit-ske-bot stackit-ske-bot Sep 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return nil, status.Error(codes.AlreadyExists, fmt.Sprintf("Multiple servers: %v already exists for the machine: %v", serverNames, req.Machine.Name))
return nil, status.Error(codes.AlreadyExists, fmt.Sprintf("multiple servers %v already exist for machine %q", serverNames, req.Machine.Name))

Fix grammar ("already exist") and use lowercase initial character consistent with standard error message styling.

}

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
Expand Down Expand Up @@ -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) {
Expand Down
57 changes: 57 additions & 0 deletions pkg/provider/create_basic_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading