-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Exec calls #58
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
Comments
realted: http://stackoverflow.com/questions/37349440/upgrade-request-required-when-running-exec-in-kubernetes I guess if we change code generator to accept headers, it should be possible to pass upgrade headers to the call. Update: Look like we need an SPDY client for python too. |
Is it feasible to have it soon? It would be nice to have a date in order to know if I can use it within my deadline. If not, do you have some workaround in mind? Thank you in advance. |
I don't have a date yet, the problem is urllib3 does not support SPDY protocol. we need to use something like python-spdylay. Unless somebody own this (or I found the time to look at it in more depth) I can't put a date on it. |
Just spend a little more time with python-spdylay. Even their client example is not working and it does not seem to be actively developed/supported. If anybody have any idea of how to talk SPDY in python, please let me know. |
More information here: kubernetes/kubernetes#24668 |
Moreover, there is kubernetes/kubernetes#7452 which stands that SPDY is deprecated and HTTP/2 might be the right option, but looks like it is still hanging in the air. Anyways, it is SPDY what is working for current versions so we need to somehow find a workaround. |
A little more digging pointed me to a library called thor. I will give it a try. If you can try it too to see if you can establish a SPDY connection with it, that would be nice. My assumption is SPDY is always secure so there should be a way to specify client-cert and other cert files that we normally set in |
You can do |
Thanks @sebgoa it look like websockets are supported as a fall-back: https://github.com/kubernetes/kubernetes/blob/master/docs/devel/api-conventions.md#websockets-and-spdy |
@mbohlool is there an example that you could provide demonstrating the use of exec via websockets? |
@mrmcmuffinz I did not have time to do it. That is why I added "Help needed" to this issue so somebody from the community hopefully help. |
@mbohlool My apologies, I thought you had some example. Do you happen to know who we can talk to, to help out? Can we work out defining what exactly it takes to get this done? |
Adding my stackoverflow question here too, for other people who stumble across this. Also has anyone looked into speaking with the developers of |
The ApiException for exec or any call needs SPDY/Websockets is informative. The right way to fix this is to catch that exception in api_client.py and start a websocket call instead. For the first step, try to write a python code that does not use this repo, and directly call into exec REST API using websocket. use kubectl proxy to get rid of authorization part and call into local host for simplification. |
I actually tried doing that but I could not figure out how to setup the authentication part of the code for the websocket. I have three pem certificates that were given to me and I'm just not sure how that fits into the picture. Thank you for your response btw. |
have you try kubectl proxy? that way you should not worry about authentication for now. it will proxy your call from localhost with the right authentication headers to the remote host. |
Ohh wow I totally forgot about the kubectl proxy command, if I do that should I switch my url to localhost in the websocket url versus the actual endpoint itself? Edit: |
Yes. lets have a working code with proxy then think about how to incorporate it into the client-python. |
Yeah I think I'm a bit further however I get a connection reset by peer error right now. I think I will need to read up on sockets in python as well as what all I need to do in order to setup the appropriate message for communicating. FYI: I'm using a package called hyper for the actual communication unless you can suggest anything better. |
Here is the issue against urllib3 for |
@mrmcmuffinz I was wondering how you are testing this. I was doing some basic testing with hyper; but the exec call needs a "SPDY/3.1" upgrade header that hyper doesn't know how to handle. If you do a connection upgrade with "SPDY/3.1" you will get this exception: https://github.com/Lukasa/hyper/blob/24b3b37d226bedb6199f22b788e066b91dbf892a/hyper/common/conncetion.py#L130 And if you do an h2 upgrade you will get this from kubernetes (since http2 is not implemented yet): |
Hi @chekolyn in all honesty I put it on hold and started looking at other clients. I just wanted to use these python client with py.test for FVT but that kinda went out the door because of this limitation. Edit: |
OK guys a gave websockets a try and I have a very limited POC I can confirm websockets works! # POC for Websocket exec calls to pods:
#
# Channel 0 is STDIN, 1 is STDOUT, 2 is STDERR (if TTY is not requested),
# and 3 is a special error channel. The server only reads from STDIN,
# writes to the other three.
import websocket
import ssl
from subprocess import check_output
def on_message(ws, message):
print 'message received ..'
print message
def on_error(ws, error):
print 'error happened .. '
print error
def on_close(ws):
print "### closed ###"
def on_open(ws):
print 'Opening Websocket connection to the server ... '
# This session_key I got, need to be passed over websocket header isntad
# of ws.send.
#ws.send(session_key)
token = check_output(["oc", "whoami", "-t"])
pod = "router-1-8u540"
exec_command = "uptime".replace(" ", "+")
url = "wss://localhost:8443/api/v1/namespaces/default/pods/%s/exec?command=%s&stderr=true&stdin=true&stdout=true&tty=false" % (pod, exec_command)
header = "Authorization: Bearer " + token
if __name__ == "__main__":
websocket.enableTrace(False)
ws = websocket.WebSocketApp(url,
on_message = on_message,
on_error = on_error,
on_close = on_close,
header = [header]
)
ws.on_open = on_open
ws.run_forever(sslopt={"cert_reqs": ssl.CERT_NONE})
# ws.on_message = on_message
# ws.run_forever(sslopt={"cert_reqs": ssl.CERT_NONE}) |
@chekolyn "oc" implies openshift? does this work in a pure kubernetes environment? |
@dims I haven't tested it on a pure k8s but I believe It should work, the oc command is just grabbing the current auth token to authenticate via headers (perhaps this is the only portion that it's OpenShift specific in the POC, you can probably remove the check_output for a valid token and it should work too ). The url api exec call is pure kubernetes and websockets calls seem to be working fine. I'm basically using this authentication strategy in the api call: https://kubernetes.io/docs/user-guide/accessing-the-cluster/#without-kubectl-proxy-post-v13x |
I still think the way it's implemented is a little hacky (not the implementation itself but the way we integrated it into the api call by checking an specific API endpoint). I would like to keep this issue a little longer so we can look back and clean it up a little. I will take another look tomorrow. |
Hi might I bring my two cents. I still think we need to fix the ged and post requests for exec as they both go down the same path. Lastly I have created a dozen or so tests for the product I'm working on but there is one thing that I don't understand, where does one access the stderr, stdout? All we get back is a response message but those elements are not accessible in any specific manner. I think the api is a bit misleading though perhaps I'm misinterpreting it. However that being said this Git issue was opened to add |
Well I don't mind creating a new issue for post work. The access to stderr and stdout separately was one of my comments on original PR too. The majority part of the task is done so I will leave it to you and @dims if you want to close this and create a new issue, but I don't think our work on exec calls is done until we solve those issues. I would like to have a similar approach to Watch here. Will try something today. |
I'm ok with either approach @mbohlool and will provide my feedback along the way as now I'm kinda a "stakeholder" in this. Any idea when we will have a new release with these changes, or at least a git tag for marking this particular milestone? |
I have a WIP PR that will send out soon. Only thing that is not working is stdin. stdout and stderr is working as well as streaming output. I will do another beta release after that PR. |
Ok, please keep me posted. |
@mrmcmuffinz I sent a partial PR. stdin is not working yet (and I am not sure why). You also show concern about get/post going the same path, what is your concern? as far as I can tell websocket does not have a method like http. so get/post is the same for websocket. |
I sent a partial PR. stdin is not working yet What is your concern for get/post going down the same path? Edit 1:
File descriptor
|
The normal HTTP call (that I assume you mean that by |
@mbohlool I meet the following error when trying to execute a command in the pod.
I tried both k8s 1.2 and 1.5 cluster. It seems the websockets is not accessable so it fails when calling WebSocket.connect() method. How to enable websockets on the k8s master node? |
I don't think the issue is the target cluster (I don't think websocket can be disabled, it is always enabled). Would you share your code (fragment)? This can be a bug in our implementation. |
|
My test code is straight forward enough. Here it is
Here is some debug trace:
After invoking websocket
Let me ask a stupid question: How to access the websocket, port 8443 or port 8080? |
@mrmcmuffinz: for no 3. It is a good idea, however API spec defined the return type as string. If you want to get stdout and stderr separately you need to call the API with resp = api.connect_get_namespaced_pod_exec(name, 'default',
command=exec_command,
stderr=True, stdin=True,
stdout=True, tty=False,
_preload_content=False)
resp.run_forever()
if resp.peek_stdout(): print("STDOUT: ", resp.read_stdout())
if resp.peek_stderr(): print("STDERR: ", resp.read_stderr()) |
@zhenhua, I guess your problem is |
@mbohlool, thanks. I use your mbohlool:exec branch to verify it on my side and it works fine. Only one thing is that, the container parameter of connect_post_namespaced_pod_exec() call is NOT optional, otherwise, it will return an 404 error of 'HTTP/1.1 404 Not Found'. |
@zhenhua documentation of the parameter says: "Defaults to only container if there is only one container in the pod." I think in case of our examples and e2e tests we have one container but you may have more than one container in your pod. Happy that it worked, I will merge the PR and cut another beta release. |
@mbohlool You are right. Actually, when creating a pod in my test, there're always have at least two containers, one is registry.access.redhat.com/rhel7/pod-infrastructure:latest and another is my test container. So I have to always specify the container name. $ docker ps CONTAINER ID IMAGE COMMAND CREATED STATUS PORTS NAMES 63aa2b961a10 registry.v2.service.xxx.org/dongyue/xxx-base-centos7:20170111 "/bin/sh -c /usr/sbin" 40 minutes ago Up 40 minutes k8s_test-rc.12ee117e_test-rc-t4052_289-ns_0b32083c-f806-11e6-b075-c81f66cd4467_1c38b8b6 be4cc79af38c registry.access.redhat.com/rhel7/pod-infrastructure:latest "/pod" 40 minutes ago Up 40 minutes k8s_POD.ae8ee9ac_test-rc-t4052_289-ns_0b32083c-f806-11e6-b075-c81f66cd4467_ffa70144 |
This commit reapplies PR kubernetes-client#120 that was removed by kubernetes-client#353. Below is the original commit message. inspired by the POC from @chekolyn * Adds a new requirement on websocket-client * Add a new class WSClient that uses WebSocketApp from the websocket-client. * Make sure we pass Authorization header * Make sure we honor the SSL settings in configuration * Some of the code will get overwritten when we generate fresh classes from swagger definition. To remind us added a e2e test so we don't lose the changes * Added a new configuration option to enable/disable failures when hostnames in certificates don't match Fixes kubernetes-client#58 Fixes kubernetes-client#409
This commit reapplies PR kubernetes-client#120 that was removed by kubernetes-client#353. Below is the original commit message. inspired by the POC from @chekolyn * Adds a new requirement on websocket-client * Add a new class WSClient that uses WebSocketApp from the websocket-client. * Make sure we pass Authorization header * Make sure we honor the SSL settings in configuration * Some of the code will get overwritten when we generate fresh classes from swagger definition. To remind us added a e2e test so we don't lose the changes * Added a new configuration option to enable/disable failures when hostnames in certificates don't match Fixes kubernetes-client#58 Fixes kubernetes-client#409
hi,
anything wrong ? thanks~ |
Hi! I'm getting this same error now in version 8.0.1 Here's the offending code:
And here's the exception that is being raised:
I'm using pipenv to manage the working environment. Here are the packages I have installed and corresponding versions:
It seems like a regression to me, but I may be wrong. Could you guys help me out figuring this out? I'm using Python v3.7.2 |
Whenever I try to open an exec call I get:
Which is related with the fact that
requests
is not SPDY compatible. Are you going to support this kind of requests in the near future?The text was updated successfully, but these errors were encountered: