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.

Sign up for GitHub

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

Jump to bottom

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
+13 −0
Open

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

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
9501679
Add support for shutdown delay
dippynark Dec 2, 2023
bf963c8
Add unit test
dippynark Dec 2, 2023
f9393cb
Add ShutdownDelay field
dippynark Dec 3, 2023
12f9864
Replace newline
dippynark Dec 3, 2023
5c3c96e
Do not disable keep-alives
dippynark Dec 3, 2023
4bd1de4
Address comments
dippynark Dec 3, 2023
384d8ce
Keep shutdown timeout
dippynark Dec 3, 2023
2b4c651
Add newline
dippynark Dec 3, 2023
3529110
Improve comments
dippynark Dec 3, 2023
dca33cb
Add back newline
dippynark Dec 3, 2023
9351414
Remove unnecessary channel
dippynark Dec 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add newline
  • Loading branch information
@dippynark
dippynark committed Dec 3, 2023
commit 2b4c651e03ff45e2ceeaf1e5e4033e13ec3cd551
1 change: 1 addition & 0 deletions pkg/webhook/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ func (s *DefaultServer) Start(ctx context.Context) error {
srv.SetKeepAlivesEnabled(false)
<-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 •
edited
Loading

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 {
Expand Down