Skip to content

Commit 77b08a8

Browse files
committed
export manager.Server to provide an official HTTP server manager runnable
Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
1 parent 2a553d6 commit 77b08a8

File tree

4 files changed

+79
-19
lines changed

4 files changed

+79
-19
lines changed

pkg/manager/internal.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,7 @@ func (cm *controllerManager) addHealthProbeServer() error {
284284
mux.Handle(cm.livenessEndpointName+"/", http.StripPrefix(cm.livenessEndpointName, cm.healthzHandler))
285285
}
286286

287-
return cm.add(&server{
287+
return cm.add(&Server{
288288
Kind: "health probe",
289289
Log: cm.logger,
290290
Server: srv,
@@ -302,7 +302,7 @@ func (cm *controllerManager) addPprofServer() error {
302302
mux.HandleFunc("/debug/pprof/symbol", pprof.Symbol)
303303
mux.HandleFunc("/debug/pprof/trace", pprof.Trace)
304304

305-
return cm.add(&server{
305+
return cm.add(&Server{
306306
Kind: "pprof",
307307
Log: cm.logger,
308308
Server: srv,
@@ -384,10 +384,10 @@ func (cm *controllerManager) Start(ctx context.Context) (err error) {
384384
}
385385
}
386386

387-
// First start any internal HTTP servers, which includes health probes, metrics and profiling if enabled.
387+
// First start any HTTP servers, which includes health probes and profiling, if enabled.
388388
//
389-
// WARNING: Internal HTTP servers MUST start before any cache is populated, otherwise it would block
390-
// conversion webhooks to be ready for serving which make the cache never get ready.
389+
// WARNING: HTTPServers includes the health probes, which MUST start before any cache is populated, otherwise
390+
// it would block conversion webhooks to be ready for serving which make the cache never get ready.
391391
if err := cm.runnables.HTTPServers.Start(cm.internalCtx); err != nil {
392392
if err != nil {
393393
return fmt.Errorf("failed to start HTTP servers: %w", err)

pkg/manager/runnable_group.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,10 @@ func newRunnables(baseContext BaseContextFunc, errChan chan error) *runnables {
5454
// The runnables added after Start are started directly.
5555
func (r *runnables) Add(fn Runnable) error {
5656
switch runnable := fn.(type) {
57-
case *server:
57+
case *Server:
58+
if runnable.NeedLeaderElection() {
59+
return r.LeaderElection.Add(fn, nil)
60+
}
5861
return r.HTTPServers.Add(fn, nil)
5962
case hasCache:
6063
return r.Caches.Add(fn, func(ctx context.Context) bool {

pkg/manager/runnable_group_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
. "github.com/onsi/ginkgo/v2"
1111
. "github.com/onsi/gomega"
1212
"k8s.io/utils/pointer"
13+
1314
"sigs.k8s.io/controller-runtime/pkg/cache/informertest"
1415
"sigs.k8s.io/controller-runtime/pkg/webhook"
1516
)
@@ -22,7 +23,7 @@ var _ = Describe("runnables", func() {
2223
})
2324

2425
It("should add HTTP servers to the appropriate group", func() {
25-
server := &server{}
26+
server := &Server{}
2627
r := newRunnables(defaultBaseContext, errCh)
2728
Expect(r.Add(server)).To(Succeed())
2829
Expect(r.HTTPServers.startQueue).To(HaveLen(1))

pkg/manager/server.go

Lines changed: 68 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,41 +21,97 @@ import (
2121
"errors"
2222
"net"
2323
"net/http"
24+
"time"
2425

2526
"github.com/go-logr/logr"
27+
28+
crlog "sigs.k8s.io/controller-runtime/pkg/log"
29+
)
30+
31+
var (
32+
_ Runnable = (*Server)(nil)
33+
_ LeaderElectionRunnable = (*Server)(nil)
2634
)
2735

28-
// server is a general purpose HTTP server Runnable for a manager
29-
// to serve some internal handlers such as health probes, metrics and profiling.
30-
type server struct {
31-
Kind string
32-
Log logr.Logger
33-
Server *http.Server
36+
// Server is a general purpose HTTP server Runnable for a manager.
37+
// It is used to serve some internal handlers for health probes and profiling,
38+
// but it can also be used to run custom servers.
39+
type Server struct {
40+
// Kind is an optional string that describes the purpose of the server. It is used in logs to distinguish
41+
// among multiple servers.
42+
Kind string
43+
44+
// Log is the logger used by the server. If not set, a logger will be derived from the context passed to Start.
45+
Log logr.Logger
46+
47+
// Server is the HTTP server to run. It is required.
48+
Server *http.Server
49+
50+
// Listener is an optional listener to use. If not set, the server start a listener using the server.Addr.
51+
// Using a listener is useful when the port reservation needs to happen in advance of this runnable starting.
3452
Listener net.Listener
53+
54+
// OnlyServeWhenLeader is an optional bool that indicates that the server should only be started when the manager is the leader.
55+
OnlyServeWhenLeader bool
56+
57+
// ShutdownTimeout is an optional duration that indicates how long to wait for the server to shutdown gracefully. If not set,
58+
// the server will wait indefinitely for all connections to close.
59+
ShutdownTimeout *time.Duration
3560
}
3661

37-
func (s *server) Start(ctx context.Context) error {
38-
log := s.Log.WithValues("kind", s.Kind, "addr", s.Listener.Addr())
62+
// Start starts the server. It will block until the server is stopped or an error occurs.
63+
func (s *Server) Start(ctx context.Context) error {
64+
log := s.Log
65+
if log.GetSink() == nil {
66+
log = crlog.FromContext(ctx)
67+
}
68+
if s.Kind != "" {
69+
log = log.WithValues("kind", s.Kind)
70+
}
71+
log = log.WithValues("addr", s.addr())
3972

4073
serverShutdown := make(chan struct{})
4174
go func() {
4275
<-ctx.Done()
4376
log.Info("shutting down server")
44-
if err := s.Server.Shutdown(context.Background()); err != nil {
77+
78+
shutdownCtx := context.Background()
79+
if s.ShutdownTimeout != nil {
80+
var shutdownCancel context.CancelFunc
81+
shutdownCtx, shutdownCancel = context.WithTimeout(context.Background(), *s.ShutdownTimeout)
82+
defer shutdownCancel()
83+
}
84+
85+
if err := s.Server.Shutdown(shutdownCtx); err != nil {
4586
log.Error(err, "error shutting down server")
4687
}
4788
close(serverShutdown)
4889
}()
4990

5091
log.Info("starting server")
51-
if err := s.Server.Serve(s.Listener); err != nil && !errors.Is(err, http.ErrServerClosed) {
92+
if err := s.serve(); err != nil && !errors.Is(err, http.ErrServerClosed) {
5293
return err
5394
}
5495

5596
<-serverShutdown
5697
return nil
5798
}
5899

59-
func (s *server) NeedLeaderElection() bool {
60-
return false
100+
// NeedLeaderElection returns true if the server should only be started when the manager is the leader.
101+
func (s *Server) NeedLeaderElection() bool {
102+
return s.OnlyServeWhenLeader
103+
}
104+
105+
func (s *Server) addr() string {
106+
if s.Listener != nil {
107+
return s.Listener.Addr().String()
108+
}
109+
return s.Server.Addr
110+
}
111+
112+
func (s *Server) serve() error {
113+
if s.Listener != nil {
114+
return s.Server.Serve(s.Listener)
115+
}
116+
return s.Server.ListenAndServe()
61117
}

0 commit comments

Comments
 (0)