Skip to content

Conversation

@GitHK
Copy link
Contributor

@GitHK GitHK commented Jul 24, 2025

What do these changes do?

Fixed an issue with the hooks, if the target container is not found an exception would have been raised.
Reported by @YuryHrytsuk

"2025-07-24T12:21:26.607Z","osparc-master-04","dy-sidecar_02d271e0-84fa-4d91-997a-44adc86ddd0b.1.ifavx6dmusfsjj60cnvoi52oz","log_level=ERROR | log_timestamp=2025-07-24 12:21:26,606 | log_source=servicelib.long_running_tasks.lrt_api:get_task_result(88) | log_uid=None | log_oec=OEC:66c1fc9fc805-1753359686606 | log_trace_id=8cd6f10fd9f2848be4445a5afb66e1a6 | log_span_id=85e1740cba321420 | log_resource.service.name= | log_trace_sampled=True | log_msg=task_id='lrt.simcore_service_dynamic_sidecar.modules.long_running_tasks.task_runs_docker_compose_down.unique' raised an exception while getting its result.
{
  ""exception_type"": ""<class 'aiodocker.exceptions.DockerError'>"",
  ""exception_string"": ""DockerError(409, 'container 922324b0e850d2a1f7af27367f3b648c74cffbd3067637088d19f52f252e286b is not running')"",
  ""exception_causes"": """",
  ""error_code"": ""OEC:66c1fc9fc805-1753359686606"",
  ""context"": {
    ""task_context"": null,
    ""task_id"": ""lrt.simcore_service_dynamic_sidecar.modules.long_running_tasks.task_runs_docker_compose_down.unique""
  },
  ""tip"": null
}
Traceback (most recent call last):
  File ""/home/scu/.venv/lib/python3.11/site-packages/servicelib/long_running_tasks/lrt_api.py"", line 78, in get_task_result
    task_result = tasks_manager.get_task_result(
                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ""/home/scu/.venv/lib/python3.11/site-packages/servicelib/long_running_tasks/task.py"", line 260, in get_task_result
    return tracked_task.task.result()
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ""/home/scu/.venv/lib/python3.11/site-packages/servicelib/long_running_tasks/task.py"", line 372, in _progress_task
    return await handler(progress, **task_kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ""/home/scu/.venv/lib/python3.11/site-packages/simcore_service_dynamic_sidecar/modules/long_running_tasks.py"", line 285, in task_runs_docker_compose_down
    await run_before_shutdown_actions(
  File ""/home/scu/.venv/lib/python3.11/site-packages/simcore_service_dynamic_sidecar/modules/long_running_tasks_utils.py"", line 32, in run_before_shutdown_actions
    await run_command_in_container(
  File ""/home/scu/.venv/lib/python3.11/site-packages/simcore_service_dynamic_sidecar/modules/container_utils.py"", line 76, in run_command_in_container
    return await asyncio.wait_for(
           ^^^^^^^^^^^^^^^^^^^^^^^
  File ""/usr/local/lib/python3.11/asyncio/tasks.py"", line 489, in wait_for
    return fut.result()
           ^^^^^^^^^^^^
  File ""/home/scu/.venv/lib/python3.11/site-packages/simcore_service_dynamic_sidecar/modules/container_utils.py"", line 26, in _execute_command
    exec_instance: Exec = await container.exec(
                          ^^^^^^^^^^^^^^^^^^^^^
  File ""/home/scu/.venv/lib/python3.11/site-packages/aiodocker/containers.py"", line 451, in exec
    data = await self.docker._query_json(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File ""/home/scu/.venv/lib/python3.11/site-packages/aiodocker/docker.py"", line 323, in _query_json
    async with self._query(
  File ""/usr/local/lib/python3.11/contextlib.py"", line 210, in __aenter__
    return await anext(self.gen)
           ^^^^^^^^^^^^^^^^^^^^^
  File ""/home/scu/.venv/lib/python3.11/site-packages/aiodocker/docker.py"", line 232, in _query
    yield await self._do_query(
          ^^^^^^^^^^^^^^^^^^^^^
  File ""/home/scu/.venv/lib/python3.11/site-packages/aiodocker/docker.py"", line 298, in _do_query
    raise DockerError(response.status, json.loads(what.decode(""utf8"")))
aiodocker.exceptions.DockerError: DockerError(409, 'container 922324b0e850d2a1f7af27367f3b648c74cffbd3067637088d19f52f252e286b is not running')",

Related issue/s

How to test

Dev-ops

@GitHK GitHK requested a review from Copilot July 24, 2025 12:51
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes an exception that occurs during container shutdown when attempting to run commands in containers that are not present or not running. The fix adds proper exception handling for Docker API errors that can occur during the shutdown process.

Key Changes

  • Added exception handling for DockerError in the shutdown command execution
  • Prevents crashes during container cleanup when containers are already stopped or removed

@GitHK GitHK self-assigned this Jul 24, 2025
@GitHK GitHK added a:dynamic-sidecar dynamic-sidecar service t:maintenance Some planned maintenance work labels Jul 24, 2025
@GitHK GitHK added this to the Engage milestone Jul 24, 2025
@GitHK GitHK changed the title 🐛 Fix exception on shutdown 🐛 Avoids raising exceptions when the target container of a hook is not found Jul 24, 2025
@GitHK GitHK marked this pull request as ready for review July 24, 2025 12:55
@codecov
Copy link

codecov bot commented Jul 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.99%. Comparing base (ba41af4) to head (e11da94).
Report is 1 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (ba41af4) and HEAD (e11da94). Click for more details.

HEAD has 31 uploads less than BASE
Flag BASE (ba41af4) HEAD (e11da94)
unittests 32 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #8156       +/-   ##
===========================================
- Coverage   88.08%   66.99%   -21.10%     
===========================================
  Files        1894      728     -1166     
  Lines       73013    33690    -39323     
  Branches     1279      176     -1103     
===========================================
- Hits        64317    22572    -41745     
- Misses       8316    11060     +2744     
+ Partials      380       58      -322     
Flag Coverage Δ
integrationtests 64.18% <100.00%> (-0.07%) ⬇️
unittests 89.18% <100.00%> (+2.47%) ⬆️
Components Coverage Δ
pkg_aws_library ∅ <ø> (∅)
pkg_celery_library ∅ <ø> (∅)
pkg_dask_task_models_library ∅ <ø> (∅)
pkg_models_library ∅ <ø> (∅)
pkg_notifications_library ∅ <ø> (∅)
pkg_postgres_database ∅ <ø> (∅)
pkg_service_integration ∅ <ø> (∅)
pkg_service_library ∅ <ø> (∅)
pkg_settings_library ∅ <ø> (∅)
pkg_simcore_sdk 77.10% <ø> (-8.01%) ⬇️
agent ∅ <ø> (∅)
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 77.05% <ø> (-14.03%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 90.08% <100.00%> (+<0.01%) ⬆️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 59.20% <ø> (-29.03%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba41af4...e11da94. Read the comment docs.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thx.

  • Looks very much as some of the issues i reported in #8145. Just in case it helps!

@GitHK
Copy link
Contributor Author

GitHK commented Jul 24, 2025

thx.

yes it's the same

@GitHK GitHK added the 🤖-automerge marks PR as ready to be merged for Mergify label Jul 24, 2025
@GitHK
Copy link
Contributor Author

GitHK commented Jul 24, 2025

@Mergifyio queue

@GitHK
Copy link
Contributor Author

GitHK commented Jul 25, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Jul 25, 2025

queue

🛑 The pull request has been removed from the queue default

The pull request can't be updated.

You can take a look at Queue: Embarked in merge queue check runs for more details about the failure.

@mergify
Copy link
Contributor

mergify bot commented Jul 25, 2025

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated.

You should update or rebase your pull request manually. If you do, this pull request will automatically be requeued once the queue conditions match again.
If you think this was a flaky issue, you can requeue the pull request, without updating it, by posting a @mergifyio requeue comment.

@GitHK
Copy link
Contributor Author

GitHK commented Jul 25, 2025

@Mergifyio requeue

@mergify
Copy link
Contributor

mergify bot commented Jul 25, 2025

requeue

✅ This pull request will be embarked automatically

The head sha of this pull request, 7e1c541, was never embarked in the merge queue.

But don't worry, Mergify will embark it automatically for you.

@mergify
Copy link
Contributor

mergify bot commented Jul 25, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at c2de0e1

@sonarqubecloud
Copy link

@mergify mergify bot merged commit c2de0e1 into ITISFoundation:master Jul 25, 2025
94 of 97 checks passed
@GitHK GitHK deleted the pr-osparc-fix-before-shutdown-error branch July 25, 2025 09:52
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
88 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:dynamic-sidecar dynamic-sidecar service t:maintenance Some planned maintenance work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Chain failures of directorv2 due to a container error with sidecars

4 participants