Skip to content

Dockerfile: Update CRIU version#38378

Closed
rst0git wants to merge 3 commits intomoby:masterfrom
rst0git:master
Closed

Dockerfile: Update CRIU version#38378
rst0git wants to merge 3 commits intomoby:masterfrom
rst0git:master

Conversation

@rst0git
Copy link
Copy Markdown
Contributor

@rst0git rst0git commented Dec 16, 2018

- What I did

  • Copied CRIU dependencies into runtime-dev
  • Updated CRIU version to 3.11
  • Add -j argument to make when building CRIU

- How to verify it

make BIND_DIR=. shell

Inside the container:

mkdir /etc/docker
echo "{\"experimental\": true}" >> /etc/docker/daemon.json
./hack/make.sh binary install-binary
dockerd

In a different terminal:

docker exec -it <container-id> bash

Where <container-id> the can be obtained from docker ps. Then inside the container:

docker run -d --name looper --security-opt seccomp:unconfined busybox  \
         /bin/sh -c 'i=0; while true; do echo $i; i=$(expr $i + 1); sleep 1; done'
docker checkpoint create looper checkpoint1
docker start --checkpoint checkpoint1 looper

- Description for the changelog
The changes in this PR make it easier to test docker's checkpoint/restore functionality with the latest version of CRIU.

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 16, 2018

Codecov Report

Merging #38378 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #38378      +/-   ##
==========================================
+ Coverage   36.54%   36.55%   +0.01%     
==========================================
  Files         608      608              
  Lines       45036    45036              
==========================================
+ Hits        16460    16465       +5     
+ Misses      26295    26292       -3     
+ Partials     2281     2279       -2

Comment thread Dockerfile Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the protobuf things needed for all off runc, criu and docker ?

(Also, are they needed at runtime or only ad build-time)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback, not all of those packages are needed at runtime.
Actually, criu is linked only with librt, libprotobuf-c, libdl, libnl-3 and libnet.
Without these installed at runtime it fails, for example, with:

$ criu --version
criu: error while loading shared libraries: libprotobuf-c.so.1: cannot open shared object file: No such file or directory

I will update the commit to install only the necessary packages.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, instead of installing these packages at runtime, it would be better to only copy the required files (i.e. libnet.so.1, libnl-3.so.200, libprotobuf-c.so.1). I will update the commit accordingly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be better to only copy the required files

@rst0git it might seem like a good idea now, but some time later than libnl-3.so.200 will become libnl-3.so.201 and everything will break, we are going to regret it (to say at least).

One other thing, if you're using a package manager you're sure dependencies are satisfied (say if libprotobuf will suddely require libxyz, package manager will take care of it), and if you're doing it manually, it's a time bomb.

So, please revert to installing needed deps during runtime.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread Dockerfile Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious, IIUC, this will limit the number of jobs at once (as the default is "infinite"), correct?

  -j [N], --jobs[=N]          Allow N jobs at once; infinite jobs with no arg.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the default value, "infinite", can quickly take over all available memory on a large build. Although criu is not very large, -j $(nproc) works well, and it is the approach that is used in the Dockerfile template for travis-ci.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explaining 👍

@rst0git rst0git force-pushed the master branch 2 times, most recently from 1385ac7 to 34a6d91 Compare December 19, 2018 16:41
@derek derek bot added the rebuild/* label Dec 19, 2018
@thaJeztah
Copy link
Copy Markdown
Member

ping @kolyshkin this looks like a PR in your area 👍 🤗

Docker is using CRIU for checkpoint and restore.
For CRIU to work within the dev environment it requires the dynamically
linked dependency libraries libnet-dev, libnl-3-dev, libprotobuf-c0-dev
to be installed.

Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
Signed-off-by: Radostin Stoyanov <rstoyanov1@gmail.com>
@rst0git
Copy link
Copy Markdown
Contributor Author

rst0git commented Dec 30, 2018

Closing in favour of cde23b2

@rst0git rst0git closed this Dec 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants