Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛♻️ Is638/fixes docker-compose operations bursts and create containers call in dy-sidecar #3187

Merged
merged 31 commits into from
Jul 18, 2022

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Jul 13, 2022

What do these changes do?

  • 🐛 FIX: for POST /containers we eliminated docker-compose rm before up. The following was observed
    • dv2 request up
    • sdc runs in background first rm followed by up
    • dv2 re-requests up (not sure why? I guess it since sdc is taking too long ...)
    • sdc runs rm (which probably removes those being created) then restart from stratch up
    • we enter on a cycle ...
    • notice that rm is a close operations and typically takes a bit longer
  • ♻️ New pytest_simcore/simcore_service_library_fixtures.py for tests with co-routines decorated with @run_sequentially_in_context
    • Used the same for all services and packages
  • ♻️ cleanup docker_compose_utils and settings
  • 🐛 FIX: All docker-compose operations in docker_compose_utils API run one at a time @run_sequentially_in_context E.g. avoids running in parallel docker-compose up and config on the same service. I.e. implements

we need to have the fix in the dy-sidecar that prevents running XX docker-compose CLI calls in parallel (currently all these run in parallel and collapses the host RAM/CPUs)

  • 🐛 FIXES flaky tests: references with dumps from sets were different

Related issue/s

How to test

pytest services/dynamic-sidecar/tests/unit/test_core_docker_compose_utils.py::test_burst_calls_to_docker_compose_config

Checklist

@pcrespov pcrespov self-assigned this Jul 13, 2022
@pcrespov pcrespov added this to the Meteora milestone Jul 13, 2022
@codecov
Copy link

codecov bot commented Jul 13, 2022

Codecov Report

Merging #3187 (a5fba4d) into master (09f85c1) will increase coverage by 1.1%.
The diff coverage is 91.0%.

❗ Current head a5fba4d differs from pull request most recent head 0b1ba85. Consider uploading reports for the commit 0b1ba85 to get more accurate results

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #3187     +/-   ##
========================================
+ Coverage    80.9%   82.0%   +1.1%     
========================================
  Files         686     736     +50     
  Lines       30152   31528   +1376     
  Branches     3940    4079    +139     
========================================
+ Hits        24400   25864   +1464     
+ Misses       4920    4829     -91     
- Partials      832     835      +3     
Flag Coverage Δ
integrationtests 66.4% <100.0%> (-0.1%) ⬇️
unittests 78.5% <89.7%> (+<0.1%) ⬆️

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

Impacted Files Coverage Δ
...es/dynamic_sidecar/docker_service_specs/sidecar.py 80.8% <ø> (+6.3%) ⬆️
...ector_v2/modules/dynamic_sidecar/scheduler/task.py 80.2% <ø> (+9.2%) ⬆️
...sidecar/src/simcore_service_dynamic_sidecar/cli.py 0.0% <0.0%> (ø)
...mcore_service_dynamic_sidecar/modules/nodeports.py 25.0% <0.0%> (ø)
...mcore_service_dynamic_sidecar/core/remote_debug.py 33.3% <50.0%> (ø)
...c/simcore_service_dynamic_sidecar/core/settings.py 95.9% <66.6%> (ø)
...rary/src/servicelib/fastapi/requests_decorators.py 80.3% <80.0%> (+1.5%) ⬆️
...ages/service-library/src/servicelib/async_utils.py 88.6% <100.0%> (-3.5%) ⬇️
...service_director_v2/api/routes/dynamic_services.py 87.2% <100.0%> (+5.8%) ⬆️
...-v2/src/simcore_service_director_v2/core/events.py 100.0% <100.0%> (ø)
... and 114 more

@pcrespov pcrespov force-pushed the is638/dy-sidecar-round-7 branch from 978b125 to d2fd484 Compare July 14, 2022 19:18
@pcrespov pcrespov added a:services-library issues on packages/service-libs a:dynamic-sidecar dynamic-sidecar service labels Jul 15, 2022
@pcrespov pcrespov changed the title WIP: ♻️ Is638/dy sidecar round 7 🐛♻️ Is638/fixes docker-compose operations bursts and create containers call in dy-sidecar Jul 15, 2022
@pcrespov pcrespov marked this pull request as ready for review July 18, 2022 08:16
@pcrespov pcrespov requested review from sanderegg and GitHK as code owners July 18, 2022 08:16
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

I'm fine if this will improve the situation for now.
Please think about my note below.

@pcrespov pcrespov force-pushed the is638/dy-sidecar-round-7 branch from 31613d2 to 279a145 Compare July 18, 2022 17:39
@pcrespov pcrespov force-pushed the is638/dy-sidecar-round-7 branch from a5fba4d to 0b1ba85 Compare July 18, 2022 18:01
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@pcrespov pcrespov merged commit 85f338a into ITISFoundation:master Jul 18, 2022
@pcrespov pcrespov deleted the is638/dy-sidecar-round-7 branch July 18, 2022 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:dynamic-sidecar dynamic-sidecar service a:services-library issues on packages/service-libs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants