redpanda: simplify setting redpanda resources

Prior to this commit it was not possible to "mix and match" the resource
requests and limits of the redpanda container. The redpanda chart also required
users to grok a new model of resources unique to the chart which made the chart
less user friendly.

This commit introduces a more user friendly and backwards compatible way of
directly controlling the requests and limits of the redpanda container. Rather
than directly exposing the `--smp`, `--memory`, `--reserve-memory`, and
`--overprovisioned` flags to the end user, this method will infer them from the
provided values with updated best practices.

redpanda-data/helm-charts#1494
K8S-325
K8S-434

.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:**
 

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",
 	}