mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 18:47:04 +09:00
sircthulhu's post-approval note on PR#3233: both the cluster.yaml
comment and the nodegroups_default_test.yaml comment attribute the
upgrade-safety guarantee to Server-Side-Apply field-manager
ownership, but this chart ships as a HelmRelease through Flux
helm-controller, which applies via the Helm SDK three-way merge —
not SSA. The stated conclusion is correct but for the wrong reason
and would send a future maintainer chasing a non-existent field-
manager fight when debugging drift.
Rewrite both comments around the two independent CAPI mechanisms
that actually carry the guarantee:
1. Create — the MachineDeployment defaulting webhook seeds
spec.replicas from the min-size autoscaler annotation.
2. Update — calculateMachineDeploymentReplicas preserves the
existing live value whenever the incoming spec has
replicas: nil (clamping into [min, max] on drift).
The chart guarantee reduces to "leave the field out of the rendered
manifest and let CAPI's update-time defaulting keep the live value
stable" — no field-manager ownership involved.
Signed-off-by: IvanHunters <xorokhotnikov@gmail.com>
247 lines
8.6 KiB
YAML
247 lines
8.6 KiB
YAML
suite: default md0 node group rendering
|
|
|
|
# When nodeGroups is left empty (chart default `nodeGroups: {}`), the
|
|
# kubernetes.nodeGroups helper must emit the built-in md0 group — this is the
|
|
# else-branch that makes md0 removable (an operator can drop md0 from their own
|
|
# nodeGroups) while a cluster that sets nothing still gets the default topology.
|
|
# A minimal VirtualMachineClusterInstancetype is mocked so the per-group
|
|
# instanceType lookup (u1.medium) returns non-nil; with no spec the template
|
|
# falls back to the default kubelet reservations, which is all this test needs.
|
|
|
|
templates:
|
|
- templates/cluster.yaml
|
|
|
|
kubernetesProvider:
|
|
scheme:
|
|
"instancetype.kubevirt.io/v1beta1/VirtualMachineClusterInstancetype":
|
|
gvr:
|
|
group: instancetype.kubevirt.io
|
|
version: v1beta1
|
|
resource: virtualmachineclusterinstancetypes
|
|
namespaced: false
|
|
"cluster.x-k8s.io/v1beta1/MachineSet":
|
|
gvr:
|
|
group: cluster.x-k8s.io
|
|
version: v1beta1
|
|
resource: machinesets
|
|
namespaced: true
|
|
"infrastructure.cluster.x-k8s.io/v1alpha1/KubevirtMachineTemplate":
|
|
gvr:
|
|
group: infrastructure.cluster.x-k8s.io
|
|
version: v1alpha1
|
|
resource: kubevirtmachinetemplates
|
|
namespaced: true
|
|
objects:
|
|
- kind: VirtualMachineClusterInstancetype
|
|
apiVersion: instancetype.kubevirt.io/v1beta1
|
|
metadata:
|
|
name: u1.medium
|
|
# Dummy CAPI objects so the MachineSet / KubevirtMachineTemplate LIST kinds
|
|
# are registered with the fake client. Shaped to be ignored by the
|
|
# old-template preservation loop (MachineSet infraRef is not a
|
|
# KubevirtMachineTemplate; the KubevirtMachineTemplate has no Cluster owner).
|
|
- kind: MachineSet
|
|
apiVersion: cluster.x-k8s.io/v1beta1
|
|
metadata:
|
|
name: ignored-ms
|
|
namespace: tenant-test
|
|
spec:
|
|
template:
|
|
spec:
|
|
infrastructureRef:
|
|
kind: Other
|
|
name: ignored
|
|
- kind: KubevirtMachineTemplate
|
|
apiVersion: infrastructure.cluster.x-k8s.io/v1alpha1
|
|
metadata:
|
|
name: ignored-kmt
|
|
namespace: tenant-test
|
|
|
|
tests:
|
|
- it: renders the default md0 group when nodeGroups is empty
|
|
release:
|
|
name: test-k8s
|
|
namespace: tenant-test
|
|
set:
|
|
_namespace:
|
|
etcd: etcd
|
|
ingress: nginx
|
|
host: example.com
|
|
_cluster:
|
|
cluster-domain: cozy.local
|
|
# nodeGroups intentionally left at the chart default ({}) so the helper
|
|
# emits the built-in md0.
|
|
asserts:
|
|
# The default md0 MachineDeployment renders (helper else-branch).
|
|
- equal:
|
|
path: metadata.name
|
|
value: test-k8s-md0
|
|
documentSelector:
|
|
path: kind
|
|
value: MachineDeployment
|
|
# Default values flow through (e.g. the built-in 20Gi system disk).
|
|
- equal:
|
|
path: spec.template.spec.virtualMachineTemplate.spec.dataVolumeTemplates[0].spec.storage.resources.requests.storage
|
|
value: "20Gi"
|
|
documentSelector:
|
|
path: kind
|
|
value: KubevirtMachineTemplate
|
|
|
|
- it: drops md0 when the user defines their own node groups (md0 is removable)
|
|
release:
|
|
name: test-k8s
|
|
namespace: tenant-test
|
|
set:
|
|
_namespace:
|
|
etcd: etcd
|
|
ingress: nginx
|
|
host: example.com
|
|
_cluster:
|
|
cluster-domain: cozy.local
|
|
# A user-supplied nodeGroups map is authoritative — it is NOT merged with
|
|
# the built-in md0 (the helper's if-branch). This is the removability the
|
|
# PR exists for: a cluster that names only its own group no longer carries
|
|
# md0.
|
|
nodeGroups:
|
|
worker0:
|
|
minReplicas: 1
|
|
maxReplicas: 3
|
|
instanceType: "u1.medium"
|
|
diskSize: 20Gi
|
|
storageClass: ""
|
|
roles:
|
|
- ingress-nginx
|
|
resources: {}
|
|
gpus: []
|
|
kubelet: {}
|
|
asserts:
|
|
# Every rendered MachineDeployment must be the user's own group. The
|
|
# selector matches *all* MachineDeployments, so a leaked default md0
|
|
# would surface here as a second doc named test-k8s-md0 that fails the
|
|
# equal — proving md0 is gone (removable) when the user names their own.
|
|
- equal:
|
|
path: metadata.name
|
|
value: test-k8s-worker0
|
|
documentSelector:
|
|
path: kind
|
|
value: MachineDeployment
|
|
matchMany: true
|
|
|
|
# Regression guard for cozystack/cozystack#3231. The chart intentionally
|
|
# OMITS `MachineDeployment.spec.replicas` so the cluster-autoscaler owns
|
|
# the field alone. Two independent CAPI mechanisms carry the guarantee:
|
|
# the defaulting webhook seeds `spec.replicas` from the
|
|
# `cluster-api-autoscaler-node-group-min-size` annotation on Create, and
|
|
# `calculateMachineDeploymentReplicas` preserves the existing live
|
|
# value on Update when the incoming spec has `replicas: nil` — so a
|
|
# `helm upgrade` re-rendering this template does not drain workers the
|
|
# autoscaler has scaled above min-size. Flux helm-controller applies
|
|
# this chart through the Helm SDK three-way merge (not SSA), so field-
|
|
# manager ownership is not the mechanism at play; the guarantee is
|
|
# purely "field absent from the rendered manifest → CAPI's update-time
|
|
# defaulting preserves the live value". Two matched cases so a future
|
|
# refactor cannot silently re-hardcode the field nor break the
|
|
# annotation-driven min/max wiring:
|
|
# * minReplicas: 0 (the render-side OIDC bats path — MachineDeployment
|
|
# emitted with the min-size annotation "0", no chart-owned replicas
|
|
# field, no worker DataVolumes provisioned until scale-out).
|
|
# * minReplicas: 2 (the kubernetes-latest / kubernetes-previous bats
|
|
# path — MachineDeployment emitted with the min-size annotation "2",
|
|
# no chart-owned replicas field, defaulting webhook fills it to 2 on
|
|
# Create).
|
|
- it: MachineDeployment omits chart-managed spec.replicas — zero-minReplicas case
|
|
release:
|
|
name: test-k8s
|
|
namespace: tenant-test
|
|
set:
|
|
_namespace:
|
|
etcd: etcd
|
|
ingress: nginx
|
|
host: example.com
|
|
_cluster:
|
|
cluster-domain: cozy.local
|
|
nodeGroups:
|
|
worker0:
|
|
minReplicas: 0
|
|
maxReplicas: 10
|
|
instanceType: "u1.medium"
|
|
diskSize: 20Gi
|
|
storageClass: ""
|
|
roles:
|
|
- worker
|
|
resources: {}
|
|
gpus: []
|
|
kubelet: {}
|
|
asserts:
|
|
# spec.replicas must not appear in the rendered manifest — CAPI's
|
|
# defaulting webhook will seed it from the annotation on Create.
|
|
- notExists:
|
|
path: spec.replicas
|
|
documentSelector:
|
|
path: kind
|
|
value: MachineDeployment
|
|
matchMany: true
|
|
# Autoscaler annotation wiring stays intact.
|
|
- equal:
|
|
path: metadata.annotations["cluster.x-k8s.io/cluster-api-autoscaler-node-group-min-size"]
|
|
value: "0"
|
|
documentSelector:
|
|
path: kind
|
|
value: MachineDeployment
|
|
matchMany: true
|
|
- equal:
|
|
path: metadata.annotations["cluster.x-k8s.io/cluster-api-autoscaler-node-group-max-size"]
|
|
value: "10"
|
|
documentSelector:
|
|
path: kind
|
|
value: MachineDeployment
|
|
matchMany: true
|
|
|
|
- it: MachineDeployment omits chart-managed spec.replicas — nonzero-minReplicas case
|
|
release:
|
|
name: test-k8s
|
|
namespace: tenant-test
|
|
set:
|
|
_namespace:
|
|
etcd: etcd
|
|
ingress: nginx
|
|
host: example.com
|
|
_cluster:
|
|
cluster-domain: cozy.local
|
|
nodeGroups:
|
|
worker0:
|
|
minReplicas: 2
|
|
maxReplicas: 10
|
|
instanceType: "u1.medium"
|
|
diskSize: 20Gi
|
|
storageClass: ""
|
|
roles:
|
|
- worker
|
|
resources: {}
|
|
gpus: []
|
|
kubelet: {}
|
|
asserts:
|
|
- notExists:
|
|
path: spec.replicas
|
|
documentSelector:
|
|
path: kind
|
|
value: MachineDeployment
|
|
matchMany: true
|
|
- equal:
|
|
path: metadata.annotations["cluster.x-k8s.io/cluster-api-autoscaler-node-group-min-size"]
|
|
value: "2"
|
|
documentSelector:
|
|
path: kind
|
|
value: MachineDeployment
|
|
matchMany: true
|
|
# Symmetric with the zero-minReplicas case above — both annotations
|
|
# must land, because CAPI's calculateMachineDeploymentReplicas
|
|
# defaults to 1 (not to minSize) if either annotation is missing.
|
|
- equal:
|
|
path: metadata.annotations["cluster.x-k8s.io/cluster-api-autoscaler-node-group-max-size"]
|
|
value: "10"
|
|
documentSelector:
|
|
path: kind
|
|
value: MachineDeployment
|
|
matchMany: true
|