From 591d7e624ac1f8e32b31efd4e756abaa81ebfb62 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Sat, 28 May 2022 10:27:50 -0700 Subject: [PATCH] Unify service stop sequence We still had some variations in how services stop. Finger, merger, and scheduler all used signal.pause in a while loop which is incompatible with stopping via the command socket (since we would always restart the pause). Sending these components a stop or graceful signal would cause them to wait forever. Instead of using signal.pause, use the thread.join methods within a while loop, and if we encounter a KeyboardInterrupt (C-c) during the join, call our exit handler and retry the join loop. This maintains the intent of the signal.pause loop (which is to make C-c exit cleanly) while also being compatible with an internal stop issued via the command socket. The stop sequence is now unified across all components. The executor has an additional complication in that it forks a process to handle streaming. To keep a C-c shutdown clean, we also handle a keyboard interrupt in the child process and use it to indicate the start of a shutdown. In the main executor process, we now close the socket which is used to keep the child running and then wait for the child to exit before the main process exits (so that the child doesn't keep running and emit a log line after the parent returns control to the terminal). Change-Id: I216b76d6aaf7ebd01fa8cca843f03fd7a3eea16d --- zuul/cmd/executor.py | 17 +++++++++++++++-- zuul/cmd/fingergw.py | 30 ++++++++++++------------------ zuul/cmd/merger.py | 20 ++++++++------------ zuul/cmd/scheduler.py | 16 ++++++++-------- zuul/lib/fingergw.py | 4 +++- 5 files changed, 46 insertions(+), 41 deletions(-) diff --git a/zuul/cmd/executor.py b/zuul/cmd/executor.py index 3c98ecd4ae..eb463348e1 100755 --- a/zuul/cmd/executor.py +++ b/zuul/cmd/executor.py @@ -67,12 +67,16 @@ class Executor(zuul.cmd.ZuulDaemonApp): # Keep running until the parent dies: pipe_read = os.fdopen(pipe_read) - pipe_read.read() + try: + pipe_read.read() + except KeyboardInterrupt: + pass self.log.info("Stopping log streamer") streamer.stop() os._exit(0) else: os.close(pipe_read) + self.log_streamer_pipe = pipe_write self.log_streamer_pid = child_pid def run(self): @@ -113,7 +117,16 @@ class Executor(zuul.cmd.ZuulDaemonApp): if self.args.nodaemon: signal.signal(signal.SIGTERM, self.exit_handler) - self.executor.join() + while True: + try: + self.executor.join() + break + except KeyboardInterrupt: + print("Ctrl + C: asking executor to exit nicely...\n") + self.exit_handler(signal.SIGINT, None) + + os.close(self.log_streamer_pipe) + os.waitpid(self.log_streamer_pid, 0) def main(): diff --git a/zuul/cmd/fingergw.py b/zuul/cmd/fingergw.py index 04e0d5cc84..2f7dbdb599 100644 --- a/zuul/cmd/fingergw.py +++ b/zuul/cmd/fingergw.py @@ -38,6 +38,10 @@ class FingerGatewayApp(zuul.cmd.ZuulDaemonApp): self.addSubCommands(parser, fingergw.COMMANDS) return parser + def exit_handler(self, signum, frame): + if self.gateway: + self.gateway.stop() + def run(self): ''' Main entry point for the FingerGatewayApp. @@ -59,28 +63,18 @@ class FingerGatewayApp(zuul.cmd.ZuulDaemonApp): self.getPidFile(), ) - self.log.info('Starting Zuul finger gateway app') self.gateway.start() if self.args.nodaemon: - # NOTE(Shrews): When running in non-daemon mode, although sending - # the 'stop' command via the command socket will shutdown the - # gateway, it's still necessary to Ctrl+C to stop the app. - while True: - try: - signal.pause() - except KeyboardInterrupt: - print("Ctrl + C: asking gateway to exit nicely...\n") - self.stop() - break - else: - self.gateway.wait() + signal.signal(signal.SIGTERM, self.exit_handler) - self.log.info('Stopped Zuul finger gateway app') - - def stop(self): - if self.gateway: - self.gateway.stop() + while True: + try: + self.gateway.join() + break + except KeyboardInterrupt: + print("Ctrl + C: asking gateway to exit nicely...\n") + self.exit_handler(signal.SIGINT, None) def main(): diff --git a/zuul/cmd/merger.py b/zuul/cmd/merger.py index 659399c854..f1e0bea88b 100755 --- a/zuul/cmd/merger.py +++ b/zuul/cmd/merger.py @@ -49,19 +49,15 @@ class Merger(zuul.cmd.ZuulDaemonApp): if self.args.nodaemon: signal.signal(signal.SIGTERM, self.exit_handler) - while True: - try: - signal.pause() - except KeyboardInterrupt: - print("Ctrl + C: asking merger to exit nicely...\n") - self.exit_handler(signal.SIGINT, None) - else: - self.merger.join() + + while True: + try: + self.merger.join() + break + except KeyboardInterrupt: + print("Ctrl + C: asking merger to exit nicely...\n") + self.exit_handler(signal.SIGINT, None) def main(): Merger().main() - - -if __name__ == "__main__": - main() diff --git a/zuul/cmd/scheduler.py b/zuul/cmd/scheduler.py index 176466cfe0..3ab93670f6 100755 --- a/zuul/cmd/scheduler.py +++ b/zuul/cmd/scheduler.py @@ -109,14 +109,14 @@ class Scheduler(zuul.cmd.ZuulDaemonApp): if self.args.nodaemon: signal.signal(signal.SIGTERM, self.exit_handler) - while True: - try: - signal.pause() - except KeyboardInterrupt: - print("Ctrl + C: asking scheduler to exit nicely...\n") - self.exit_handler(signal.SIGINT, None) - else: - self.sched.join() + + while True: + try: + self.sched.join() + break + except KeyboardInterrupt: + print("Ctrl + C: asking scheduler to exit nicely...\n") + self.exit_handler(signal.SIGINT, None) def main(): diff --git a/zuul/lib/fingergw.py b/zuul/lib/fingergw.py index 90eb032ea5..ad945c1b7d 100644 --- a/zuul/lib/fingergw.py +++ b/zuul/lib/fingergw.py @@ -217,6 +217,7 @@ class FingerGateway(object): raise def start(self): + self.log.info("Starting finger gateway") kwargs = dict( user=self.user, pid_file=self.pid_file, @@ -260,6 +261,7 @@ class FingerGateway(object): self.log.info("Finger gateway is started") def stop(self): + self.log.info("Stopping finger gateway") self.component_info.state = self.component_info.STOPPED if self.server: @@ -283,7 +285,7 @@ class FingerGateway(object): self.log.info("Finger gateway is stopped") - def wait(self): + def join(self): ''' Wait on the gateway to shutdown. '''