From f9524528d744ec32b6cf5d501c26e2682884ab54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Amand?= Date: Mon, 16 Mar 2026 14:27:37 +0100 Subject: [PATCH 1/3] fix: add missing client test suite and fix test case --- pkg/client/client_suite_test.go | 13 +++++++++++++ pkg/client/sdk_test.go | 2 +- 2 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 pkg/client/client_suite_test.go diff --git a/pkg/client/client_suite_test.go b/pkg/client/client_suite_test.go new file mode 100644 index 00000000..78b821c2 --- /dev/null +++ b/pkg/client/client_suite_test.go @@ -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") +} diff --git a/pkg/client/sdk_test.go b/pkg/client/sdk_test.go index 21ea9dfd..4979e14c 100644 --- a/pkg/client/sdk_test.go +++ b/pkg/client/sdk_test.go @@ -122,7 +122,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")) }) }) From a1942c7ec05e90cd9f2c23058b78328363c61bfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Amand?= Date: Mon, 16 Mar 2026 14:28:35 +0100 Subject: [PATCH 2/3] feat: Add InternalIPs to CreateMachineResponse --- pkg/client/mock/client.go | 4 ++- pkg/client/sdk.go | 2 ++ pkg/client/sdk_test.go | 60 +++++++++++++++++++++++++++++++ pkg/client/stackit.go | 8 +++-- pkg/provider/create.go | 49 +++++++++++++++++-------- pkg/provider/create_basic_test.go | 34 ++++++++++++++++++ 6 files changed, 139 insertions(+), 18 deletions(-) diff --git a/pkg/client/mock/client.go b/pkg/client/mock/client.go index bb623de3..43a7fbf2 100644 --- a/pkg/client/mock/client.go +++ b/pkg/client/mock/client.go @@ -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) { diff --git a/pkg/client/sdk.go b/pkg/client/sdk.go index 27174edd..a306ac85 100644 --- a/pkg/client/sdk.go +++ b/pkg/client/sdk.go @@ -332,6 +332,8 @@ func convertSDKNICtoNIC(nic *iaas.NIC) *NIC { ID: nic.GetId(), NetworkID: nic.GetNetworkId(), AllowedAddresses: addresses, + IPv4: nic.GetIpv4(), + IPv6: nic.GetIpv6(), } } diff --git a/pkg/client/sdk_test.go b/pkg/client/sdk_test.go index 4979e14c..c2ca1131 100644 --- a/pkg/client/sdk_test.go +++ b/pkg/client/sdk_test.go @@ -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() { @@ -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() { diff --git a/pkg/client/stackit.go b/pkg/client/stackit.go index 21643426..41b4810a 100644 --- a/pkg/client/stackit.go +++ b/pkg/client/stackit.go @@ -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"` } diff --git a/pkg/provider/create.go b/pkg/provider/create.go index f79c5465..f7b87fcf 100644 --- a/pkg/provider/create.go +++ b/pkg/provider/create.go @@ -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" @@ -27,10 +28,14 @@ import ( // Returns: // - ProviderID: Unique identifier in format "stackit:///" // - 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) @@ -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) + 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)) } @@ -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 } @@ -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{ @@ -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 diff --git a/pkg/provider/create_basic_test.go b/pkg/provider/create_basic_test.go index ce1b86b5..c6a47d14 100644 --- a/pkg/provider/create_basic_test.go +++ b/pkg/provider/create_basic_test.go @@ -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 @@ -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) { From d39fd9f1b7252f805c4ed5e558f046b721898c8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Amand?= Date: Fri, 20 Mar 2026 12:25:12 +0100 Subject: [PATCH 3/3] chore: Refactor NIC fetching in CreateMachine (PR feedback) Co-authored-by: Felix Breuer --- pkg/client/sdk_test.go | 27 +++++++++++++-------------- pkg/provider/create.go | 40 ++++++++++++++++++++++------------------ 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/pkg/client/sdk_test.go b/pkg/client/sdk_test.go index c2ca1131..cb74ff30 100644 --- a/pkg/client/sdk_test.go +++ b/pkg/client/sdk_test.go @@ -8,8 +8,7 @@ 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" + iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api" ) var _ = Describe("SDK Client Helpers", func() { @@ -208,10 +207,10 @@ 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"), + Id: new("nic-1"), + NetworkId: new("net-1"), + Ipv4: new("10.0.0.5"), + Ipv6: new("fd00::1"), } result := convertSDKNICtoNIC(sdkNIC) @@ -224,9 +223,9 @@ var _ = Describe("SDK Type Conversion Helpers", func() { 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"), + Id: new("nic-1"), + NetworkId: new("net-1"), + Ipv4: new("10.0.0.5"), } result := convertSDKNICtoNIC(sdkNIC) @@ -237,8 +236,8 @@ var _ = Describe("SDK Type Conversion Helpers", func() { It("should handle a NIC with neither IPv4 nor IPv6", func() { sdkNIC := &iaas.NIC{ - Id: ptr.To("nic-1"), - NetworkId: ptr.To("net-1"), + Id: new("nic-1"), + NetworkId: new("net-1"), } result := convertSDKNICtoNIC(sdkNIC) @@ -250,9 +249,9 @@ var _ = Describe("SDK Type Conversion Helpers", func() { 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{ + Id: new("nic-1"), + NetworkId: new("net-1"), + AllowedAddresses: []iaas.AllowedAddressesInner{ {String: &addr}, }, } diff --git a/pkg/provider/create.go b/pkg/provider/create.go index f7b87fcf..930600b5 100644 --- a/pkg/provider/create.go +++ b/pkg/provider/create.go @@ -94,20 +94,10 @@ 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)) } - nics, err := p.client.GetNICsForServer(ctx, projectID, providerSpec.Region, server.ID) + nics, err := p.patchNetworkInterfaces(ctx, projectID, server.ID, providerSpec) 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)) + 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:/// @@ -265,17 +255,28 @@ func (p *Provider) getServerByName(ctx context.Context, projectID, region, serve return nil, nil } -func (p *Provider) patchNetworkInterface(ctx context.Context, projectID string, nics []*client.NIC, providerSpec *api.ProviderSpec) error { +func (p *Provider) patchNetworkInterfaces(ctx context.Context, projectID, serverID string, providerSpec *api.ProviderSpec) ([]*client.NIC, error) { + nics, err := p.client.GetNICsForServer(ctx, projectID, providerSpec.Region, serverID) + if err != nil { + return nil, fmt.Errorf("failed to get NICs for server %q: %w", serverID, err) + } + + if len(nics) == 0 { + return nil, fmt.Errorf("no NICs found for server %q", serverID) + } + if len(providerSpec.AllowedAddresses) == 0 { - return nil + return nics, nil } + result := make([]*client.NIC, 0, len(nics)) 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 if providerSpec.Networking != nil { // only process interfaces that are either in the configured network (NetworkID) or are defined in NICIDs if providerSpec.Networking.NetworkID != nic.NetworkID && !slices.Contains(providerSpec.Networking.NICIDs, nic.ID) { + result = append(result, nic) continue } } @@ -290,17 +291,20 @@ func (p *Provider) patchNetworkInterface(ctx context.Context, projectID string, } if !updateNic { + result = append(result, nic) continue } - if _, err := p.client.UpdateNIC(ctx, projectID, providerSpec.Region, nic.NetworkID, nic.ID, nic.AllowedAddresses); err != nil { - return fmt.Errorf("failed to update allowed addresses for NIC %s: %w", nic.ID, err) + updatedNic, err := p.client.UpdateNIC(ctx, projectID, providerSpec.Region, nic.NetworkID, nic.ID, nic.AllowedAddresses) + if err != nil { + return nil, fmt.Errorf("failed to update allowed addresses for NIC %s: %w", nic.ID, err) } klog.V(2).Infof("Updated allowed addresses for NIC %s to %v", nic.ID, nic.AllowedAddresses) + result = append(result, updatedNic) } - return nil + return result, nil } func (p *Provider) WaitUntilServerRunning(ctx context.Context, projectID, region, serverID string) error {