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

Feature Request: detach-keys option/setting for pod terminal #1011

Open
Nuru opened this issue Jan 24, 2021 · 9 comments
Open

Feature Request: detach-keys option/setting for pod terminal #1011

Nuru opened this issue Jan 24, 2021 · 9 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@Nuru
Copy link

Nuru commented Jan 24, 2021

This is a revival of #693 which was sort of duplicated by #878 but both went stale before being addressed. #878 labeled this behavior a bug and put it on the backlog. I consider it a feature request, but am happy to let SIGCLI decide.

See Update 2023-04-04 below

kubectl run -it, kubectl exec -it, and kubectl attach -it connect to a Docker container in a manner parallel to docker run -it and docker attach. All these commands by default listen for the key sequence ctrl-p,ctrl-q to cause the command to detach from the container. However, unlike kubectl, the docker commands have a [--detach-keys option] and configuration property to allow you to specify an alternate key sequence to use instead of the default. kubectl is missing this option. Please implement it.

The default is problematic with bash because by default, ctrl-p on the bash command line brings up the previous command from the history list. Users typing ctrl-p once expect to see the previous command, but get nothing until they type another character, because the ctrl-p (being the first character in a potential 2-character command) is held back until the next character is sent. If the next character is ctrl-q the connection is closed and the container never sees the ctrl-p. If the next character is something else, then the ctrl-p and the next character are sent as soon as the second character is typed. If that character is ctrl-p, then the user will get the to see the command before the previous command, as they have just typed ctrl-p,ctrl-p. So getting to the immediately previous command requires typing ctrl-p followed by some other character that will not alter the command line. This is annoying at best.

Whatever issues people may have with the default detach key sequence in docker are alleviated by the presence of the --detach-keys option and the detachKeys property of the docker configuration. However kubectl does not have this option.

Please add a --detach-keys option that operates the same way as docker --detach-keys does.

Update 2023-04-04

Although the latest (v1.26) documentation still says you can detach using Ctrl+P,Ctrl+Q, this no longer seems to be the case. It seems you have to kill the attached process or close the connection with Ctrl+D. Why this is or what changed is not documented. So this is now also a request to update the documentation to reflect current behavior and to show the recommended way to end a session.

See also:

@Nuru Nuru added the kind/feature Categorizes issue or PR as related to a new feature. label Jan 24, 2021
@k8s-ci-robot k8s-ci-robot added the needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. label Jan 24, 2021
@eddiezane
Copy link
Member

Sorry this keeps getting closed. We will certainly accept a PR on this if anyone wants to pick it up.

/triage accepted
/priority backlog
/help

cc @her

@k8s-ci-robot
Copy link
Contributor

@eddiezane:
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:

Sorry this keeps getting closed. We will certainly accept a PR on this if anyone wants to pick it up.

/triage accepted
/priority backlog
/help

cc @her

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.

@k8s-ci-robot k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/backlog Higher priority than priority/awaiting-more-evidence. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Feb 3, 2021
@eddiezane
Copy link
Member

/lifecycle frozen

@k8s-ci-robot k8s-ci-robot added the lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. label Feb 3, 2021
@book987
Copy link

book987 commented Mar 5, 2021

Looks like this is a huge change, since kubernetes doesn't prepared for detach keys for now. This cannot achived by simply adding options to cli. I propose close this issue and reopen in kubernetes.

https://github.com/kubernetes/kubernetes/blob/master/pkg/kubelet/cri/streaming/server.go#L65

*pls point out if i'm wrong, thanks!

@Nuru
Copy link
Author

Nuru commented Mar 5, 2021

@book987 @mthome There is already an issue in Kubernetes for this feature, but it got closed as stale (as did #693 and #878 here). If you can get it re-opened and linked, that would be great.

We should not close the issue here, because even after support is added in kubelet we will still need to add the option in kubectl.

@bayeslearner
Copy link

As a workaround, create the pod using k run ... <-- bash> or k run -it <-- bash> , and debug it using exec -it.

@k8s-triage-robot
Copy link

This issue has not been updated in over 1 year, and should be re-triaged.

You can:

  • Confirm that this issue is still relevant with /triage accepted (org members only)
  • Close this issue with /close

For more details on the triage process, see https://www.kubernetes.dev/docs/guide/issue-triage/

/remove-triage accepted

@k8s-ci-robot k8s-ci-robot added needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. and removed triage/accepted Indicates an issue or PR is ready to be actively worked on. labels Mar 31, 2023
@Nuru
Copy link
Author

Nuru commented Apr 5, 2023

@eddiezane: Please see my updates to the Feature Request and add /triage accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. lifecycle/frozen Indicates that an issue or PR should not be auto-closed due to staleness. needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

No branches or pull requests

7 participants