ws_client.py could not handle binary as stdout

k8s python client: 11.0.0
python 3

We're using following block copying files/dirs from pod to local, however, ws_client would decode(utf-8) data frame from socket into str, which would break the function. Enhancement needed for function def update(self, timeout=0) to store data based on type into channels.

    exec_command = ['tar', 'cf', '-', '/var']

    with TemporaryFile() as tar_buffer:
        resp = stream(api_instance.connect_get_namespaced_pod_exec, pod_name, namespace,
                      command=exec_command,
                      stderr=True, stdin=False,
                      stdout=True, tty=False,
                      _preload_content=False)

        while resp.is_open():
            resp.update(timeout=1)
            if resp.peek_stdout():
                out=resp.read_stdout()
                tar_buffer.write(out)
            if resp.peek_stderr():
                print("STDERR: %s" % resp.read_stderr())

        resp.close()

        tar_buffer.flush()
        tar_buffer.seek(0)

        with tarfile.open(fileobj=tar_buffer, mode='r:') as tar:
            print('members', tar.getmembers())

/assign

We should only decode the response if opcode == ABNF.OPCODE_TEXT (websocket-client module). Currently we are decoding binary payload as well:

python-base/stream/ws_client.py

Lines 207 to 210 in bf5c599

    
           elif op_code == ABNF.OPCODE_BINARY or op_code == ABNF.OPCODE_TEXT: 
        
               data = frame.data 
        
               if six.PY3: 
        
                   data = data.decode("utf-8", "replace")

We also need a e2e test case for this. I will work on this when I get a chance, or someone else may pick it up.

/help

@roycaihw:
This request has been marked as needing help from a contributor.

Please ensure the request meets the requirements listed here.

If this request no longer meets these requirements, the label can be removed
by commenting with the /remove-help command.

In response to this:

We should only decode the response if opcode == ABNF.OPCODE_TEXT (websocket-client module). Currently we are decoding binary payload as well:

python-base/stream/ws_client.py

Lines 207 to 210 in bf5c599

elif op_code == ABNF.OPCODE_BINARY or op_code == ABNF.OPCODE_TEXT:

data = frame.data

if six.PY3:

data = data.decode("utf-8", "replace")

We also need a e2e test case for this. I will work on this when I get a chance, or someone else may pick it up.

/help

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

/assign

Here is a total curve ball observation.

I think the wrapping of the pod exec api needs to be completely reimplemented following the python subprocess.Popen constructor calling sequence as close as possible. This new implementation should return an object that then acts as close as possible to an instance of subprocess.Popen. This is a well thought out library for interacting with processes. Then python programmers that know how to interact with subprocess.Popen will be up and running doing the same with pod exec.

In response to this specific issue, subprocess.Popen by default handles stdin, stdout, and stderr as binary streams. The text=True argument specifies that these streams should be text streams.

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-contributor-experience at kubernetes/community.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

	elif op_code == ABNF.OPCODE_BINARY or op_code == ABNF.OPCODE_TEXT:
	data = frame.data
	if six.PY3:
	data = data.decode("utf-8", "replace")