From 0b3ce9804687977dc3fa46598cca493fe24e7a41 Mon Sep 17 00:00:00 2001 From: Maxim Zhiltsov Date: Fri, 16 Dec 2022 16:27:16 +0300 Subject: [PATCH] Fix the missing env variable issue (#5467) Fixes #5448, #5453, #5457 - Relaxed env var requirements in the images, no mandatory ones - RQ workers now use a custom python class for remote debugging - Factored out common remote debugging implementation --- .vscode/launch.json | 6 +- Dockerfile | 3 +- cvat/rqworker.py | 56 ++++++++++++++++++ cvat/simpleworker.py | 28 --------- cvat/utils/remote_debugger.py | 59 +++++++++++++++++++ cvat/wsgi.py | 41 +++---------- .../en/docs/contributing/running-tests.md | 4 +- supervisord/worker.default.conf | 9 +-- supervisord/worker.low.conf | 9 +-- 9 files changed, 133 insertions(+), 82 deletions(-) create mode 100644 cvat/rqworker.py delete mode 100644 cvat/simpleworker.py create mode 100644 cvat/utils/remote_debugger.py diff --git a/.vscode/launch.json b/.vscode/launch.json index 6fe6a2954d5..19e05d37109 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -144,7 +144,7 @@ "rqworker", "default", "--worker-class", - "cvat.simpleworker.SimpleWorker", + "cvat.rqworker.SimpleWorker", ], "django": true, "cwd": "${workspaceFolder}", @@ -179,7 +179,7 @@ "rqworker", "low", "--worker-class", - "cvat.simpleworker.SimpleWorker", + "cvat.rqworker.SimpleWorker", ], "django": true, "cwd": "${workspaceFolder}", @@ -198,7 +198,7 @@ "rqworker", "webhooks", "--worker-class", - "cvat.simpleworker.SimpleWorker", + "cvat.rqworker.SimpleWorker", ], "django": true, "cwd": "${workspaceFolder}", diff --git a/Dockerfile b/Dockerfile index be138b14fd0..121778ac350 100644 --- a/Dockerfile +++ b/Dockerfile @@ -150,8 +150,7 @@ COPY --from=build-image /opt/ffmpeg /usr # These variables are required for supervisord substitutions in files # This library allows remote python debugging with VS Code -ARG CVAT_DEBUG_ENABLED='no' -ENV CVAT_DEBUG_PORT='' +ARG CVAT_DEBUG_ENABLED RUN if [ "${CVAT_DEBUG_ENABLED}" = 'yes' ]; then \ python3 -m pip install --no-cache-dir debugpy; \ fi diff --git a/cvat/rqworker.py b/cvat/rqworker.py new file mode 100644 index 00000000000..b21a8c8f7bd --- /dev/null +++ b/cvat/rqworker.py @@ -0,0 +1,56 @@ +# Copyright (C) 2018-2022 Intel Corporation +# Copyright (C) 2022 CVAT.ai Corporation +# +# SPDX-License-Identifier: MIT + +from rq import Worker + +import cvat.utils.remote_debugger as debug + + +DefaultWorker = Worker + + +class BaseDeathPenalty: + def __init__(self, timeout, exception, **kwargs): + pass + + def __enter__(self): + pass + + def __exit__(self, exc_type, exc_value, traceback): + pass + + +class SimpleWorker(Worker): + """ + Allows to work with at most 1 worker thread. Useful for debugging. + """ + + death_penalty_class = BaseDeathPenalty + + def main_work_horse(self, *args, **kwargs): + raise NotImplementedError("Test worker does not implement this method") + + def execute_job(self, *args, **kwargs): + """Execute job in same thread/process, do not fork()""" + return self.perform_job(*args, **kwargs) + + +if debug.is_debugging_enabled(): + class RemoteDebugWorker(SimpleWorker): + """ + Support for VS code debugger + """ + + def __init__(self, *args, **kwargs): + self.__debugger = debug.RemoteDebugger() + super().__init__(*args, **kwargs) + + def execute_job(self, *args, **kwargs): + """Execute job in same thread/process, do not fork()""" + self.__debugger.attach_current_thread() + + return super().execute_job(*args, **kwargs) + + DefaultWorker = RemoteDebugWorker diff --git a/cvat/simpleworker.py b/cvat/simpleworker.py deleted file mode 100644 index 139f5836e69..00000000000 --- a/cvat/simpleworker.py +++ /dev/null @@ -1,28 +0,0 @@ - -# Copyright (C) 2018-2022 Intel Corporation -# -# SPDX-License-Identifier: MIT - -from rq import Worker - - -class BaseDeathPenalty: - def __init__(self, timeout, exception, **kwargs): - pass - - def __enter__(self): - pass - - def __exit__(self, exc_type, exc_value, traceback): - pass - - -class SimpleWorker(Worker): - death_penalty_class = BaseDeathPenalty - - def main_work_horse(self, *args, **kwargs): - raise NotImplementedError("Test worker does not implement this method") - - def execute_job(self, *args, **kwargs): - """Execute job in same thread/process, do not fork()""" - return self.perform_job(*args, **kwargs) diff --git a/cvat/utils/remote_debugger.py b/cvat/utils/remote_debugger.py new file mode 100644 index 00000000000..b4d01baf3c3 --- /dev/null +++ b/cvat/utils/remote_debugger.py @@ -0,0 +1,59 @@ +# Copyright (C) 2022 CVAT.ai Corporation +# +# SPDX-License-Identifier: MIT + +import os + + +def is_debugging_enabled() -> bool: + return os.environ.get('CVAT_DEBUG_ENABLED') == 'yes' + +if is_debugging_enabled(): + import debugpy + + class RemoteDebugger: + """ + Support for VS code debugger. + + Supports both single- and multi-thread scenarios. + + Read docs: https://github.com/microsoft/debugpy + Read more: https://modwsgi.readthedocs.io/en/develop/user-guides/debugging-techniques.html + """ + + ENV_VAR_PORT = 'CVAT_DEBUG_PORT' + ENV_VAR_WAIT = 'CVAT_DEBUG_WAIT' + __debugger_initialized = False + + @classmethod + def _singleton_init(cls): + if cls.__debugger_initialized: + return + + try: + port = int(os.environ[cls.ENV_VAR_PORT]) + + # The only intended use is in Docker. + # Using 127.0.0.1 will not allow host connections + addr = ('0.0.0.0', port) # nosec - B104:hardcoded_bind_all_interfaces + + # Debugpy is a singleton + # We put it in the main thread of the process and then report new threads + debugpy.listen(addr) + + # In most cases it makes no sense to debug subprocesses + # Feel free to enable if needed. + debugpy.configure({"subProcess": False}) + + if os.environ.get(cls.ENV_VAR_WAIT) == 'yes': + debugpy.wait_for_client() + except Exception as ex: + raise Exception("failed to set debugger") from ex + + cls.__debugger_initialized = True + + def __init__(self) -> None: + self._singleton_init() + + def attach_current_thread(self) -> None: + debugpy.debug_this_thread() diff --git a/cvat/wsgi.py b/cvat/wsgi.py index b41c346073b..54dd33aa260 100644 --- a/cvat/wsgi.py +++ b/cvat/wsgi.py @@ -14,55 +14,30 @@ """ import os - from django.core.wsgi import get_wsgi_application +import cvat.utils.remote_debugger as debug + + os.environ.setdefault("DJANGO_SETTINGS_MODULE", "cvat.settings.{}" \ .format(os.environ.get("DJANGO_CONFIGURATION", "development"))) application = get_wsgi_application() -if os.environ.get('CVAT_DEBUG_ENABLED') == 'yes': - import debugpy - - class Debugger: +if debug.is_debugging_enabled(): + class DebuggerApp: """ Support for VS code debugger - - Read docs: https://github.com/microsoft/debugpy - Read more: https://modwsgi.readthedocs.io/en/develop/user-guides/debugging-techniques.html """ - ENV_VAR_PORT = 'CVAT_DEBUG_PORT' - ENV_VAR_WAIT = 'CVAT_DEBUG_WAIT' - def __init__(self, obj): self.__object = obj - - port = int(os.environ[self.ENV_VAR_PORT]) - - # The only intended use is in Docker. - # Using 127.0.0.1 will not allow host connections - addr = ('0.0.0.0', port) # nosec - B104:hardcoded_bind_all_interfaces - - try: - # Debugpy is a singleton - # We put it in the main thread of the process and then report new threads - debugpy.listen(addr) - - # In most cases it makes no sense to debug subprocesses - # Feel free to enable if needed. - debugpy.configure({"subProcess": False}) - - if os.environ.get(self.ENV_VAR_WAIT) == 'yes': - debugpy.wait_for_client() - except Exception as ex: - print("failed to set debugger:", ex) + self.__debugger = debug.RemoteDebugger() def __call__(self, *args, **kwargs): - debugpy.debug_this_thread() + self.__debugger.attach_current_thread() return self.__object(*args, **kwargs) - application = Debugger(application) + application = DebuggerApp(application) diff --git a/site/content/en/docs/contributing/running-tests.md b/site/content/en/docs/contributing/running-tests.md index ac7bd1402a6..7c870fbb6d9 100644 --- a/site/content/en/docs/contributing/running-tests.md +++ b/site/content/en/docs/contributing/running-tests.md @@ -76,8 +76,8 @@ pytest ./tests/python --rebuild **Debugging** -Currently, this is only supported in docker-compose deployments, which should be -enough to fix errors arising in REST API tests. +Currently, this is only supported in `docker-compose`-based deployments, +which should be enough to fix errors arising in REST API tests. To debug a server deployed with Docker, you need to do the following: diff --git a/supervisord/worker.default.conf b/supervisord/worker.default.conf index 4f3bf7e89ca..e5a8f58c0d5 100644 --- a/supervisord/worker.default.conf +++ b/supervisord/worker.default.conf @@ -24,13 +24,8 @@ autorestart=true [program:rqworker_default] command=%(ENV_HOME)s/wait-for-it.sh %(ENV_CVAT_REDIS_HOST)s:6379 -t 0 -- bash -ic " \ - if [ \"%(ENV_CVAT_DEBUG_ENABLED)s\" = 'yes' ]; then \ - _DEBUG_CMD=\"-m debugpy --listen 0.0.0.0:%(ENV_CVAT_DEBUG_PORT)s\"; - else - _DEBUG_CMD=\"\"; - fi && \ - \ - exec python3 ${_DEBUG_CMD} %(ENV_HOME)s/manage.py rqworker -v 3 default \ + exec python3 %(ENV_HOME)s/manage.py rqworker -v 3 default \ + --worker-class cvat.rqworker.DefaultWorker \ " environment=SSH_AUTH_SOCK="/tmp/ssh-agent.sock" numprocs=%(ENV_NUMPROCS)s diff --git a/supervisord/worker.low.conf b/supervisord/worker.low.conf index 04d39a5acb1..da47564747d 100644 --- a/supervisord/worker.low.conf +++ b/supervisord/worker.low.conf @@ -24,13 +24,8 @@ autorestart=true [program:rqworker_low] command=%(ENV_HOME)s/wait-for-it.sh %(ENV_CVAT_REDIS_HOST)s:6379 -t 0 -- bash -ic " \ - if [ \"%(ENV_CVAT_DEBUG_ENABLED)s\" = 'yes' ]; then \ - _DEBUG_CMD=\"-m debugpy --listen 0.0.0.0:%(ENV_CVAT_DEBUG_PORT)s\"; - else - _DEBUG_CMD=\"\"; - fi && \ - \ - exec python3 ${_DEBUG_CMD} %(ENV_HOME)s/manage.py rqworker -v 3 low \ + exec python3 %(ENV_HOME)s/manage.py rqworker -v 3 low \ + --worker-class cvat.rqworker.DefaultWorker \ " environment=SSH_AUTH_SOCK="/tmp/ssh-agent.sock" numprocs=%(ENV_NUMPROCS)s