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