redpanda: simplify setting redpanda resources

.changes/unreleased/chart-redpanda-Added-20250108-153147.yaml

@@ -0,0 +1,22 @@
+project: chart-redpanda
+kind: Added
+body: |
+  Added `resources.limits` and `resources.requests` as an alternative method of managing the redpanda container's resources.
+
+    When both `resources.limits` and `resources.requests` are specified, the
+    redpanda container's `resources` will be set to the provided values and all
+    other keys of `resources` will be ignored. Instead, all other values will be
+    inferred from the limits and requests.
+
+    This allows fine grain control of resources. i.e. It is now possible to set
+    CPU requests without setting limits:
+
+    ```yaml
+    resources:
+    limits: {} # Specified but no cpu or memory values provided
+    requests:
+      cpu: 5 # Only CPU requests
+    ```
+
+    For more details see [redpanda's values.yaml](./charts/redpanda/values.yaml).
+time: 2025-01-08T15:31:47.946476-05:00

charts/redpanda/CHANGELOG.md

@@ -5,6 +5,28 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
 and is generated by [Changie](https://github.com/miniscruff/changie).
 
 
+## Unreleased
+### Added
+* Added `resources.limits` and `resources.requests` as an alternative method of managing the redpanda container's resources.
+
+  When both `resources.limits` and `resources.requests` are specified, the
+  redpanda container's `resources` will be set to the provided values and all
+  other keys of `resources` will be ignored. Instead, all other values will be
+  inferred from the limits and requests.
+
+  This allows fine grain control of resources. i.e. It is now possible to set
+  CPU requests without setting limits:
+
+  ```yaml
+  resources:
+  limits: {} # Specified but no cpu or memory values provided
+  requests:
+    cpu: 5 # Only CPU requests
+  ```
+
+  For more details see [redpanda's values.yaml](./charts/redpanda/values.yaml).
+
+
 ### [5.9.18](https://github.com/redpanda-data/helm-charts/releases/tag/redpanda-5.9.18) - 2024-12-20
 #### Added
 #### Changed

charts/redpanda/README.md

@@ -621,7 +621,42 @@ Enable for features that need extra privileges. If you use the Redpanda Operator
 
 ### [resources](https://artifacthub.io/packages/helm/redpanda-data/redpanda?modal=values&path=resources)
 
-Pod resource management. This section simplifies resource allocation by providing a single location where resources are defined. Helm sets these resource values within the `statefulset.yaml` and `configmap.yaml` templates.  The default values are for a development environment. Production-level values and other considerations are documented, where those values are different from the default. For details, see the [Pod resources documentation](https://docs.redpanda.com/docs/manage/kubernetes/manage-resources/).
+Pod resource management.
+This section simplifies resource allocation for the redpanda container by
+providing a single location where resources are defined.
+
+Resources may be specified by either setting `resources.cpu` and
+`resources.memory` (the default) or by setting `resources.requests` and
+`resources.limits`.
+
+For details on `resources.cpu` and `resources.memory`, see their respective
+documentation below.
+
+When `resources.limits` and `resources.requests` are set, the redpanda
+container's resources will be set to exactly the provided values. This allows
+users to granularly control limits and requests to best suit their use case.
+For example: `resources.requests.cpu` may be set without setting
+`resources.limits.cpu` to avoid the potential of CPU throttling.
+
+Redpanda's resource related CLI flags will then be calculated as follows:
+* `--smp max(1, floor(coalesce(resources.requests.cpu, resources.limits.cpu)))`
+* `--memory coalesce(resources.requests.memory, resources.limits.memory) * 90%`
+* `--reserve-memory 0`
+* `--overprovisioned coalesce(resources.requests.cpu, resources.limits.cpu) < 1000m`
+
+If neither a request nor a limit is provided for cpu or memory, the
+corresponding flag will be omitted. As a result, setting `resources.limits`
+and `resources.requests` to `{}` will result in redpanda being run without
+`--smp` or `--memory`. (This is not recommended).
+
+If the computed CLI flags are undesirable, they may be overridden by
+specifying the desired value through `statefulset.additionalRedpandaCmdFlags`.
+
+The default values are for a development environment.
+Production-level values and other considerations are documented,
+where those values are different from the default.
+For details,
+see the [Pod resources documentation](https://docs.redpanda.com/docs/manage/kubernetes/manage-resources/).
 
 **Default:**
 
@@ -961,7 +996,7 @@ To create `Guaranteed` Pods for Redpanda brokers, provide both requests and limi
 
 ### [statefulset.sideCars.controllers.image.tag](https://artifacthub.io/packages/helm/redpanda-data/redpanda?modal=values&path=statefulset.sideCars.controllers.image.tag)
 
-**Default:** `"v2.3.5-24.3.2"`
+**Default:** `"v2.3.3-24.3.1"`
 
 ### [statefulset.sideCars.controllers.metricsAddress](https://artifacthub.io/packages/helm/redpanda-data/redpanda?modal=values&path=statefulset.sideCars.controllers.metricsAddress)
 

charts/redpanda/configmap.tpl.go

@@ -359,10 +359,12 @@ func rpkNodeConfig(dot *helmette.Dot) map[string]any {
 		schemaRegistryTLS = tls
 	}
 
+	lockMemory, overprovisioned, flags := RedpandaAdditionalStartFlags(&values)
+
 	result := map[string]any{
-		"overprovisioned":        values.Resources.GetOverProvisionValue(),
-		"enable_memory_locking":  ptr.Deref(values.Resources.Memory.EnableMemoryLocking, false),
-		"additional_start_flags": RedpandaAdditionalStartFlags(dot),
+		"additional_start_flags": flags,
+		"enable_memory_locking":  lockMemory,
+		"overprovisioned":        overprovisioned,
 		"kafka_api": map[string]any{
 			"brokers": brokerList,
 			"tls":     brokerTLS,
@@ -610,39 +612,60 @@ func createInternalListenerCfg(port int32) map[string]any {
 	}
 }
 
-// RedpandaAdditionalStartFlags returns a string list of flags suitable for use
+// RedpandaAdditionalStartFlags returns a string slice of flags suitable for use
 // as `additional_start_flags`. User provided flags will override any of those
 // set by default.
-func RedpandaAdditionalStartFlags(dot *helmette.Dot) []string {
-	values := helmette.Unwrap[Values](dot.Values)
-
+func RedpandaAdditionalStartFlags(values *Values) (bool, bool, []string) {
 	// All `additional_start_flags` that are set by the chart.
-	chartFlags := values.Resources.GetRedpandaStartFlags()
-	chartFlags["default-log-level"] = values.Logging.LogLevel
+	flags := values.Resources.GetRedpandaFlags()
+	flags["--default-log-level"] = values.Logging.LogLevel
 
-	// If in developer_mode, don't set reserve-memory.
+	// Unclear why this is done aside from historical reasons.
+	// Legacy comment: If in developer_mode, don't set reserve-memory.
 	if values.Config.Node["developer_mode"] == true {
-		delete(chartFlags, "reserve-memory")
+		delete(flags, "reserve-memory")
 	}
 
-	// Check to see if there are any flags overriding the defaults set by the
-	// chart.
-	for flag := range chartFlags {
-		for _, userFlag := range values.Statefulset.AdditionalRedpandaCmdFlags {
-			if helmette.RegexMatch(fmt.Sprintf("^--%s", flag), userFlag) {
-				delete(chartFlags, flag)
-			}
-		}
+	for key, value := range ParseCLIArgs(values.Statefulset.AdditionalRedpandaCmdFlags) {
+		flags[key] = value
+	}
+
+	enabledOptions := map[string]bool{
+		"true": true,
+		"1":    true,
+		"":     true,
+	}
+
+	// Due to a buglet in rpk, we need to set lock-memory and overprovisioned
+	// via their fields in redpanda.yaml instead of additional_start_flags.
+	// https://github.com/redpanda-data/helm-charts/pull/1622#issuecomment-2577922409
+	lockMemory := false
+	if value, ok := flags["lock-memory"]; ok {
+		lockMemory = enabledOptions[value]
+		delete(flags, "lock-memory")
+	}
+
+	overprovisioned := false
+	if value, ok := flags["overprovisioned"]; ok {
+		overprovisioned = enabledOptions[value]
+		delete(flags, "overprovisioned")
 	}
 
 	// Deterministically order out list and add in values supplied flags.
-	keys := helmette.Keys(chartFlags)
-	helmette.SortAlpha(keys)
+	keys := helmette.Keys(flags)
+	keys = helmette.SortAlpha(keys)
 
-	flags := []string{}
+	var rendered []string
 	for _, key := range keys {
-		flags = append(flags, fmt.Sprintf("--%s=%s", key, chartFlags[key]))
+		value := flags[key]
+		// Support flags that don't have values (`--overprovisioned`) by
+		// letting them be specified as key: ""
+		if value == "" {
+			rendered = append(rendered, fmt.Sprintf("--%s", key))
+		} else {
+			rendered = append(rendered, fmt.Sprintf("--%s=%s", key, value))
+		}
 	}
 
-	return append(flags, values.Statefulset.AdditionalRedpandaCmdFlags...)
+	return lockMemory, overprovisioned, rendered
 }

charts/redpanda/helpers.go

@@ -568,3 +568,54 @@ func mergeContainer(original corev1.Container, override applycorev1.ContainerApp
 	merged.VolumeMounts = mergeSliceBy(original.VolumeMounts, override.VolumeMounts, "name", mergeVolumeMount)
 	return merged
 }
+
+// ParseCLIArgs parses a slice of strings intended for rpk's
+// `additional_start_flags` field into a map to allow merging slices of flags
+// or introspection thereof.
+// Flags without values are not differentiated between flags with values of an
+// empty string. e.g. --value and --value=” are represented the same way.
+func ParseCLIArgs(args []string) map[string]string {
+	parsed := map[string]string{}
+
+	// NB: templates/gotohelm don't supported c style for loops (or ++) which
+	// is the ideal for this situation. The janky code you see is a rough
+	// equivalent for the following:
+	// for i := 0; i < len(args); i++ {
+	i := -1          // Start at -1 so our increment can be at the start of the loop.
+	for range args { // Range needs to range over something and we'll always have < len(args) iterations.
+		i = i + 1
+		if i >= len(args) {
+			break
+		}
+
+		// All flags should start with - or --.
+		// If not present, skip this value.
+		if !strings.HasPrefix(args[i], "-") {
+			continue
+		}
+
+		flag := args[i]
+
+		// Handle values like: `--flag value` or `--flag=value`
+		// There's no strings.Index in sprig, so RegexSplit is the next best
+		// option.
+		spl := helmette.RegexSplit(" |=", flag, 2)
+		if len(spl) == 2 {
+			parsed[spl[0]] = spl[1]
+			continue
+		}
+
+		// If no ' ' or =, consume the next value if it's not formatted like a
+		// flag: `--flag`, `value`
+		if i+1 < len(args) && !strings.HasPrefix(args[i+1], "-") {
+			parsed[flag] = args[i+1]
+			i = i + 1
+			continue
+		}
+
+		// Otherwise, assume this is a bare flag and assign it an empty string.
+		parsed[flag] = ""
+	}
+
+	return parsed
+}

charts/redpanda/helpers_test.go

@@ -210,3 +210,58 @@ func TestStrategicMergePatch_nopanic(t *testing.T) {
 		})
 	})
 }
+
+func TestParseCLIArgs(t *testing.T) {
+	testCases := []struct {
+		In  []string
+		Out map[string]string
+	}{
+		{
+			In:  []string{},
+			Out: map[string]string{},
+		},
+		{
+			In: []string{
+				"-foo=bar",
+				"-baz 1",
+				"--help", "topic",
+			},
+			Out: map[string]string{
+				"-foo":   "bar",
+				"-baz":   "1",
+				"--help": "topic",
+			},
+		},
+		{
+			In: []string{
+				"invalid",
+				"-valid=bar",
+				"--trailing spaces ",
+				"--bare=",
+				"ignored-perhaps-confusingly",
+				"--final",
+			},
+			Out: map[string]string{
+				"-valid":     "bar",
+				"--trailing": "spaces ",
+				"--bare":     "",
+				"--final":    "",
+			},
+		},
+	}
+
+	for _, tc := range testCases {
+		assert.Equal(t, tc.Out, redpanda.ParseCLIArgs(tc.In))
+	}
+
+	t.Run("NotPanics", rapid.MakeCheck(func(t *rapid.T) {
+		// We could certainly be more inventive with
+		// the inputs here but this is more of a
+		// fuzz test than a property test.
+		in := rapid.SliceOf(rapid.String()).Draw(t, "input")
+
+		assert.NotPanics(t, func() {
+			redpanda.ParseCLIArgs(in)
+		})
+	}))
+}

charts/redpanda/servicemonitor.go

@@ -29,7 +29,7 @@ func ServiceMonitor(dot *helmette.Dot) *monitoringv1.ServiceMonitor {
 		Interval:    values.Monitoring.ScrapeInterval,
 		Path:        "/public_metrics",
 		Port:        "admin",
-		EnableHttp2: values.Monitoring.EnableHttp2,
+		EnableHttp2: values.Monitoring.EnableHTTP2,
 		Scheme:      "http",
 	}