From f243c8877978cf3d328c536b36cdf0ef5ca08a37 Mon Sep 17 00:00:00 2001 From: Steffen Butzer Date: Fri, 25 Jan 2019 00:34:42 +0100 Subject: [PATCH 1/3] windows/service: implement graceful shutdown when run as windows service - Fixes https://github.com/kubernetes/kubernetes/issues/72900 The issue here originally is that os.Exit() is called which exits the process too early (before svc.Execute updates the status to stopped). This is picked up as service error and leads to restarting, if restart-on-fail is configured for the windows service. svc.Execute already guarantees that the application is exited after, so that os.Exit call would be unnecessary. This rework also adds graceful shutdown, which also resolves the underlying root cause. The graceful shutdown is not guaranteed to succeed, since the service controller can decide to kill the service any time after exceeding a shutdown timeout. --- pkg/windows/service/service.go | 19 ++++++++++++++----- .../src/k8s.io/apiserver/pkg/server/signal.go | 16 ++++++++++++---- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/pkg/windows/service/service.go b/pkg/windows/service/service.go index a5bffa1822e1..28b531ae77c4 100644 --- a/pkg/windows/service/service.go +++ b/pkg/windows/service/service.go @@ -19,8 +19,7 @@ limitations under the License. package service import ( - "os" - + "k8s.io/apiserver/pkg/server" "k8s.io/klog" "golang.org/x/sys/windows" @@ -80,9 +79,19 @@ Loop: case svc.Interrogate: s <- c.CurrentStatus case svc.Stop, svc.Shutdown: - s <- svc.Status{State: svc.Stopped} - // TODO: Stop the kubelet gracefully instead of killing the process - os.Exit(0) + klog.Infof("Service stopping") + // We need to translate this request into a signal that can be handled by the the signal handler + // handling shutdowns normally (currently apiserver/pkg/server/signal.go). + // If we do not do this, our main threads won't be notified of the upcoming shutdown. + // Since Windows services do not use any console, we cannot simply generate a CTRL_BREAK_EVENT + // but need a dedicated notification mechanism. + server.RequestShutdown() + + // Free up the control handler and let us terminate as gracefully as possible. + // If that takes too long, the service controller will kill the remaining threads. + // As per https://docs.microsoft.com/en-us/windows/desktop/services/service-control-handler-function + s <- svc.Status{State: svc.StopPending} + break Loop } } } diff --git a/staging/src/k8s.io/apiserver/pkg/server/signal.go b/staging/src/k8s.io/apiserver/pkg/server/signal.go index 6f0cff4baed8..6f3edbba470c 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/signal.go +++ b/staging/src/k8s.io/apiserver/pkg/server/signal.go @@ -22,6 +22,7 @@ import ( ) var onlyOneSignalHandler = make(chan struct{}) +var shutdownHandler = make(chan os.Signal, 2) // SetupSignalHandler registered for SIGTERM and SIGINT. A stop channel is returned // which is closed on one of these signals. If a second signal is caught, the program @@ -30,14 +31,21 @@ func SetupSignalHandler() <-chan struct{} { close(onlyOneSignalHandler) // panics when called twice stop := make(chan struct{}) - c := make(chan os.Signal, 2) - signal.Notify(c, shutdownSignals...) + signal.Notify(shutdownHandler, shutdownSignals...) go func() { - <-c + <-shutdownHandler close(stop) - <-c + <-shutdownHandler os.Exit(1) // second signal. Exit directly. }() return stop } + +// RequestShutdown emulates a received event that is considered as shutdown signal (SIGTERM/SIGINT) +func RequestShutdown() { + select { + case shutdownHandler <- shutdownSignals[0]: + default: + } +} From afdfe8d558c17a4e3ea4f6c7f066897b47bbf989 Mon Sep 17 00:00:00 2001 From: Steffen Butzer Date: Fri, 1 Feb 2019 19:16:11 +0100 Subject: [PATCH 2/3] windows/svc: workaround-exit mechanism that works for signal-less binaries --- pkg/windows/service/service.go | 17 ++++++++++++++++- .../src/k8s.io/apiserver/pkg/server/signal.go | 18 +++++++++++++----- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/pkg/windows/service/service.go b/pkg/windows/service/service.go index 28b531ae77c4..47961b842ef3 100644 --- a/pkg/windows/service/service.go +++ b/pkg/windows/service/service.go @@ -19,6 +19,9 @@ limitations under the License. package service import ( + "os" + "time" + "k8s.io/apiserver/pkg/server" "k8s.io/klog" @@ -85,12 +88,24 @@ Loop: // If we do not do this, our main threads won't be notified of the upcoming shutdown. // Since Windows services do not use any console, we cannot simply generate a CTRL_BREAK_EVENT // but need a dedicated notification mechanism. - server.RequestShutdown() + graceful := server.RequestShutdown() // Free up the control handler and let us terminate as gracefully as possible. // If that takes too long, the service controller will kill the remaining threads. // As per https://docs.microsoft.com/en-us/windows/desktop/services/service-control-handler-function s <- svc.Status{State: svc.StopPending} + + // If we cannot exit gracefully, we really only can exit our process, so atleast the + // service manager will think that we gracefully exited. At the time of writing this comment this is + // needed for applications that do not use signals (e.g. kube-proxy) + if !graceful { + go func() { + // Ensure the SCM was notified (The operation above (send to s) was received and communicated to the + // service control manager - so it doesn't look like the service crashes) + time.Sleep(1 * time.Second) + os.Exit(0) + }() + } break Loop } } diff --git a/staging/src/k8s.io/apiserver/pkg/server/signal.go b/staging/src/k8s.io/apiserver/pkg/server/signal.go index 6f3edbba470c..0ea19d6606d0 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/signal.go +++ b/staging/src/k8s.io/apiserver/pkg/server/signal.go @@ -22,7 +22,7 @@ import ( ) var onlyOneSignalHandler = make(chan struct{}) -var shutdownHandler = make(chan os.Signal, 2) +var shutdownHandler chan os.Signal // SetupSignalHandler registered for SIGTERM and SIGINT. A stop channel is returned // which is closed on one of these signals. If a second signal is caught, the program @@ -30,6 +30,8 @@ var shutdownHandler = make(chan os.Signal, 2) func SetupSignalHandler() <-chan struct{} { close(onlyOneSignalHandler) // panics when called twice + shutdownHandler = make(chan os.Signal, 2) + stop := make(chan struct{}) signal.Notify(shutdownHandler, shutdownSignals...) go func() { @@ -43,9 +45,15 @@ func SetupSignalHandler() <-chan struct{} { } // RequestShutdown emulates a received event that is considered as shutdown signal (SIGTERM/SIGINT) -func RequestShutdown() { - select { - case shutdownHandler <- shutdownSignals[0]: - default: +// This returns whether a handler was notified +func RequestShutdown() bool { + if shutdownHandler != nil { + select { + case shutdownHandler <- shutdownSignals[0]: + return true + default: + } } + + return false } From c2b771d5c46ca3b2f58f72595fe07a4e56a63762 Mon Sep 17 00:00:00 2001 From: Steffen Butzer Date: Thu, 7 Feb 2019 16:58:37 +0100 Subject: [PATCH 3/3] windows/svc: address failing test by updating bazel BUILD --- pkg/windows/service/BUILD | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/windows/service/BUILD b/pkg/windows/service/BUILD index e64c40488b43..e518a527080b 100644 --- a/pkg/windows/service/BUILD +++ b/pkg/windows/service/BUILD @@ -11,6 +11,7 @@ go_library( importpath = "k8s.io/kubernetes/pkg/windows/service", deps = select({ "@io_bazel_rules_go//go/platform:windows": [ + "//staging/src/k8s.io/apiserver/pkg/server:go_default_library", "//vendor/golang.org/x/sys/windows:go_default_library", "//vendor/golang.org/x/sys/windows/svc:go_default_library", "//vendor/k8s.io/klog:go_default_library",