Catch SIGTERM or SIGINT and send offline message #13858

jessicamack · 2023-04-13T19:24:29Z

SUMMARY

Handle SIGTERM and SIGINT in the hearbeet daemon. Relates to #13320

ISSUE TYPE

Bug, Docs Fix or other nominal change

COMPONENT NAME

API

AWX VERSION

awx: 21.14.1

ADDITIONAL INFORMATION

fosterseth · 2023-04-13T20:31:07Z

awx/main/management/commands/run_heartbeet.py

@@ -56,6 +57,8 @@ def do_hearbeat_loop(self):
                logger.debug('Sending heartbeat')
                conn.notify('web_heartbeet', self.construct_payload())
                time.sleep(settings.BROADCAST_WEBSOCKET_BEACON_FROM_WEB_RATE_SECONDS)
+                signal.signal(signal.SIGTERM, conn.notify('web_heartbeet', self.construct_payload(action='offline')))


the signal only needs to be called once I think. if so we may move this to before the while True loop

do we need something in the loop to check that the signal had been received and break out of the loop too?

yeah you're right we need to do a sys.exit
we can follow the pattern here https://github.com/fosterseth/awx/blob/e42461d96ffe3979c8d5c4b9e978cee0809fef35/awx/main/scheduler/task_manager.py#L121

the signal only needs to be called once I think. if so we may move this to before the while True loop

my understanding is that if we want to catch both we need to call signal on both similar to how we do in base.py https://github.com/ansible/awx/blob/devel/awx/main/dispatch/worker/base.py#L129-L130

@jessicamack you're right we need both for SIGTERM and SIGINT

what I meant is that I don't think we need to put them into the while True block

AlanCoding · 2023-04-14T13:38:23Z

I tried to test this locally. Steps:

run docker-compose
in 2nd tab, exec into the awx_devel container, awx-manage run_heartbeeet, leave it
in 3rd tab, exec in again, ps aux --forest and find process, kill 1234

When I do this, in the 2nd tab, I find this:

(awx) bash-5.1$ awx-manage run_heartbeet

   **********   **********
 ************* *************
*****************************
 ***********HEART***********
  *************************
     *******************
       *************** _._
         *********** /`._ `'.      __
           *******   \  .\| \   _'`  `)
             ***  (``_)  \| ).'` /`- /
              *   `\ `;\_ `\\//`-'` /
                    \ `'.'.| /  __/`
                     `'--v_|/`'`
                         __||-._
                      /'` `-``  `'\\
                     /         .'` )
                     \  BEET  '    )
                      \.          /
                        '.     /'`
                           `) |
                            //
                            '(.
                             `\`.
                               ``
Traceback (most recent call last):
  File "/usr/local/bin/awx-manage", line 9, in <module>
    load_entry_point('awx', 'console_scripts', 'awx-manage')()
  File "/awx_devel/awx/__init__.py", line 200, in manage
    execute_from_command_line(sys.argv)
  File "/var/lib/awx/venv/awx/lib64/python3.9/site-packages/django/core/management/__init__.py", line 419, in execute_from_command_line
    utility.execute()
  File "/var/lib/awx/venv/awx/lib64/python3.9/site-packages/django/core/management/__init__.py", line 413, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/var/lib/awx/venv/awx/lib64/python3.9/site-packages/django/core/management/base.py", line 354, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/var/lib/awx/venv/awx/lib64/python3.9/site-packages/django/core/management/base.py", line 398, in execute
    output = self.handle(*args, **options)
  File "/awx_devel/awx/main/management/commands/run_heartbeet.py", line 70, in handle
    self.do_hearbeat_loop()
  File "/awx_devel/awx/main/management/commands/run_heartbeet.py", line 60, in do_hearbeat_loop
    signal.signal(signal.SIGTERM, conn.notify('web_heartbeet', self.construct_payload(action='offline')))
  File "/usr/lib64/python3.9/signal.py", line 56, in signal
    handler = _signal.signal(_enum_to_int(signalnum), _enum_to_int(handler))
TypeError: signal handler must be signal.SIG_IGN, signal.SIG_DFL, or a callable object

Before your branch, it printed "Terminated" and exited, which is desirable.

AlanCoding · 2023-04-20T01:12:12Z

you might have a problem with an outdated black version for the api-lint check failure. Just a guess.

relrod · 2023-04-24T01:59:17Z

awx/main/management/commands/run_heartbeet.py

+            signal.signal(signal.SIGTERM, self.notify_listener_and_exit)
+            signal.signal(signal.SIGINT, self.notify_listener_and_exit)


It might make more sense to register these in handle() instead.

And we can drop the TODO above handle() then.

signal catches have been moved to handle() and TODO has been removed

awx/main/management/commands/run_heartbeet.py

relrod · 2023-04-25T22:50:29Z

awx/main/management/commands/run_heartbeet.py

+    def notify_listener_and_exit(self, *args):
+        with pg_bus_conn(new_connection=False) as conn:
+            conn.notify('web_heartbeet', self.construct_payload(action='offline'))
+        sys.exit(1)


This is the normal way to exit the program, so we probably should exit with 0 instead.

relrod

One more inline comment. I think after that fix, this is good to go.

TheRealHaoLiu · 2023-04-26T13:55:54Z

related to #13322

Signed-off-by: jessicamack <jmack@redhat.com>

This fixes two different exceptions in wsrelay. - One resulted from heartbeet getting ability in ansible#13858 to gracefully shut down. When we saw the message come through, we didn't fully clean up the connection to the web node. - The second resulted when Redis disappeared. We still want to exit in that case, but it's better to log a message and exit gracefully instead of crashing out. Signed-off-by: Rick Elrod <rick@elrod.me>

This fixes two different exceptions in wsrelay. * One resulted from heartbeet getting ability in ansible#13858 to gracefully shut down. When we saw the message come through, we didn't fully clean up the connection to the web node. * The second resulted when Redis disappeared. We still want to exit in that case, but it's better to log a message and exit gracefully instead of crashing out. Signed-off-by: Rick Elrod <rick@elrod.me>

This fixes two different exceptions in wsrelay. * One resulted from heartbeet getting ability in #13858 to gracefully shut down. When we saw the message come through, we didn't fully clean up the connection to the web node. * The second resulted when Redis disappeared. We still want to exit in that case, but it's better to log a message and exit gracefully instead of crashing out. Signed-off-by: Rick Elrod <rick@elrod.me>

Catch SIGTERM or SIGINT and send offline message

jessicamack requested a review from TheRealHaoLiu April 13, 2023 19:24

jessicamack marked this pull request as draft April 13, 2023 19:24

github-actions bot added the component:api label Apr 13, 2023

fosterseth reviewed Apr 13, 2023

View reviewed changes

jessicamack requested review from AlanCoding and fosterseth April 19, 2023 20:37

jessicamack force-pushed the 13322-catch-sigterm branch from ea26861 to 24fce59 Compare April 19, 2023 20:38

jessicamack marked this pull request as ready for review April 19, 2023 20:38

relrod reviewed Apr 24, 2023

View reviewed changes

awx/main/management/commands/run_heartbeet.py Show resolved Hide resolved

jessicamack requested a review from relrod April 24, 2023 14:52

relrod reviewed Apr 25, 2023

View reviewed changes

relrod approved these changes Apr 25, 2023

View reviewed changes

jessicamack added 7 commits April 26, 2023 11:37

catch SIGTERM or SIGINT and send offline message

919d1e5

Signed-off-by: jessicamack <jmack@redhat.com>

fix signal handler. black reformats

3fe64ad

Signed-off-by: jessicamack <jmack@redhat.com>

updated black and ran again to fix lint formatting

d739a4a

Signed-off-by: jessicamack <jmack@redhat.com>

removed TODO. moved signal catches to handle()

397d58c

Signed-off-by: jessicamack <jmack@redhat.com>

move with block inside of while to free up persistent db connection

3025ef0

Signed-off-by: jessicamack <jmack@redhat.com>

reverse previous commit to break into separate PR

6ea3b20

Signed-off-by: jessicamack <jmack@redhat.com>

change exit code

d282393

Signed-off-by: jessicamack <jmack@redhat.com>

jessicamack force-pushed the 13322-catch-sigterm branch from 7799c7d to d282393 Compare April 26, 2023 15:38

jessicamack merged commit 66a3cb6 into ansible:devel Apr 26, 2023
14 checks passed

relrod mentioned this pull request Apr 27, 2023

wsrelay: handle heartbeet shutdown and redis drop #13923

Closed

relrod mentioned this pull request May 11, 2023

[wsrelay] Handle heartbeet shutdown and redis drop #13991

Merged

AlanCoding pushed a commit to AlanCoding/awx that referenced this pull request Jun 5, 2023

Merge pull request ansible#13858 from jessicamack/13322-catch-sigterm

f2574dc

Catch SIGTERM or SIGINT and send offline message

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Catch SIGTERM or SIGINT and send offline message #13858

Catch SIGTERM or SIGINT and send offline message #13858

jessicamack commented Apr 13, 2023

fosterseth Apr 13, 2023

AlanCoding Apr 14, 2023

fosterseth Apr 14, 2023 •

edited

jessicamack Apr 14, 2023

fosterseth Apr 17, 2023

AlanCoding commented Apr 14, 2023

AlanCoding commented Apr 20, 2023

relrod Apr 24, 2023

relrod Apr 24, 2023

jessicamack Apr 24, 2023

relrod Apr 25, 2023 •

edited

relrod left a comment

TheRealHaoLiu commented Apr 26, 2023

		signal.signal(signal.SIGTERM, self.notify_listener_and_exit)
		signal.signal(signal.SIGINT, self.notify_listener_and_exit)

Catch SIGTERM or SIGINT and send offline message #13858

Catch SIGTERM or SIGINT and send offline message #13858

Conversation

jessicamack commented Apr 13, 2023

SUMMARY

ISSUE TYPE

COMPONENT NAME

AWX VERSION

ADDITIONAL INFORMATION

fosterseth Apr 13, 2023

Choose a reason for hiding this comment

AlanCoding Apr 14, 2023

Choose a reason for hiding this comment

fosterseth Apr 14, 2023 • edited

Choose a reason for hiding this comment

jessicamack Apr 14, 2023

Choose a reason for hiding this comment

fosterseth Apr 17, 2023

Choose a reason for hiding this comment

AlanCoding commented Apr 14, 2023

AlanCoding commented Apr 20, 2023

relrod Apr 24, 2023

Choose a reason for hiding this comment

relrod Apr 24, 2023

Choose a reason for hiding this comment

jessicamack Apr 24, 2023

Choose a reason for hiding this comment

relrod Apr 25, 2023 • edited

Choose a reason for hiding this comment

relrod left a comment

Choose a reason for hiding this comment

TheRealHaoLiu commented Apr 26, 2023

fosterseth Apr 14, 2023 •

edited

relrod Apr 25, 2023 •

edited