redpanda: respect default_topic_replications if explicitly set

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

CHANGELOG.md

@@ -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

charts/redpanda/Chart.yaml

@@ -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

charts/redpanda/README.md

@@ -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.
 
@@ -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)

charts/redpanda/chart_template_test.go

@@ -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)
@@ -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)
 
@@ -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

charts/redpanda/ci/05-one-node-cluster-tls-sasl-values.yaml

@@ -30,7 +30,6 @@ auth:
 
 config:
   cluster:
-    default_topic_replications: 3
     kafka_nodelete_topics: ['audit', 'consumer_offsets', '_schemas', 'my_sample_topic']
 
 storage:

charts/redpanda/post_upgrade_job.go

@@ -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 != "" {
@@ -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()))
 	}

charts/redpanda/templates/post_upgrade_job.yaml

@@ -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 -}}
@@ -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") -}}

charts/redpanda/templates/tests/test-kafka-nodelete.yaml

@@ -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 }}
@@ -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
 

charts/redpanda/templates/tests/test-kafka-produce-consume.yaml

@@ -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