gvisor icon indicating copy to clipboard operation
gvisor copied to clipboard

`docker update` command fails with runsc

Open isaac-mcfadyen opened this issue 2 years ago • 9 comments

Description

  • docker update is mainly useful for updating CPU/memory quotas on running containers.
  • When updating an existing container via docker update (which is using the runsc runtime), the command crashes with an exit code of 128.
  • It seems that Docker is trying (and failing) to call runsc with invalid flags/commands during docker update?

Steps to reproduce

  • Install the runsc runtime for Docker.
  • Run a docker container: docker run --name=nginx -d --runtime=runsc nginx
  • Update its CPU quota or really any attribute allowed by docker update: docker update nginx --cpu-period 1000
  • Observe command failing - it seems that Docker is trying to call runsc with invalid flags?

runsc version

runsc version release-20231009.0
spec: 1.1.0-rc.1

docker version (if using docker)

Client: Docker Engine - Community
 Version:           24.0.6
 API version:       1.43
 Go version:        go1.20.7
 Git commit:        ed223bc
 Built:             Mon Sep  4 12:31:44 2023
 OS/Arch:           linux/amd64
 Context:           default

Server: Docker Engine - Community
 Engine:
  Version:          24.0.6
  API version:      1.43 (minimum version 1.12)
  Go version:       go1.20.7
  Git commit:       1a79695
  Built:            Mon Sep  4 12:31:44 2023
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.6.24
  GitCommit:        61f9fd88f79f081d64d6fa3bb1a0dc71ec870523
 runc:
  Version:          1.1.9
  GitCommit:        v1.1.9-0-gccaecfc
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

uname

Linux isaacs-desktop 5.15.0-86-generic #96-Ubuntu SMP Wed Sep 20 08:23:49 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

kubectl (if using Kubernetes)

N/A

repo state (if built from source)

N/A

runsc debug logs (if available)

N/A, issue isn't with syscalls/sandbox

isaac-mcfadyen avatar Oct 17 '23 16:10 isaac-mcfadyen

runsc does not implement the update command. We should add support for it, but probably won't have cycles. Happy to get OSS contributions for this!

References: runc man page: https://github.com/opencontainers/runc/blob/main/man/runc-update.8.md runc implementation: https://github.com/opencontainers/runc/blob/main/update.go

ayushr2 avatar Oct 23 '23 17:10 ayushr2

Hi, I'd love to contribute to this feature!

kevinmingtarja avatar Dec 09 '23 10:12 kevinmingtarja

Cool! Do you need help getting started?

EtiennePerot avatar Dec 11 '23 21:12 EtiennePerot

Hi! Yeah perhaps do you have a high level overview on which parts of the codebase this change will touch and where to start. Thanks!

kevinmingtarja avatar Dec 12 '23 00:12 kevinmingtarja

I would start by looking through https://cs.opensource.google/gvisor/gvisor/+/master:runsc/cmd/ directory. All runsc commands are implemented there. Then see the runc implementation to understand what the update command does and implement the same in runsc.

ayushr2 avatar Dec 12 '23 05:12 ayushr2

Got it, that sounds good. Thank you for the pointers!

kevinmingtarja avatar Dec 12 '23 06:12 kevinmingtarja

Hi folks, some updates! I just created a draft PR for this.

I've made it work with cgroup v2 on my local (for cgroup v1 it's still a stub), and wanted to get some high level feedback first on the implementation before I proceed, to make sure I'm on the right track. I'll be adding tests as well over the next few days while waiting for feedback. Thanks!

Sample run (editing cpuset-cpus):

$ docker run --runtime=$(git branch --show-current)-d --rm --name=test-update --cpus=2 -p 8888:80 -d nginx
b3a6ed3787eda646a5d3dafcf03bcc5795cf6fb1a4449918543594134a4def10
$ cat /sys/fs/cgroup/system.slice/docker-b3a6ed3787eda646a5d3dafcf03bcc5795cf6fb1a4449918543594134a4def10.scope/cpuset.cpus

$ docker update --cpuset-cpus 1,2,5 b3a6ed3787eda646a5d3dafcf03bcc5795cf6fb1a4449918543594134a4def10
b3a6ed3787eda646a5d3dafcf03bcc5795cf6fb1a4449918543594134a4def10
$ cat /sys/fs/cgroup/system.slice/docker-b3a6ed3787eda646a5d3dafcf03bcc5795cf6fb1a4449918543594134a4def10.scope/cpuset.cpus
1-2,5
$ sudo cat /var/run/docker/runtime-runc/moby/b3a6ed3787eda646a5d3dafcf03bcc5795cf6fb1a4449918543594134a4def10_sandbox:b3a6ed3787eda646a5d3dafcf03bcc5795cf6fb1a4449918543594134a4def10.state | jq | grep "1,2,5"
          "cpus": "1,2,5"

kevinmingtarja avatar Jan 09 '24 18:01 kevinmingtarja

wanted to get some high level feedback first on the implementation before I proceed, to make sure I'm on the right track

I think you are on the right track!

ayushr2 avatar Jan 09 '24 19:01 ayushr2