From b1d0852d7661d44642c05d5ac899862eef5e88b5 Mon Sep 17 00:00:00 2001 From: "Per G. da Silva" Date: Wed, 10 Jun 2026 11:36:39 +0200 Subject: [PATCH] NO-ISSUE: Add opt-in pprof endpoint to machine-controller-manager Add an opt-in pprof profiling endpoint to the AWS machine controller manager, gated behind the --enable-pprof flag (default: disabled). The pprof server listens on plaintext HTTP at 127.0.0.1:6060 (loopback only). TLS termination and authentication are expected to be handled by a kube-rbac-proxy sidecar configured in the machine-api-operator deployment, consistent with how other serving endpoints are secured across machine-api controllers. Changes: - Add pprofServer runnable (cmd/manager/pprof.go) that serves net/http/pprof handlers via the manager lifecycle with graceful shutdown on context cancellation - Add --enable-pprof and --pprof-bind-address CLI flags - Unit tests for server response, graceful shutdown, and leader election opt-out Signed-off-by: Per G. da Silva Co-Authored-By: Claude Opus 4.6 (1M context) --- cmd/manager/main.go | 21 ++++++++ cmd/manager/main_test.go | 105 +++++++++++++++++++++++++++++++++++++++ cmd/manager/pprof.go | 44 ++++++++++++++++ 3 files changed, 170 insertions(+) create mode 100644 cmd/manager/main_test.go create mode 100644 cmd/manager/pprof.go diff --git a/cmd/manager/main.go b/cmd/manager/main.go index 18412767f..83e6cc6d9 100644 --- a/cmd/manager/main.go +++ b/cmd/manager/main.go @@ -16,6 +16,7 @@ package main import ( "flag" "fmt" + _ "net/http/pprof" "os" "strings" "time" @@ -97,6 +98,18 @@ func main() { "The address for health checking.", ) + enablePprof := flag.Bool( + "enable-pprof", + false, + "Enable the pprof profiling endpoint.", + ) + + pprofAddr := flag.String( + "pprof-bind-address", + "127.0.0.1:6060", + "The address for serving pprof profiling endpoints (requires --enable-pprof).", + ) + // Sets up feature gates (version from build time, default 4 for unknown) // Default should be changed to 5 once we branch for 5 majorVersion := version.Version.Major @@ -231,6 +244,14 @@ func main() { klog.Fatal(err) } + if *enablePprof { + if err := mgr.Add(&pprofServer{ + Addr: *pprofAddr, + }); err != nil { + klog.Fatalf("Error adding pprof server: %v", err) + } + } + // Start the Cmd err = mgr.Start(ctrl.SetupSignalHandler()) if err != nil { diff --git a/cmd/manager/main_test.go b/cmd/manager/main_test.go new file mode 100644 index 000000000..9d0ae4d72 --- /dev/null +++ b/cmd/manager/main_test.go @@ -0,0 +1,105 @@ +package main + +import ( + "context" + "fmt" + "net" + "net/http" + "testing" + "time" +) + +func TestPprofServerResponds(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + addr := listener.Addr().String() + listener.Close() + + srv := &pprofServer{ + Addr: addr, + } + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + errCh := make(chan error, 1) + go func() { + errCh <- srv.Start(ctx) + }() + + var resp *http.Response + for i := 0; i < 20; i++ { + resp, err = http.Get(fmt.Sprintf("http://%s/debug/pprof/", addr)) + if err == nil { + break + } + time.Sleep(50 * time.Millisecond) + } + if err != nil { + t.Fatalf("pprof server did not respond: %v", err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Errorf("expected status 200, got %d", resp.StatusCode) + } + + cancel() + select { + case err := <-errCh: + if err != nil { + t.Errorf("server returned error on shutdown: %v", err) + } + case <-time.After(10 * time.Second): + t.Fatal("server did not shut down within 10 seconds") + } +} + +func TestPprofServerGracefulShutdown(t *testing.T) { + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("failed to listen: %v", err) + } + addr := listener.Addr().String() + listener.Close() + + srv := &pprofServer{ + Addr: addr, + } + + ctx, cancel := context.WithCancel(context.Background()) + + errCh := make(chan error, 1) + go func() { + errCh <- srv.Start(ctx) + }() + + for i := 0; i < 20; i++ { + resp, getErr := http.Get(fmt.Sprintf("http://%s/debug/pprof/", addr)) + if getErr == nil { + resp.Body.Close() + break + } + time.Sleep(50 * time.Millisecond) + } + + cancel() + + select { + case err := <-errCh: + if err != nil { + t.Errorf("server returned error on shutdown: %v", err) + } + case <-time.After(5 * time.Second): + t.Fatal("server did not shut down within 5 seconds") + } +} + +func TestPprofServerNeedLeaderElection(t *testing.T) { + srv := &pprofServer{} + if srv.NeedLeaderElection() { + t.Error("pprofServer.NeedLeaderElection() returned true, expected false") + } +} diff --git a/cmd/manager/pprof.go b/cmd/manager/pprof.go new file mode 100644 index 000000000..23fd83e02 --- /dev/null +++ b/cmd/manager/pprof.go @@ -0,0 +1,44 @@ +package main + +import ( + "context" + "fmt" + "net/http" + "time" + + "k8s.io/klog/v2" +) + +type pprofServer struct { + Addr string +} + +func (s *pprofServer) Start(ctx context.Context) error { + srv := &http.Server{ + Addr: s.Addr, + Handler: http.DefaultServeMux, + } + klog.Infof("Starting pprof server on %s", s.Addr) + + errCh := make(chan error, 1) + go func() { + if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed { + errCh <- err + } + close(errCh) + }() + + select { + case err := <-errCh: + return fmt.Errorf("pprof server failed: %w", err) + case <-ctx.Done(): + } + + shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + return srv.Shutdown(shutdownCtx) +} + +func (s *pprofServer) NeedLeaderElection() bool { + return false +}