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

Pod exec enhancements #328

Merged
merged 4 commits into from
Aug 9, 2024
Merged

Conversation

olivier-matz-6wind
Copy link
Contributor

This draft pull request provides some enhancements to the pod_exec example and ws API.
The last commit is an API break, so you may not want it.

Comments welcome, of course!

Fixes #322

Rework this example to be closer to the one provided by the official kubernetes
python api. The busybox pod is now created if it does not exist, making the
example easier to use. Also, use contexts to ensure resources are properly
released.

This prepares the work for next commits that will add more cases in this
pod_exec example.

Signed-off-by: Olivier Matz <[email protected]>
Copy link

codecov bot commented Aug 7, 2024

Codecov Report

Attention: Patch coverage is 93.75000% with 1 line in your changes missing coverage. Please review.

Project coverage is 28.12%. Comparing base (2126b1d) to head (c3a3128).

Files Patch % Lines
kubernetes_asyncio/stream/ws_client.py 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #328      +/-   ##
==========================================
+ Coverage   28.11%   28.12%   +0.01%     
==========================================
  Files         779      779              
  Lines       92138    92153      +15     
==========================================
+ Hits        25904    25919      +15     
  Misses      66234    66234              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Owner

@tomplus tomplus left a comment

Choose a reason for hiding this comment

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

Excellent job! I really appreciate that. Thank you.

kubernetes_asyncio/stream/ws_client.py Outdated Show resolved Hide resolved
kubernetes_asyncio/stream/ws_client.py Show resolved Hide resolved
kubernetes_asyncio/stream/ws_client.py Show resolved Hide resolved
examples/pod_exec.py Show resolved Hide resolved
@olivier-matz-6wind
Copy link
Contributor Author

Excellent job! I really appreciate that. Thank you.

Thank you for the feedback!

When the websocket API is used to execute a command on a pod, the status is sent
over the ERROR_CHANNEL on termination. Add a helper to parse this information
that returns the exit code of the command. This helper can only be used if
_preload_content=False.

The pod_exec example will be updated in next commit to make use of this new
helper.

Signed-off-by: Olivier Matz <[email protected]>
Introduce an example that shows how the WsApiClient can be used to interactively
execute a command on a pod. The example is similar to what is done in the
official kubernetes python api.

Signed-off-by: Olivier Matz <[email protected]>
The ClientSession.ws_connect() method is synchronous and returns a
_RequestContextManager which takes a coroutine as parameter (here,
ClientSession._ws_connect()).

This context manager is in charge of closing the connection in its __aexit__()
method, so it has to be used with "async with".

However, this context manager can also be awaited as it has an __await__()
method. In this case, it will await the _ws_connect() coroutine. This is what is
done in the current code, but the connection will not be released.

Remove the "await" to return the context manager, so that the user can use it
with "async with", which will properly release resources.

This is the documented way of using ws_connect():
https://docs.aiohttp.org/en/stable/client_quickstart.html#websockets

Signed-off-by: Olivier Matz <[email protected]>
@olivier-matz-6wind olivier-matz-6wind marked this pull request as ready for review August 9, 2024 08:13
@tomplus tomplus linked an issue Aug 9, 2024 that may be closed by this pull request
Copy link
Owner

@tomplus tomplus left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.
I'm merging it now and it will be released with the next "big" version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cannot retrieve the command exit code after a pod exec Add example for using the exec endpoint with stdin
2 participants