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

Improve kubectl cp, so it doesn't require the tar binary in the container #58512

Open
luksa opened this issue Jan 19, 2018 · 46 comments
Open

Improve kubectl cp, so it doesn't require the tar binary in the container #58512

luksa opened this issue Jan 19, 2018 · 46 comments
Labels
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. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/node Categorizes an issue or PR as relevant to SIG Node.

Comments

@luksa
Copy link
Contributor

luksa commented Jan 19, 2018

Uncomment only one, leave it on its own line:

/kind feature

What happened:
Kubectl cp currently requires the container we're copying into to include the tar binary. This is problematic when the container image is minimal and only includes the main binary run in the container and nothing else.

What you expected to happen:
Docker now has docker cp, which can copy files into a running container without any prerequisites on the container itself. Kubectl cp could use that mechanism. Obviously, this will require introducing a new feature into CRI, so it's not a small task.

Why we need this:
This will enable users to debug an existing (running) container, which is based on the scratch image and contains nothing else but the main app binary. Users would be able to get any binary they need into the container. An alternative solution could be to mount an additional volume (possibly from another container image) into a running pod (if that feature is ever implemented).

@k8s-ci-robot k8s-ci-robot added the needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. label Jan 19, 2018
@dims
Copy link
Member

dims commented Jan 19, 2018

@luksa : did you figure out which Docker API is to be used?

/sig node

@k8s-ci-robot k8s-ci-robot added sig/node Categorizes an issue or PR as relevant to SIG Node. and removed needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Jan 19, 2018
@luksa
Copy link
Contributor Author

luksa commented Jan 19, 2018

Copy from container: GET /containers/(id or name)/archive
Copy into container: PUT /containers/(id or name)/archive

This is from https://docs.docker.com/engine/api/v1.20/

@cyphar
Copy link

cyphar commented Mar 28, 2018

Ideally we would add a CRI method that allows the kubelet (and thus Kubernetes) to fetch the stream of an archive of a given path inside a container and to extract a stream of an archive to a path inside a container -- so that cri-o could support this improved kubectl cp just as well.

It should be noted though that (in my experience) the semantics of "what path to output to" (especially for single-file copy operations) are a little bit odd. Docker made a few mistakes historically with this, so I'd prefer to not repeat them in Kubernetes.

@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 Jun 26, 2018
@talolierwork
Copy link

+1

@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 Aug 1, 2018
@yue9944882
Copy link
Member

@kubernetes/sig-node-feature-requests

@k8s-ci-robot k8s-ci-robot added the kind/feature Categorizes issue or PR as related to a new feature. label Aug 1, 2018
@yue9944882
Copy link
Member

/remove-lifecycle rotten

@k8s-ci-robot k8s-ci-robot removed the lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. label Aug 1, 2018
@yujuhong
Copy link
Contributor

yujuhong commented Aug 2, 2018

@luksa will the WIP "debug container" feature satisfy your use case? kubernetes/enhancements#277

/cc @verb

@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 Oct 31, 2018
@luksa
Copy link
Contributor Author

luksa commented Nov 1, 2018

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Nov 1, 2018
@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 Jan 30, 2019
@goddenrich
Copy link

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jul 1, 2020
@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 Sep 29, 2020
@lindhe
Copy link

lindhe commented Sep 29, 2020

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 29, 2020
@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 Dec 28, 2020
@lindhe
Copy link

lindhe commented Dec 28, 2020

/remove-lifecycle stale

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Dec 28, 2020
@BenTheElder
Copy link
Member

I think the best option is to have CRI support this, CRI could e.g. use a trusted host tar implementation and make various improvements. With dockershim deprecated it will be feasible to only focus on doing this in CRI.

To get a change like this in, you probably need to file an enhancement under SIG node and work with that group to make it happen.
I don't have anywhere near enough free time to make this happen currently, but I think this needs to be fixed rather than say removing this functionality because we have issues securing it.
/lifecycle frozen

@zhangguanzhang
Copy link

zhangguanzhang commented Oct 26, 2022

$ echo 123 > txt
$ kubectl exec -ti xxx -- sh -c 'cat - > /tmp/file' < txt
Unable to use a TTY - input is not a terminal or the right kind of file
$ kubectl exec -ti xxx -- cat /tmp/file
123

@RichardoC
Copy link

Instead of doing this via the CRI, couldn't the kubelet get the file(s) directly and do the taring directly itself and this be possible via a new API? There's pretty good golang standard support for that (e.g. https://gist.github.com/maximilien/328c9ac19ab0a158a8df)

@RichardoC
Copy link

An alternative would be writing the host tar to memory in the container's memory space and using something like https://magisterquis.github.io/2018/03/31/in-memory-only-elf-execution.html via the kubelet, though that's probably a can of worms we don't want to open.

@BenTheElder
Copy link
Member

Instead of doing this via the CRI, couldn't the kubelet get the file(s) directly and do the taring directly itself and this be possible via a new API?

Kubelet leaves the implementation of the container filesystem up to the CRI implementation at this point (since dockershim is now gone we only have CRI mode), so we still need some way to access the container filesystem. So most likely any variation suggested above would require changes to kubelet, CRI API, and the CRI implementations. It will require a KEP in SIG Node to make a change like this.

@RichardoC
Copy link

I think the best option is to have CRI support this, CRI could e.g. use a trusted host tar implementation and make various improvements. With dockershim deprecated it will be feasible to only focus on doing this in CRI.

To get a change like this in, you probably need to file an enhancement under SIG node and work with that group to make it happen. I don't have anywhere near enough free time to make this happen currently, but I think this needs to be fixed rather than say removing this functionality because we have issues securing it. /lifecycle frozen

The enhancement link doesn't work, Github 404s

@BenTheElder
Copy link
Member

BenTheElder commented Jan 4, 2023

Sorry that should be https://github.com/kubernetes/enhancements, the "s" got dropped from the link. Also known as "KEPs" -- the process for Kubernetes features. https://github.com/kubernetes/enhancements/tree/master/keps

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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. sig/cli Categorizes an issue or PR as relevant to SIG CLI. sig/node Categorizes an issue or PR as relevant to SIG Node.
Projects
Status: Needs Triage
Development

No branches or pull requests