Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add support for controller-manager webhook shutdown delay #2601

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Prev Previous commit
Next Next commit
Keep shutdown timeout
  • Loading branch information
dippynark committed Dec 3, 2023
commit 384d8ce13504fefd34a103c71a5b1baad1c75093
13 changes: 4 additions & 9 deletions pkg/webhook/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,20 +250,15 @@ func (s *DefaultServer) Start(ctx context.Context) error {
idleConnsClosed := make(chan struct{})
go func() {
<-ctx.Done()
log.Info("Shutting down webhook server with timeout of 1 minute")
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
defer cancel()
// Disable HTTP keep-alives to close persistent connections after next
// HTTP request. Clients may reconnect until routes are updated and
// server listeners are closed however this should start gradually
// migrating clients to server instances that are not about to shutdown
srv.SetKeepAlivesEnabled(false)
// Wait for the specified shutdown delay or until the shutdown context
// expires, whichever happens first
select {
case <-time.After(s.Options.ShutdownDelay):
case <-ctx.Done():
}
<-time.After(s.Options.ShutdownDelay)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<-time.After(s.Options.ShutdownDelay)
time.Sleep(s.Options.ShutdownDelay)

We don't need to create a channel, I think.

Copy link
Author

@dippynark dippynark Dec 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, fixed

log.Info("Shutting down webhook server with timeout of 1 minute")
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Minute)
defer cancel()
if err := srv.Shutdown(ctx); err != nil {
// Error from closing listeners, or context timeout
log.Error(err, "error shutting down the HTTP server")
Expand Down