Skip to content

Commit

Permalink
redpanda: respect default_topic_replications if explicitly set
Browse files Browse the repository at this point in the history 
Prior to this commit, `default_topic_replications` could be unexpectedly
modified by the chart based on some internal math. This behavior was opaque and
frustrating to users attempting to finely tune their cluster configuration.

This commit removes the default value of `default_topic_replications` in
`values.yaml` and instead sets it within the chart only if
`default_topic_replications` is NOT provided. This preserves the existing
behavior of setting `default_topic_replications` to 3 or 1 based on
`statefulset.replicas`.

Fixes #1501 K8S-334
  • Loading branch information
@chrisseto
chrisseto committed Sep 13, 2024
1 parent 0a4c2b5 commit 0a6ebce
Show file tree
Hide file tree
Showing 18 changed files with 1,917 additions and 2,053 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@
#### Fixed
#### Removed

### [5.9.3](https://github.com/redpanda-data/helm-charts/releases/tag/redpanda-5.9.3) - 2024-09-11
#### Added
* Add basic bootstrap user support (#1513)
#### Changed
#### Fixed
* When specified, `truststore_file` is no longer propagated to client configurations.
* If provided, `config.cluster.default_topic_replications` is now respected regardless of the value of `statefulset.replicas`.
#### Removed

### [5.9.1](https://github.com/redpanda-data/helm-charts/releases/tag/redpanda-5.9.1) - 2024-8-19
#### Added
#### Changed
Expand Down
2 changes: 1 addition & 1 deletion charts/redpanda/Chart.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ type: application
# The chart version and the app version are not the same and will not track
# together. The chart version is a semver representation of changes to this
# chart.
version: 5.9.2
version: 5.9.3

# The app version is the default version of Redpanda to install.
# ** NOTE for maintainers: please ensure the artifacthub image annotation is updated before merging
Expand Down
4 changes: 2 additions & 2 deletions charts/redpanda/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
description: Find the default values and descriptions of settings in the Redpanda Helm chart.
---

![Version: 5.9.2](https://img.shields.io/badge/Version-5.9.2-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v24.2.3](https://img.shields.io/badge/AppVersion-v24.2.3-informational?style=flat-square)
![Version: 5.9.3](https://img.shields.io/badge/Version-5.9.3-informational?style=flat-square) ![Type: application](https://img.shields.io/badge/Type-application-informational?style=flat-square) ![AppVersion: v24.2.3](https://img.shields.io/badge/AppVersion-v24.2.3-informational?style=flat-square)

This page describes the official Redpanda Helm Chart. In particular, this page describes the contents of the chart’s [`values.yaml` file](https://github.com/redpanda-data/helm-charts/blob/main/charts/redpanda/values.yaml). Each of the settings is listed and described on this page, along with any default values.

Expand Down Expand Up @@ -172,7 +172,7 @@ This section contains various settings supported by Redpanda that may not work c
**Default:**

```
{"cluster":{"default_topic_replications":3},"node":{"crash_loop_limit":5},"pandaproxy_client":{},"rpk":{},"schema_registry_client":{},"tunable":{"compacted_log_segment_size":67108864,"group_topic_partitions":16,"kafka_batch_max_bytes":1048576,"kafka_connection_rate_limit":1000,"log_segment_size":134217728,"log_segment_size_max":268435456,"log_segment_size_min":16777216,"max_compacted_log_segment_size":536870912,"topic_partitions_per_shard":1000}}
{"cluster":{},"node":{"crash_loop_limit":5},"pandaproxy_client":{},"rpk":{},"schema_registry_client":{},"tunable":{"compacted_log_segment_size":67108864,"group_topic_partitions":16,"kafka_batch_max_bytes":1048576,"kafka_connection_rate_limit":1000,"log_segment_size":134217728,"log_segment_size_max":268435456,"log_segment_size_min":16777216,"max_compacted_log_segment_size":536870912,"topic_partitions_per_shard":1000}}
```

### [config.node](https://artifacthub.io/packages/helm/redpanda-data/redpanda?modal=values&path=config.node)
Expand Down
56 changes: 51 additions & 5 deletions charts/redpanda/chart_template_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,14 @@ func TestTemplate(t *testing.T) {
require.NoError(t, err)
AssertFieldEquals(t, params, out)

case `ASSERT-FIELD-CONTAINS`:
require.NoError(t, err)
AssertFieldContains(t, params, out)

case `ASSERT-FIELD-NOT-CONTAINS`:
require.NoError(t, err)
AssertFieldNotContains(t, params, out)

case `ASSERT-VALID-RPK-CONFIGURATION`:
require.NoError(t, err)
AssertValidRPKConfiguration(t, out)
Expand Down Expand Up @@ -435,6 +443,47 @@ func AssertFieldEquals(t *testing.T, params []json.RawMessage, manifests []byte)
require.NoError(t, json.Unmarshal(params[1], &key))
require.NoError(t, json.Unmarshal(params[2], &fieldPath))

execJSONPath(t, manifests, gvk, key, fieldPath, func(result any) {
actual, err := json.Marshal(result)
require.NoError(t, err)

require.JSONEq(t, string(fieldValue), string(actual), "%q", fieldPath)
})
}

func AssertFieldContains(t *testing.T, params []json.RawMessage, manifests []byte) {
var gvk string
var key string
var fieldPath string
var contained any

require.NoError(t, json.Unmarshal(params[0], &gvk))
require.NoError(t, json.Unmarshal(params[1], &key))
require.NoError(t, json.Unmarshal(params[2], &fieldPath))
require.NoError(t, json.Unmarshal(params[3], &contained))

execJSONPath(t, manifests, gvk, key, fieldPath, func(result any) {
assert.Contains(t, result, contained)
})
}

func AssertFieldNotContains(t *testing.T, params []json.RawMessage, manifests []byte) {
var gvk string
var key string
var fieldPath string
var contained any

require.NoError(t, json.Unmarshal(params[0], &gvk))
require.NoError(t, json.Unmarshal(params[1], &key))
require.NoError(t, json.Unmarshal(params[2], &fieldPath))
require.NoError(t, json.Unmarshal(params[3], &contained))

execJSONPath(t, manifests, gvk, key, fieldPath, func(result any) {
assert.NotContains(t, result, contained)
})
}

func execJSONPath(t *testing.T, manifests []byte, gvk, key, jsonPath string, fn func(any)) {
objs, err := kube.DecodeYAML(manifests, redpanda.Scheme)
require.NoError(t, err)

Expand All @@ -452,16 +501,13 @@ func AssertFieldEquals(t *testing.T, params []json.RawMessage, manifests []byte)

// See https://kubernetes.io/docs/reference/kubectl/jsonpath/
path := jsonpath.New("").AllowMissingKeys(true)
require.NoError(t, path.Parse(fieldPath))
require.NoError(t, path.Parse(jsonPath))

results, err := path.FindResults(obj)
require.NoError(t, err)

for _, result := range results {
actual, err := json.Marshal(result[0].Interface())
require.NoError(t, err)

require.JSONEq(t, string(fieldValue), string(actual), "%q", fieldPath)
fn(result[0].Interface())
}

return
Expand Down
1 change: 0 additions & 1 deletion charts/redpanda/ci/05-one-node-cluster-tls-sasl-values.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ auth:

config:
cluster:
default_topic_replications: 3
kafka_nodelete_topics: ['audit', 'consumer_offsets', '_schemas', 'my_sample_topic']

storage:
Expand Down
4 changes: 2 additions & 2 deletions charts/redpanda/configmap.tpl.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ func BootstrapFile(dot *helmette.Dot) string {
bootstrap = helmette.Merge(bootstrap, values.AuditLogging.Translate(dot, values.Auth.IsSASLEnabled()))
bootstrap = helmette.Merge(bootstrap, values.Logging.Translate())
bootstrap = helmette.Merge(bootstrap, values.Config.Tunable.Translate())
bootstrap = helmette.Merge(bootstrap, values.Config.Cluster.Translate(values.Statefulset.Replicas, false))
bootstrap = helmette.Merge(bootstrap, values.Config.Cluster.Translate())
bootstrap = helmette.Merge(bootstrap, values.Auth.Translate(values.Auth.IsSASLEnabled()))

return helmette.ToYaml(bootstrap)
Expand All @@ -84,7 +84,7 @@ func RedpandaConfigFile(dot *helmette.Dot, includeSeedServer bool) string {
redpanda = helmette.Merge(redpanda, values.AuditLogging.Translate(dot, values.Auth.IsSASLEnabled()))
redpanda = helmette.Merge(redpanda, values.Logging.Translate())
redpanda = helmette.Merge(redpanda, values.Config.Tunable.Translate())
redpanda = helmette.Merge(redpanda, values.Config.Cluster.Translate(values.Statefulset.Replicas, true))
redpanda = helmette.Merge(redpanda, values.Config.Cluster.Translate())
redpanda = helmette.Merge(redpanda, values.Auth.Translate(values.Auth.IsSASLEnabled()))
redpanda = helmette.Merge(redpanda, values.Config.Node.Translate())

Expand Down
17 changes: 10 additions & 7 deletions charts/redpanda/post_upgrade_job.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,13 +86,6 @@ func PostUpgradeJobScript(dot *helmette.Dot) string {
for key, value := range values.Config.Cluster {
asInt64, isInt64 := helmette.AsIntegral[int64](value)

if key == "default_topic_replications" && isInt64 {
r := int64(values.Statefulset.Replicas)
// This calculates the closest odd number less than or equal to r: 1=1, 2=1, 3=3, ...
r = (r + (r % 2)) - 1
asInt64 = helmette.Min(asInt64, int64(r))
}

if asBool, ok := value.(bool); ok && asBool {
script = append(script, fmt.Sprintf("rpk cluster config set %s %t", key, asBool))
} else if asStr, ok := value.(string); ok && asStr != "" {
Expand All @@ -106,6 +99,16 @@ func PostUpgradeJobScript(dot *helmette.Dot) string {
}
}

// If default_topic_replications is not set and we have at least 3 Brokers,
// upgrade from redpanda's default of 1 to 3 so, when possible, topics are
// HA by default.
// See also:
// - https://github.com/redpanda-data/helm-charts/issues/583
// - https://github.com/redpanda-data/helm-charts/issues/1501
if _, ok := values.Config.Cluster["default_topic_replications"]; !ok && values.Statefulset.Replicas >= 3 {
script = append(script, "rpk cluster config set default_topic_replications 3")
}

if _, ok := values.Config.Cluster["storage_min_free_bytes"]; !ok {
script = append(script, fmt.Sprintf("rpk cluster config set storage_min_free_bytes %d", values.Storage.StorageMinFreeBytes()))
}
Expand Down
4 changes: 2 additions & 2 deletions charts/redpanda/templates/_configmap.go.tpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
{{- $bootstrap = (merge (dict ) $bootstrap (get (fromJson (include "redpanda.AuditLogging.Translate" (dict "a" (list $values.auditLogging $dot (get (fromJson (include "redpanda.Auth.IsSASLEnabled" (dict "a" (list $values.auth) ))) "r")) ))) "r")) -}}
{{- $bootstrap = (merge (dict ) $bootstrap (get (fromJson (include "redpanda.Logging.Translate" (dict "a" (list $values.logging) ))) "r")) -}}
{{- $bootstrap = (merge (dict ) $bootstrap (get (fromJson (include "redpanda.TunableConfig.Translate" (dict "a" (list $values.config.tunable) ))) "r")) -}}
{{- $bootstrap = (merge (dict ) $bootstrap (get (fromJson (include "redpanda.ClusterConfig.Translate" (dict "a" (list $values.config.cluster ($values.statefulset.replicas | int) false) ))) "r")) -}}
{{- $bootstrap = (merge (dict ) $bootstrap (get (fromJson (include "redpanda.ClusterConfig.Translate" (dict "a" (list $values.config.cluster) ))) "r")) -}}
{{- $bootstrap = (merge (dict ) $bootstrap (get (fromJson (include "redpanda.Auth.Translate" (dict "a" (list $values.auth (get (fromJson (include "redpanda.Auth.IsSASLEnabled" (dict "a" (list $values.auth) ))) "r")) ))) "r")) -}}
{{- $_is_returning = true -}}
{{- (dict "r" (toYaml $bootstrap)) | toJson -}}
Expand All @@ -52,7 +52,7 @@
{{- $redpanda = (merge (dict ) $redpanda (get (fromJson (include "redpanda.AuditLogging.Translate" (dict "a" (list $values.auditLogging $dot (get (fromJson (include "redpanda.Auth.IsSASLEnabled" (dict "a" (list $values.auth) ))) "r")) ))) "r")) -}}
{{- $redpanda = (merge (dict ) $redpanda (get (fromJson (include "redpanda.Logging.Translate" (dict "a" (list $values.logging) ))) "r")) -}}
{{- $redpanda = (merge (dict ) $redpanda (get (fromJson (include "redpanda.TunableConfig.Translate" (dict "a" (list $values.config.tunable) ))) "r")) -}}
{{- $redpanda = (merge (dict ) $redpanda (get (fromJson (include "redpanda.ClusterConfig.Translate" (dict "a" (list $values.config.cluster ($values.statefulset.replicas | int) true) ))) "r")) -}}
{{- $redpanda = (merge (dict ) $redpanda (get (fromJson (include "redpanda.ClusterConfig.Translate" (dict "a" (list $values.config.cluster) ))) "r")) -}}
{{- $redpanda = (merge (dict ) $redpanda (get (fromJson (include "redpanda.Auth.Translate" (dict "a" (list $values.auth (get (fromJson (include "redpanda.Auth.IsSASLEnabled" (dict "a" (list $values.auth) ))) "r")) ))) "r")) -}}
{{- $redpanda = (merge (dict ) $redpanda (get (fromJson (include "redpanda.NodeConfig.Translate" (dict "a" (list $values.config.node) ))) "r")) -}}
{{- $_ := (get (fromJson (include "redpanda.configureListeners" (dict "a" (list $redpanda $dot) ))) "r") -}}
Expand Down
26 changes: 3 additions & 23 deletions charts/redpanda/templates/_values.go.tpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1246,35 +1246,15 @@

{{- define "redpanda.ClusterConfig.Translate" -}}
{{- $c := (index .a 0) -}}
{{- $replicas := (index .a 1) -}}
{{- $skipDefaultTopic := (index .a 2) -}}
{{- range $_ := (list 1) -}}
{{- $_is_returning := false -}}
{{- $result := (dict ) -}}
{{- range $k, $v := $c -}}
{{- if (and (eq $k "default_topic_replications") (not $skipDefaultTopic)) -}}
{{- $r := ($replicas | int) -}}
{{- $input := ($r | int) -}}
{{- $tmp_tuple_15 := (get (fromJson (include "_shims.compact" (dict "a" (list (get (fromJson (include "_shims.asintegral" (dict "a" (list $v) ))) "r")) ))) "r") -}}
{{- $tmp_tuple_15 := (get (fromJson (include "_shims.compact" (dict "a" (list (get (fromJson (include "_shims.typetest" (dict "a" (list "bool" $v false) ))) "r")) ))) "r") -}}
{{- $ok_17 := $tmp_tuple_15.T2 -}}
{{- $num_16 := ($tmp_tuple_15.T1 | int) -}}
{{- $b_16 := $tmp_tuple_15.T1 -}}
{{- if $ok_17 -}}
{{- $input = $num_16 -}}
{{- end -}}
{{- $tmp_tuple_16 := (get (fromJson (include "_shims.compact" (dict "a" (list (get (fromJson (include "_shims.asnumeric" (dict "a" (list $v) ))) "r")) ))) "r") -}}
{{- $ok_19 := $tmp_tuple_16.T2 -}}
{{- $f_18 := ($tmp_tuple_16.T1 | float64) -}}
{{- if $ok_19 -}}
{{- $input = ($f_18 | int) -}}
{{- end -}}
{{- $_ := (set $result $k (min ($input | int64) (((sub ((add $r (((mod $r (2 | int)) | int))) | int) (1 | int)) | int) | int64))) -}}
{{- continue -}}
{{- end -}}
{{- $tmp_tuple_17 := (get (fromJson (include "_shims.compact" (dict "a" (list (get (fromJson (include "_shims.typetest" (dict "a" (list "bool" $v false) ))) "r")) ))) "r") -}}
{{- $ok_21 := $tmp_tuple_17.T2 -}}
{{- $b_20 := $tmp_tuple_17.T1 -}}
{{- if $ok_21 -}}
{{- $_ := (set $result $k $b_20) -}}
{{- $_ := (set $result $k $b_16) -}}
{{- continue -}}
{{- end -}}
{{- if (not (empty $v)) -}}
Expand Down
14 changes: 7 additions & 7 deletions charts/redpanda/templates/post_upgrade_job.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@
{{- $tmp_tuple_1 := (get (fromJson (include "_shims.compact" (dict "a" (list (get (fromJson (include "_shims.asintegral" (dict "a" (list $value) ))) "r")) ))) "r") -}}
{{- $isInt64 := $tmp_tuple_1.T2 -}}
{{- $asInt64 := ($tmp_tuple_1.T1 | int64) -}}
{{- if (and (eq $key "default_topic_replications") $isInt64) -}}
{{- $r := (($values.statefulset.replicas | int) | int64) -}}
{{- $r = ((sub (((add $r (((mod $r (2 | int64)) | int64))) | int64)) (1 | int64)) | int64) -}}
{{- $asInt64 = (min $asInt64 ($r | int64)) -}}
{{- end -}}
{{- $tmp_tuple_2 := (get (fromJson (include "_shims.compact" (dict "a" (list (get (fromJson (include "_shims.typetest" (dict "a" (list "bool" $value false) ))) "r")) ))) "r") -}}
{{- $ok_2 := $tmp_tuple_2.T2 -}}
{{- $asBool_1 := $tmp_tuple_2.T1 -}}
Expand Down Expand Up @@ -62,9 +57,14 @@
{{- if $_is_returning -}}
{{- break -}}
{{- end -}}
{{- $tmp_tuple_5 := (get (fromJson (include "_shims.compact" (dict "a" (list (get (fromJson (include "_shims.dicttest" (dict "a" (list $values.config.cluster "storage_min_free_bytes" (coalesce nil)) ))) "r")) ))) "r") -}}
{{- $tmp_tuple_5 := (get (fromJson (include "_shims.compact" (dict "a" (list (get (fromJson (include "_shims.dicttest" (dict "a" (list $values.config.cluster "default_topic_replications" (coalesce nil)) ))) "r")) ))) "r") -}}
{{- $ok_7 := $tmp_tuple_5.T2 -}}
{{- if (not $ok_7) -}}
{{- if (and (not $ok_7) (ge ($values.statefulset.replicas | int) (3 | int))) -}}
{{- $script = (concat (default (list ) $script) (list "rpk cluster config set default_topic_replications 3")) -}}
{{- end -}}
{{- $tmp_tuple_6 := (get (fromJson (include "_shims.compact" (dict "a" (list (get (fromJson (include "_shims.dicttest" (dict "a" (list $values.config.cluster "storage_min_free_bytes" (coalesce nil)) ))) "r")) ))) "r") -}}
{{- $ok_8 := $tmp_tuple_6.T2 -}}
{{- if (not $ok_8) -}}
{{- $script = (concat (default (list ) $script) (list (printf "rpk cluster config set storage_min_free_bytes %d" ((get (fromJson (include "redpanda.Storage.StorageMinFreeBytes" (dict "a" (list $values.storage) ))) "r") | int64)))) -}}
{{- end -}}
{{- if (get (fromJson (include "redpanda.RedpandaAtLeast_23_2_1" (dict "a" (list $dot) ))) "r") -}}
Expand Down
9 changes: 2 additions & 7 deletions charts/redpanda/templates/tests/test-auditLogging.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ spec:
old_setting=${-//[^x]/}
audit_topic_name="_redpanda.audit_log"
expected_partitions={{ .Values.auditLogging.partitions }}
# sasl configurations
set +x
IFS=":" read -r {{ include "rpk-sasl-environment-variables" . }} < <(grep "" $(find /etc/secrets/users/* -print))
Expand All @@ -62,12 +62,7 @@ spec:
{{- end }}
export {{ include "rpk-sasl-environment-variables" . }}
if [[ -n "$old_setting" ]]; then set -x; fi
{{- $i := .Values.statefulset.replicas }}
{{- $default_topic_replicas := sub (add $i (mod $i 2)) 1 }}
# wait for post-upgrade job to update the default_topic_replications value
timeout 600 bash -c "until [[ $(rpk cluster config get default_topic_replications) = {{ $default_topic_replicas }} ]]; do sleep 1; done"
# now run the to determine if we have the right results
# should describe topic without error
rpk topic describe ${audit_topic_name}
Expand Down
8 changes: 2 additions & 6 deletions charts/redpanda/templates/tests/test-kafka-nodelete.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,7 @@ spec:
export {{ include "rpk-sasl-environment-variables" . }}
if [[ -n "$old_setting" ]]; then set -x; fi
{{- end }}
{{- $i := .Values.statefulset.replicas }}
{{- $default_topic_replicas := sub (add $i (mod $i 2)) 1 }}
# wait for post-upgrade job to update the default_topic_replications value
timeout 120 bash -c "until [[ $(rpk cluster config get default_topic_replications) = {{ $default_topic_replicas }} ]]; do sleep 1; done"


exists=$(rpk topic list | grep my_sample_topic | awk '{print $1}')
if [[ "$exists" != "my_sample_topic" ]]; then
until rpk topic create my_sample_topic {{ $cloudStorageFlags }}
Expand All @@ -85,7 +81,7 @@ spec:
{{- end }}
sleep 2
rpk topic consume my_sample_topic -n 1 | grep "Pandas are awesome!"

# now check if we can delete the topic (we should not)
rpk topic delete my_sample_topic

Expand Down
4 changes: 0 additions & 4 deletions charts/redpanda/templates/tests/test-kafka-produce-consume.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,6 @@ spec:
export {{ include "rpk-sasl-environment-variables" . }}
if [[ -n "$old_setting" ]]; then set -x; fi
{{- end }}
{{- $i := .Values.statefulset.replicas }}
{{- $default_topic_replicas := sub (add $i (mod $i 2)) 1 }}
# wait for post-upgrade job to update the default_topic_replications value
timeout 600 bash -c "until [[ $(rpk cluster config get default_topic_replications) = {{ $default_topic_replicas }} ]]; do sleep 1; done"
until rpk topic create produce.consume.test.$POD_NAME {{ $cloudStorageFlags }}
do sleep 2
done
Expand Down
Loading

0 comments on commit 0a6ebce

Please sign in to comment.