From cf50164ac87ae9578a2e0bc8da490ae30006e3b7 Mon Sep 17 00:00:00 2001 From: Brent Graveland Date: Thu, 10 Sep 2026 19:19:25 -0600 Subject: [PATCH] feat(operator): derive GOMEMLIMIT and expose pprof on loopback Go derives GOMAXPROCS from the cgroup CPU quota on its own, but has no equivalent for memory: golang/go#75164 is still an open proposal, so GOMEMLIMIT defaults to math.MaxInt64 and a reconcile burst OOMKills instead of making the collector work harder. Derive it from the limit the downward API projects, using resourceFieldRef so it resolves after any kustomize patch and cannot drift from the limit it is derived from. Defer to an explicit GOMEMLIMIT when one is set. The manifest supplies MEMORY_LIMIT_BYTES unconditionally, so an overlay that lowers GOMEMLIMIT to leave more headroom would otherwise be overwritten with no signal beyond the "resolved runtime limits" line. Add --pprof-bind-address, enabled by default, set to 127.0.0.1:6060 in the shipped manifest. If it's not enabled by default, then it wouldn't be available when we most need it. Binding to localhost means we have to port-forward to access it, so k8s authentication is the guard. The resource values replace untouched kubebuilder scaffold defaults (128Mi / 500m), not measured ones, and stay provisional until those profiles exist. The floor on the memory limit is arithmetic rather than empirical: GOMEMLIMIT excludes the binary's own mapping, ~58MiB stripped, so at 128Mi the 90% handed to the runtime could not fit alongside it. No CPU limit on purpose, since a limit also sets GOMAXPROCS (500m pinned it to 2) and throttling the collector is the wrong way to fail when GOMEMLIMIT is what keeps the process inside its memory limit. Anchor the multigres-operator ignore rule while here: unanchored it matched any path component, which also hid cmd/multigres-operator/. Signed-off-by: Brent Graveland --- .gitignore | 4 +- cmd/multigres-operator/main.go | 69 +++++++++++++++++++++++++++++ cmd/multigres-operator/main_test.go | 60 +++++++++++++++++++++++++ config/manager/manager.yaml | 42 +++++++++++++++--- 4 files changed, 168 insertions(+), 7 deletions(-) create mode 100644 cmd/multigres-operator/main_test.go diff --git a/.gitignore b/.gitignore index e92342bc..b964e5dc 100644 --- a/.gitignore +++ b/.gitignore @@ -17,6 +17,9 @@ cover.out *cover.out cover.html coverage/ +# ad-hoc `go build ./cmd/multigres-operator` drops the binary at the repo root. +# Anchored, because unanchored it also matches (and hides) cmd/multigres-operator/. +/multigres-operator # kubebuilder bin/ @@ -27,7 +30,6 @@ kubeconfig.yaml # MacOS .DS_Store -multigres-operator # Results of Agent runs agent-docs/ diff --git a/cmd/multigres-operator/main.go b/cmd/multigres-operator/main.go index 9e1b906f..4283fcf3 100644 --- a/cmd/multigres-operator/main.go +++ b/cmd/multigres-operator/main.go @@ -24,6 +24,9 @@ import ( "flag" "os" "path/filepath" + goruntime "runtime" + "runtime/debug" + "strconv" // Import all Kubernetes client auth plugins (e.g. Azure, GCP, OIDC, etc.) _ "k8s.io/client-go/plugin/pkg/client/auth" @@ -87,6 +90,7 @@ func main() { var metricsAddr string var enableLeaderElection bool var probeAddr string + var pprofAddr string var secureMetrics bool var enableHTTP2 bool var tlsOpts []func(*tls.Config) @@ -130,6 +134,15 @@ func main() { ":8081", "The address the probe endpoint binds to.", ) + flag.StringVar( + &pprofAddr, + "pprof-bind-address", + "", + "The address the pprof endpoint binds to. Empty disables it. The endpoint is "+ + "unauthenticated, serves this process's argv, and lets any caller consume CPU "+ + "via /debug/pprof/profile, so bind it to loopback and reach it with "+ + "'kubectl port-forward', e.g. '127.0.0.1:6060'.", + ) flag.BoolVar( &enableLeaderElection, "leader-elect", @@ -193,6 +206,41 @@ func main() { ctrl.SetLogger(zap.New(zap.UseFlagOptions(&opts))) + // The Go runtime derives GOMAXPROCS from the cgroup CPU quota on its own + // (go1.25+), but has no equivalent for memory: golang/go#75164 is still an + // open proposal, so GOMEMLIMIT defaults to math.MaxInt64 and a reconcile + // burst OOMKills instead of making the collector work harder. Derive it + // from the limit the downward API projects in manager.yaml. + rawMemLimit := os.Getenv("MEMORY_LIMIT_BYTES") + // Cases in precedence order. An absent MEMORY_LIMIT_BYTES is the ordinary + // case outside a container and needs no comment, since "resolved runtime + // limits" below reports the outcome either way. + switch memLimit, ok := goMemLimitFromEnv(rawMemLimit); { + case os.Getenv("GOMEMLIMIT") != "": + // Leave it alone. The runtime has already applied it, and whoever set it + // chose that headroom on purpose; the manifest supplies + // MEMORY_LIMIT_BYTES unconditionally, so deriving from it here would + // silently overwrite their choice. A malformed GOMEMLIMIT never reaches + // this point: the runtime treats it as fatal before main runs. + case ok: + debug.SetMemoryLimit(memLimit) + case rawMemLimit != "": + // Someone tried to set a limit and silently did not. + setupLog.Info( + "MEMORY_LIMIT_BYTES is not a positive integer; leaving GOMEMLIMIT effectively unlimited", + "value", + rawMemLimit, + ) + } + + // Emit the resolved values rather than the configured ones, so a profile + // captured later arrives with the runtime limits that produced it. + setupLog.Info("resolved runtime limits", + "GOMEMLIMIT", debug.SetMemoryLimit(-1), + "GOMAXPROCS", goruntime.GOMAXPROCS(0), + "numCPU", goruntime.NumCPU(), + ) + if imageUpdateStrategy != string(images.UpdateImmediate) && imageUpdateStrategy != string(images.UpdateLazy) { setupLog.Error( @@ -335,6 +383,7 @@ func main() { Scheme: scheme, Metrics: metricsServerOptions, HealthProbeBindAddress: probeAddr, + PprofBindAddress: pprofAddr, LeaderElection: enableLeaderElection, LeaderElectionID: "multigres-operator.multigres.com", // RELEASE LEADER ON CANCEL: Enables faster failover during rolling upgrades @@ -553,3 +602,23 @@ func certsExist(dir string) bool { _, errKey := os.Stat(filepath.Join(dir, "tls.key")) return !os.IsNotExist(errCrt) && !os.IsNotExist(errKey) } + +// memLimitRatio is the fraction of the container memory limit handed to the Go +// runtime. The remainder has to cover what GOMEMLIMIT explicitly excludes, +// dominated by the ~58MiB mapping of the binary itself, so a limit low enough +// that 10% of it is smaller than the binary cannot be satisfied at all. +const memLimitRatio = 0.9 + +// goMemLimitFromEnv converts a container memory limit in bytes, as projected by +// the downward API, into a GOMEMLIMIT value. It reports false when the value is +// absent or unusable; that is the unlimited case, not an error, since the +// operator must still run outside a container and under manifests predating +// this variable. Note that a container with no memory limit gets the node's +// allocatable memory here rather than nothing, which is the intended reading. +func goMemLimitFromEnv(raw string) (int64, bool) { + limit, err := strconv.ParseInt(raw, 10, 64) + if err != nil || limit <= 0 { + return 0, false + } + return int64(float64(limit) * memLimitRatio), true +} diff --git a/cmd/multigres-operator/main_test.go b/cmd/multigres-operator/main_test.go new file mode 100644 index 00000000..ea56ffb3 --- /dev/null +++ b/cmd/multigres-operator/main_test.go @@ -0,0 +1,60 @@ +package main + +import "testing" + +func TestGoMemLimitFromEnv(t *testing.T) { + tests := map[string]struct { + raw string + want int64 + wantOK bool + }{ + "derives 90% of a 512Mi limit": { + raw: "536870912", + want: 483183820, + wantOK: true, + }, + "unset means unlimited": { + raw: "", + wantOK: false, + }, + "non-numeric means unlimited": { + raw: "512Mi", + wantOK: false, + }, + "zero means unlimited": { + raw: "0", + wantOK: false, + }, + "negative means unlimited": { + raw: "-1", + wantOK: false, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + got, ok := goMemLimitFromEnv(tc.raw) + if ok != tc.wantOK { + t.Fatalf("goMemLimitFromEnv(%q) ok = %v, want %v", tc.raw, ok, tc.wantOK) + } + if got != tc.want { + t.Fatalf("goMemLimitFromEnv(%q) = %d, want %d", tc.raw, got, tc.want) + } + }) + } +} + +// A node's allocatable memory is what the downward API projects when the +// container declares no memory limit, so the conversion has to stay in range +// for values far larger than any limit we would set deliberately. +func TestGoMemLimitFromEnvDoesNotOverflowAtNodeScale(t *testing.T) { + const nodeAllocatable = 512 * 1024 * 1024 * 1024 // 512GiB + + got, ok := goMemLimitFromEnv("549755813888") + if !ok { + t.Fatal("goMemLimitFromEnv() rejected a node-sized limit") + } + if got <= 0 || got >= nodeAllocatable { + t.Fatalf("goMemLimitFromEnv() = %d, want a positive value below %d", got, nodeAllocatable) + } +} diff --git a/config/manager/manager.yaml b/config/manager/manager.yaml index 20fe7f11..32394cb8 100644 --- a/config/manager/manager.yaml +++ b/config/manager/manager.yaml @@ -68,6 +68,16 @@ spec: args: - --leader-elect - --health-probe-bind-address=:8081 + # On by default because it cannot usefully be enabled on demand: + # changing args rolls the Deployment, so the process holding the heap + # under investigation is replaced by a fresh one. A slow leak takes + # days to reappear, and after an OOMKill the evidence is already gone. + # + # Loopback only: the endpoint is unauthenticated, serves this + # process's argv, and lets any caller burn CPU via + # /debug/pprof/profile. Reach it with a port-forward, which the API + # server authenticates and RBAC authorizes. + - --pprof-bind-address=127.0.0.1:6060 env: - name: POD_NAMESPACE valueFrom: @@ -77,6 +87,16 @@ spec: valueFrom: fieldRef: fieldPath: spec.serviceAccountName + # Go derives GOMAXPROCS from the cgroup CPU quota by itself, but has no + # equivalent for memory (golang/go#75164). Project the limit so the + # binary can set GOMEMLIMIT from it; resourceFieldRef resolves after any + # kustomize patch, so this cannot drift from the limit below. + - name: MEMORY_LIMIT_BYTES + valueFrom: + resourceFieldRef: + containerName: manager + resource: limits.memory + divisor: "1" image: controller:latest name: manager ports: [] @@ -98,15 +118,25 @@ spec: port: 8081 initialDelaySeconds: 5 periodSeconds: 10 - # TODO(user): Configure the resources accordingly based on the project requirements. - # More info: https://kubernetes.io/docs/concepts/configuration/manage-resources-containers/ + # Provisional, pending the heap and allocation profiles that + # --pprof-bind-address now makes possible. These replace untouched + # kubebuilder scaffold values (128Mi / 500m), not measured ones. + # + # The floor on the memory limit is arithmetic rather than empirical: + # GOMEMLIMIT excludes the binary's own mapping, which is ~58MiB + # stripped, so at 128Mi the 90% the operator hands the runtime could + # not fit alongside it. + # + # No CPU limit on purpose. Five controllers run at + # MaxConcurrentReconciles 20, and throttling the collector is the wrong + # way to fail when GOMEMLIMIT is what keeps the process inside its + # memory limit. A CPU limit also sets GOMAXPROCS: 500m pinned it to 2. resources: limits: - cpu: 500m - memory: 128Mi + memory: 512Mi requests: - cpu: 10m - memory: 64Mi + cpu: 100m + memory: 256Mi volumeMounts: - mountPath: /var/run/secrets/webhook name: cert-dir