Add the option to use docker containers #181

achow101 · 2018-05-29T04:19:50Z

This PR adds another VM type: docker containers.

Since vmbuilder appears to be borked in recent versions (at least for how we are using it) and people have had issues with lxc in the past (including myself), I have added the option of using docker containers instead of LXC, KVM, or vbox. Setting up and using the docker containers is just as easy and seamless as the setup for lxc and kvm, except it should actually work since the docker people know what they are doing and have made it easy to actually setup and interact with docker containers.

make-base-vm now has a --docker switch to enable docker containers. Like with lxc, a USE_DOCKER environment variable needs to be set for the other scripts to use docker.

I have tested this with a build of Bitcoin Core 0.16.1rc1. The host is Ubuntu 18.04 while the container uses Ubuntu 14.04 as specified by the Bitcoin Core gitian descriptors. The binaries built matched those built by others using lxc or kvm (I am currently unable to compare with my own lxc and kvm builds because neither work for me).

While this PR supports using debian instead of ubuntu, I have no idea whether it actually works and I don't have anything to test that with.

Requires a gitian-builder version with devrandom/gitian-builder#181

codablock

👍 Your timing couldn't be better :) I was already thinking about implementing the same. Found one error so far (see inline comments).

I'm still testing this and can later report if I was able to create deterministic builds.

codablock · 2018-05-29T11:40:25Z

bin/make-base-vm

@@ -70,6 +71,10 @@ if [ $# != 0 ] ; then
        VBOX=1
        shift 1
        ;;
+      --docker)
+        DOCKER=1


Doesn't have a default value set on top of this script (like LXC and KVM). This later makes $DOCKER = "1" fail when not using docker.

maflcko

Concept ACK

maflcko · 2018-05-29T15:52:55Z

bin/make-base-vm

+
+ENV DEBIAN_FRONTEND=noninteractive
+RUN apt-get update
+RUN apt-get --no-install-recommends -y install $addpkg


This should be combined with the previous statement to avoid rare caching issues that would cause the install to fail.

C.f. https://docs.docker.com/develop/develop-images/dockerfile_best-practices/#run

devrandom · 2018-05-29T18:14:31Z

I like this in concept, but I have some concerns about trust.

I worry that there is no way to validate that the pulled docker image (e.g. Debian) hasn't been tampered with. If everybody builds using docker, that will become a single point of potential compromise.

Any thoughts on this?

devrandom · 2018-05-29T18:15:33Z

To clarify, I contrast this with distribution supplied CD/DVD images, which (I believe) have more eyes on them.

achow101 · 2018-05-29T20:03:19Z

I worry that there is no way to validate that the pulled docker image (e.g. Debian) hasn't been tampered with. If everybody builds using docker, that will become a single point of potential compromise.

The main information page for the Ubuntu docker images is at https://hub.docker.com/r/_/ubuntu/. This page has links to the repo which contains the stuff for building the ubuntu docker images, including the Ubuntu images from Canonical. So you can clone that repo, build the docker image yourself, and use docker image inspect to verify that the built image matches the one downloaded from the docker image repos.

The only problem with this is that the docker image used is from https://partner-images.canonical.com/core/ which is not where the normal official Ubuntu images are downloaded from.

For debian, the main information page has a section that describes how the images are made deterministically so you can do the same process and then check that the build docker image matches the downloaded one.

To clarify, I contrast this with distribution supplied CD/DVD images, which (I believe) have more eyes on them.

That is certainly the case. I don't think these docker images have as many eyes on them as the normal distributions.

You can also specify a specific image by its SHA256 hash, so you can verify that a particular image is legit, and then just use that one only by specifying its hash.

devrandom · 2018-05-30T00:37:50Z

That's very good to know, thank you.

devrandom · 2018-05-30T00:42:57Z

libexec/start-target

@@ -34,4 +36,7 @@ case $VMSW in
    VBoxManage startvm "Gitian-${2}" --type headless
    echo "Gitian-${2}" > var/target.vmname
    ;;
+  DOCKER)
+    docker run -t -d -P --name gitian-target base-$SUFFIX:latest > /dev/null


Why -t - is the pty used somehow?

Why -P - we're not exposing any ports?

-t is necessary for the container to stay up.

-P is not necessary, I have removed it.

devrandom · 2018-05-30T00:44:38Z

bin/make-base-vm

+
+RUN useradd -ms /bin/bash -U ubuntu
+USER ubuntu:ubuntu
+WORKDIR /home/ubuntu


Is that right, or should it be $DISTRO on these three lines instead of ubuntu?

That should be $DISTRO, fixed.

codablock · 2018-05-30T07:02:40Z

bin/make-base-vm

+FROM $DISTRO:$SUITE
+
+ENV DEBIAN_FRONTEND=noninteractive
+RUN apt-get update && apt-get --no-install-recommends -y install $addpkg


Does it make sense to run a dist-upgrade here as well? This seems to be done later by gitian-builder nevertheless, but doing it here as well would speed up the later upgrade.

I think we should keep the base vm image as close to what lxc and kvm modes would generate. AFAICT, they don't do dist-upgrade either.

codablock · 2018-05-30T07:04:39Z

bin/make-base-vm

+CMD ["/bin/bash"]
+EOF
+
+  docker build -f $OUT.Dockerfile -t $OUT .


Adding --pull here would also speed up later dist-upgrades.

Requires a gitian-builder version with devrandom/gitian-builder#181 This will also switch to use Docker by default.

* gitian-build.sh: fix signProg being recognized as two parameters * Support docker based Gitian builds Requires a gitian-builder version with devrandom/gitian-builder#181 This will also switch to use Docker by default. * Switch back to using lxc as default until upstream gitian-builder is ready

devrandom · 2018-05-30T22:54:04Z

bin/make-base-vm

+USER $DISTRO:$DISTRO
+WORKDIR /home/$DISTRO
+
+CMD ["/bin/bash"]


I assume this is why there is the -t flag. If you want it to just stay up, why not explicitly have a wait+sleep loop here? It seems cleaner.

I don't think we can do that. The line is supposed to be of the form CMD [“executable”, “param1”, “param2”…]. I don't know how such a loop would be written in that form.

CMD ["bash", "-c", "while true; do sleep 1; done"] should work

Actually, you can do CMD ["sleep", "infinity"]

make-base-vm can be run with --docker to create a dockerfile and docker image for the specified arch and suite.

start-target creates and starts a docker container from the image created by make-base-vm stop-target stops and removes the docker container from start-target copy-from-target, copy-to-target, and on-target can execute commands and copy files to and from the docker container started by start-target. make-clean-vm does nothing because nothing needs to be done to make a clean docker container.

devrandom

Will merge in the next couple of days.

devrandom · 2018-05-31T18:11:49Z

After reading
https://docs.docker.com/engine/security/trust/content_trust/#image-tags-and-content-trust it seems we should provide a way to specify the digest that will go into FROM $DISTRO:$SUITE@$DIGEST.

I'll go ahead and merge this, but would be great if a PR could be submitted for that, and Bitcoin and other Gitian descriptors are updated to specify the digest.

27d0130 update gitignore for docker (Andrew Chow) 8847ca1 update readme for docker (Andrew Chow) 3934150 Have libexec scripts be able to communicate with a docker container (Andrew Chow) 1ca59fd Option for make-base-vm to create docker image (Andrew Chow)

achow101 · 2018-05-31T18:16:56Z

it seems we should provide a way to specify the digest that will go into FROM $DISTRO:$SUITE@$DIGEST.

Well $SUITE could include the digest too, e.g. ubuntu@sha256:b8855dc848e2622653ab557d1ce2f4c34218a9380cceaa51ced85c5f3c8eb201

devrandom · 2018-05-31T18:44:03Z

That may confuse the user, since it will stop working if they switch to another virtualization. Seems like it's virtualization specific, so maybe it should be something like DOCKER_IMAGE_DIGEST.

devrandom · 2018-05-31T18:44:44Z

To be more precise, the Gitian descriptors were meant to be virtualization tech independent.

achow101 · 2018-05-31T18:51:40Z

Ah, good point. I will open a PR for that later today or tomorrow.

devrandom · 2018-06-01T05:01:41Z

Great.

* gitian-build.sh: fix signProg being recognized as two parameters * Support docker based Gitian builds Requires a gitian-builder version with devrandom/gitian-builder#181 This will also switch to use Docker by default. * Switch back to using lxc as default until upstream gitian-builder is ready

achow101 requested a review from devrandom as a code owner May 29, 2018 04:19

achow101 force-pushed the docker branch from e70057a to 898fa93 Compare May 29, 2018 04:23

codablock added a commit to codablock/dash that referenced this pull request May 29, 2018

Support docker based Gitian builds

a4293fa

Requires a gitian-builder version with devrandom/gitian-builder#181

codablock reviewed May 29, 2018

View reviewed changes

codablock mentioned this pull request May 29, 2018

Add docker support when doing Gitian builds dashpay/dash#2084

Merged

achow101 force-pushed the docker branch from 898fa93 to 86ab408 Compare May 29, 2018 15:40

maflcko approved these changes May 29, 2018

View reviewed changes

achow101 force-pushed the docker branch from 86ab408 to 0b5ec57 Compare May 29, 2018 16:36

devrandom reviewed May 30, 2018

View reviewed changes

achow101 force-pushed the docker branch 2 times, most recently from f081e69 to 0b44462 Compare May 30, 2018 01:24

codablock reviewed May 30, 2018

View reviewed changes

codablock added a commit to codablock/dash that referenced this pull request May 30, 2018

Support docker based Gitian builds

f208660

Requires a gitian-builder version with devrandom/gitian-builder#181 This will also switch to use Docker by default.

achow101 force-pushed the docker branch from 0b44462 to 451a57c Compare May 30, 2018 15:06

devrandom reviewed May 30, 2018

View reviewed changes

achow101 added 4 commits May 30, 2018 20:17

Option for make-base-vm to create docker image

1ca59fd

make-base-vm can be run with --docker to create a dockerfile and docker image for the specified arch and suite.

update readme for docker

8847ca1

update gitignore for docker

27d0130

achow101 force-pushed the docker branch from 451a57c to 27d0130 Compare May 31, 2018 00:17

devrandom approved these changes May 31, 2018

View reviewed changes

devrandom merged commit 27d0130 into devrandom:master May 31, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the option to use docker containers #181

Add the option to use docker containers #181

achow101 commented May 29, 2018 •

edited

Loading

codablock left a comment

codablock May 29, 2018

achow101 May 29, 2018

maflcko left a comment

maflcko May 29, 2018

achow101 May 29, 2018

devrandom commented May 29, 2018

devrandom commented May 29, 2018 •

edited

Loading

achow101 commented May 29, 2018 •

edited

Loading

devrandom commented May 30, 2018

devrandom May 30, 2018

achow101 May 30, 2018

devrandom May 30, 2018

achow101 May 30, 2018

codablock May 30, 2018

achow101 May 30, 2018

codablock May 30, 2018

achow101 May 30, 2018

devrandom May 30, 2018

achow101 May 30, 2018

devrandom May 31, 2018

devrandom May 31, 2018

achow101 May 31, 2018

devrandom left a comment

devrandom commented May 31, 2018

achow101 commented May 31, 2018

devrandom commented May 31, 2018

devrandom commented May 31, 2018

achow101 commented May 31, 2018

devrandom commented Jun 1, 2018

Add the option to use docker containers #181

Add the option to use docker containers #181

Conversation

achow101 commented May 29, 2018 • edited Loading

codablock left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maflcko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devrandom commented May 29, 2018

devrandom commented May 29, 2018 • edited Loading

achow101 commented May 29, 2018 • edited Loading

devrandom commented May 30, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devrandom left a comment

Choose a reason for hiding this comment

devrandom commented May 31, 2018

achow101 commented May 31, 2018

devrandom commented May 31, 2018

devrandom commented May 31, 2018

achow101 commented May 31, 2018

devrandom commented Jun 1, 2018

achow101 commented May 29, 2018 •

edited

Loading

devrandom commented May 29, 2018 •

edited

Loading

achow101 commented May 29, 2018 •

edited

Loading