Skip to content

Commit dea35f7

Browse files
myasnikovdaniilclaude
myasnikovdaniil
and
claude
committed
Merge origin/main into feat/e2e-chainsaw-example (post-#3233)
Main's #3233 landed mid-reconciliation and touched the two OIDC bats again: nodeGroups (and storageClass) dropped from the render-side test CRs — the schema default {} now renders one zero-replica MachineDeployment via CAPI defaulting, so the tests provision zero worker DataVolumes (#3231). Resolve the modify/delete as deleted and mirror the fixture change (and its rationale) into the ported kubernetes-oidc-system / kubernetes-oidc-customconfig Chainsaw suites. Assisted-By: Claude <[email protected]> Co-Authored-By: Claude Fable 5 <[email protected]> Signed-off-by: Myasnikov Daniil <[email protected]>
2 parents 1f9555a + 0c7b598 commit dea35f7

10 files changed

Lines changed: 195 additions & 50 deletions

File tree

‎api/apps/v1alpha1/kubernetes/types.go‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎hack/e2e-chainsaw/kubernetes-oidc-customconfig/kubernetes-oidc-byo.yaml‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
# nodeGroups (and storageClass) intentionally omitted — this is a render-side
2+
# test that only asserts on the KamajiControlPlane + the tenant-supplied
3+
# issuer AuthenticationConfiguration Secret. With the schema default `{}`,
4+
# the chart helper emits the default `md0` group with `minReplicas: 0` —
5+
# exactly one MachineDeployment renders (API-surface completeness) but the
6+
# chart no longer manages `spec.replicas`, so CAPI's defaulting webhook seeds
7+
# it to 0 from the autoscaler min-size annotation. Result: zero
8+
# KubevirtMachines, zero worker DataVolumes, no DRBD churn between CI runs
9+
# (see #3231).
110
apiVersion: apps.cozystack.io/v1alpha1
211
kind: Kubernetes
312
metadata:
@@ -13,17 +22,6 @@ spec:
1322
resources: {}
1423
resourcesPreset: small
1524
replicas: 1
16-
nodeGroups:
17-
md0:
18-
diskSize: 20Gi
19-
gpus: []
20-
instanceType: u1.micro
21-
maxReplicas: 1
22-
minReplicas: 0
23-
resources: {}
24-
roles:
25-
- worker
26-
storageClass: replicated
2725
version: v1.35
2826
oidc:
2927
mode: CustomConfig

‎hack/e2e-chainsaw/kubernetes-oidc-system/kubernetes-oidc-system.yaml‎

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
# nodeGroups (and storageClass) intentionally omitted — this is a render-side
2+
# test that only asserts on the KamajiControlPlane + the cozy-realm Keycloak
3+
# objects. With the schema default `{}`, the chart helper emits the default
4+
# `md0` group with `minReplicas: 0` — exactly one MachineDeployment renders
5+
# (API-surface completeness) but the chart no longer manages `spec.replicas`,
6+
# so CAPI's defaulting webhook seeds it to 0 from the autoscaler min-size
7+
# annotation. Result: zero KubevirtMachines, zero worker DataVolumes, no DRBD
8+
# churn overlapping the kubernetes-previous suite (see #3231).
19
apiVersion: apps.cozystack.io/v1alpha1
210
kind: Kubernetes
311
metadata:
@@ -13,17 +21,6 @@ spec:
1321
resources: {}
1422
resourcesPreset: small
1523
replicas: 1
16-
nodeGroups:
17-
md0:
18-
diskSize: 20Gi
19-
gpus: []
20-
instanceType: u1.micro
21-
maxReplicas: 1
22-
minReplicas: 0
23-
resources: {}
24-
roles:
25-
- worker
26-
storageClass: replicated
2724
version: v1.35
2825
oidc:
2926
mode: System

‎packages/apps/kubernetes/README.md‎

Lines changed: 23 additions & 23 deletions
Large diffs are not rendered by default.

‎packages/apps/kubernetes/templates/_helpers.tpl‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,16 @@ re-add a baked-in default md0 on top of the user's groups, and because
124124
Kubernetes strips null values the default could never be removed. With the
125125
default applied only when the map is empty, users can freely choose their own
126126
node groups (and omit md0).
127+
128+
The default carries `minReplicas: 0`, which now propagates through to
129+
`MachineDeployment.spec.replicas: 0` (see the templates/cluster.yaml comment
130+
next to that field). Consequence: an empty-`nodeGroups` install provisions
131+
NO workers until either the cluster-autoscaler responds to an unschedulable
132+
Pod or an operator explicitly scales the group up. If the operator plans to
133+
enable the ingress-nginx addon, either supply an explicit nodeGroup with
134+
`roles: [ingress-nginx]` and `minReplicas >= 1`, or accept that the
135+
autoscaler will bring the default md0 up in response to the ingress-nginx
136+
controller Pods becoming Pending on install.
127137
*/}}
128138
{{- define "kubernetes.nodeGroups" -}}
129139
{{- if .Values.nodeGroups -}}

‎packages/apps/kubernetes/templates/cluster.yaml‎

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -743,7 +743,29 @@ metadata:
743743
{{- end }}
744744
spec:
745745
clusterName: {{ $.Release.Name }}
746-
replicas: 2
746+
# `spec.replicas` is intentionally omitted so the field is owned by
747+
# the cluster-autoscaler alone. Two independent CAPI mechanisms —
748+
# not Server-Side-Apply field managers — carry the guarantee:
749+
# 1. Create: the MachineDeployment defaulting webhook seeds
750+
# `spec.replicas` from the `cluster-api-autoscaler-node-group-
751+
# min-size` annotation (see
752+
# https://cluster-api.sigs.k8s.io/tasks/automated-machine-management/autoscaling).
753+
# 2. Update: `calculateMachineDeploymentReplicas` preserves the
754+
# existing live value (`oldMD.Spec.Replicas`) whenever the
755+
# incoming spec has `replicas: nil`, clamping into
756+
# `[min, max]` if it drifts outside — so a cluster the
757+
# autoscaler holds above min-size is not drained by a
758+
# `helm upgrade` that would otherwise re-render this field.
759+
# Flux helm-controller applies this chart through the Helm SDK
760+
# three-way merge (not SSA), so field-manager ownership is not the
761+
# mechanism at play. The chart guarantee reduces to "leave the
762+
# field out of the rendered manifest and let CAPI's update-time
763+
# defaulting keep the live value stable". Emitting a chart-managed
764+
# `replicas: N` here (as this chart did before
765+
# cozystack/cozystack#3231) drains workers on every `helm upgrade`
766+
# action until the autoscaler notices unschedulable Pods and
767+
# scales back up — a real disruption window on every routine
768+
# platform bump.
747769
strategy:
748770
rollingUpdate:
749771
maxSurge: {{ $group.maxReplicas }}

‎packages/apps/kubernetes/tests/nodegroups_default_test.yaml‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,3 +126,121 @@ tests:
126126
path: kind
127127
value: MachineDeployment
128128
matchMany: true
129+
130+
# Regression guard for cozystack/cozystack#3231. The chart intentionally
131+
# OMITS `MachineDeployment.spec.replicas` so the cluster-autoscaler owns
132+
# the field alone. Two independent CAPI mechanisms carry the guarantee:
133+
# the defaulting webhook seeds `spec.replicas` from the
134+
# `cluster-api-autoscaler-node-group-min-size` annotation on Create, and
135+
# `calculateMachineDeploymentReplicas` preserves the existing live
136+
# value on Update when the incoming spec has `replicas: nil` — so a
137+
# `helm upgrade` re-rendering this template does not drain workers the
138+
# autoscaler has scaled above min-size. Flux helm-controller applies
139+
# this chart through the Helm SDK three-way merge (not SSA), so field-
140+
# manager ownership is not the mechanism at play; the guarantee is
141+
# purely "field absent from the rendered manifest → CAPI's update-time
142+
# defaulting preserves the live value". Two matched cases so a future
143+
# refactor cannot silently re-hardcode the field nor break the
144+
# annotation-driven min/max wiring:
145+
# * minReplicas: 0 (the render-side OIDC bats path — MachineDeployment
146+
# emitted with the min-size annotation "0", no chart-owned replicas
147+
# field, no worker DataVolumes provisioned until scale-out).
148+
# * minReplicas: 2 (the kubernetes-latest / kubernetes-previous bats
149+
# path — MachineDeployment emitted with the min-size annotation "2",
150+
# no chart-owned replicas field, defaulting webhook fills it to 2 on
151+
# Create).
152+
- it: MachineDeployment omits chart-managed spec.replicas — zero-minReplicas case
153+
release:
154+
name: test-k8s
155+
namespace: tenant-test
156+
set:
157+
_namespace:
158+
etcd: etcd
159+
ingress: nginx
160+
host: example.com
161+
_cluster:
162+
cluster-domain: cozy.local
163+
nodeGroups:
164+
worker0:
165+
minReplicas: 0
166+
maxReplicas: 10
167+
instanceType: "u1.medium"
168+
diskSize: 20Gi
169+
storageClass: ""
170+
roles:
171+
- worker
172+
resources: {}
173+
gpus: []
174+
kubelet: {}
175+
asserts:
176+
# spec.replicas must not appear in the rendered manifest — CAPI's
177+
# defaulting webhook will seed it from the annotation on Create.
178+
- notExists:
179+
path: spec.replicas
180+
documentSelector:
181+
path: kind
182+
value: MachineDeployment
183+
matchMany: true
184+
# Autoscaler annotation wiring stays intact.
185+
- equal:
186+
path: metadata.annotations["cluster.x-k8s.io/cluster-api-autoscaler-node-group-min-size"]
187+
value: "0"
188+
documentSelector:
189+
path: kind
190+
value: MachineDeployment
191+
matchMany: true
192+
- equal:
193+
path: metadata.annotations["cluster.x-k8s.io/cluster-api-autoscaler-node-group-max-size"]
194+
value: "10"
195+
documentSelector:
196+
path: kind
197+
value: MachineDeployment
198+
matchMany: true
199+
200+
- it: MachineDeployment omits chart-managed spec.replicas — nonzero-minReplicas case
201+
release:
202+
name: test-k8s
203+
namespace: tenant-test
204+
set:
205+
_namespace:
206+
etcd: etcd
207+
ingress: nginx
208+
host: example.com
209+
_cluster:
210+
cluster-domain: cozy.local
211+
nodeGroups:
212+
worker0:
213+
minReplicas: 2
214+
maxReplicas: 10
215+
instanceType: "u1.medium"
216+
diskSize: 20Gi
217+
storageClass: ""
218+
roles:
219+
- worker
220+
resources: {}
221+
gpus: []
222+
kubelet: {}
223+
asserts:
224+
- notExists:
225+
path: spec.replicas
226+
documentSelector:
227+
path: kind
228+
value: MachineDeployment
229+
matchMany: true
230+
- equal:
231+
path: metadata.annotations["cluster.x-k8s.io/cluster-api-autoscaler-node-group-min-size"]
232+
value: "2"
233+
documentSelector:
234+
path: kind
235+
value: MachineDeployment
236+
matchMany: true
237+
# Symmetric with the zero-minReplicas case above — both annotations
238+
# must land, because CAPI's calculateMachineDeploymentReplicas
239+
# defaults to 1 (not to minSize) if either annotation is missing.
240+
- equal:
241+
path: metadata.annotations["cluster.x-k8s.io/cluster-api-autoscaler-node-group-max-size"]
242+
value: "10"
243+
documentSelector:
244+
path: kind
245+
value: MachineDeployment
246+
matchMany: true

‎packages/apps/kubernetes/values.schema.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
}
1818
},
1919
"nodeGroups": {
20-
"description": "Worker nodes configuration map. When left empty, a single default node group `md0` (one `ingress-nginx` worker) is provisioned. Provide your own groups to take full control — they are not merged with the default, so you may name and omit groups freely.",
20+
"description": "Worker nodes configuration map. When left empty, a default node group `md0` is emitted with `minReplicas: 0` and `roles: [ingress-nginx]` — the MachineDeployment renders but provisions no workers until an unschedulable Pod triggers the cluster-autoscaler (or an operator scales the group manually). Enabling `addons.ingressNginx.enabled: true` on a CR with default `nodeGroups: {}` therefore requires either supplying a nodeGroup with `roles: [ingress-nginx]` and `minReplicas \u003e= 1`, or waiting for the autoscaler to bring up the default md0 in response to the ingress-nginx controller Pods becoming Pending. Provide your own groups to take full control — they are not merged with the default, so you may name and omit groups freely.",
2121
"type": "object",
2222
"default": {},
2323
"additionalProperties": {

‎packages/apps/kubernetes/values.yaml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ storageClass: replicated
8989
## @field {[]GPU} gpus - List of GPUs to attach (NVIDIA driver requires at least 4 GiB RAM).
9090
## @field {Kubelet} [kubelet] - Kubelet resource reservations for this node group.
9191

92-
## @param {map[string]NodeGroup} nodeGroups - Worker nodes configuration map. When left empty, a single default node group `md0` (one `ingress-nginx` worker) is provisioned. Provide your own groups to take full control — they are not merged with the default, so you may name and omit groups freely.
92+
## @param {map[string]NodeGroup} nodeGroups - Worker nodes configuration map. When left empty, a default node group `md0` is emitted with `minReplicas: 0` and `roles: [ingress-nginx]` — the MachineDeployment renders but provisions no workers until an unschedulable Pod triggers the cluster-autoscaler (or an operator scales the group manually). Enabling `addons.ingressNginx.enabled: true` on a CR with default `nodeGroups: {}` therefore requires either supplying a nodeGroup with `roles: [ingress-nginx]` and `minReplicas >= 1`, or waiting for the autoscaler to bring up the default md0 in response to the ingress-nginx controller Pods becoming Pending. Provide your own groups to take full control — they are not merged with the default, so you may name and omit groups freely.
9393
nodeGroups: {}
9494

9595
##

‎packages/system/kubernetes-rd/cozyrds/kubernetes.yaml‎

Lines changed: 1 addition & 1 deletion
Large diffs are not rendered by default.

0 commit comments

Comments
 (0)