Skip to content

Commit 7abb0a2

Browse files
samuelstolicnySamuel STOLICNY (contractor)
andauthored
feat: make zone field optional for uniform zone distribution (#1947)
## Summary - Remove required zone validation from `DynamicNodePool.Validate()`, making `providerSpec.zone` optional - Add `TestOptionalZone` test to verify nodepools pass validation with and without zone - Update documentation to reflect optional zone field and automatic zone distribution ## Related Issue Partially addresses #1629 (Uniformly distribute nodes across zones by default) This PR completes the following exit criteria: - [x] `providerSpec.zone` is optional - [x] docs are updated Remaining work (in berops/claudie-config#19): - [x] nodes from a single nodepool are uniformly distributed across all zones in the region ## Documentation Updates - Updated API reference to mark zone as optional with explanation of automatic distribution - Updated provider docs (AWS, Azure, GCP, OCI) with zone-optional examples - Added note to Hetzner docs that zone is still required (no automatic distribution support) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Zone field is now optional; omitting it distributes nodes across all available zones for improved availability. * **Documentation** * Updated API reference and provider docs to clarify zone optionality and auto-distribution across AWS, Azure, GCP, and OCI. * **Tests** * Added validation tests to cover optional zone handling. * **Chores** * Updated project ignore rules and improved cross-platform build scripting helper. <sub>✏️ Tip: You can customize this high-level summary in your review settings.</sub> <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Samuel STOLICNY (contractor) <samuel.stolicny@amadeus.com>
1 parent deba9df commit 7abb0a2

10 files changed

Lines changed: 62 additions & 35 deletions

File tree

.gitignore

Lines changed: 32 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,43 @@
1+
# IDE and editors
12
.idea/
2-
*.ini
3+
.vscode/
4+
5+
# Terraform
36
.terraform*
4-
playground
5-
clusters
6-
.vscode
7-
mongodb
87
*.tfstate*
9-
*.pem
8+
9+
# Local development
10+
playground/
11+
clusters/
12+
mongodb/
13+
bin/
14+
15+
# Kubernetes and cluster files
1016
kubeone.yaml
1117
cluster.tar.gz
1218
cluster-kubeconfig
19+
*.pem
1320
*.conf
21+
22+
# Build artifacts
1423
__debug_bin
15-
services/testing-framework/test-sets
16-
services/terraformer/templates
24+
25+
# OS-specific
26+
.DS_Store
27+
28+
# Python/Documentation
1729
/venv/
1830
/site/
19-
bin
31+
32+
# Configuration files
33+
*.ini
2034
.manifest*
21-
.DS_Store
22-
/services/terraformer/server/cache
35+
36+
# Service-specific
37+
services/testing-framework/test-sets/
38+
services/terraformer/templates/
39+
/services/terraformer/server/cache/
40+
41+
# Claude
42+
.claude/
43+
CLAUDE.md

Makefile

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,15 @@ datastoreStop:
7474
TARGETARCH = $$(go env GOHOSTARCH)
7575
REV = $$(git rev-parse --short HEAD)
7676
SERVICES = $$(command ls services/)
77+
# macOS (BSD) sed requires -i '' while GNU sed uses -i
78+
SED_INPLACE := $(shell if [ "$$(uname)" = "Darwin" ]; then echo "sed -i ''"; else echo "sed -i"; fi)
7779
containerimgs:
78-
sed -i "s/image: ghcr.io\/berops\/claudie\/autoscaler-adapter/&:$(REV)/" services/kuber/templates/cluster-autoscaler.goyaml
80+
$(SED_INPLACE) "s/image: ghcr.io\/berops\/claudie\/autoscaler-adapter/&:$(REV)/" services/kuber/templates/cluster-autoscaler.goyaml
7981
for service in $(SERVICES) ; do \
8082
echo " --- building $$service --- "; \
8183
DOCKER_BUILDKIT=1 docker build --build-arg=TARGETARCH="$(TARGETARCH)" -t "ghcr.io/berops/claudie/$$service:$(REV)" -f ./services/$$service/Dockerfile . ; \
8284
done
83-
sed -i "s/adapter:.*$$/adapter/" services/kuber/templates/cluster-autoscaler.goyaml
85+
$(SED_INPLACE) "s/adapter:.*$$/adapter/" services/kuber/templates/cluster-autoscaler.goyaml
8486

8587
kind-load-images:
8688
for service in $(SERVICES) ; do \

docs/input-manifest/api-reference.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,9 @@ Provider spec is an additional specification built on top of the data from any o
296296

297297
Region of the nodepool.
298298

299-
- `zone`
299+
- `zone` *(optional)*
300300

301-
Zone of the nodepool.
301+
Zone of the nodepool. If not specified, nodes are automatically distributed across all available zones in the region using round-robin assignment. This provides better availability and fault tolerance by spreading nodes across multiple availability zones.
302302

303303
## Autoscaler Configuration
304304

docs/input-manifest/example.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ spec:
8585
# providerSpec: # Provider specification for this nodepool.
8686
# name: # Name of the provider instance, referencing one of the providers define above.
8787
# region: # Region of the nodepool.
88-
# zone: # Zone of the nodepool.
88+
# zone: # Zone of the nodepool (optional). If omitted, nodes are distributed across zones automatically.
8989
# count: # Static number of nodes in this nodepool.
9090
# serverType: # Machine type of the nodes in this nodepool.
9191
# image: # OS image of the nodes in the nodepool.

docs/input-manifest/providers/aws.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ spec:
117117
name: aws-1
118118
# Region of the nodepool.
119119
region: eu-central-1
120-
# Availability zone of the nodepool.
120+
# Zone is optional. If omitted, nodes are distributed across all availability zones.
121121
zone: eu-central-1a
122122
count: 1
123123
# Instance type name.
@@ -132,7 +132,7 @@ spec:
132132
name: aws-1
133133
# Region of the nodepool.
134134
region: eu-west-2
135-
# Availability zone of the nodepool.
135+
# Zone is optional. If omitted, nodes are distributed across all availability zones.
136136
zone: eu-west-2a
137137
count: 2
138138
# Instance type name.
@@ -148,7 +148,7 @@ spec:
148148
name: aws-1
149149
# Region of the nodepool.
150150
region: eu-west-2
151-
# Availability zone of the nodepool.
151+
# Zone is optional. If omitted, nodes are distributed across all availability zones.
152152
zone: eu-west-2a
153153
count: 2
154154
# Instance type name.

docs/input-manifest/providers/azure.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ spec:
114114
name: azure-1
115115
# Location of the nodepool.
116116
region: North Europe
117-
# Zone of the nodepool.
117+
# Zone is optional. If omitted, nodes are distributed across zones.
118118
zone: "1"
119119
count: 2
120120
# VM size name.
@@ -128,7 +128,7 @@ spec:
128128
name: azure-1
129129
# Location of the nodepool.
130130
region: Germany West Central
131-
# Zone of the nodepool.
131+
# Zone is optional. If omitted, nodes are distributed across zones.
132132
zone: "1"
133133
count: 2
134134
# VM size name.
@@ -143,7 +143,7 @@ spec:
143143
name: azure-1
144144
# Location of the nodepool.
145145
region: North Europe
146-
# Zone of the nodepool.
146+
# Zone is optional. If omitted, nodes are distributed across zones.
147147
zone: "1"
148148
count: 2
149149
# VM size name.

docs/input-manifest/providers/gcp.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ spec:
102102
name: gcp-1
103103
# Region of the nodepool.
104104
region: europe-west1
105-
# Zone of the nodepool.
105+
# Zone is optional. If omitted, nodes are distributed across all zones in the region.
106106
zone: europe-west1-c
107107
count: 1
108108
# Machine type name.
@@ -116,7 +116,7 @@ spec:
116116
name: gcp-1
117117
# Region of the nodepool.
118118
region: europe-west3
119-
# Zone of the nodepool.
119+
# Zone is optional. If omitted, nodes are distributed across all zones in the region.
120120
zone: europe-west3-a
121121
count: 2
122122
# Machine type name.
@@ -131,7 +131,7 @@ spec:
131131
name: gcp-1
132132
# Region of the nodepool.
133133
region: europe-west2
134-
# Zone of the nodepool.
134+
# Zone is optional. If omitted, nodes are distributed across all zones in the region.
135135
zone: europe-west2-a
136136
count: 2
137137
# Machine type name.

docs/input-manifest/providers/oci.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ spec:
151151
name: oci-1
152152
# Region of the nodepool.
153153
region: eu-milan-1
154-
# Availability domain of the nodepool.
154+
# Zone is optional. If omitted, nodes are distributed across all availability domains.
155155
zone: hsVQ:EU-MILAN-1-AD-1
156156
count: 1
157157
# VM shape name.
@@ -167,7 +167,7 @@ spec:
167167
name: oci-1
168168
# Region of the nodepool.
169169
region: eu-frankfurt-1
170-
# Availability domain of the nodepool.
170+
# Zone is optional. If omitted, nodes are distributed across all availability domains.
171171
zone: hsVQ:EU-FRANKFURT-1-AD-1
172172
count: 2
173173
# VM shape name.
@@ -184,7 +184,7 @@ spec:
184184
name: oci-1
185185
# Region of the nodepool.
186186
region: eu-frankfurt-1
187-
# Availability domain of the nodepool.
187+
# Zone is optional. If omitted, nodes are distributed across all availability domains.
188188
zone: hsVQ:EU-FRANKFURT-1-AD-2
189189
count: 2
190190
# VM shape name.

internal/api/manifest/validate_node_pool.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,6 @@ func (d *DynamicNodePool) Validate(m *Manifest) error {
126126
}
127127

128128
validate := validator.New()
129-
validate.RegisterStructValidation(func(sl validator.StructLevel) {
130-
dnp := sl.Current().Interface().(DynamicNodePool)
131-
132-
if dnp.ProviderSpec.Zone == "" {
133-
sl.ReportError(dnp.ProviderSpec.Zone, "Zone", "Zone", "required", "")
134-
}
135-
}, DynamicNodePool{})
136129

137130
if err := validate.RegisterValidation("external_net", validateExternalNet); err != nil {
138131
return err

internal/api/manifest/validate_test.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,8 @@ var (
212212
testNpDiskSizeSuccessfulFifty = DynamicNodePool{Name: "control-1", Count: 10, ServerType: "small", Image: "ubuntu", ProviderSpec: ProviderSpec{Name: "foo", Region: "north", Zone: "1"}, StorageDiskSize: newIntP(50)}
213213
testNpDiskSizeSuccessfulDefault = DynamicNodePool{Name: "control-1", Count: 10, ServerType: "small", Image: "ubuntu", ProviderSpec: ProviderSpec{Name: "foo", Region: "north", Zone: "1"}}
214214
testNpDiskSizeSuccessfulFail = DynamicNodePool{Name: "control-1", Count: 10, ServerType: "small", Image: "ubuntu", ProviderSpec: ProviderSpec{Name: "foo", Region: "north", Zone: "1"}, StorageDiskSize: newIntP(10)}
215+
testNodepoolWithoutZone = &DynamicNodePool{Name: "Test", ServerType: "s1", Image: "ubuntu", StorageDiskSize: newIntP(50), Count: 1, ProviderSpec: ProviderSpec{Name: "p1", Region: "a"}}
216+
testNodepoolWithZone = &DynamicNodePool{Name: "Test", ServerType: "s1", Image: "ubuntu", StorageDiskSize: newIntP(50), Count: 1, ProviderSpec: ProviderSpec{Name: "p1", Region: "a", Zone: "1"}}
215217
)
216218

217219
func newIntP(a int32) *int32 {
@@ -318,3 +320,12 @@ func TestStorageDiskSize(t *testing.T) {
318320
r.NoError(testNpDiskSizeSuccessfulDefault.Validate(&Manifest{}))
319321
r.Error(testNpDiskSizeSuccessfulFail.Validate(&Manifest{}))
320322
}
323+
324+
// TestOptionalZone tests that the zone field is optional for uniform zone distribution.
325+
func TestOptionalZone(t *testing.T) {
326+
r := require.New(t)
327+
// Zone should be optional - nodepools without zone should pass validation
328+
r.NoError(testNodepoolWithoutZone.Validate(&Manifest{}))
329+
// Nodepools with zone should still pass validation
330+
r.NoError(testNodepoolWithZone.Validate(&Manifest{}))
331+
}

0 commit comments

Comments
 (0)