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
Open
39 changes: 29 additions & 10 deletions pkg/manager/signals/signal.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,45 @@ import (
"context"
"os"
"os/signal"
"time"
)

var onlyOneSignalHandler = make(chan struct{})
var (
onlyOneSignalHandler = make(chan struct{})
// Define global signal channel for testing
signalCh = make(chan os.Signal, 2)
)

// SetupSignalHandler registers for SIGTERM and SIGINT. A context is returned
// which is canceled on one of these signals. If a second signal is caught, the program
// is terminated with exit code 1.
func SetupSignalHandler() context.Context {
// SetupSignalHandlerWithDelay registers for SIGTERM and SIGINT. A context is
// returned which is canceled on one of these signals after waiting for the
// specified delay. In particular, the delay can be used to give external
// Kubernetes controllers (such as kube-proxy) time to observe the termination
// of this manager before starting shutdown of any webhook servers to avoid
// receiving connection attempts after closing webhook listeners. If a second
// signal is caught, the program is terminated with exit code 1.
func SetupSignalHandlerWithDelay(delay time.Duration) context.Context {
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need this? IMHO this doesn't make sense because if this is needed or not is not a property of the binary, but of the environment. Kubernetes allows to use a preStopHook for this and recently even merged native sleep support for this precise use-case: kubernetes/kubernetes#119026

Copy link
Author

Choose a reason for hiding this comment

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

Ah nice, I wasn't aware of this sleep lifecycle hook feature! I agree with this but until Kubernetes v1.28 (I think the first version where the sleep lifecycle hook is available) the application otherwise needs to ship with a sleep binary so I think it'd still be worth having to deal with the long tail of Kubernetes versions.

However, happy for this to be closed and wait for the sleep hook for a native solution!

close(onlyOneSignalHandler) // panics when called twice

ctx, cancel := context.WithCancel(context.Background())

c := make(chan os.Signal, 2)
signal.Notify(c, shutdownSignals...)
signal.Notify(signalCh, shutdownSignals...)
go func() {
<-c
cancel()
<-c
<-signalCh
// Cancel the context after delaying for the specified duration but
// avoid blocking if a second signal is caught
go func() {
<-time.After(delay)
cancel()
}()
<-signalCh
os.Exit(1) // second signal. Exit directly.
}()

return ctx
}

// SetupSignalHandler is a special case of SetupSignalHandlerWithDelay with no
// delay for backwards compatibility
func SetupSignalHandler() context.Context {
return SetupSignalHandlerWithDelay(time.Duration(0))
}
61 changes: 14 additions & 47 deletions pkg/manager/signals/signal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,7 @@ limitations under the License.
package signals

import (
"fmt"
"os"
"os/signal"
"sync"
"time"

. "github.com/onsi/ginkgo/v2"
Expand All @@ -31,53 +28,23 @@ var _ = Describe("runtime signal", func() {

Context("SignalHandler Test", func() {

It("test signal handler", func() {
ctx := SetupSignalHandler()
task := &Task{
ticker: time.NewTicker(time.Second * 2),
}
c := make(chan os.Signal, 1)
signal.Notify(c, os.Interrupt)
task.wg.Add(1)
go func(c chan os.Signal) {
defer task.wg.Done()
task.Run(c)
}(c)
It("test signal handler with delay", func() {
delay := time.Second
ctx := SetupSignalHandlerWithDelay(delay)

select {
case sig := <-c:
fmt.Printf("Got %s signal. Aborting...\n", sig)
case _, ok := <-ctx.Done():
Expect(ok).To(BeFalse())
}
// Save time before sending signal
beforeSendingSignal := time.Now()

// Send signal
signalCh <- os.Interrupt

_, ok := <-ctx.Done()
// Verify that the channel was closed
Expect(ok).To(BeFalse())
// Verify that our delay was respected
Expect(time.Since(beforeSendingSignal)).To(BeNumerically(">=", delay))
})

})

})

type Task struct {
wg sync.WaitGroup
ticker *time.Ticker
}

func (t *Task) Run(c chan os.Signal) {
for {
go sendSignal(c)
handle()
}
}

func handle() {
for i := 0; i < 5; i++ {
fmt.Print("#")
time.Sleep(time.Millisecond * 100)
}
fmt.Println()
}

func sendSignal(stopChan chan os.Signal) {
fmt.Printf("...")
time.Sleep(1 * time.Second)
stopChan <- os.Interrupt
}