Skip to content

Commit 12e64a9

Browse files
lexfreilexfrei
authored
fix(rabbitmq): right-size the default resources preset to s1.nano (#3936)
## What this PR does Raises the RabbitMQ default resource preset from `t1.nano` (128Mi memory) to `s1.nano` (250m CPU / 512Mi memory). A default-sized RabbitMQ 4.2 broker on Cozystack 1.6.1 was OOMKilled with exit 137, reached 10 restarts, and never became Ready. A canary on a larger preset reached 3/3 Ready with zero restarts; its working set peaked at 125.8Mi. Against the 128Mi `t1.nano` ceiling that is 98% of the limit in steady state, and cgroup accounting (working set plus page cache) crosses it under load. The canary and its storage were removed after validation. ### Sizing `s1.nano` keeps the CPU of `t1.nano` (250m) and quadruples memory to 512Mi — a 4x margin over the observed 125.8Mi peak. An earlier revision of this PR proposed `u1.nano` (1Gi). Review correctly objected that 1Gi reserves 8x the observed working set with no measured peak to justify it, and asked to either report that peak or right-size the default. The peak is reported above, and the default is right-sized to the smallest preset that clears the ceiling with a real margin rather than to the first one that is comfortably large. ### Upgrade impact cozystack-api applies OpenAPI schema defaults on the read path and stores what the client sent: `Get` runs `applySpecDefaults` on its way out (`pkg/registry/apps/application/rest.go:1232`) and the write puts `app.Spec` into `HelmRelease.spec.values` (`rest.go:1652`). That splits existing brokers in two. **Brokers with no stored `resourcesPreset`** pick up the new chart default on the next Helm reconcile. For them this upgrade is: - a rolling restart of every replica, and - a rise in guaranteed memory reservation from 128Mi to 512Mi per replica, because the sanitize helper sets memory request equal to limit — roughly 1.1Gi extra reserved per 3-replica instance. **Brokers whose values went through a read-modify-write keep `t1.nano` and are not resized.** `Update` opens with `r.Get` (`rest.go:489`), so a GET-then-PUT — `kubectl edit` is the unambiguous case, and any client that reads before writing behaves the same — freezes the schema default of the moment into the stored values, and a stored value beats the chart default. Those brokers are not restarted, not resized, and keep hitting the ceiling this PR raises. Nothing downstream can separate a deliberate `t1.nano` from one a round trip baked in, so this is documented rather than migrated. After the upgrade the affected brokers are the ones that still report the old preset: ``` kubectl get rabbitmq -A -o custom-columns=NS:.metadata.namespace,NAME:.metadata.name,PRESET:.spec.resourcesPreset ``` The same command before the upgrade tells you nothing: the cozyrd default is still `t1.nano` then, so the read path fills it in for both groups. Afterwards, anything reading `t1.nano` carries a stored value; setting `resourcesPreset: s1.nano` explicitly on the ones that should move is the fix. PVCs are untouched, so there is no data loss, but node headroom should be checked before upgrading. Also worth knowing when reading the parameter table: the preset does not only apply to a fully omitted `resources`. `cozy-lib.resources.defaultingSanitize` merges it key by key, so a broker pinned to `resources: {cpu: 700m}` keeps its CPU and moves from 128Mi to 512Mi with everyone else. The field descriptions now say so, and a test pins it. The values, generated schema, ApplicationDefinition, typed API, and README all carry the same default. Tests cover the new default, an explicit legacy override, explicit resources, and a partially set `resources` map inheriting the preset for what it leaves out. Validation: - `helm template` with the chart's `cozy-lib` dependency renders `limits 250m / 512Mi` and `requests 0.025 / 536870912` for the default; an explicit `t1.nano` renders `128Mi / 134217728`; explicit `700m / 2Gi` renders `0.07 / 2147483648`. The unit test asserts exactly these values. - Provider counterpart: cozystack/terraform-provider-cozystack#34. ### Screenshots Not a UI change. ### Downstream repositories The default is restated by the Terraform provider, so the linked provider PR updates it and documents the state-plan implication. No other trigger-map entry matches this diff. - [ ] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [x] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: cozystack/terraform-provider-cozystack#34 - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: ### Release note ```release-note fix(rabbitmq): raise the default resource preset from t1.nano to s1.nano so default-sized brokers are no longer OOMKilled. Brokers with no stored `resourcesPreset` are restarted on upgrade and their guaranteed memory reservation grows from 128Mi to 512Mi per replica; check node headroom before upgrading. Brokers whose values already pin `resourcesPreset`, including ones where a read-modify-write through the API froze the old default into them, keep their old size: list them after the upgrade with `kubectl get rabbitmq -A -o custom-columns=NAME:.metadata.name,PRESET:.spec.resourcesPreset` and set `resourcesPreset: s1.nano` explicitly on the ones that should move. The preset also fills any resource a partially set `resources` leaves out, so a broker pinned to cpu alone moves too. A tenant that opted into `resourceQuotas` and sits near its memory ceiling has the extra reservation rejected by quota admission, which shows up as a StatefulSet that never scales rather than as pods left Pending. ```
2 parents 7d07bbe + 78f01d5 commit 12e64a9

6 files changed

Lines changed: 92 additions & 21 deletions

File tree

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

Lines changed: 3 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/apps/rabbitmq/README.md‎

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,10 @@ The scope is **definitions only** — vhosts, users, permissions, queues, exchan
2424
| Name | Description | Type | Value |
2525
| ---------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ---------- | --------- |
2626
| `replicas` | Number of RabbitMQ replicas. | `int` | `3` |
27-
| `resources` | Explicit CPU and memory configuration for each RabbitMQ replica. When omitted, the preset defined in `resourcesPreset` is applied. | `object` | `{}` |
27+
| `resources` | Explicit CPU and memory configuration for each RabbitMQ replica. Every resource left unset here is taken from `resourcesPreset`. | `object` | `{}` |
2828
| `resources.cpu` | CPU available to each replica. | `quantity` | `""` |
2929
| `resources.memory` | Memory (RAM) available to each replica. | `quantity` | `""` |
30-
| `resourcesPreset` | Default sizing preset used when `resources` is omitted. | `string` | `t1.nano` |
30+
| `resourcesPreset` | Default sizing preset. It supplies every resource `resources` does not set, not only a `resources` left empty entirely. | `string` | `s1.nano` |
3131
| `size` | Persistent Volume Claim size available for application data. | `quantity` | `10Gi` |
3232
| `storageClass` | StorageClass used to store the data. | `string` | `""` |
3333
| `external` | Enable external access from outside the cluster. | `bool` | `false` |
@@ -139,13 +139,7 @@ resources:
139139
`resourcesPreset` sets named CPU and memory configurations for each replica.
140140
This setting is ignored if the corresponding `resources` value is set.
141141

142-
| Preset name | CPU | memory |
143-
|-------------|--------|---------|
144-
| `nano` | `100m` | `128Mi` |
145-
| `micro` | `250m` | `256Mi` |
146-
| `small` | `500m` | `512Mi` |
147-
| `medium` | `500m` | `1Gi` |
148-
| `large` | `1` | `2Gi` |
149-
| `xlarge` | `2` | `4Gi` |
150-
| `2xlarge` | `4` | `8Gi` |
142+
Presets follow a cloud-style `<series>.<size>` naming convention. Five series cover the full CPU-to-memory ratio range (`t1` 1:0.5, `c1` 1:1, `s1` 1:2, `u1` 1:4, `m1` 1:8) and each series ships eight sizes (`nano` through `4xlarge`). The legacy flat names (`nano`, `micro`, `small`, `medium`, `large`, `xlarge`, `2xlarge`) remain accepted as deprecated aliases of their 1:1 instance-type equivalents.
143+
144+
See [`docs/operations/resource-presets.md`](../../../docs/operations/resource-presets.md) for the full size matrix and the legacy-to-instance-type mapping.
151145

‎packages/apps/rabbitmq/tests/resources_test.yaml‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
suite: RabbitMQ default resources
2+
3+
templates:
4+
- templates/rabbitmq.yaml
5+
6+
tests:
7+
- it: defaults each RabbitMQ replica to the s1.nano preset
8+
release:
9+
name: rabbitmq-test
10+
namespace: tenant-test
11+
asserts:
12+
- equal:
13+
path: spec.resources.limits.cpu
14+
value: 250m
15+
- equal:
16+
path: spec.resources.limits.memory
17+
value: 512Mi
18+
- equal:
19+
path: spec.resources.requests.cpu
20+
value: "0.025"
21+
- equal:
22+
path: spec.resources.requests.memory
23+
value: "536870912"
24+
25+
- it: preserves t1.nano as an explicit backward-compatible override
26+
release:
27+
name: rabbitmq-test
28+
namespace: tenant-test
29+
set:
30+
resourcesPreset: t1.nano
31+
asserts:
32+
- equal:
33+
path: spec.resources.limits.cpu
34+
value: 250m
35+
- equal:
36+
path: spec.resources.limits.memory
37+
value: 128Mi
38+
39+
- it: prefers explicit resources over the default preset
40+
release:
41+
name: rabbitmq-test
42+
namespace: tenant-test
43+
set:
44+
resources:
45+
cpu: 700m
46+
memory: 2Gi
47+
asserts:
48+
- equal:
49+
path: spec.resources.limits.cpu
50+
value: 700m
51+
- equal:
52+
path: spec.resources.limits.memory
53+
value: 2Gi
54+
- equal:
55+
path: spec.resources.requests.cpu
56+
value: "0.07"
57+
- equal:
58+
path: spec.resources.requests.memory
59+
value: "2147483648"
60+
61+
- it: fills the preset into every resource a partial resources map leaves unset
62+
release:
63+
name: rabbitmq-test
64+
namespace: tenant-test
65+
set:
66+
resources:
67+
cpu: 700m
68+
asserts:
69+
- equal:
70+
path: spec.resources.limits.cpu
71+
value: 700m
72+
- equal:
73+
path: spec.resources.limits.memory
74+
value: 512Mi
75+
- equal:
76+
path: spec.resources.requests.memory
77+
value: "536870912"

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
"default": 3
99
},
1010
"resources": {
11-
"description": "Explicit CPU and memory configuration for each RabbitMQ replica. When omitted, the preset defined in `resourcesPreset` is applied.",
11+
"description": "Explicit CPU and memory configuration for each RabbitMQ replica. Every resource left unset here is taken from `resourcesPreset`.",
1212
"type": "object",
1313
"default": {},
1414
"properties": {
@@ -41,9 +41,9 @@
4141
}
4242
},
4343
"resourcesPreset": {
44-
"description": "Default sizing preset used when `resources` is omitted.",
44+
"description": "Default sizing preset. It supplies every resource `resources` does not set, not only a `resources` left empty entirely.",
4545
"type": "string",
46-
"default": "t1.nano",
46+
"default": "s1.nano",
4747
"enum": [
4848
"t1.nano",
4949
"t1.micro",

‎packages/apps/rabbitmq/values.yaml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@
5858
## @param {int} replicas - Number of RabbitMQ replicas.
5959
replicas: 3
6060

61-
## @param {Resources} [resources] - Explicit CPU and memory configuration for each RabbitMQ replica. When omitted, the preset defined in `resourcesPreset` is applied.
61+
## @param {Resources} [resources] - Explicit CPU and memory configuration for each RabbitMQ replica. Every resource left unset here is taken from `resourcesPreset`.
6262
resources: {}
6363

64-
## @param {ResourcesPreset} resourcesPreset="t1.nano" - Default sizing preset used when `resources` is omitted.
65-
resourcesPreset: "t1.nano"
64+
## @param {ResourcesPreset} resourcesPreset="s1.nano" - Default sizing preset. It supplies every resource `resources` does not set, not only a `resources` left empty entirely.
65+
resourcesPreset: "s1.nano"
6666

6767
## @param {quantity} size - Persistent Volume Claim size available for application data.
6868
size: 10Gi

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

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

0 commit comments

Comments
 (0)