feat: Add InternalIPs to CreateMachineResponse (#140)

* fix: add missing client test suite and fix test case

* feat: Add InternalIPs to CreateMachineResponse

* chore: Refactor NIC fetching in CreateMachine (PR feedback)

Co-authored-by: Felix Breuer <f.breuer94@gmail.com>

---------

Co-authored-by: Felix Breuer <f.breuer94@gmail.com>

Author: jamand

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")
+}

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

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(),
 	}
 }
 

pkg/client/sdk_test.go

@@ -8,6 +8,7 @@ import (
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	"github.com/stackitcloud/stackit-sdk-go/core/oapierror"
+	iaas "github.com/stackitcloud/stackit-sdk-go/services/iaas/v2api"
 )
 
 var _ = Describe("SDK Client Helpers", func() {
@@ -122,7 +123,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"))
 			})
 		})
@@ -203,6 +204,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:        new("nic-1"),
+				NetworkId: new("net-1"),
+				Ipv4:      new("10.0.0.5"),
+				Ipv6:      new("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:        new("nic-1"),
+				NetworkId: new("net-1"),
+				Ipv4:      new("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:        new("nic-1"),
+				NetworkId: new("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:        new("nic-1"),
+				NetworkId: new("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() {

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

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://<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)
@@ -89,9 +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))
 	}
 
-	if err := p.patchNetworkInterface(ctx, projectID, server.ID, 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))
+	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>
@@ -101,6 +107,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 +220,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,26 +255,28 @@ 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 {
-	if len(providerSpec.AllowedAddresses) == 0 {
-		return nil
-	}
-
+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 fmt.Errorf("failed to get NICs for server %q: %w", serverID, err)
+		return nil, 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)
+		return nil, fmt.Errorf("no NICs found for server %q", serverID)
 	}
 
+	if len(providerSpec.AllowedAddresses) == 0 {
+		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
 			}
 		}
@@ -269,17 +291,20 @@ func (p *Provider) patchNetworkInterface(ctx context.Context, projectID, serverI
 		}
 
 		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 {

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