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

Status callback for exec is never called when stdin closes #465

Closed
mochja opened this issue Jun 13, 2020 · 12 comments
Closed

Status callback for exec is never called when stdin closes #465

mochja opened this issue Jun 13, 2020 · 12 comments
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.

Comments

@mochja
Copy link

mochja commented Jun 13, 2020

Is it possible to close stdin for Exec without closing ws connection?

StatusStream is not received if WS connection is closed as well as additional data on stdout, stderr streams is lost.

@mochja mochja changed the title Status callback never called when stdin closes Status callback for exec is never called when stdin closes Jun 13, 2020
@brendandburns
Copy link
Contributor

You could try sending an explicit control-D character across the stream. That should close the stream in the application.

@mochja
Copy link
Author

mochja commented Jun 14, 2020

@brendandburns neat, that does work indeed, but only when tty is also attached.

With tty I am not able to correctly receive binary data for exec command.

tar xf - -C /tmp
tar: Refusing to read archive contents from terminal (missing -f option?)
tar: Error is not recoverable: exiting now
{
  metadata: {},
  status: 'Failure',
  message: 'command terminated with non-zero exit code: Error executing in Docker Container: 2',
  reason: 'NonZeroExitCode',
  details: { causes: [ [Object] ] }
}

@brendandburns
Copy link
Contributor

Makes sense ctrl-d is a terminal character after all.

This error is coming from the tar command itself. I'm not sure why that's happening, but I don't think it's related to this library.

@mochja
Copy link
Author

mochja commented Jun 17, 2020

@brendandburns you are correct it will be related to tty itself, but the fact with result is as kubectl exec is not behaving like that when stdin is closed. I was digging into kubectl exec and I figured out it's using SPDY and multiple streams for different io instead of single websocket connection.

I think issue could be solved with #411 or maybe there is way for ws protocol to close streams some other way.

kubernetes/kubernetes#89899

@brendandburns
Copy link
Contributor

I'm obviously not certain, but I think tar is giving up before stdin is even considered. I'm uncertain about the difference between tar xf - ... vs tar -x -f - but you may be since that is referenced in the error message.

@mochja
Copy link
Author

mochja commented Jun 19, 2020

It does not matter what executable I try to run, even cat breaks up in similiar manner as if you try to paste binary data trough regular terminal. I believe I would have to somehow escape the data so the terminal could recognize 0x04 as a byte and as a ctrl-d. I am not sure if thats possible (I was not able to find anything about that), but could also solve the issue.

@bankyadam
Copy link

bankyadam commented Aug 6, 2020

Did you find out something @mochja? I'm trying to upload some file with exec to a pod and I experience the same you do.

@mochja
Copy link
Author

mochja commented Aug 6, 2020

@bankyadam I did not, I think there are only two proper ways to handle this:

  • Use SPDY protocol instead of WS
  • Implement closing streams into WS protocol itself

I was experimenting by removing following lines

stdin.on('end', () => {
ws.close();
});

and if I am remembering correctly it was somewhat working.

@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 4, 2020
@fejta-bot
Copy link

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

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Dec 4, 2020
@fejta-bot
Copy link

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-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@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-testing, kubernetes/test-infra and/or fejta.
/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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed.
Projects
None yet
Development

No branches or pull requests

5 participants