diff --git a/cmd/cainjector/app/BUILD.bazel b/cmd/cainjector/app/BUILD.bazel index 3cbf75a7c..0d69d73f5 100644 --- a/cmd/cainjector/app/BUILD.bazel +++ b/cmd/cainjector/app/BUILD.bazel @@ -11,6 +11,7 @@ go_library( "//pkg/controller/cainjector:go_default_library", "//pkg/logs:go_default_library", "//pkg/util:go_default_library", + "//pkg/util/profiling:go_default_library", "@com_github_go_logr_logr//:go_default_library", "@com_github_spf13_cobra//:go_default_library", "@com_github_spf13_pflag//:go_default_library", diff --git a/cmd/cainjector/app/start.go b/cmd/cainjector/app/start.go index 23b8770b2..c5c7c2075 100644 --- a/cmd/cainjector/app/start.go +++ b/cmd/cainjector/app/start.go @@ -20,6 +20,8 @@ import ( "context" "fmt" "io" + "net" + "net/http" "time" "github.com/go-logr/logr" @@ -34,6 +36,7 @@ import ( "github.com/jetstack/cert-manager/pkg/controller/cainjector" logf "github.com/jetstack/cert-manager/pkg/logs" "github.com/jetstack/cert-manager/pkg/util" + "github.com/jetstack/cert-manager/pkg/util/profiling" ) type InjectorControllerOptions struct { @@ -47,6 +50,12 @@ type InjectorControllerOptions struct { StdOut io.Writer StdErr io.Writer + // EnablePprof determines whether Go profiler should be run. + EnablePprof bool + // PprofAddr is the address at which Go profiler will be run if enabled. + // The profiler should never be exposed on a public address. + PprofAddr string + // logger to be used by this controller log logr.Logger } @@ -74,6 +83,9 @@ func (o *InjectorControllerOptions) AddFlags(fs *pflag.FlagSet) { fs.DurationVar(&o.RetryPeriod, "leader-election-retry-period", cmdutil.DefaultLeaderElectionRetryPeriod, ""+ "The duration the clients should wait between attempting acquisition and renewal "+ "of a leadership. This is only applicable if leader election is enabled.") + + fs.BoolVar(&o.EnablePprof, "enable-profiling", cmdutil.DefaultEnableProfiling, "Enable Go profiler (pprof) should be run.") + fs.StringVar(&o.PprofAddr, "profiler-address", cmdutil.DefaultProfilerAddr, "Address of the Go profiler (pprof) if enabled. This should never be exposed on a public interface.") } func NewInjectorControllerOptions(out, errOut io.Writer) *InjectorControllerOptions { @@ -134,6 +146,39 @@ func (o InjectorControllerOptions) RunInjectorController(ctx context.Context) er g, gctx := errgroup.WithContext(ctx) + // if a PprofAddr is provided, start the pprof listener + if o.EnablePprof { + pprofListener, err := net.Listen("tcp", o.PprofAddr) + if err != nil { + return err + } + + profilerMux := http.NewServeMux() + // Add pprof endpoints to this mux + profiling.Install(profilerMux) + o.log.V(logf.InfoLevel).Info("running go profiler on", "address", o.PprofAddr) + server := &http.Server{ + Handler: profilerMux, + } + g.Go(func() error { + <-gctx.Done() + // allow a timeout for graceful shutdown + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + if err := server.Shutdown(ctx); err != nil { + return err + } + return nil + }) + g.Go(func() error { + if err := server.Serve(pprofListener); err != http.ErrServerClosed { + return err + } + return nil + }) + } + g.Go(func() (err error) { defer func() { o.log.Error(err, "manager goroutine exited") diff --git a/cmd/controller/app/BUILD.bazel b/cmd/controller/app/BUILD.bazel index 3c9f3b986..d376127a1 100644 --- a/cmd/controller/app/BUILD.bazel +++ b/cmd/controller/app/BUILD.bazel @@ -34,6 +34,7 @@ go_library( "//pkg/metrics:go_default_library", "//pkg/util:go_default_library", "//pkg/util/feature:go_default_library", + "//pkg/util/profiling:go_default_library", "@com_github_spf13_cobra//:go_default_library", "@io_k8s_api//core/v1:go_default_library", "@io_k8s_apimachinery//pkg/api/errors:go_default_library", diff --git a/cmd/controller/app/controller.go b/cmd/controller/app/controller.go index 4b1d97205..31145ba6b 100644 --- a/cmd/controller/app/controller.go +++ b/cmd/controller/app/controller.go @@ -59,6 +59,7 @@ import ( "github.com/jetstack/cert-manager/pkg/metrics" "github.com/jetstack/cert-manager/pkg/util" utilfeature "github.com/jetstack/cert-manager/pkg/util/feature" + "github.com/jetstack/cert-manager/pkg/util/profiling" ) const controllerAgentName = "cert-manager" @@ -84,11 +85,12 @@ func Run(opts *options.ControllerOptions, stopCh <-chan struct{}) error { enabledControllers := opts.EnabledControllers() log.Info(fmt.Sprintf("enabled controllers: %s", enabledControllers.List())) - ln, err := net.Listen("tcp", opts.MetricsListenAddress) + // Start metrics server + metricsLn, err := net.Listen("tcp", opts.MetricsListenAddress) if err != nil { return fmt.Errorf("failed to listen on prometheus address %s: %v", opts.MetricsListenAddress, err) } - server := ctx.Metrics.NewServer(ln, opts.EnablePprof) + metricsServer := ctx.Metrics.NewServer(metricsLn) g.Go(func() error { <-rootCtx.Done() @@ -96,19 +98,52 @@ func Run(opts *options.ControllerOptions, stopCh <-chan struct{}) error { ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - if err := server.Shutdown(ctx); err != nil { + if err := metricsServer.Shutdown(ctx); err != nil { return err } return nil }) g.Go(func() error { - log.V(logf.InfoLevel).Info("starting metrics server", "address", ln.Addr()) - if err := server.Serve(ln); err != http.ErrServerClosed { + log.V(logf.InfoLevel).Info("starting metrics server", "address", metricsLn.Addr()) + if err := metricsServer.Serve(metricsLn); err != http.ErrServerClosed { return err } return nil }) + // Start profiler if it is enabled + if opts.EnablePprof { + profilerLn, err := net.Listen("tcp", opts.PprofAddress) + if err != nil { + return fmt.Errorf("failed to listen on profiler address %s: %v", opts.PprofAddress, err) + } + profilerMux := http.NewServeMux() + // Add pprof endpoints to this mux + profiling.Install(profilerMux) + profilerServer := &http.Server{ + Handler: profilerMux, + } + + g.Go(func() error { + <-rootCtx.Done() + // allow a timeout for graceful shutdown + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + if err := profilerServer.Shutdown(ctx); err != nil { + return err + } + return nil + }) + g.Go(func() error { + log.V(logf.InfoLevel).Info("starting profiler", "address", profilerLn.Addr()) + if err := profilerServer.Serve(profilerLn); err != http.ErrServerClosed { + return err + } + return nil + }) + } + elected := make(chan struct{}) if opts.LeaderElect { g.Go(func() error { diff --git a/cmd/controller/app/options/options.go b/cmd/controller/app/options/options.go index d777130bc..e233c6413 100644 --- a/cmd/controller/app/options/options.go +++ b/cmd/controller/app/options/options.go @@ -102,8 +102,10 @@ type ControllerOptions struct { // The host and port address, separated by a ':', that the Prometheus server // should expose metrics on. MetricsListenAddress string - // EnablePprof controls whether net/http/pprof handlers are registered with - // the HTTP listener. + // PprofAddress is the address on which Go profiler will run. Should be + // in form :. + PprofAddress string + // EnablePprof determines whether pprof should be enabled. EnablePprof bool DNS01CheckRetryPeriod time.Duration @@ -235,7 +237,8 @@ func NewControllerOptions() *ControllerOptions { EnableCertificateOwnerRef: defaultEnableCertificateOwnerRef, MetricsListenAddress: defaultPrometheusMetricsServerAddress, DNS01CheckRetryPeriod: defaultDNS01CheckRetryPeriod, - EnablePprof: false, + EnablePprof: cmdutil.DefaultEnableProfiling, + PprofAddress: cmdutil.DefaultProfilerAddr, } } @@ -340,8 +343,10 @@ func (s *ControllerOptions) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&s.MetricsListenAddress, "metrics-listen-address", defaultPrometheusMetricsServerAddress, ""+ "The host and port that the metrics endpoint should listen on.") - fs.BoolVar(&s.EnablePprof, "enable-profiling", false, ""+ + fs.BoolVar(&s.EnablePprof, "enable-profiling", cmdutil.DefaultEnableProfiling, ""+ "Enable profiling for controller.") + fs.StringVar(&s.PprofAddress, "profiler-address", cmdutil.DefaultProfilerAddr, + "The host and port that Go profiler should listen on, i.e localhost:6060. Ensure that profiler is not exposed on a public address. Profiler will be served at /debug/pprof.") } func (o *ControllerOptions) Validate() error { diff --git a/cmd/util/defaults.go b/cmd/util/defaults.go index 6bb2c40dc..d3d0bf60e 100644 --- a/cmd/util/defaults.go +++ b/cmd/util/defaults.go @@ -26,4 +26,7 @@ const ( DefaultLeaderElectionLeaseDuration = 60 * time.Second DefaultLeaderElectionRenewDeadline = 40 * time.Second DefaultLeaderElectionRetryPeriod = 15 * time.Second + + DefaultEnableProfiling = false + DefaultProfilerAddr = "localhost:6060" ) diff --git a/cmd/webhook/app/options/BUILD.bazel b/cmd/webhook/app/options/BUILD.bazel index 168810fad..1d01de9a4 100644 --- a/cmd/webhook/app/options/BUILD.bazel +++ b/cmd/webhook/app/options/BUILD.bazel @@ -6,6 +6,7 @@ go_library( importpath = "github.com/jetstack/cert-manager/cmd/webhook/app/options", visibility = ["//visibility:public"], deps = [ + "//cmd/util:go_default_library", "@com_github_spf13_pflag//:go_default_library", "@io_k8s_component_base//cli/flag:go_default_library", ], diff --git a/cmd/webhook/app/options/options.go b/cmd/webhook/app/options/options.go index 9693daed0..f94efa15a 100644 --- a/cmd/webhook/app/options/options.go +++ b/cmd/webhook/app/options/options.go @@ -21,6 +21,15 @@ import ( "github.com/spf13/pflag" cliflag "k8s.io/component-base/cli/flag" + + cmdutil "github.com/jetstack/cert-manager/cmd/util" +) + +const ( + // Default port on which /validate, /mutate, /convert endpoints will be served + defaultListeningPort = 6443 + // Default health check port + defaultHealthPort = 6080 ) type WebhookOptions struct { @@ -56,12 +65,19 @@ type WebhookOptions struct { // MinTLSVersion is the minimum TLS version supported. // Values are from tls package constants (https://golang.org/pkg/crypto/tls/#pkg-constants). MinTLSVersion string + + // EnablePprof determines whether pprof is enabled. + EnablePprof bool + + // Address on which /debug/pprof endpoint will be served if enabled. Default is + // localhost:6060. + PprofAddress string } func (o *WebhookOptions) AddFlags(fs *pflag.FlagSet) { // TODO: rename secure-port to listen-port - fs.IntVar(&o.ListenPort, "secure-port", 6443, "port number to listen on for secure TLS connections") - fs.IntVar(&o.HealthzPort, "healthz-port", 6080, "port number to listen on for insecure healthz connections") + fs.IntVar(&o.ListenPort, "secure-port", defaultListeningPort, "port number to listen on for secure TLS connections") + fs.IntVar(&o.HealthzPort, "healthz-port", defaultHealthPort, "port number to listen on for insecure healthz connections") fs.StringVar(&o.TLSCertFile, "tls-cert-file", "", "path to the file containing the TLS certificate to serve with") fs.StringVar(&o.TLSKeyFile, "tls-private-key-file", "", "path to the file containing the TLS private key to serve with") fs.StringVar(&o.DynamicServingCASecretNamespace, "dynamic-serving-ca-secret-namespace", "", "namespace of the secret used to store the CA that signs serving certificates") @@ -71,7 +87,10 @@ func (o *WebhookOptions) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&o.APIServerHost, "api-server-host", "", ""+ "Optional apiserver host address to connect to. If not specified, autoconfiguration "+ "will be attempted.") - + fs.BoolVar(&o.EnablePprof, "enable-profiling", cmdutil.DefaultEnableProfiling, ""+ + "Enable profiling for controller.") + fs.StringVar(&o.PprofAddress, "profiler-address", cmdutil.DefaultProfilerAddr, + "Address of the Go profiler (pprof). This should never be exposed on a public interface. If this flag is not set, the profiler is not run.") tlsCipherPossibleValues := cliflag.TLSCipherPossibleValues() fs.StringSliceVar(&o.TLSCipherSuites, "tls-cipher-suites", o.TLSCipherSuites, "Comma-separated list of cipher suites for the server. "+ @@ -81,6 +100,7 @@ func (o *WebhookOptions) AddFlags(fs *pflag.FlagSet) { fs.StringVar(&o.MinTLSVersion, "tls-min-version", o.MinTLSVersion, "Minimum TLS version supported. "+ "Possible values: "+strings.Join(tlsPossibleVersions, ", ")) + } func FileTLSSourceEnabled(o WebhookOptions) bool { diff --git a/cmd/webhook/app/webhook.go b/cmd/webhook/app/webhook.go index 71936070e..bf2c56791 100644 --- a/cmd/webhook/app/webhook.go +++ b/cmd/webhook/app/webhook.go @@ -85,7 +85,8 @@ func NewServerWithOptions(log logr.Logger, opts options.WebhookOptions) (*server return &server.Server{ ListenAddr: fmt.Sprintf(":%d", opts.ListenPort), HealthzAddr: fmt.Sprintf(":%d", opts.HealthzPort), - EnablePprof: true, + PprofAddr: opts.PprofAddress, + EnablePprof: opts.EnablePprof, CertificateSource: source, CipherSuites: opts.TLSCipherSuites, MinTLSVersion: opts.MinTLSVersion, diff --git a/pkg/metrics/BUILD.bazel b/pkg/metrics/BUILD.bazel index a0865caf3..7873d47cd 100644 --- a/pkg/metrics/BUILD.bazel +++ b/pkg/metrics/BUILD.bazel @@ -13,7 +13,6 @@ go_library( "//pkg/apis/certmanager/v1:go_default_library", "//pkg/apis/meta/v1:go_default_library", "//pkg/logs:go_default_library", - "//pkg/util/profiling:go_default_library", "@com_github_go_logr_logr//:go_default_library", "@com_github_prometheus_client_golang//prometheus:go_default_library", "@com_github_prometheus_client_golang//prometheus/promhttp:go_default_library", diff --git a/pkg/metrics/metrics.go b/pkg/metrics/metrics.go index 651bac69e..c51ed4117 100644 --- a/pkg/metrics/metrics.go +++ b/pkg/metrics/metrics.go @@ -37,7 +37,6 @@ import ( "github.com/prometheus/client_golang/prometheus/promhttp" cmmeta "github.com/jetstack/cert-manager/pkg/apis/meta/v1" - "github.com/jetstack/cert-manager/pkg/util/profiling" ) const ( @@ -158,7 +157,7 @@ func New(log logr.Logger, c clock.Clock) *Metrics { } // Start will register the Prometheus metrics, and start the Prometheus server -func (m *Metrics) NewServer(ln net.Listener, enablePprof bool) *http.Server { +func (m *Metrics) NewServer(ln net.Listener) *http.Server { m.registry.MustRegister(m.clockTimeSeconds) m.registry.MustRegister(m.certificateExpiryTimeSeconds) m.registry.MustRegister(m.certificateRenewalTimeSeconds) @@ -169,9 +168,6 @@ func (m *Metrics) NewServer(ln net.Listener, enablePprof bool) *http.Server { mux := http.NewServeMux() mux.Handle("/metrics", promhttp.HandlerFor(m.registry, promhttp.HandlerOpts{})) - if enablePprof { - profiling.Install(mux) - } server := &http.Server{ Addr: ln.Addr().String(), diff --git a/pkg/webhook/server/server.go b/pkg/webhook/server/server.go index 9e2b63b7c..d1eb20c77 100644 --- a/pkg/webhook/server/server.go +++ b/pkg/webhook/server/server.go @@ -84,8 +84,9 @@ type Server struct { // If not specified, the healthz endpoint will not be exposed. HealthzAddr string - // EnablePprof controls whether net/http/pprof handlers are registered with - // the HTTP listener. + // PprofAddr is the address the pprof endpoint should be served on if enabled. + PprofAddr string + // EnablePprof determines whether pprof is enabled. EnablePprof bool // Scheme is used to decode/encode request/response payloads. @@ -134,12 +135,12 @@ func (s *Server) Run(stopCh <-chan struct{}) error { return err } - mux := http.NewServeMux() - mux.HandleFunc("/healthz", s.handleHealthz) - mux.HandleFunc("/livez", s.handleLivez) + healthMux := http.NewServeMux() + healthMux.HandleFunc("/healthz", s.handleHealthz) + healthMux.HandleFunc("/livez", s.handleLivez) s.Log.V(logf.InfoLevel).Info("listening for insecure healthz connections", "address", s.HealthzAddr) server := &http.Server{ - Handler: mux, + Handler: healthMux, } g.Go(func() error { <-gctx.Done() @@ -160,6 +161,39 @@ func (s *Server) Run(stopCh <-chan struct{}) error { }) } + // if a PprofAddr is provided, start the pprof listener + if s.EnablePprof { + pprofListener, err := net.Listen("tcp", s.PprofAddr) + if err != nil { + return err + } + + profilerMux := http.NewServeMux() + // Add pprof endpoints to this mux + profiling.Install(profilerMux) + s.Log.V(logf.InfoLevel).Info("running go profiler on", "address", s.PprofAddr) + server := &http.Server{ + Handler: profilerMux, + } + g.Go(func() error { + <-gctx.Done() + // allow a timeout for graceful shutdown + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + if err := server.Shutdown(ctx); err != nil { + return err + } + return nil + }) + g.Go(func() error { + if err := server.Serve(pprofListener); err != http.ErrServerClosed { + return err + } + return nil + }) + } + // create a listener for actual webhook requests listener, err := net.Listen("tcp", s.ListenAddr) if err != nil { @@ -194,16 +228,12 @@ func (s *Server) Run(stopCh <-chan struct{}) error { } s.listener = listener - mux := http.NewServeMux() - mux.HandleFunc("/validate", s.handle(s.validate)) - mux.HandleFunc("/mutate", s.handle(s.mutate)) - mux.HandleFunc("/convert", s.handle(s.convert)) - if s.EnablePprof { - profiling.Install(mux) - s.Log.V(logf.InfoLevel).Info("registered pprof handlers") - } + serverMux := http.NewServeMux() + serverMux.HandleFunc("/validate", s.handle(s.validate)) + serverMux.HandleFunc("/mutate", s.handle(s.mutate)) + serverMux.HandleFunc("/convert", s.handle(s.convert)) server := &http.Server{ - Handler: mux, + Handler: serverMux, } g.Go(func() error { <-gctx.Done() diff --git a/test/integration/certificates/metrics_controller_test.go b/test/integration/certificates/metrics_controller_test.go index 4d1d64194..782117eee 100644 --- a/test/integration/certificates/metrics_controller_test.go +++ b/test/integration/certificates/metrics_controller_test.go @@ -68,7 +68,7 @@ func TestMetricsController(t *testing.T) { if err != nil { t.Fatal(err) } - server := metricsHandler.NewServer(ln, false) + server := metricsHandler.NewServer(ln) doneCh := make(chan struct{}) go func() {