Skip to content

Conversation

@GitHK
Copy link
Contributor

@GitHK GitHK commented Aug 25, 2025

What do these changes do?

Since the hostname is no longer the same as the ID of the container the command was not executed in the correct runtime.

Related issue/s

How to test

Dev-ops

@GitHK GitHK self-assigned this Aug 25, 2025
@GitHK GitHK added the a:agent agent service label Aug 25, 2025
@GitHK GitHK added this to the Voyager milestone Aug 25, 2025
@GitHK GitHK requested a review from Copilot August 25, 2025 13:38
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 issue where the agent was not executing commands in the correct container due to hostname changes that no longer match container IDs. The fix replaces the hostname-based container identification with a Docker API-based approach that dynamically determines the container ID by matching the container's IP address.

Key changes:

  • Replace hostname-based container identification with Docker socket API lookup
  • Add IP-based container resolution using Docker's /containers/json endpoint
  • Update tests to mock the new container identification logic

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
services/agent/src/simcore_service_agent/services/backup.py Implements new container ID resolution using Docker API and IP matching instead of hostname
services/agent/tests/unit/test_services_backup.py Updates test fixtures and adds mocking for the new container IP resolution logic

@sonarqubecloud
Copy link

@codecov
Copy link

codecov bot commented Aug 25, 2025

Codecov Report

❌ Patch coverage is 81.25000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 66.79%. Comparing base (236d3a1) to head (9b41a31).
⚠️ Report is 1 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (236d3a1) and HEAD (9b41a31). Click for more details.

HEAD has 31 uploads less than BASE
Flag BASE (236d3a1) HEAD (9b41a31)
unittests 32 1
Additional details and impacted files
@@             Coverage Diff             @@
##           master    #8256       +/-   ##
===========================================
- Coverage   88.02%   66.79%   -21.24%     
===========================================
  Files        1919      765     -1154     
  Lines       74311    34976    -39335     
  Branches     1305      175     -1130     
===========================================
- Hits        65415    23363    -42052     
- Misses       8503    11556     +3053     
+ Partials      393       57      -336     
Flag Coverage Δ
integrationtests 63.89% <ø> (-0.05%) ⬇️
unittests 93.53% <81.25%> (+6.85%) ⬆️
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.01% <ø> (-8.02%) ⬇️
agent 93.53% <81.25%> (-0.38%) ⬇️
api_server ∅ <ø> (∅)
autoscaling ∅ <ø> (∅)
catalog ∅ <ø> (∅)
clusters_keeper ∅ <ø> (∅)
dask_sidecar ∅ <ø> (∅)
datcore_adapter ∅ <ø> (∅)
director ∅ <ø> (∅)
director_v2 77.97% <ø> (-12.89%) ⬇️
dynamic_scheduler ∅ <ø> (∅)
dynamic_sidecar 87.19% <ø> (-2.91%) ⬇️
efs_guardian ∅ <ø> (∅)
invitations ∅ <ø> (∅)
payments ∅ <ø> (∅)
resource_usage_tracker ∅ <ø> (∅)
storage ∅ <ø> (∅)
webclient ∅ <ø> (∅)
webserver 58.63% <ø> (-29.45%) ⬇️

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 236d3a1...9b41a31. 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.

@GitHK GitHK marked this pull request as ready for review August 25, 2025 13:49
@YuryHrytsuk
Copy link
Contributor

Thanks for a fix 🙏

Could you also explain, why so many logs were generated? (200_000 in 5 min) Out of curiosity

@GitHK
Copy link
Contributor Author

GitHK commented Aug 25, 2025

Thanks for a fix 🙏

Could you also explain, why so many logs were generated? (200_000 in 5 min) Out of curiosity

@YuryHrytsuk Because it's trying to remove 4 x number of volumes. So I think it can't remove volumes since quite a while that's why there are so many logs

@mergify
Copy link
Contributor

mergify bot commented Aug 25, 2025

🧪 CI Insights

Here's what we observed from your CI run for 9b41a31.

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on base branch Retries 🔍 CI Insights 📄 Logs
CI system-tests You had a 23% chance of failing… lucky you! 🎲 Flaky Configure an automatic retry View View

@GitHK GitHK merged commit 13b6062 into ITISFoundation:master Aug 25, 2025
91 of 95 checks passed
@GitHK GitHK deleted the pr-osparc-fix-agent-container-name branch August 25, 2025 14:11
@YuryHrytsuk
Copy link
Contributor

YuryHrytsuk commented Aug 25, 2025

Thanks for a fix 🙏
Could you also explain, why so many logs were generated? (200_000 in 5 min) Out of curiosity

@YuryHrytsuk Because it's trying to remove 4 x number of volumes. So I think it can't remove volumes since quite a while that's why there are so many logs

Thanks for explanation!

Is there a way to avoid this behaviour? So that we do not generate hundeds of thousands of logs in couple of minutes if this goes wrong?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:agent agent service

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants