Skip to content

Commit 8be7fb2

Browse files
yrahman82yrahman82
committed
feat(redis): expose min-replicas-to-write and min-replicas-max-lag
Lets an operator bound the writes lost when an isolated Redis master is failed over: the master refuses writes while fewer replicas than configured are connected and recent. Both values are unset by default, so the rendered Redis config is unchanged. Redis treats 0 in either directive as disabling the check, so the chart refuses minReplicasMaxLag 0 next to a replica requirement instead of silently turning the protection off. Negative values are refused by the schema (minimum 0), since Redis rejects them at CONFIG SET and the operator would stop healing the Sentinels of that instance. A requirement of replicas or more is refused at render: a master with replicas pods has replicas-1 replicas, so it would refuse every write for good. Assisted-by: LLM Signed-off-by: Yasar Rahman <[email protected]>
1 parent 33ca8b6 commit 8be7fb2

8 files changed

Lines changed: 157 additions & 4 deletions

File tree

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

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

‎api/apps/v1alpha1/redis/zz_generated.deepcopy.go‎

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

‎packages/apps/redis/README.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,11 @@ Service utilizes the freshworks-oss Redis Operator (a maintained fork of the arc
3939

4040
### Application-specific parameters
4141

42-
| Name | Description | Type | Value |
43-
| ------------- | --------------------------- | ------ | ------ |
44-
| `authEnabled` | Enable password generation. | `bool` | `true` |
42+
| Name | Description | Type | Value |
43+
| -------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------ | ------ |
44+
| `authEnabled` | Enable password generation. | `bool` | `true` |
45+
| `minReplicasToWrite` | Redis `min-replicas-to-write`: the master refuses writes unless at least this many replicas are connected and within `minReplicasMaxLag`. Unset by default, which leaves the Redis default (disabled). With `replicas: 2` and a value of 1 the master rejects writes while its only replica is down, trading availability for a bound on the writes lost if an isolated master is failed over. Replication stays asynchronous, so this bounds the loss, it does not remove it. Must be 0 or more and below `replicas` (a master with `replicas` pods has `replicas - 1` replicas), otherwise every write is refused; the chart refuses such a value. | `*int` | `null` |
46+
| `minReplicasMaxLag` | Redis `min-replicas-max-lag`, in seconds: a replica whose last acknowledgement is older than this does not count towards `minReplicasToWrite`. Unset by default, which leaves the Redis default (10). Has no effect unless `minReplicasToWrite` is set. Must be at least 1 when `minReplicasToWrite` is set: Redis treats 0 as disabling the check, so the chart refuses it. | `*int` | `null` |
4547

4648

4749
### Sentinel parameters

‎packages/apps/redis/templates/redisfailover.yaml‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,22 @@ spec:
105105
- appendonly no
106106
- save ""
107107
{{- end }}
108+
{{- /* A master with `replicas` pods has replicas-1 replicas, so a requirement of
109+
`replicas` or more can never be met and the master would refuse every write. */}}
110+
{{- if and (not (kindIs "invalid" .Values.minReplicasToWrite)) (ge (.Values.minReplicasToWrite | int) (.Values.replicas | int)) }}
111+
{{- fail (printf "minReplicasToWrite (%d) must be below replicas (%d): the master has replicas-1 replicas, so the master would refuse every write" (.Values.minReplicasToWrite | int) (.Values.replicas | int)) }}
112+
{{- end }}
113+
{{- if not (kindIs "invalid" .Values.minReplicasToWrite) }}
114+
- min-replicas-to-write {{ .Values.minReplicasToWrite | int }}
115+
{{- end }}
116+
{{- /* Redis treats 0 in either directive as "check disabled" (redis.conf), so a lag of 0
117+
next to a replica requirement would silently turn the protection off. */}}
118+
{{- if and (not (kindIs "invalid" .Values.minReplicasToWrite)) (gt (.Values.minReplicasToWrite | int) 0) (not (kindIs "invalid" .Values.minReplicasMaxLag)) (le (.Values.minReplicasMaxLag | int) 0) }}
119+
{{- fail "minReplicasMaxLag must be at least 1 when minReplicasToWrite is set: Redis treats 0 as disabling the check" }}
120+
{{- end }}
121+
{{- if not (kindIs "invalid" .Values.minReplicasMaxLag) }}
122+
- min-replicas-max-lag {{ .Values.minReplicasMaxLag | int }}
123+
{{- end }}
108124
{{- if .Values.authEnabled }}
109125
auth:
110126
secretPath: {{ .Release.Name }}-auth

‎packages/apps/redis/tests/min_replicas_test.yaml‎

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
suite: Redis min-replicas write safety
2+
3+
# Both settings are opt-in: unset keeps the Redis defaults, so existing
4+
# instances render the same customConfig as before.
5+
6+
release:
7+
name: my-redis
8+
namespace: tenant-test
9+
10+
set:
11+
_cluster:
12+
cluster-domain: cluster.local
13+
_namespace:
14+
host: example.com
15+
16+
templates:
17+
- templates/redisfailover.yaml
18+
19+
tests:
20+
- it: "renders neither directive by default"
21+
documentSelector:
22+
path: kind
23+
value: RedisFailover
24+
asserts:
25+
- notContains:
26+
path: spec.redis.customConfig
27+
content: min-replicas-to-write 1
28+
- matchRegex:
29+
path: spec.redis.customConfig[0]
30+
pattern: ^tcp-keepalive 0$
31+
- lengthEqual:
32+
path: spec.redis.customConfig
33+
count: 2
34+
35+
- it: "renders min-replicas-to-write when set"
36+
set:
37+
minReplicasToWrite: 1
38+
documentSelector:
39+
path: kind
40+
value: RedisFailover
41+
asserts:
42+
- contains:
43+
path: spec.redis.customConfig
44+
content: min-replicas-to-write 1
45+
- notContains:
46+
path: spec.redis.customConfig
47+
content: min-replicas-max-lag 5
48+
49+
- it: "renders both directives when both are set"
50+
set:
51+
minReplicasToWrite: 1
52+
minReplicasMaxLag: 5
53+
documentSelector:
54+
path: kind
55+
value: RedisFailover
56+
asserts:
57+
- contains:
58+
path: spec.redis.customConfig
59+
content: min-replicas-to-write 1
60+
- contains:
61+
path: spec.redis.customConfig
62+
content: min-replicas-max-lag 5
63+
64+
- it: "renders an explicit 0 so the setting can be disabled"
65+
set:
66+
minReplicasToWrite: 0
67+
documentSelector:
68+
path: kind
69+
value: RedisFailover
70+
asserts:
71+
- contains:
72+
path: spec.redis.customConfig
73+
content: min-replicas-to-write 0
74+
75+
- it: refuses a zero lag next to a replica requirement
76+
set:
77+
minReplicasToWrite: 1
78+
minReplicasMaxLag: 0
79+
asserts:
80+
- failedTemplate:
81+
errorPattern: "minReplicasMaxLag must be at least 1 when minReplicasToWrite is set"
82+
83+
- it: refuses a negative minReplicasToWrite (schema minimum 0)
84+
set:
85+
minReplicasToWrite: -1
86+
asserts:
87+
- failedTemplate: {}
88+
89+
# minReplicasMaxLag alone, so only the schema minimum can refuse it: with
90+
# minReplicasToWrite set, the template's lag check would refuse it anyway.
91+
- it: refuses a negative minReplicasMaxLag (schema minimum 0)
92+
set:
93+
minReplicasMaxLag: -1
94+
asserts:
95+
- failedTemplate: {}
96+
97+
- it: refuses a requirement that can never be met (minReplicasToWrite >= replicas)
98+
set:
99+
replicas: 2
100+
minReplicasToWrite: 2
101+
asserts:
102+
- failedTemplate:
103+
errorPattern: "minReplicasToWrite \\(2\\) must be below replicas \\(2\\)"

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,16 @@
161161
"type": "boolean",
162162
"default": true
163163
},
164+
"minReplicasToWrite": {
165+
"description": "Redis `min-replicas-to-write`: the master refuses writes unless at least this many replicas are connected and within `minReplicasMaxLag`. Unset by default, which leaves the Redis default (disabled). With `replicas: 2` and a value of 1 the master rejects writes while its only replica is down, trading availability for a bound on the writes lost if an isolated master is failed over. Replication stays asynchronous, so this bounds the loss, it does not remove it. Must be 0 or more and below `replicas` (a master with `replicas` pods has `replicas - 1` replicas), otherwise every write is refused; the chart refuses such a value.",
166+
"type": "integer",
167+
"minimum": 0
168+
},
169+
"minReplicasMaxLag": {
170+
"description": "Redis `min-replicas-max-lag`, in seconds: a replica whose last acknowledgement is older than this does not count towards `minReplicasToWrite`. Unset by default, which leaves the Redis default (10). Has no effect unless `minReplicasToWrite` is set. Must be at least 1 when `minReplicasToWrite` is set: Redis treats 0 as disabling the check, so the chart refuses it.",
171+
"type": "integer",
172+
"minimum": 0
173+
},
164174
"sentinel": {
165175
"description": "Sentinel configuration. When `sentinel.resources` is empty and `sentinel.resourcesPreset` is unset, the three Sentinel pods are sized like the Redis pods (`resources` and `resourcesPreset`). An empty `sentinel.resources` map counts as unset, as an empty top-level `resources` does.",
166176
"type": "object",

‎packages/apps/redis/values.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ tls: {}
113113
## @param {bool} authEnabled - Enable password generation.
114114
authEnabled: true
115115

116+
## @param {*int} [minReplicasToWrite] - Redis `min-replicas-to-write`: the master refuses writes unless at least this many replicas are connected and within `minReplicasMaxLag`. Unset by default, which leaves the Redis default (disabled). With `replicas: 2` and a value of 1 the master rejects writes while its only replica is down, trading availability for a bound on the writes lost if an isolated master is failed over. Replication stays asynchronous, so this bounds the loss, it does not remove it. Must be 0 or more and below `replicas` (a master with `replicas` pods has `replicas - 1` replicas), otherwise every write is refused; the chart refuses such a value.
117+
## @minimum 0
118+
119+
## @param {*int} [minReplicasMaxLag] - Redis `min-replicas-max-lag`, in seconds: a replica whose last acknowledgement is older than this does not count towards `minReplicasToWrite`. Unset by default, which leaves the Redis default (10). Has no effect unless `minReplicasToWrite` is set. Must be at least 1 when `minReplicasToWrite` is set: Redis treats 0 as disabling the check, so the chart refuses it.
120+
## @minimum 0
121+
116122
##
117123
## @section Sentinel parameters
118124
##

0 commit comments

Comments
 (0)