Support Cluster API failure domains for availability zone spreading - #397
Open
pulish wants to merge 3 commits into
Open
Support Cluster API failure domains for availability zone spreading#397pulish wants to merge 3 commits into
pulish wants to merge 3 commits into
Conversation
Add an opt-in spec.failureDomains field to IonosCloudCluster, mirrored to status.failureDomains so Cluster API's core controllers assign Machine.spec.failureDomain round-robin across the configured zones. The IonosCloudMachine server creation path now honors that assignment, overriding the default AUTO placement with the assigned ZONE_1/ZONE_2 when no explicit availabilityZone is set on the machine spec. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
pulish
requested review from
WassimKallel,
gauravgahlot,
gfariasalves-ionos,
jriedel-ionos,
mcbenjemaa,
piepmatz,
schegi,
tinashe-mundangepfupfu and
wikkyk
as code owners
August 13, 2026 10:31
There was a problem hiding this comment.
Pull request overview
This PR adds opt-in Cluster API failure domain support so that Machines can be spread across IONOS Cloud availability zones, and ensures the Machine controller honors Cluster API’s Machine.spec.failureDomain assignment when the infra machine is otherwise configured for AUTO placement.
Changes:
- Add
spec.failureDomainstoIonosCloudClusterand mirror it intostatus.failureDomains(for consumption by Cluster API core controllers). - Resolve server
availabilityZonefrom the owningMachine.spec.failureDomainwhen the infra machine spec usesAUTO, while keeping explicit non-AUTOmachine settings authoritative. - Regenerate CRDs/deepcopy code and add unit tests for both failure domain syncing and availability zone resolution.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/service/cloud/server.go | Derives effective server availability zone from Machine.spec.failureDomain when infra spec is AUTO. |
| internal/service/cloud/server_test.go | Adds unit tests covering availability zone resolution precedence and edge cases. |
| internal/controller/ionoscloudcluster_controller.go | Mirrors spec.failureDomains into status.failureDomains for Cluster API consumption. |
| internal/controller/ionoscloudcluster_controller_test.go | Adds unit tests validating syncFailureDomains behavior. |
| config/crd/bases/infrastructure.cluster.x-k8s.io_ionoscloudclustertemplates.yaml | Updates template CRD schema to include failureDomains validation. |
| config/crd/bases/infrastructure.cluster.x-k8s.io_ionoscloudclusters.yaml | Updates cluster CRD schema for spec.failureDomains and status.failureDomains. |
| api/v1alpha1/zz_generated.deepcopy.go | Regenerates deepcopy logic for new spec/status fields. |
| api/v1alpha1/ionoscloudcluster_types.go | Adds FailureDomains to spec and FailureDomains to status with validation markers. |
Files not reviewed (1)
- api/v1alpha1/zz_generated.deepcopy.go: Generated file
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+69
to
+73
| // Leave unset to keep the current behavior of letting IONOS Cloud automatically place VMs. | ||
| //+kubebuilder:validation:MaxItems=2 | ||
| //+kubebuilder:validation:XValidation:rule="self.all(z, z in ['ZONE_1', 'ZONE_2'])",message="failureDomains may only contain ZONE_1 or ZONE_2" | ||
| //+optional | ||
| FailureDomains []AvailabilityZone `json:"failureDomains,omitempty"` |
Author
There was a problem hiding this comment.
Good catch — added a CEL uniqueness rule (size(self) < 2 || self[0] != self[1]) and regenerated the CRDs in e67036c.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
spec.failureDomains []AvailabilityZone(restricted toZONE_1/ZONE_2) toIonosCloudCluster, mirrored intostatus.failureDomains(clusterv1.FailureDomains, all markedControlPlane: true) so Cluster API's coreKubeadmControlPlane/MachineDeploymentcontrollers can round-robinMachine.spec.failureDomainacross the configured zones — the same contract AWS/Azure/GCP providers implement.IonosCloudMachineserver creation now resolves the effectiveavailabilityZonefrom the owningMachine.spec.failureDomainwhenever the machine spec is left at the defaultAUTO, so Cluster API's assignment actually lands as real VM placement. An explicit non-AUTOavailabilityZoneon the machine spec always takes precedence.make generate/make manifests.Test plan
go build ./...go vet ./...go test ./... -short(all packages pass)resolveAvailabilityZone(server_test.go) andsyncFailureDomains(ionoscloudcluster_controller_test.go)spec.failureDomains: [ZONE_1, ZONE_2]on itsIonosCloudCluster) and confirm new machines land in the expected zones🤖 Generated with Claude Code