refactor(loadbalancer): target pool ownership

cloud/client.go

@@ -51,9 +51,9 @@ type Client interface {
 
 	ListAPIServerLoadBalancersByTags(ctx context.Context, tags map[string]string) ([]*LoadBalancer, error)
 
-	EnsureAPIServerLoadBalancerTarget(ctx context.Context, input LoadBalancerTargetInput) error
-
-	DeleteAPIServerLoadBalancerTarget(ctx context.Context, input LoadBalancerTargetInput) error
+	// SetAPIServerLoadBalancerTargets replaces the contents of the API-server
+	// target pool. STACKIT rejects an empty pool.
+	SetAPIServerLoadBalancerTargets(ctx context.Context, loadBalancerID string, port int32, targets []LoadBalancerTargetInput) error
 
 	DeleteAPIServerLoadBalancer(ctx context.Context, id string) error
 }

cloud/fake/client.go

@@ -47,8 +47,7 @@ type Client struct {
 	FailNextFindServer    error
 	FailNextEnsureLB      error
 	FailNextDeleteLB      error
-	FailNextEnsureTarget  error
-	FailNextDeleteTarget  error
+	FailNextSetTargets    error
 	FailNextGetNetwork    error
 	FailNextEnsureBastion error
 	FailNextDeleteBastion error
@@ -483,36 +482,37 @@ func (c *Client) ListAPIServerLoadBalancersByTags(
 	return loadBalancers, nil
 }
 
-func (c *Client) EnsureAPIServerLoadBalancerTarget(_ context.Context, input cloud.LoadBalancerTargetInput) error {
+func (c *Client) SetAPIServerLoadBalancerTargets(
+	_ context.Context,
+	loadBalancerID string,
+	port int32,
+	targets []cloud.LoadBalancerTargetInput,
+) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	if err := consume(&c.FailNextEnsureTarget); err != nil {
+	if err := consume(&c.FailNextSetTargets); err != nil {
 		return err
 	}
-	entry, ok := c.loadBalancers[input.LoadBalancerID]
-	if !ok {
-		return fmt.Errorf("load balancer %q: %w", input.LoadBalancerID, cloud.ErrNotFound)
-	}
-	if entry.targets[bootstrapTargetName] == bootstrapTargetIP {
-		delete(entry.targets, bootstrapTargetName)
+	if loadBalancerID == "" || port <= 0 {
+		return fmt.Errorf("load balancer ID and target port are required: %w", cloud.ErrInvalidInput)
 	}
-	entry.targets[input.Name] = input.IP
-	return nil
-}
-
-func (c *Client) DeleteAPIServerLoadBalancerTarget(_ context.Context, input cloud.LoadBalancerTargetInput) error {
-	c.mu.Lock()
-	defer c.mu.Unlock()
-
-	if err := consume(&c.FailNextDeleteTarget); err != nil {
-		return err
+	if len(targets) == 0 {
+		return fmt.Errorf("at least one target is required: %w", cloud.ErrInvalidInput)
 	}
-	entry, ok := c.loadBalancers[input.LoadBalancerID]
+	entry, ok := c.loadBalancers[loadBalancerID]
 	if !ok {
-		return fmt.Errorf("load balancer %q: %w", input.LoadBalancerID, cloud.ErrNotFound)
+		return fmt.Errorf("load balancer %q: %w", loadBalancerID, cloud.ErrNotFound)
 	}
-	delete(entry.targets, input.Name)
+	replaced := make(map[string]string, len(targets))
+	for _, target := range targets {
+		// The real API rejects these; accepting them would hide a regression.
+		if target.Name == "" || target.IP == "" {
+			return fmt.Errorf("target name and target IP are required: %w", cloud.ErrInvalidInput)
+		}
+		replaced[target.Name] = target.IP
+	}
+	entry.targets = replaced
 	return nil
 }
 
@@ -604,6 +604,17 @@ func (c *Client) LoadBalancerTargetCount(id string) int {
 	return len(entry.targets)
 }
 
+// LoadBalancerTargetIPs returns one target pool as name to IP (test helper).
+func (c *Client) LoadBalancerTargetIPs(id string) map[string]string {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	entry, ok := c.loadBalancers[id]
+	if !ok {
+		return nil
+	}
+	return copyTags(entry.targets)
+}
+
 func mapContains(haystack, needle map[string]string) bool {
 	for k, v := range needle {
 		if haystack[k] != v {

cloud/sdk_client.go

@@ -11,12 +11,14 @@ You may obtain a copy of the License at
 package cloud
 
 import (
+	"cmp"
 	"context"
 	"encoding/base64"
 	"errors"
 	"fmt"
 	"net/http"
 	"os"
+	"slices"
 	"strings"
 
 	"github.com/stackitcloud/stackit-sdk-go/core/config"
@@ -568,11 +570,31 @@ func (c *SDKClient) ListAPIServerLoadBalancersByTags(
 	return matched, nil
 }
 
-func (c *SDKClient) EnsureAPIServerLoadBalancerTarget(ctx context.Context, input LoadBalancerTargetInput) error {
-	if input.LoadBalancerID == "" || input.Name == "" || input.IP == "" {
-		return fmt.Errorf("%w: load balancer ID, target name, and target IP are required", ErrInvalidInput)
+func (c *SDKClient) SetAPIServerLoadBalancerTargets(
+	ctx context.Context,
+	loadBalancerID string,
+	port int32,
+	targets []LoadBalancerTargetInput,
+) error {
+	if loadBalancerID == "" || port <= 0 {
+		return fmt.Errorf("%w: load balancer ID and target port are required", ErrInvalidInput)
+	}
+	// STACKIT NLB target pools must contain at least one target.
+	if len(targets) == 0 {
+		return fmt.Errorf("%w: at least one target is required", ErrInvalidInput)
+	}
+	desired := make([]lb.Target, 0, len(targets))
+	for _, targetInput := range targets {
+		if targetInput.Name == "" || targetInput.IP == "" {
+			return fmt.Errorf("%w: target name and target IP are required", ErrInvalidInput)
+		}
+		target := lb.NewTarget()
+		target.SetDisplayName(targetInput.Name)
+		target.SetIp(targetInput.IP)
+		desired = append(desired, *target)
 	}
-	loadBalancer, err := c.lbClient.DefaultAPI.GetLoadBalancer(ctx, c.projectID, c.region, input.LoadBalancerID).Execute()
+
+	loadBalancer, err := c.lbClient.DefaultAPI.GetLoadBalancer(ctx, c.projectID, c.region, loadBalancerID).Execute()
 	if err != nil {
 		return classifySDKError("get load balancer", err)
 	}
@@ -581,68 +603,37 @@ func (c *SDKClient) EnsureAPIServerLoadBalancerTarget(ctx context.Context, input
 		return fmt.Errorf(
 			"%w: load balancer %q has no %q target pool",
 			ErrNotFound,
-			input.LoadBalancerID,
+			loadBalancerID,
 			apiserverTargetPoolName,
 		)
 	}
 
-	targets := withoutBootstrapTarget(targetPool.GetTargets())
-	for i := range targets {
-		if targets[i].GetDisplayName() == input.Name || targets[i].GetIp() == input.IP {
-			targets[i].SetDisplayName(input.Name)
-			targets[i].SetIp(input.IP)
-			return c.updateAPIServerTargetPool(ctx, input.LoadBalancerID, targetPool, targets, input.Port)
-		}
+	// Every cluster reconcile ends up here; skip the update call when the pool
+	// already matches.
+	if targetPool.GetTargetPort() == port && sameTargets(targetPool.GetTargets(), desired) {
+		return nil
 	}
-
-	target := lb.NewTarget()
-	target.SetDisplayName(input.Name)
-	target.SetIp(input.IP)
-	targets = append(targets, *target)
-	return c.updateAPIServerTargetPool(ctx, input.LoadBalancerID, targetPool, targets, input.Port)
+	return c.updateAPIServerTargetPool(ctx, loadBalancerID, targetPool, desired, port)
 }
 
-func withoutBootstrapTarget(targets []lb.Target) []lb.Target {
-	out := targets[:0]
-	for _, target := range targets {
-		if target.GetDisplayName() == bootstrapTargetName {
-			continue
-		}
-		out = append(out, target)
+func sameTargets(current, desired []lb.Target) bool {
+	if len(current) != len(desired) {
+		return false
 	}
-	return out
+	return slices.Equal(sortedTargetKeys(current), sortedTargetKeys(desired))
 }
 
-func (c *SDKClient) DeleteAPIServerLoadBalancerTarget(ctx context.Context, input LoadBalancerTargetInput) error {
-	if input.LoadBalancerID == "" || input.Name == "" {
-		return fmt.Errorf("%w: load balancer ID and target name are required", ErrInvalidInput)
-	}
-	loadBalancer, err := c.lbClient.DefaultAPI.GetLoadBalancer(ctx, c.projectID, c.region, input.LoadBalancerID).Execute()
-	if err != nil {
-		return classifySDKError("get load balancer", err)
-	}
-	targetPool := apiServerTargetPool(loadBalancer)
-	if targetPool == nil {
-		return nil
-	}
+type targetKey struct{ name, ip string }
 
-	targets := targetPool.GetTargets()
-	out := make([]lb.Target, 0, len(targets))
+func sortedTargetKeys(targets []lb.Target) []targetKey {
+	keys := make([]targetKey, 0, len(targets))
 	for _, target := range targets {
-		if target.GetDisplayName() == input.Name {
-			continue
-		}
-		out = append(out, target)
-	}
-	if len(out) == len(targets) {
-		return nil
-	}
-	if len(out) == 0 {
-		// STACKIT NLB target pools must contain at least one target. Leave the
-		// last target in place; deleting the load balancer removes it.
-		return nil
+		keys = append(keys, targetKey{name: target.GetDisplayName(), ip: target.GetIp()})
 	}
-	return c.updateAPIServerTargetPool(ctx, input.LoadBalancerID, targetPool, out, input.Port)
+	slices.SortFunc(keys, func(a, b targetKey) int {
+		return cmp.Or(strings.Compare(a.name, b.name), strings.Compare(a.ip, b.ip))
+	})
+	return keys
 }
 
 func (c *SDKClient) findLoadBalancerByTags(ctx context.Context, tags map[string]string) (*LoadBalancer, error) {

cloud/sdk_client_integration_test.go

@@ -69,7 +69,6 @@ func TestSDKClientLoadBalancerCreateDeleteIntegration(t *testing.T) {
 	loadBalancer := createIntegrationLoadBalancer(t, client, networkID, LoadBalancerTargetInput{
 		Name: "capistackit-initial-target",
 		IP:   targetIP,
-		Port: 6443,
 	})
 
 	if err := client.DeleteAPIServerLoadBalancer(context.Background(), loadBalancer.ID); err != nil {
@@ -81,28 +80,26 @@ func TestSDKClientLoadBalancerTargetIntegration(t *testing.T) {
 	client := newIntegrationClient(t)
 	networkID := requiredIntegrationEnv(t, envIntegrationNetworkID)
 	targetIP := requiredIntegrationEnv(t, envIntegrationTargetIP)
+	initialIP := integrationInitialTargetIP(targetIP)
 	loadBalancer := createIntegrationLoadBalancer(t, client, networkID, LoadBalancerTargetInput{
 		Name: "capistackit-initial-target",
-		IP:   integrationInitialTargetIP(targetIP),
-		Port: 6443,
+		IP:   initialIP,
 	})
 	t.Cleanup(func() {
 		if err := client.DeleteAPIServerLoadBalancer(context.Background(), loadBalancer.ID); err != nil && !IsNotFound(err) {
 			t.Logf("DeleteAPIServerLoadBalancer() cleanup error = %v", err)
 		}
 	})
 
-	target := LoadBalancerTargetInput{
-		LoadBalancerID: loadBalancer.ID,
-		Name:           "capistackit-integration-target",
-		IP:             targetIP,
-		Port:           6443,
+	targets := []LoadBalancerTargetInput{
+		{Name: "capistackit-initial-target", IP: initialIP},
+		{Name: "capistackit-integration-target", IP: targetIP},
 	}
-	if err := client.EnsureAPIServerLoadBalancerTarget(context.Background(), target); err != nil {
-		t.Fatalf("EnsureAPIServerLoadBalancerTarget() error = %v", err)
+	if err := client.SetAPIServerLoadBalancerTargets(context.Background(), loadBalancer.ID, 6443, targets); err != nil {
+		t.Fatalf("SetAPIServerLoadBalancerTargets() error = %v", err)
 	}
-	if err := client.DeleteAPIServerLoadBalancerTarget(context.Background(), target); err != nil {
-		t.Fatalf("DeleteAPIServerLoadBalancerTarget() error = %v", err)
+	if err := client.SetAPIServerLoadBalancerTargets(context.Background(), loadBalancer.ID, 6443, targets[:1]); err != nil {
+		t.Fatalf("SetAPIServerLoadBalancerTargets() shrink error = %v", err)
 	}
 }
 

cloud/sdk_client_test.go

@@ -211,7 +211,7 @@ func TestSDKClientEnsureAPIServerLoadBalancerUsesBootstrapTargetWhenInitialTarge
 	assertNestedStringField(t, createPayload, []string{"listeners", "0", "targetPool"}, apiserverTargetPoolName)
 }
 
-func TestSDKClientLoadBalancerTargetUpdates(t *testing.T) {
+func TestSDKClientSetsAPIServerTargetPool(t *testing.T) {
 	var mu sync.Mutex
 	targets := []any{
 		map[string]any{"displayName": "cp-0", "ip": "10.0.0.10"},
@@ -247,30 +247,57 @@ func TestSDKClientLoadBalancerTargetUpdates(t *testing.T) {
 	}))
 
 	client := newTestSDKClient(t, server.URL)
-	input := LoadBalancerTargetInput{
-		LoadBalancerID: "apiserver-test",
-		Name:           "cp-1",
-		IP:             "10.0.0.11",
-		Port:           6443,
+	desired := []LoadBalancerTargetInput{
+		{Name: "cp-0", IP: "10.0.0.10"},
+		{Name: "cp-1", IP: "10.0.0.11"},
 	}
-	if err := client.EnsureAPIServerLoadBalancerTarget(context.Background(), input); err != nil {
-		t.Fatalf("EnsureAPIServerLoadBalancerTarget() error = %v", err)
+	if err := client.SetAPIServerLoadBalancerTargets(context.Background(), "apiserver-test", 6443, desired); err != nil {
+		t.Fatalf("SetAPIServerLoadBalancerTargets() error = %v", err)
 	}
-	if err := client.DeleteAPIServerLoadBalancerTarget(context.Background(), input); err != nil {
-		t.Fatalf("DeleteAPIServerLoadBalancerTarget() error = %v", err)
+	if len(updatePayloads) != 1 {
+		t.Fatalf("got %d update payloads, want 1", len(updatePayloads))
 	}
+	assertNestedStringField(t, updatePayloads[0], []string{"targets", "0", "displayName"}, "cp-0")
+	assertNestedStringField(t, updatePayloads[0], []string{"targets", "1", "displayName"}, "cp-1")
+	assertNestedStringField(t, updatePayloads[0], []string{"targets", "1", "ip"}, "10.0.0.11")
 
+	// An unchanged pool must not turn into a write.
+	if err := client.SetAPIServerLoadBalancerTargets(context.Background(), "apiserver-test", 6443, desired); err != nil {
+		t.Fatalf("SetAPIServerLoadBalancerTargets() repeat error = %v", err)
+	}
+	if len(updatePayloads) != 1 {
+		t.Fatalf("got %d update payloads after an unchanged set, want 1", len(updatePayloads))
+	}
+
+	if err := client.SetAPIServerLoadBalancerTargets(
+		context.Background(),
+		"apiserver-test",
+		6443,
+		desired[:1],
+	); err != nil {
+		t.Fatalf("SetAPIServerLoadBalancerTargets() shrink error = %v", err)
+	}
 	if len(updatePayloads) != 2 {
 		t.Fatalf("got %d update payloads, want 2", len(updatePayloads))
 	}
-	assertNestedStringField(t, updatePayloads[0], []string{"targets", "1", "displayName"}, "cp-1")
-	assertNestedStringField(t, updatePayloads[0], []string{"targets", "1", "ip"}, "10.0.0.11")
 	if got := nestedValue(t, updatePayloads[1], []string{"targets"}).([]any); len(got) != 1 {
-		t.Fatalf("delete target payload targets = %#v, want one remaining target", got)
+		t.Fatalf("shrunk target payload targets = %#v, want one remaining target", got)
 	}
 	assertNestedStringField(t, updatePayloads[1], []string{"targets", "0", "displayName"}, "cp-0")
 }
 
+func TestSDKClientRejectsEmptyAPIServerTargetPool(t *testing.T) {
+	server := newSDKTestServer(t, http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+		t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String())
+	}))
+
+	client := newTestSDKClient(t, server.URL)
+	err := client.SetAPIServerLoadBalancerTargets(context.Background(), "apiserver-test", 6443, nil)
+	if !IsInvalidInput(err) {
+		t.Fatalf("SetAPIServerLoadBalancerTargets() error = %v, want ErrInvalidInput", err)
+	}
+}
+
 func TestSDKClientClassifiesHTTPStatusCodes(t *testing.T) {
 	tests := []struct {
 		name   string