feat: retrieve, verify and apply availability zones correctly

Signed-off-by: Jan Larwig <jan.larwig@digits.schwarz>

Author: tuunit

Makefile

@@ -23,6 +23,7 @@ STACKIT_WORKLOAD_CNI ?= cilium
 CILIUM_VERSION ?= 1.19.4
 CILIUM_VALUES ?= templates/addons/cilium-values.yaml
 CALICO_MANIFEST ?= https://raw.githubusercontent.com/projectcalico/calico/v3.30.0/manifests/calico.yaml
+ENABLE_WEBHOOKS ?= false
 # YEAR defines the year value used for substituting the YEAR placeholder in the boilerplate header.
 YEAR ?= $(shell date +%Y)
 
@@ -187,7 +188,7 @@ build: manifests generate fmt vet ## Build manager binary.
 
 .PHONY: run
 run: manifests generate fmt vet ## Run a controller from your host.
-	go run ./cmd/manager/main.go
+	ENABLE_WEBHOOKS=$(ENABLE_WEBHOOKS) go run ./cmd/manager/main.go
 
 # If you wish to build the manager image targeting other platforms you can use the --platform flag.
 # (i.e. docker build --platform linux/arm64). However, you must enable docker buildKit for it.

cloud/client.go

@@ -56,6 +56,8 @@ type Client interface {
 	SetAPIServerLoadBalancerTargets(ctx context.Context, loadBalancerID string, port int32, targets []LoadBalancerTargetInput) error
 
 	DeleteAPIServerLoadBalancer(ctx context.Context, id string) error
+
+	ListAvailabilityZones(ctx context.Context) ([]string, error)
 }
 
 // Factory builds a Client from credentials (raw bytes from the configured

cloud/fake/client.go

@@ -53,6 +53,14 @@ type Client struct {
 	FailNextDeleteBastion error
 	FailNextEnsureNodeSSH error
 	FailNextDeleteNodeSSH error
+	FailNextListAZ        error
+
+	// AvailabilityZones is returned by ListAvailabilityZones if non-empty.
+	// Defaults to eu01-1, eu01-2, eu01-3.
+	AvailabilityZones []string
+
+	// LastCreateServerInput records the most recent input passed to CreateServer.
+	LastCreateServerInput cloud.CreateServerInput
 
 	// Before* hooks, if non-nil, run before the call they belong to. They let a
 	// test observe API server state at the exact moment a cloud call would
@@ -189,6 +197,7 @@ func (c *Client) CreateServer(_ context.Context, input cloud.CreateServerInput)
 	}
 
 	id := c.genID()
+	c.LastCreateServerInput = input
 	zone := input.AvailabilityZone
 	if zone == "" {
 		zone = "eu01-1"
@@ -716,6 +725,18 @@ func idOfSecurityGroup(securityGroup *cloud.SecurityGroup) string {
 	return securityGroup.ID
 }
 
+func (c *Client) ListAvailabilityZones(_ context.Context) ([]string, error) {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+	if err := consume(&c.FailNextListAZ); err != nil {
+		return nil, err
+	}
+	if len(c.AvailabilityZones) > 0 {
+		return slices.Clone(c.AvailabilityZones), nil
+	}
+	return []string{"eu01-1", "eu01-2", "eu01-3"}, nil
+}
+
 func cloneServer(s *cloud.Server) *cloud.Server {
 	out := *s
 	if s.Addresses != nil {

cloud/sdk_client.go

@@ -203,6 +203,17 @@ func (c *SDKClient) CreateServer(ctx context.Context, input CreateServerInput) (
 	return c.serverFromSDK(ctx, server), nil
 }
 
+func (c *SDKClient) ListAvailabilityZones(ctx context.Context) ([]string, error) {
+	resp, err := c.iaasClient.DefaultAPI.ListAvailabilityZones(ctx, c.region).Execute()
+	if err != nil {
+		return nil, classifySDKError("list availability zones", err)
+	}
+	if resp == nil || resp.Items == nil {
+		return nil, nil
+	}
+	return resp.GetItems(), nil
+}
+
 func (c *SDKClient) DeleteServer(ctx context.Context, id string) error {
 	if err := c.iaasClient.DefaultAPI.DeleteServer(ctx, c.projectID, c.region, id).Execute(); err != nil {
 		return classifySDKError("delete server", err)

cloud/sdk_client_test.go

@@ -707,3 +707,24 @@ func TestSDKClientEnsureBastionDeduplicatesRepeatedCIDRs(t *testing.T) {
 			len(createdRuleCIDRs), createdRuleCIDRs)
 	}
 }
+
+func TestSDKClientListAvailabilityZones(t *testing.T) {
+	server := newSDKTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Method == http.MethodGet && strings.HasSuffix(r.URL.Path, "/availability-zones") {
+			writeJSON(t, w, map[string]any{
+				"items": []any{"eu01-1", "eu01-2", "eu01-3"},
+			})
+			return
+		}
+		t.Fatalf("unexpected request: %s %s", r.Method, r.URL.String())
+	}))
+
+	client := newTestSDKClient(t, server.URL)
+	zones, err := client.ListAvailabilityZones(context.Background())
+	if err != nil {
+		t.Fatalf("ListAvailabilityZones() error = %v", err)
+	}
+	if len(zones) != 3 || zones[0] != "eu01-1" || zones[1] != "eu01-2" || zones[2] != "eu01-3" {
+		t.Fatalf("ListAvailabilityZones() = %v, want [eu01-1 eu01-2 eu01-3]", zones)
+	}
+}

cmd/manager/main.go

@@ -183,6 +183,7 @@ func main() {
 		os.Exit(1)
 	}
 
+	// nolint:staticcheck // SA1019 ignore this deprecation as long as cluster-api doesn't support the newer recorder
 	record.InitFromRecorder(mgr.GetEventRecorderFor("cluster-api-provider-stackit"))
 
 	if err := (&controller.StackitClusterReconciler{

controller/stackitcluster_controller_test.go

@@ -969,6 +969,35 @@ var _ = Describe("StackitCluster Controller", func() {
 		requests := reconciler.filterForStackitClusterResources(ctx, cluster)
 		Expect(requests).To(BeEmpty())
 	})
+
+	It("discovers failure domains dynamically and filters out metro (-m) zones", func() {
+		fakeCloud.AvailabilityZones = []string{"eu01-2", "eu01-m", "eu01-1", "eu01-3"}
+
+		result, err := reconciler.Reconcile(ctx, request)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(result).To(Equal(reconcile.Result{}))
+
+		got := &infrav1.StackitCluster{}
+		Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed())
+		Expect(got.Status.FailureDomains).To(Equal([]clusterv1.FailureDomain{
+			{Name: "eu01-1", ControlPlane: new(true), Attributes: map[string]string{"region": "eu01"}},
+			{Name: "eu01-2", ControlPlane: new(true), Attributes: map[string]string{"region": "eu01"}},
+			{Name: "eu01-3", ControlPlane: new(true), Attributes: map[string]string{"region": "eu01"}},
+		}))
+	})
+
+	It("reports an error condition when listing availability zones fails", func() {
+		fakeCloud.FailNextListAZ = fmt.Errorf("cloud failure: %w", cloud.ErrTransient)
+
+		result, err := reconciler.Reconcile(ctx, request)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(result.RequeueAfter).To(BeNumerically(">", 0))
+
+		got := &infrav1.StackitCluster{}
+		Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed())
+		Expect(got.Status.Ready).To(BeFalse())
+		expectCondition(got.Status.Conditions, infrav1.ClusterReadyCondition, metav1.ConditionFalse, "AvailabilityZonesError")
+	})
 })
 
 func expectBastionCloudInit(

controller/stackitcluster_infrastructure.go

@@ -15,6 +15,8 @@ import (
 	"errors"
 	"fmt"
 	"net/netip"
+	"slices"
+	"strings"
 	"time"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -48,7 +50,7 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS
 			return ctrl.Result{}, fmt.Errorf("persist finalizer: %w", err)
 		}
 	}
-	stackitCluster.Status.FailureDomains = stackitFailureDomains(stackitCluster.Spec.Region)
+	clusterScope.SetFailureDomains(stackitFailureDomains(stackitCluster.Spec.Region))
 
 	cloudClient, err := util.BuildCloudClient(ctx, r.Client, r.CloudClientFactory, stackitCluster)
 	if err != nil {
@@ -71,6 +73,21 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS
 		infrav1.ClusterCredentialsReadyCondition,
 	)
 
+	zones, err := cloudClient.ListAvailabilityZones(ctx)
+	if err != nil {
+		stackitCluster.Status.Ready = false
+		return util.CloudFailureResult(
+			&stackitCluster.Status.Conditions,
+			stackitCluster.Generation,
+			"AvailabilityZonesError",
+			err,
+			retryableErrorRequeueAfter,
+			true,
+			infrav1.ClusterReadyCondition,
+		)
+	}
+	clusterScope.SetFailureDomains(failureDomainsFromZones(stackitCluster.Spec.Region, zones))
+
 	network, err := cloudClient.GetNetwork(ctx, stackitCluster.Spec.Network.ID)
 	if err != nil {
 		log.Error(err, "Failed to get network",
@@ -272,6 +289,31 @@ func (r *StackitClusterReconciler) reconcileNormal(ctx context.Context, clusterS
 	return ctrl.Result{}, nil
 }
 
+func failureDomainsFromZones(region string, zones []string) []clusterv1.FailureDomain {
+	controlPlane := true
+	failureDomains := make([]clusterv1.FailureDomain, 0, len(zones))
+	for _, zone := range zones {
+		// Filter out virtual/metro zones (such as eu01-m or any zone with suffix -m)
+		if strings.HasSuffix(zone, "-m") {
+			continue
+		}
+		failureDomains = append(failureDomains, clusterv1.FailureDomain{
+			Name:         zone,
+			ControlPlane: &controlPlane,
+			Attributes: map[string]string{
+				"region": region,
+			},
+		})
+	}
+	if len(failureDomains) == 0 {
+		return stackitFailureDomains(region)
+	}
+	slices.SortFunc(failureDomains, func(a, b clusterv1.FailureDomain) int {
+		return strings.Compare(a.Name, b.Name)
+	})
+	return failureDomains
+}
+
 func stackitFailureDomains(region string) []clusterv1.FailureDomain {
 	controlPlane := true
 	return []clusterv1.FailureDomain{

controller/stackitmachine_controller_test.go

@@ -285,6 +285,70 @@ var _ = Describe("StackitMachine Controller", func() {
 			expectCondition(got.Status.Conditions, infrav1.MachineReadyCondition, metav1.ConditionFalse, "InvalidFailureDomain")
 		})
 
+		//nolint:dupl // Failure domain resolution cases test explicit failureDomain vs cluster default.
+		It("respects Machine.spec.failureDomain when StackitMachine.spec.availabilityZone is empty", func() {
+			got := &infrav1.StackitMachine{}
+			Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed())
+			got.Spec.AvailabilityZone = ""
+			Expect(k8sClient.Update(ctx, got)).To(Succeed())
+
+			ownerMachine := &clusterv1.Machine{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: machineName, Namespace: namespace}, ownerMachine)).To(Succeed())
+			ownerMachine.Spec.FailureDomain = "eu01-2"
+			Expect(k8sClient.Update(ctx, ownerMachine)).To(Succeed())
+
+			result, err := reconciler.Reconcile(ctx, request)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result).To(Equal(reconcile.Result{}))
+			Expect(fakeCloud.ServerCount()).To(Equal(1))
+			Expect(fakeCloud.LastCreateServerInput.AvailabilityZone).To(Equal("eu01-2"))
+
+			Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed())
+			Expect(got.Status.Ready).To(BeTrue())
+			Expect(got.Status.AvailabilityZone).To(Equal("eu01-2"))
+		})
+
+		//nolint:dupl // Failure domain resolution cases test explicit failureDomain vs cluster default.
+		It("defaults to the first failure domain when neither failureDomain nor availabilityZone is set", func() {
+			got := &infrav1.StackitMachine{}
+			Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed())
+			got.Spec.AvailabilityZone = ""
+			Expect(k8sClient.Update(ctx, got)).To(Succeed())
+
+			ownerMachine := &clusterv1.Machine{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: machineName, Namespace: namespace}, ownerMachine)).To(Succeed())
+			ownerMachine.Spec.FailureDomain = ""
+			Expect(k8sClient.Update(ctx, ownerMachine)).To(Succeed())
+
+			result, err := reconciler.Reconcile(ctx, request)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result).To(Equal(reconcile.Result{}))
+			Expect(fakeCloud.ServerCount()).To(Equal(1))
+			Expect(fakeCloud.LastCreateServerInput.AvailabilityZone).To(Equal("eu01-1"))
+
+			Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed())
+			Expect(got.Status.Ready).To(BeTrue())
+			Expect(got.Status.AvailabilityZone).To(Equal("eu01-1"))
+		})
+
+		It("does not create a VM when Machine.spec.failureDomain is outside published failure domains", func() {
+			ownerMachine := &clusterv1.Machine{}
+			Expect(k8sClient.Get(ctx, types.NamespacedName{Name: machineName, Namespace: namespace}, ownerMachine)).To(Succeed())
+			ownerMachine.Spec.FailureDomain = "eu01-9"
+			Expect(k8sClient.Update(ctx, ownerMachine)).To(Succeed())
+
+			result, err := reconciler.Reconcile(ctx, request)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(result).To(Equal(reconcile.Result{}))
+			Expect(fakeCloud.ServerCount()).To(Equal(0))
+
+			got := &infrav1.StackitMachine{}
+			Expect(k8sClient.Get(ctx, stackitKey, got)).To(Succeed())
+			Expect(got.Status.Ready).To(BeFalse())
+			expectCondition(got.Status.Conditions, infrav1.MachineInstanceReadyCondition, metav1.ConditionFalse, "InvalidFailureDomain")
+			expectCondition(got.Status.Conditions, infrav1.MachineReadyCondition, metav1.ConditionFalse, "InvalidFailureDomain")
+		})
+
 		It("requeues on unauthorized credentials and recovers once they are corrected", func() {
 			reconciler.CloudClientFactory = func(context.Context, cloud.Credentials) (cloud.Client, error) {
 				return nil, fmt.Errorf("authenticate: %w", cloud.ErrUnauthorized)

controller/stackitmachine_infrastructure.go

@@ -184,16 +184,28 @@ func (r *StackitMachineReconciler) reconcileNormal(ctx context.Context, machineS
 }
 
 func validateMachineAvailabilityZone(machineScope *scope.MachineScope) error {
-	availabilityZone := machineScope.StackitMachine.Spec.AvailabilityZone
-	if availabilityZone == "" || len(machineScope.StackitCluster.Status.FailureDomains) == 0 {
+	clusterFailureDomains := machineScope.StackitCluster.Status.FailureDomains
+	if len(clusterFailureDomains) == 0 {
 		return nil
 	}
-	for _, failureDomain := range machineScope.StackitCluster.Status.FailureDomains {
-		if failureDomain.Name == availabilityZone {
-			return nil
+	knownAZ := func(zone string) bool {
+		for _, failureDomain := range clusterFailureDomains {
+			if failureDomain.Name == zone {
+				return true
+			}
 		}
+		return false
+	}
+	if az := machineScope.StackitMachine.Spec.AvailabilityZone; az != "" && !knownAZ(az) {
+		return fmt.Errorf("availabilityZone %q is not published in StackitCluster status.failureDomains", az)
 	}
-	return fmt.Errorf("availabilityZone %q is not published in StackitCluster status.failureDomains", availabilityZone)
+	if machineScope.Machine != nil && machineScope.Machine.Spec.FailureDomain != "" {
+		fd := machineScope.Machine.Spec.FailureDomain
+		if !knownAZ(fd) {
+			return fmt.Errorf("machine failureDomain %q is not published in StackitCluster status.failureDomains", fd)
+		}
+	}
+	return nil
 }
 
 func (r *StackitMachineReconciler) reconcileDelete(ctx context.Context, machineScope *scope.MachineScope) error {
@@ -335,7 +347,7 @@ func (r *StackitMachineReconciler) ensureServer(
 		Region:           machineScope.StackitCluster.Spec.Region,
 		ImageID:          stackitMachine.Spec.ImageID,
 		MachineType:      stackitMachine.Spec.MachineType,
-		AvailabilityZone: stackitMachine.Spec.AvailabilityZone,
+		AvailabilityZone: machineScope.AvailabilityZone(),
 		SSHKeyName:       stackitMachine.Spec.SSHKeyName,
 		NetworkID:        stackitMachine.Spec.Network.ID,
 		SecurityGroups:   stackitMachine.Spec.SecurityGroups,