From c0f8224f2b10c8984839c651a45373bcd6f26da0 Mon Sep 17 00:00:00 2001 From: Xavier Lange Date: Wed, 1 Jul 2026 22:48:02 -0400 Subject: [PATCH] feat: set a CPU request on the etcd container (Burstable QoS) Set a CPU request (never a limit) of 50m on the etcd container so the pod is Burstable instead of BestEffort. Without a request etcd gets the kernel-floor cpu.shares of 2 and is the first workload evicted under node pressure; a small request raises the scheduling floor without ever throttling etcd. Overridable via a new --etcd-cpu-request flag (default "50m"). Empty string or "0" applies no request (original BestEffort behavior); a malformed quantity is treated as unset so a bad flag cannot wedge cluster creation. Controller-level knob, no API/CRD change. Documented in the flag help and docs/operator-flags.md (linked from the README). Table-driven test asserts the request is applied (with no CPU limit), an override is honored, and empty/"0"/malformed disable it. Co-Authored-By: Claude Fable 5 Signed-off-by: Xavier Lange --- README.md | 8 +++ cmd/main.go | 11 ++++ docs/operator-flags.md | 42 ++++++++++++ internal/controller/utils.go | 51 +++++++++++++-- internal/controller/utils_test.go | 102 ++++++++++++++++++++++++++++++ 5 files changed, 210 insertions(+), 4 deletions(-) create mode 100644 docs/operator-flags.md diff --git a/README.md b/README.md index bac1320f..2533701d 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,14 @@ make deploy IMG=/etcd-operator:tag > **NOTE**: If you encounter RBAC errors, you may need to grant yourself cluster-admin privileges or be logged in as admin. +**Tune the manager (optional):** + +The manager exposes process-level flags, including `--etcd-cpu-request` (CPU +request on the etcd container, default `50m`, which makes etcd Burstable instead +of BestEffort). See [docs/operator-flags.md](docs/operator-flags.md) for details +and tuning guidance. + + **Create instances of your solution** You can apply the samples (examples) from the config/sample: diff --git a/cmd/main.go b/cmd/main.go index a04983ec..e90b1a10 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -64,6 +64,7 @@ func main() { var probeAddr string var secureMetrics bool var enableHTTP2 bool + var etcdCPURequest string var tlsOpts []func(*tls.Config) flag.StringVar(&imageRegistry, "image-registry", "gcr.io/etcd-development/etcd", "The container registry to pull etcd images from. Defaults to gcr.io/etcd-development/etcd.") @@ -77,6 +78,11 @@ func main() { "If set, the metrics endpoint is served securely via HTTPS. Use --metrics-secure=false to use HTTP instead.") flag.BoolVar(&enableHTTP2, "enable-http2", false, "If set, HTTP/2 will be enabled for the metrics and webhook servers") + flag.StringVar(&etcdCPURequest, "etcd-cpu-request", controller.DefaultEtcdCPURequest, + "CPU request set on the etcd container. A request (not a limit) lifts the etcd pod from "+ + "BestEffort to Burstable QoS and raises its cpu.shares floor without ever throttling "+ + "etcd. Set to \"\" or \"0\" to apply no request (original BestEffort behavior) so the "+ + "effect can be A/B-measured. Defaults to "+controller.DefaultEtcdCPURequest+".") opts := zap.Options{ Development: true, } @@ -85,6 +91,11 @@ func main() { ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + // Apply the etcd CPU request knob to the controller package. This is an + // operator-wide tuning lever (identical for every cluster), so it is a flag + // rather than a CRD field. See controller.EtcdCPURequest. + controller.EtcdCPURequest = etcdCPURequest + // if the enable-http2 flag is false (the default), http/2 should be disabled // due to its vulnerabilities. More specifically, disabling http/2 will // prevent from being vulnerable to the HTTP/2 Stream Cancellation and diff --git a/docs/operator-flags.md b/docs/operator-flags.md new file mode 100644 index 00000000..731382cb --- /dev/null +++ b/docs/operator-flags.md @@ -0,0 +1,42 @@ +# Operator Flags + +These flags configure the etcd-operator **manager process** itself (the +controller), as opposed to per-cluster settings expressed on the `EtcdCluster` +custom resource. They are passed as command-line arguments to the operator +binary (see `cmd/main.go`). + +## `--etcd-cpu-request` + +- **Type:** string (Kubernetes resource quantity) +- **Default:** `50m` + +CPU **request** (never a limit) set on the etcd container of every managed +cluster. + +Without any request, the etcd pod lands in the **BestEffort** QoS class. That +gives its cgroup the kernel-floor `cpu.shares` of `2` and makes it the first +workload the kubelet evicts under node memory pressure. Setting even a tiny CPU +request lifts the pod to **Burstable**, raises `cpu.shares` to ~`51` (a +scheduling floor that only matters under contention), and — because it is a +request and not a limit — **never throttles** etcd. + +`50m` is deliberately tiny: it is a scheduling floor, not a reservation. It is +expressed as a controller-level flag rather than a CRD field because it is an +operator tuning lever that is identical for every cluster, which keeps it out of +the `EtcdCluster` API. + +Set the value to an empty string (`""`) or `"0"` to apply **no** request, +restoring the original BestEffort behavior. This makes the effect easy to +A/B-measure and tune per fleet. A malformed quantity is treated the same as +unset (no request) so a typo can never wedge cluster creation. + +```sh +# Default: 50m request, Burstable QoS. +manager --etcd-cpu-request=50m + +# Opt out: no request, BestEffort QoS (original behavior). +manager --etcd-cpu-request="" + +# Larger floor for busy clusters. +manager --etcd-cpu-request=200m +``` diff --git a/internal/controller/utils.go b/internal/controller/utils.go index 3092c379..7c4ef075 100644 --- a/internal/controller/utils.go +++ b/internal/controller/utils.go @@ -37,8 +37,27 @@ import ( const ( etcdDataDir = "/var/lib/etcd" volumeName = "etcd-data" + + // DefaultEtcdCPURequest is the default CPU *request* applied to the etcd + // container. With no request the etcd pod lands in the BestEffort QoS class, + // which gives its cgroup the kernel-floor cpu.shares of 2 and makes it the + // first thing the kubelet evicts under node pressure. A request lifts the pod + // to Burstable, raises cpu.shares to ~51 (a scheduling floor that only bites + // under contention), and — because it is a request, not a limit — never + // throttles etcd. 50m is deliberately tiny: it is a floor, not a reservation. + DefaultEtcdCPURequest = "50m" ) +// EtcdCPURequest is the CPU request applied to the etcd container, settable from +// the operator's --etcd-cpu-request flag. Defaults to DefaultEtcdCPURequest. +// +// An empty string or "0" disables the request entirely, restoring the original +// BestEffort behavior so the effect can be A/B-measured and tuned per fleet. +// This is intentionally a controller-level knob rather than a CRD field: it is +// an operator tuning lever, identical for every cluster, so keeping it out of +// the API avoids a CRD change. +var EtcdCPURequest = DefaultEtcdCPURequest + type etcdClusterState string const ( @@ -46,6 +65,29 @@ const ( etcdClusterStateExisting etcdClusterState = "existing" ) +// etcdContainerResources builds the ResourceRequirements for the etcd container. +// +// When EtcdCPURequest is a non-empty, non-zero quantity it sets a CPU *request* +// (never a limit) so the pod is Burstable rather than BestEffort. An empty or +// "0" value yields zero-valued requirements, preserving the original BestEffort +// behavior. A malformed quantity is treated the same as unset so a bad flag can +// never wedge cluster creation. +func etcdContainerResources() corev1.ResourceRequirements { + if EtcdCPURequest == "" || EtcdCPURequest == "0" { + return corev1.ResourceRequirements{} + } + qty, err := resource.ParseQuantity(EtcdCPURequest) + if err != nil || qty.IsZero() { + log.Printf("invalid --etcd-cpu-request %q, leaving etcd container without a CPU request: %v", EtcdCPURequest, err) + return corev1.ResourceRequirements{} + } + return corev1.ResourceRequirements{ + Requests: corev1.ResourceList{ + corev1.ResourceCPU: qty, + }, + } +} + func reconcileStatefulSet(ctx context.Context, logger logr.Logger, ec *ecv1alpha1.EtcdCluster, c client.Client, replicas int32, scheme *runtime.Scheme) (*appsv1.StatefulSet, error) { // prepare/update configmap for StatefulSet @@ -143,10 +185,11 @@ func createOrPatchStatefulSet(ctx context.Context, logger logr.Logger, ec *ecv1a podSpec := corev1.PodSpec{ Containers: []corev1.Container{ { - Name: "etcd", - Command: []string{"/usr/local/bin/etcd"}, - Args: createArgs(ec.Name, ec.Spec.EtcdOptions), - Image: fmt.Sprintf("%s:%s", ec.Spec.ImageRegistry, ec.Spec.Version), + Name: "etcd", + Command: []string{"/usr/local/bin/etcd"}, + Args: createArgs(ec.Name, ec.Spec.EtcdOptions), + Image: fmt.Sprintf("%s:%s", ec.Spec.ImageRegistry, ec.Spec.Version), + Resources: etcdContainerResources(), Env: []corev1.EnvVar{ { Name: "POD_NAME", diff --git a/internal/controller/utils_test.go b/internal/controller/utils_test.go index 67592316..87e71552 100644 --- a/internal/controller/utils_test.go +++ b/internal/controller/utils_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/require" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -1090,3 +1091,104 @@ func TestCreateCMCertificateConfig(t *testing.T) { }) } } + +// getEtcdContainer returns the etcd container from a StatefulSet, or fails the test. +func getEtcdContainer(t *testing.T, sts *appsv1.StatefulSet) corev1.Container { + t.Helper() + for _, c := range sts.Spec.Template.Spec.Containers { + if c.Name == "etcd" { + return c + } + } + t.Fatalf("no etcd container found in StatefulSet %q", sts.Name) + return corev1.Container{} +} + +func TestEtcdContainerCPURequest(t *testing.T) { + ctx := t.Context() + logger := log.FromContext(ctx) + + scheme := runtime.NewScheme() + require.NoError(t, corev1.AddToScheme(scheme)) + require.NoError(t, ecv1alpha1.AddToScheme(scheme)) + require.NoError(t, appsv1.AddToScheme(scheme)) + + // Preserve and restore the package-level knob so this test does not leak + // state into other tests in the package. + orig := EtcdCPURequest + defer func() { EtcdCPURequest = orig }() + + tests := []struct { + name string + cpuRequest string + wantRequest bool + wantQty string + }{ + { + name: "default 50m sets a CPU request (Burstable)", + cpuRequest: DefaultEtcdCPURequest, + wantRequest: true, + wantQty: "50m", + }, + { + name: "explicit override is honored", + cpuRequest: "100m", + wantRequest: true, + wantQty: "100m", + }, + { + name: "empty string disables the request (BestEffort)", + cpuRequest: "", + wantRequest: false, + }, + { + name: "zero disables the request (BestEffort)", + cpuRequest: "0", + wantRequest: false, + }, + { + name: "malformed quantity is treated as unset (BestEffort)", + cpuRequest: "not-a-quantity", + wantRequest: false, + }, + } + + for i, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + EtcdCPURequest = tt.cpuRequest + + fakeClient := fake.NewClientBuilder().WithScheme(scheme).Build() + name := fmt.Sprintf("test-etcd-cpu-%d", i) + ec := &ecv1alpha1.EtcdCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: name, + Namespace: "default", + }, + Spec: ecv1alpha1.EtcdClusterSpec{ + Size: 3, + Version: "3.5.17", + }, + } + + err := createOrPatchStatefulSet(ctx, logger, ec, fakeClient, 3, scheme) + require.NoError(t, err) + + sts := &appsv1.StatefulSet{} + require.NoError(t, fakeClient.Get(ctx, client.ObjectKey{Name: name, Namespace: "default"}, sts)) + + c := getEtcdContainer(t, sts) + cpu, hasCPU := c.Resources.Requests[corev1.ResourceCPU] + + if tt.wantRequest { + require.True(t, hasCPU, "expected etcd container to carry a CPU request") + assert.True(t, cpu.Equal(resource.MustParse(tt.wantQty)), + "expected CPU request %s, got %s", tt.wantQty, cpu.String()) + // It is a request, never a limit: etcd must not be throttled. + _, hasLimit := c.Resources.Limits[corev1.ResourceCPU] + assert.False(t, hasLimit, "etcd container must not carry a CPU limit") + } else { + assert.False(t, hasCPU, "expected no CPU request (BestEffort), got %s", cpu.String()) + } + }) + } +}