Fix non-zero exit code when receiving remote shutdown #8650

lyzlisa · 2023-11-19T00:15:07Z

Description

Possibly fixes #8540.

This issue likely surfaced with #7544. Since WorkerShutdown inherits from SystemExit, providing a non-integer value results in an exit code of 1. This exception is then propagated to

celery/celery/worker/worker.py

Line 209 in bad2750

self.stop(exitcode=exc.code)

which causes a non-zero exit code upon a graceful shutdown request of the worker.

codecov · 2023-11-19T03:53:26Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (8e5efc2) 87.33% compared to head (b44d88b) 87.33%.
Report is 4 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #8650   +/-   ##
=======================================
  Coverage   87.33%   87.33%           
=======================================
  Files         148      148           
  Lines       18515    18515           
  Branches     3163     3163           
=======================================
  Hits        16170    16170           
  Misses       2060     2060           
  Partials      285      285

Flag	Coverage Δ
unittests	`87.30% <100.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Nusnus · 2023-11-19T08:15:05Z

@auvipy check out this PR!
Very interesting catch!

Indeed msg is incorrect as it is being treated as True meaning 1 meaning error.

Nusnus · 2023-11-19T08:16:19Z

Do you want to include it in 5.3.6 @auvipy?
If you agree, I'll take care of reviewing it more thoroughly ASAP.

auvipy · 2023-11-19T08:18:45Z

Im open to include it

Nusnus · 2023-11-19T08:20:45Z

Im open to include it

Awesome!
On it!

Nusnus · 2023-11-19T11:18:13Z

I am working on a new massive version for the pytest-celery plugin (currently at alpha stage, soon in beta!) & smoke tests for increased automatic QA for Celery - @lyzlisa I'll share a glimpse of its capabilities, testing your PR.

Test Case

def test_shutdown_exit_with_zero(self, celery_setup: CeleryTestSetup):
    celery_setup.app.control.shutdown()
    while celery_setup.worker.container.status != "exited":
        celery_setup.worker.container.reload()
    assert celery_setup.worker.container.attrs['State']['ExitCode'] == 0

Running on `main`

tox -e 3.12-smoke -- -k test_shutdown

ROOT: will run in automatically provisioned tox, host /Users/nusnus/.pyenv/versions/3.12.0/envs/celery_py312/bin/python3.12 is missing [requires (has)]: tox-gh-actions
ROOT: provision> .tox/.tox/bin/python -m tox -e 3.12-smoke -- -k test_shutdown
ROOT: tox-gh-actions won't override envlist because tox is not running in GitHub Actions
.pkg: _optional_hooks> python /Users/nusnus/dev/GitHub/celery/.tox/.tox/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_editable> python /Users/nusnus/dev/GitHub/celery/.tox/.tox/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: build_editable> python /Users/nusnus/dev/GitHub/celery/.tox/.tox/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
3.12-smoke: install_package> python -I -m pip install --force-reinstall --no-deps /Users/nusnus/dev/GitHub/celery/.tox/.tmp/package/22/celery-5.3.5-0.editable-py3-none-any.whl
3.12-smoke: commands[0]> pytest -xsv t/smoke -k test_shutdown
======================================================================= test session starts =======================================================================
platform darwin -- Python 3.12.0, pytest-7.4.3, pluggy-1.3.0 -- /Users/nusnus/dev/GitHub/celery/.tox/3.12-smoke/bin/python
cachedir: .tox/3.12-smoke/.pytest_cache
rootdir: /Users/nusnus/dev/GitHub/celery
configfile: pyproject.toml
plugins: docker-tools-3.1.3, timeout-2.2.0, github-actions-annotate-failures-0.2.0, rerunfailures-12.0, celery-1.0.0a1, order-1.1.0, click-1.1.0, subtests-0.11.0, cov-4.1.0, xdist-3.3.1
collected 49 items / 47 deselected / 2 selected

t/smoke/tests/test_control.py::test_control::test_shutdown[celery_setup_worker-celery_redis_broker-celery_redis_backend] Creating network pytest-bf32cae0-58ca-4046-aff8-72af07c58daf
Fetching redis:latest
Waiting for container to be ready.RedisContainer::reverent_bhabha is ready.

Creating network pytest-e16252dc-76c0-4b84-8c2c-b15ce708afb5
Building .......................................................................................................................................................................
Waiting for container to be ready.RedisContainer::beautiful_jemison is ready.

Waiting for container to be ready.RedisContainer::magical_jemison is ready.

Creating volume pytest-1c8a3bcc-027a-41f2-9e81-ecfc91e7d628
Waiting for container to be ready.Waiting for SmokeWorkerContainer::suspicious_hodgkin to get ready...
SmokeWorkerContainer::suspicious_hodgkin is ready.

FAILED

============================================================================ FAILURES =============================================================================
____________________________________ test_control.test_shutdown[celery_setup_worker-celery_redis_broker-celery_redis_backend] _____________________________________

self = <test_control.test_control object at 0x1098c2420>, celery_setup = <pytest_celery.api.setup.CeleryTestSetup object at 0x1098ea870>

    def test_shutdown(self, celery_setup: CeleryTestSetup):
        celery_setup.app.control.shutdown()
        while celery_setup.worker.container.status != "exited":
            celery_setup.worker.container.reload()
>       assert celery_setup.worker.container.attrs['State']['ExitCode'] == 0
E       assert 1 == 0

t/smoke/tests/test_control.py:13: AssertionError
-------------------------------------------------------------- redis_test_container: reverent_bhabha --------------------------------------------------------------
1:C 19 Nov 2023 11:01:32.063 # WARNING Memory overcommit must be enabled! Without it, a background save or replication may fail under low memory condition. Being disabled, it can also cause failures without low memory condition, see https://github.com/jemalloc/jemalloc/issues/1328. To fix this issue add 'vm.overcommit_memory = 1' to /etc/sysctl.conf and then reboot or run the command 'sysctl vm.overcommit_memory=1' for this to take effect.
1:C 19 Nov 2023 11:01:32.064 * oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
1:C 19 Nov 2023 11:01:32.064 * Redis version=7.2.3, bits=64, commit=00000000, modified=0, pid=1, just started
1:C 19 Nov 2023 11:01:32.064 # Warning: no config file specified, using the default config. In order to specify a config file use redis-server /path/to/redis.conf
1:M 19 Nov 2023 11:01:32.064 * monotonic clock: POSIX clock_gettime
1:M 19 Nov 2023 11:01:32.064 * Running mode=standalone, port=6379.
1:M 19 Nov 2023 11:01:32.065 * Server initialized
1:M 19 Nov 2023 11:01:32.065 * Ready to accept connections tcp
------------------------------------------------------------- default_redis_broker: beautiful_jemison -------------------------------------------------------------
1:C 19 Nov 2023 11:01:45.277 # WARNING Memory overcommit must be enabled! Without it, a background save or replication may fail under low memory condition. Being disabled, it can also cause failures without low memory condition, see https://github.com/jemalloc/jemalloc/issues/1328. To fix this issue add 'vm.overcommit_memory = 1' to /etc/sysctl.conf and then reboot or run the command 'sysctl vm.overcommit_memory=1' for this to take effect.
1:C 19 Nov 2023 11:01:45.279 * oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
1:C 19 Nov 2023 11:01:45.279 * Redis version=7.2.3, bits=64, commit=00000000, modified=0, pid=1, just started
1:C 19 Nov 2023 11:01:45.279 # Warning: no config file specified, using the default config. In order to specify a config file use redis-server /path/to/redis.conf
1:M 19 Nov 2023 11:01:45.279 * monotonic clock: POSIX clock_gettime
1:M 19 Nov 2023 11:01:45.279 * Running mode=standalone, port=6379.
1:M 19 Nov 2023 11:01:45.280 * Server initialized
1:M 19 Nov 2023 11:01:45.280 * Ready to accept connections tcp
------------------------------------------------------------- default_redis_backend: magical_jemison --------------------------------------------------------------
1:C 19 Nov 2023 11:01:45.558 # WARNING Memory overcommit must be enabled! Without it, a background save or replication may fail under low memory condition. Being disabled, it can also cause failures without low memory condition, see https://github.com/jemalloc/jemalloc/issues/1328. To fix this issue add 'vm.overcommit_memory = 1' to /etc/sysctl.conf and then reboot or run the command 'sysctl vm.overcommit_memory=1' for this to take effect.
1:C 19 Nov 2023 11:01:45.559 * oO0OoO0OoO0Oo Redis is starting oO0OoO0OoO0Oo
1:C 19 Nov 2023 11:01:45.559 * Redis version=7.2.3, bits=64, commit=00000000, modified=0, pid=1, just started
1:C 19 Nov 2023 11:01:45.559 # Warning: no config file specified, using the default config. In order to specify a config file use redis-server /path/to/redis.conf
1:M 19 Nov 2023 11:01:45.560 * monotonic clock: POSIX clock_gettime
1:M 19 Nov 2023 11:01:45.560 * Running mode=standalone, port=6379.
1:M 19 Nov 2023 11:01:45.560 * Server initialized
1:M 19 Nov 2023 11:01:45.560 * Ready to accept connections tcp
---------------------------------------------------------- default_worker_container: suspicious_hodgkin -----------------------------------------------------------

 -------------- smoke_tests_worker@44272ced703c v5.3.5 (emerald-rush)
--- ***** -----
-- ******* ---- Linux-6.4.16-linuxkit-aarch64-with-glibc2.36 2023-11-19 11:01:46
- *** --- * ---
- ** ---------- [config]
- ** ---------- .> app:         celery_test_app:0xffffb16171d0
- ** ---------- .> transport:   redis://f75391404d30:6379/0
- ** ---------- .> results:     redis://6dd5b5ff1b9f/0
- *** --- * --- .> concurrency: 10 (prefork)
-- ******* ---- .> task events: OFF (enable -E to monitor tasks in this worker)
--- ***** -----
 -------------- [queues]
                .> smoke_tests_queue exchange=smoke_tests_queue(direct) key=smoke_tests_queue


[tasks]
  . pytest_celery.vendors.worker.tasks.ping
  . t.integration.tasks.add
  . t.integration.tasks.add_chord_to_chord
  . t.integration.tasks.add_ignore_result
  . t.integration.tasks.add_not_typed
  . t.integration.tasks.add_replaced
  . t.integration.tasks.add_to_all
  . t.integration.tasks.add_to_all_to_chord
  . t.integration.tasks.build_chain_inside_task
  . t.integration.tasks.chain_add
  . t.integration.tasks.chord_add
  . t.integration.tasks.collect_ids
  . t.integration.tasks.delayed_sum
  . t.integration.tasks.delayed_sum_with_soft_guard
  . t.integration.tasks.errback_new_style
  . t.integration.tasks.errback_old_style
  . t.integration.tasks.fail
  . t.integration.tasks.fail_replaced
  . t.integration.tasks.fail_unpickleable
  . t.integration.tasks.identity
  . t.integration.tasks.ids
  . t.integration.tasks.mul
  . t.integration.tasks.print_unicode
  . t.integration.tasks.raise_error
  . t.integration.tasks.rebuild_signature
  . t.integration.tasks.redis_count
  . t.integration.tasks.redis_echo
  . t.integration.tasks.redis_echo_group_id
  . t.integration.tasks.replace_with_chain
  . t.integration.tasks.replace_with_chain_which_raises
  . t.integration.tasks.replace_with_empty_chain
  . t.integration.tasks.replace_with_stamped_task
  . t.integration.tasks.replaced_with_me
  . t.integration.tasks.retry
  . t.integration.tasks.retry_once
  . t.integration.tasks.retry_once_headers
  . t.integration.tasks.retry_once_priority
  . t.integration.tasks.retry_unpickleable
  . t.integration.tasks.return_exception
  . t.integration.tasks.return_nested_signature_chain_chain
  . t.integration.tasks.return_nested_signature_chain_chord
  . t.integration.tasks.return_nested_signature_chain_group
  . t.integration.tasks.return_nested_signature_chord_chain
  . t.integration.tasks.return_nested_signature_chord_chord
  . t.integration.tasks.return_nested_signature_chord_group
  . t.integration.tasks.return_nested_signature_group_chain
  . t.integration.tasks.return_nested_signature_group_chord
  . t.integration.tasks.return_nested_signature_group_group
  . t.integration.tasks.return_priority
  . t.integration.tasks.return_properties
  . t.integration.tasks.second_order_replace1
  . t.integration.tasks.second_order_replace2
  . t.integration.tasks.sleeping
  . t.integration.tasks.tsum
  . t.integration.tasks.write_to_file_and_return_int
  . t.integration.tasks.xsum
  . t.smoke.tasks.long_running_task
  . t.smoke.tasks.noop
  . t.smoke.tasks.replace_with_task

/celery/celery/worker/consumer/consumer.py:508: CPendingDeprecationWarning: The broker_connection_retry configuration setting will no longer determine
whether broker connection retries are made during startup in Celery 6.0 and above.
If you wish to retain the existing behavior for retrying connections on startup,
you should set broker_connection_retry_on_startup to True.
  warnings.warn(

[2023-11-19 11:01:46,709: WARNING/MainProcess] /celery/celery/worker/consumer/consumer.py:508: CPendingDeprecationWarning: The broker_connection_retry configuration setting will no longer determine
whether broker connection retries are made during startup in Celery 6.0 and above.
If you wish to retain the existing behavior for retrying connections on startup,
you should set broker_connection_retry_on_startup to True.
  warnings.warn(

Connected to redis://f75391404d30:6379/0
[2023-11-19 11:01:46,717: INFO/MainProcess] Connected to redis://f75391404d30:6379/0
/celery/celery/worker/consumer/consumer.py:508: CPendingDeprecationWarning: The broker_connection_retry configuration setting will no longer determine
whether broker connection retries are made during startup in Celery 6.0 and above.
If you wish to retain the existing behavior for retrying connections on startup,
you should set broker_connection_retry_on_startup to True.
  warnings.warn(

[2023-11-19 11:01:46,718: WARNING/MainProcess] /celery/celery/worker/consumer/consumer.py:508: CPendingDeprecationWarning: The broker_connection_retry configuration setting will no longer determine
whether broker connection retries are made during startup in Celery 6.0 and above.
If you wish to retain the existing behavior for retrying connections on startup,
you should set broker_connection_retry_on_startup to True.
  warnings.warn(

[2023-11-19 11:01:46,719: INFO/MainProcess] mingle: searching for neighbors
mingle: searching for neighbors
mingle: all alone
[2023-11-19 11:01:47,724: INFO/MainProcess] mingle: all alone
[2023-11-19 11:01:47,733: INFO/MainProcess] smoke_tests_worker@44272ced703c ready.
smoke_tests_worker@44272ced703c ready.
Got shutdown from remote
[2023-11-19 11:01:48,124: WARNING/MainProcess] Got shutdown from remote
[2023-11-19 11:01:49,135: WARNING/MainProcess] Got shutdown from remote
Got shutdown from remote
===================================================================== short test summary info =====================================================================
FAILED t/smoke/tests/test_control.py::test_control::test_shutdown[celery_setup_worker-celery_redis_broker-celery_redis_backend] - assert 1 == 0
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! stopping after 1 failures !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
================================================================ 1 failed, 47 deselected in 20.47s ================================================================
3.12-smoke: exit 1 (21.63 seconds) /Users/nusnus/dev/GitHub/celery> pytest -xsv t/smoke -k test_shutdown pid=66809
.pkg: _exit> python /Users/nusnus/dev/GitHub/celery/.tox/.tox/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
  3.12-smoke: FAIL code 1 (23.11=setup[1.48]+cmd[21.63] seconds)
  evaluation failed :( (23.42 seconds)

Running on PR

tox -e 3.12-smoke -- -k test_shutdown

ROOT: will run in automatically provisioned tox, host /Users/nusnus/.pyenv/versions/3.12.0/envs/celery_py312/bin/python3.12 is missing [requires (has)]: tox-gh-actions
ROOT: provision> .tox/.tox/bin/python -m tox -e 3.12-smoke -- -k test_shutdown
ROOT: tox-gh-actions won't override envlist because tox is not running in GitHub Actions
.pkg: _optional_hooks> python /Users/nusnus/dev/GitHub/celery/.tox/.tox/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_editable> python /Users/nusnus/dev/GitHub/celery/.tox/.tox/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: build_editable> python /Users/nusnus/dev/GitHub/celery/.tox/.tox/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
3.12-smoke: install_package> python -I -m pip install --force-reinstall --no-deps /Users/nusnus/dev/GitHub/celery/.tox/.tmp/package/23/celery-5.3.5-0.editable-py3-none-any.whl
3.12-smoke: commands[0]> pytest -xsv t/smoke -k test_shutdown
======================================================================= test session starts =======================================================================
platform darwin -- Python 3.12.0, pytest-7.4.3, pluggy-1.3.0 -- /Users/nusnus/dev/GitHub/celery/.tox/3.12-smoke/bin/python
cachedir: .tox/3.12-smoke/.pytest_cache
rootdir: /Users/nusnus/dev/GitHub/celery
configfile: pyproject.toml
plugins: docker-tools-3.1.3, timeout-2.2.0, github-actions-annotate-failures-0.2.0, rerunfailures-12.0, celery-1.0.0a1, order-1.1.0, click-1.1.0, subtests-0.11.0, cov-4.1.0, xdist-3.3.1
collected 49 items / 47 deselected / 2 selected

t/smoke/tests/test_control.py::test_control::test_shutdown[celery_setup_worker-celery_redis_broker-celery_redis_backend] Fetching redis:latest
Creating network pytest-0e247ca2-5ae3-4b6b-97de-d472582465c9
Waiting for container to be ready.RedisContainer::frosty_swanson is ready.

Creating network pytest-d50e1813-86b2-4aa3-8abb-1ea504d04025
Waiting for container to be ready.RedisContainer::flamboyant_liskov is ready.

Waiting for container to be ready.RedisContainer::reverent_beaver is ready.

Creating volume pytest-c5d0bdf5-a86c-4380-a752-aa90506fc0ef
Building ........................................................................................................................................................................
Waiting for container to be ready.Waiting for SmokeWorkerContainer::compassionate_curran to get ready...
SmokeWorkerContainer::compassionate_curran is ready.

PASSED
t/smoke/tests/test_control.py::test_control::test_shutdown[celery_setup_worker-celery_rabbitmq_broker-celery_redis_backend] Creating network pytest-32f0aaeb-f0af-45fb-97aa-b12c44719e4f
Waiting for container to be ready.........RabbitMQContainer::happy_cohen is ready.

Waiting for container to be ready.RedisContainer::romantic_booth is ready.

Creating volume pytest-6ab71c4c-bfa8-4920-bd6a-b2f46a2b6f4a
Waiting for container to be ready.Waiting for SmokeWorkerContainer::nervous_goldberg to get ready...
SmokeWorkerContainer::nervous_goldberg is ready.

PASSED

================================================================ 2 passed, 47 deselected in 34.89s ================================================================
.pkg: _exit> python /Users/nusnus/dev/GitHub/celery/.tox/.tox/lib/python3.12/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
  3.12-smoke: OK (36.86=setup[0.78]+cmd[36.08] seconds)
  congratulations :) (37.09 seconds)```

Nusnus

Very important bugfix !!
APPROVED

auvipy · 2023-11-19T12:22:39Z

thanks a lot! both of you!!!

Fix non-zero exit code when receiving remote shutdown

b44d88b

Nusnus self-requested a review November 19, 2023 08:21

Nusnus approved these changes Nov 19, 2023

View reviewed changes

Nusnus merged commit 709c5e7 into celery:main Nov 19, 2023
33 checks passed

Nusnus mentioned this pull request Nov 19, 2023

Celery v5.3.6 release #8642

Closed

10 tasks

auvipy added this to the 5.3.x milestone Nov 19, 2023

auvipy added the PR Type: Bugfix label Nov 19, 2023

lyzlisa deleted the fix-worker-shutdown branch November 20, 2023 00:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix non-zero exit code when receiving remote shutdown #8650

Fix non-zero exit code when receiving remote shutdown #8650

lyzlisa commented Nov 19, 2023

codecov bot commented Nov 19, 2023 •

edited

Nusnus commented Nov 19, 2023

Nusnus commented Nov 19, 2023

auvipy commented Nov 19, 2023

Nusnus commented Nov 19, 2023

Nusnus commented Nov 19, 2023

Nusnus left a comment

auvipy commented Nov 19, 2023

Fix non-zero exit code when receiving remote shutdown #8650

Fix non-zero exit code when receiving remote shutdown #8650

Conversation

lyzlisa commented Nov 19, 2023

Description

codecov bot commented Nov 19, 2023 • edited

Codecov Report

Nusnus commented Nov 19, 2023

Nusnus commented Nov 19, 2023

auvipy commented Nov 19, 2023

Nusnus commented Nov 19, 2023

Nusnus commented Nov 19, 2023

Test Case

Running on main

Running on PR

Nusnus left a comment

Choose a reason for hiding this comment

auvipy commented Nov 19, 2023

codecov bot commented Nov 19, 2023 •

edited

Running on `main`