Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 13 additions & 0 deletions pkg/client/client_suite_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package client

import (
"testing"

. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)

func TestClient(t *testing.T) {
RegisterFailHandler(Fail)
RunSpecs(t, "Client Suite")
}
4 changes: 3 additions & 1 deletion pkg/client/mock/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@ func (m *StackitClient) GetNICsForServer(ctx context.Context, projectID, region,
if m.GetNICsFunc != nil {
return m.GetNICsFunc(ctx, projectID, region, serverID)
}
return []*client.NIC{}, nil
return []*client.NIC{
{ID: "default-nic-id", NetworkID: "default-network-id"},
}, nil
}

func (m *StackitClient) UpdateNIC(ctx context.Context, projectID, region, networkID, nicID string, allowedAddresses []string) (*client.NIC, error) {
Expand Down
2 changes: 2 additions & 0 deletions pkg/client/sdk.go
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,8 @@ func convertSDKNICtoNIC(nic *iaas.NIC) *NIC {
ID: nic.GetId(),
NetworkID: nic.GetNetworkId(),
AllowedAddresses: addresses,
IPv4: nic.GetIpv4(),
IPv6: nic.GetIpv6(),
}
}

Expand Down
62 changes: 61 additions & 1 deletion pkg/client/sdk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import (
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
"github.com/stackitcloud/stackit-sdk-go/core/oapierror"
"github.com/stackitcloud/stackit-sdk-go/services/iaas"
"k8s.io/utils/ptr"
)

var _ = Describe("SDK Client Helpers", func() {
Expand Down Expand Up @@ -122,7 +124,7 @@ var _ = Describe("SDK Type Conversion Helpers", func() {
result := convertLabelsToSDK(labels)

Expect(result).NotTo(BeNil())
Expect(result).To(HaveLen(2))
Expect(result).To(HaveLen(1))
Expect(result["kubernetes.io/machine"]).To(Equal("test-machine"))
})
})
Expand Down Expand Up @@ -203,6 +205,64 @@ var _ = Describe("SDK Type Conversion Helpers", func() {
})
})

Describe("convertSDKNICtoNIC", func() {
It("should populate IPv4 and IPv6 from SDK NIC", func() {
sdkNIC := &iaas.NIC{
Id: ptr.To("nic-1"),
NetworkId: ptr.To("net-1"),
Ipv4: ptr.To("10.0.0.5"),
Ipv6: ptr.To("fd00::1"),
}

result := convertSDKNICtoNIC(sdkNIC)

Expect(result.ID).To(Equal("nic-1"))
Expect(result.NetworkID).To(Equal("net-1"))
Expect(result.IPv4).To(Equal("10.0.0.5"))
Expect(result.IPv6).To(Equal("fd00::1"))
})

It("should handle a NIC with only IPv4", func() {
sdkNIC := &iaas.NIC{
Id: ptr.To("nic-1"),
NetworkId: ptr.To("net-1"),
Ipv4: ptr.To("10.0.0.5"),
}

result := convertSDKNICtoNIC(sdkNIC)

Expect(result.IPv4).To(Equal("10.0.0.5"))
Expect(result.IPv6).To(BeEmpty())
})

It("should handle a NIC with neither IPv4 nor IPv6", func() {
sdkNIC := &iaas.NIC{
Id: ptr.To("nic-1"),
NetworkId: ptr.To("net-1"),
}

result := convertSDKNICtoNIC(sdkNIC)

Expect(result.IPv4).To(BeEmpty())
Expect(result.IPv6).To(BeEmpty())
})

It("should populate AllowedAddresses", func() {
addr := "10.0.0.0/8"
sdkNIC := &iaas.NIC{
Id: ptr.To("nic-1"),
NetworkId: ptr.To("net-1"),
AllowedAddresses: &[]iaas.AllowedAddressesInner{
{String: &addr},
},
}

result := convertSDKNICtoNIC(sdkNIC)

Expect(result.AllowedAddresses).To(ConsistOf("10.0.0.0/8"))
})
})

Describe("NewStackitClient", func() {
Context("with STACKIT_NO_AUTH enabled", func() {
It("should create client successfully without authentication", func() {
Expand Down
8 changes: 5 additions & 3 deletions pkg/client/stackit.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,9 @@ type Server struct {

// NIC represents a STACKIT network interface
type NIC struct {
ID string
NetworkID string
AllowedAddresses []string
ID string `json:"id"`
NetworkID string `json:"networkId"`
AllowedAddresses []string `json:"allowedAddresses,omitempty"`
IPv4 string `json:"ipv4,omitempty"`
IPv6 string `json:"ipv6,omitempty"`
}
49 changes: 35 additions & 14 deletions pkg/provider/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/stackitcloud/machine-controller-manager-provider-stackit/pkg/client"
api "github.com/stackitcloud/machine-controller-manager-provider-stackit/pkg/provider/apis"
"github.com/stackitcloud/machine-controller-manager-provider-stackit/pkg/provider/apis/validation"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/klog/v2"
"k8s.io/utils/ptr"
Expand All @@ -27,10 +28,14 @@ import (
// Returns:
// - ProviderID: Unique identifier in format "stackit://<projectId>/<serverId>"
// - NodeName: Name that the VM will register with in Kubernetes (matches Machine name)
// - Addresses: Internal IP addresses of the server's NICs (NodeInternalIP)
//
// Error codes:
// - InvalidArgument: Invalid ProviderSpec or missing required fields
// - Internal: Failed to create server or communicate with STACKIT API
// Error codes (see machine_error_codes.md for retry semantics):
// - InvalidArgument (no retry): Invalid ProviderSpec fields or missing required values
// - Internal (no retry): Malformed ProviderSpec JSON or failed to initialize STACKIT client
// - Unavailable (retry): Transient API failure (create/get server, get NICs, patch NIC)
// - ResourceExhausted (no retry): No capacity available (e.g. "no valid host was found")
// - DeadlineExceeded (retry): Server did not reach ACTIVE state within the polling timeout
func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineRequest) (*driver.CreateMachineResponse, error) {
// Log messages to track request
klog.V(2).Infof("Machine creation request has been received for %q", req.Machine.Name)
Expand Down Expand Up @@ -89,7 +94,18 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR
return nil, status.Error(codes.DeadlineExceeded, fmt.Sprintf("failed waiting for server to be ACTIVE: %v", err))
}

if err := p.patchNetworkInterface(ctx, projectID, server.ID, providerSpec); err != nil {
nics, err := p.client.GetNICsForServer(ctx, projectID, providerSpec.Region, server.ID)
Copy link
Member

Choose a reason for hiding this comment

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

In order to keep the main create function as clean as possible, i think its best to keep the GetNICs function in the patchNetworkInterface function but have it return the patched interfaces.
the UpdateNic Function also returns the updated NIC, which we currently just throw away. Just use these, put them in a list and return them :)
And maybe also rename patchNetworkInterface to patchNetworkInterfaces while doing so :D

if err != nil {
klog.Errorf("Failed to get NICs for server %q: %v", req.Machine.Name, err)
return nil, status.Error(codes.Unavailable, fmt.Sprintf("failed to get NICs for server: %v", err))
}

if len(nics) == 0 {
klog.Errorf("No NICs found for server %q (ID: %s)", req.Machine.Name, server.ID)
return nil, status.Error(codes.Unavailable, fmt.Sprintf("no NICs found for server %q", server.ID))
}

if err := p.patchNetworkInterface(ctx, projectID, nics, providerSpec); err != nil {
klog.Errorf("Failed to patch network interface for server %q: %v", req.Machine.Name, err)
return nil, status.Error(codes.Unavailable, fmt.Sprintf("failed to patch network interface for server: %v", err))
}
Expand All @@ -101,6 +117,7 @@ func (p *Provider) CreateMachine(ctx context.Context, req *driver.CreateMachineR
return &driver.CreateMachineResponse{
ProviderID: providerID,
NodeName: req.Machine.Name,
Addresses: nicAddresses(nics),
}, nil
}

Expand Down Expand Up @@ -213,6 +230,19 @@ func (p *Provider) createServerRequest(req *driver.CreateMachineRequest, provide
return createReq
}

func nicAddresses(nics []*client.NIC) []corev1.NodeAddress {
var addresses []corev1.NodeAddress
for _, nic := range nics {
if nic.IPv4 != "" {
addresses = append(addresses, corev1.NodeAddress{Type: corev1.NodeInternalIP, Address: nic.IPv4})
}
if nic.IPv6 != "" {
addresses = append(addresses, corev1.NodeAddress{Type: corev1.NodeInternalIP, Address: nic.IPv6})
}
}
return addresses
}

func (p *Provider) getServerByName(ctx context.Context, projectID, region, serverName string) (*client.Server, error) {
// Check if the server got already created
labelSelector := map[string]string{
Expand All @@ -235,20 +265,11 @@ func (p *Provider) getServerByName(ctx context.Context, projectID, region, serve
return nil, nil
}

func (p *Provider) patchNetworkInterface(ctx context.Context, projectID, serverID string, providerSpec *api.ProviderSpec) error {
func (p *Provider) patchNetworkInterface(ctx context.Context, projectID string, nics []*client.NIC, providerSpec *api.ProviderSpec) error {
if len(providerSpec.AllowedAddresses) == 0 {
return nil
}

nics, err := p.client.GetNICsForServer(ctx, projectID, providerSpec.Region, serverID)
if err != nil {
return fmt.Errorf("failed to get NICs for server %q: %w", serverID, err)
}

if len(nics) == 0 {
return fmt.Errorf("failed to find NIC for server %q", serverID)
}

for _, nic := range nics {
// if networking is not set, server is inside the default network
// just patch the interface since the server should only have one
Expand Down
34 changes: 34 additions & 0 deletions pkg/provider/create_basic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,24 @@ var _ = Describe("CreateMachine", func() {
Expect(capturedReq.ImageID).To(Equal("12345678-1234-1234-1234-123456789abc"))
})

It("should return internal IPs from NICs in Addresses", func() {
mockClient.GetNICsFunc = func(_ context.Context, _, _, _ string) ([]*client.NIC, error) {
return []*client.NIC{
{ID: "nic-1", NetworkID: "net-1", IPv4: "10.0.0.5", IPv6: "fd00::1"},
{ID: "nic-2", NetworkID: "net-2", IPv4: "10.0.1.5"},
}, nil
}

resp, err := provider.CreateMachine(ctx, req)

Expect(err).NotTo(HaveOccurred())
Expect(resp.Addresses).To(ConsistOf(
corev1.NodeAddress{Type: corev1.NodeInternalIP, Address: "10.0.0.5"},
corev1.NodeAddress{Type: corev1.NodeInternalIP, Address: "fd00::1"},
corev1.NodeAddress{Type: corev1.NodeInternalIP, Address: "10.0.1.5"},
))
})

It("should poll GetServer until server is ACTIVE", func() {
getServerCallCount := 0

Expand Down Expand Up @@ -210,6 +228,22 @@ var _ = Describe("CreateMachine", func() {
})
})

Context("when server has no NICs", func() {
It("should return Unavailable error", func() {
mockClient.GetNICsFunc = func(_ context.Context, _, _, _ string) ([]*client.NIC, error) {
return []*client.NIC{}, 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.Unavailable))
Expect(err.Error()).To(ContainSubstring("no NICs found"))
})
})

Context("when STACKIT API fails", func() {
It("should return Internal error on API failure", func() {
mockClient.CreateServerFunc = func(_ context.Context, _, _ string, _ *client.CreateServerRequest) (*client.Server, error) {
Expand Down