-
Notifications
You must be signed in to change notification settings - Fork 27
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
✨ adding docker-api-proxy
service ⚠️
#7070
✨ adding docker-api-proxy
service ⚠️
#7070
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #7070 +/- ##
==========================================
- Coverage 87.06% 87.04% -0.02%
==========================================
Files 1647 1642 -5
Lines 64308 64181 -127
Branches 1093 1188 +95
==========================================
- Hits 55987 55865 -122
+ Misses 8008 8003 -5
Partials 313 313
Continue to review full report in Codecov by Sentry.
|
…socket-via-http-interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some early comments
services/docker-api-proxy/tests/integration/test_docker_api_proxy.py
Outdated
Show resolved
Hide resolved
services/docker-api-proxy/tests/integration/test_docker_api_proxy_autenticated.py
Outdated
Show resolved
Hide resolved
services/docker-api-proxy/tests/integration/test_docker_api_proxy_autenticated.py
Outdated
Show resolved
Hide resolved
services/docker-api-proxy/tests/integration/test_docker_api_proxy.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
much better thanks!
There are a few things to still fix/think about:
- dockerfile in root mode
- who closes the aiohttp client?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your work!
Let's discuss security implications before going ahead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@GitHK please create a ticket to add this service to ops repository and assign to me
…socket-via-http-interface
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!, a few last comments
…socket-via-http-interface
|
@Mergifyio queue |
🛑 The pull request has been removed from the queue
|
This pull request has been removed from the queue for the following reason: The pull request can't be updated You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it. If you want to requeue this pull request, you need to post a comment with the text: |
What do these changes do?
Why yet another service? It allows to use the docker API from a manager node.
Why is this important? Some services like the dynamic-scheduler require access to it in order to create and remove services and overlay networks (swarm scope network).
Bonuses
dynamic-scheduler
to thedocker-api-proxy
Related issue/s
docker-api-proxy
service to the stack that can allows to use the docker socket remotely form another service not running on a simcore node or potentially inside the cluster #7145docker-api-proxy
osparc-ops-environments#965docker-api-network
internal #7222docker-api-proxy
service osparc-ops-environments#969How to test
Dev-ops checklist