Skip to content
This repository has been archived by the owner on Mar 25, 2018. It is now read-only.

Quick how-to use this containers #6

Merged
merged 1 commit into from Dec 10, 2014
Merged

Quick how-to use this containers #6

merged 1 commit into from Dec 10, 2014

Conversation

ghostbar
Copy link
Contributor

@ghostbar ghostbar commented Dec 6, 2014

I think this will be useful for any contributor to the project.

@rvagg
Copy link
Member

rvagg commented Dec 6, 2014

This is the full command we have in Jenkins FYI, might be worth updating with some of these options:

docker run -a stdin -a stdout -a stdin -t --rm -v `pwd`:/opt/node/ node-forward-ci:node-${DISTRO}

@rvagg
Copy link
Member

rvagg commented Dec 6, 2014

@ghostbar would you like to become a contributor to the containers work? I can add you as a committer here and give you access to the containers build servers to help keep them maintained if you're up for it.

See the contributors stuff in io.js although I'm not a fan of the strict git stuff so we won't be following any of that here.

@ghostbar
Copy link
Contributor Author

ghostbar commented Dec 7, 2014

Awesome, yes @rvagg I would happily help out as a commiter helping to keep them updated

@ghostbar
Copy link
Contributor Author

ghostbar commented Dec 7, 2014

@rvagg I would suggest copying the files with ADD and build a container image + run it, which will not consume too much time on jenkins. The reason of the suggestion is that it creates permission issues with the host volumes and you won't necessarily want the output of the make (eg: out/) in your host but only the stdout messages. Am I right? Not only that, adding a volume inherits the permissions as well, so the command I was using gave a permission issues because iojs user didn't have enough permissions to write in that directory, and if I change permissions of the mounted volume that will be reflected on the host directory.

Pros of creating an image per each build:

  • Image will be up-to-date with all the packages from the system (the most important ones: libc6 and gcc/g++).
  • Wont leave traces of the build in the host system (If used with ADD on the Dockerfile).
  • Wont have permissions issues.

Cons of creating an image per each build:

  • A little more time, but more time nonetheless.
  • If jenkins makes parallel builds in the same system will have to find a way to avoid collision on the image names.

How could this be done?:

  • A very cool option would be to git clone it directly from github on the build step per image.
  • Creating a data-container than then would be used as the base-point for mounting this volume to the different hosts. (This would be: download once, use it as much as required) (PS: This is my preferred version)

Option 1 can handle parallel builds, Option 2 doesn't.

Let me know your thoughts.

@rvagg rvagg mentioned this pull request Dec 8, 2014
@retrohacker
Copy link
Contributor

@ghostbar unsure about using ADD for these files. Just to make sure I have the context right, you are suggesting putting the source code directly into the image instead of mounting it as a volume, correct?

@rvagg
Copy link
Member

rvagg commented Dec 9, 2014

I was pondering this yesterday and thinking that perhaps a better option is to try and get a --builddir or similar into io.js so that the object files can be dumped elsewhere. This might be tricky with GYP but I really don't know yet.
ADD is going to create quite a bit of overhead here and will make it impractical to run them from Docker Hub and then you'd have to rebuild the image each time you want to test (right?). Another option is to build in a cp -a or similar into the CMD for the images.

@ghostbar
Copy link
Contributor Author

ghostbar commented Dec 9, 2014

Yes, I have been checking and the issue is the permission stuff with the mounted volume. Can't we run make with root user right away? What are the cons? The container should be unprivileged anyway (for this we should make sure that #9 works on an unprivileged container) so building it with the root inside the container shouldn't do harm.

@retrohacker
Copy link
Contributor

Agree on all points @rvagg.

cp might be a good solution.

Also, agree with @ghostbar on letting the build run as root, the only reason I could see otherwise is if we want to make sure the project can be built and pass tests as a non-root user.

@ghostbar
Copy link
Contributor Author

ghostbar commented Dec 9, 2014

Another solution I have seen is somehow firing the container and then with exec or something copying the files on run time.

I don't know what cp you guys are referring to, hope it's not docker cp since that one only works container -> host and not viceversa AFAIK.

@retrohacker
Copy link
Contributor

adding cp -a /opt/iojs/ /internal/iojs as a bootstrap (either as an ENTRYPOINT or as part of CMD) Basically copying the mounted volume into the container at runtime, can take ownership from there.

@rvagg
Copy link
Member

rvagg commented Dec 9, 2014

I didn't want to run as root because I frankly don't trust the Docker security model yet! I don't know if their new privileged / unprivileged system is workable yet and whether it covers enough of the exposed system endpoints mounted by a container. I'm willing to go with advice from you guys from this if you feel confident enough but just keep in mind that the whole point of doing these in containers is to isolate untrusted code that comes in from pull requests so we can run any old crap someone wants to put in a PR, even if it's an rm -rf / in the configure script.

@ghostbar
Copy link
Contributor Author

ghostbar commented Dec 9, 2014

Yeah, I think the cp -a on the CMD is a good solution :-) @wblankenship .

Indeed, @rvagg, running it as a user is a good solution to avoid any zero-day exploit in the docker security model.

@retrohacker
Copy link
Contributor

I'm not a security guy, by any means. Will leave that judgement to somebody else. "Breaking out" of containers has been documented in the wild before: http://blog.docker.com/2014/06/docker-container-breakout-proof-of-concept-exploit/

@rmg
Copy link

rmg commented Dec 9, 2014

What is the time difference between an image per build:

FROM base-build-image
ADD checkout-path /internal/iojs
$ docker build -t build-NN
$ docker run build-NN ...

And copying the source as the first step of the run inside the container:

FROM base-build-image
# assume source is mounted at /vol/src at run time
CMD [ "cp", "-a", "/opt/iojs", "/internal/iojs" ]

docker run -v /checkout/path:/vol/src base-build-image ...

If the difference isn't much, the former seems easier to follow (to me, at least) and might allow for slightly easier postmortems.

A refinement on that might be to put the Dockerfile in an empty directory with a git archive generated tarball and do ADD iojs.tar.gz /internal/iojs, which might be faster for cache (in)validation at least.

@rvagg
Copy link
Member

rvagg commented Dec 9, 2014

The easiest workflow will involve having a directory with the source in it and pointing something to it and say "run this, report errors back". i.e. the current Jenkins setup follows a standard flow of a fresh git checkout for a given commit and then invoking an arbitrary command to build & test. In this case it's just a docker ....

I'm not really fussed about whether it's an ADD or cp -a or whatever but these are the competing objectives as I see it:

  • security--the top objective here and the reason we're even considering Docker
  • ease of setup on the build machines, pulling from Docker Hub seems to be a winner here
  • ease of execution, a single docker run makes sense here but perhaps it's just as easy to do a two-step build & execute?
  • ease of cleanup, docker run has an --rm which is perfect for this because the last thing we want is aufs blobs laying around from each run, a docker build leave an artifact to manually clean up
  • sharability, i.e. allowing people to grab the images or Dockerfile to run tests for themselves--this is not a high priority but kind of nice

Am I missing anything else?

@ghostbar
Copy link
Contributor Author

ghostbar commented Dec 9, 2014

@rvagg, nope, that is it. I'm gonna work on the cp -a part on the CMD some time tonight and drop a PR about it after checking it out on my machine.

@ghostbar
Copy link
Contributor Author

ghostbar commented Dec 9, 2014

This one is ready to go, @rvagg let me know if you're OK with it and I'll merge it; I know @wblankenship already checked it.

@ryanstevens
Copy link

Cool- I like the direction / strategy we are going here. I've been kinda waiting for the dust to settle here before I started building OS X

Not sure how much of this train of thought will translate to OS X inside Virtual Box... But it nonetheless provides solid direction.

iojs/build
===

These images are used to verify pull requests against [https://github.com/iojs/io.js](https://github.com/iojs/io.js)
Copy link
Member

Choose a reason for hiding this comment

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

need to mention libuv here, the goals are the same even though the implementation is slightly different

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call. @ghostbar would you mind opening a pull request for this against another branch on this repo? Would help multiple people work on this branch.

Copy link
Member

Choose a reason for hiding this comment

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

for context, the libuv folk are very diverse and it's completely separate from Node these days so there's a tiny bit of sensitivity about this build project being tied to Node / io.js and not being focused on libuv, so I'd like to make sure they know they are being looked after here and are not an afterthought

@ghostbar ghostbar merged commit a4d4e1f into nodejs:master Dec 10, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants