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

Build dependencies with docker #1200

Merged
merged 4 commits into from
Apr 12, 2018
Merged

Build dependencies with docker #1200

merged 4 commits into from
Apr 12, 2018

Conversation

gbirke
Copy link
Member

@gbirke gbirke commented Mar 14, 2018

Use Docker images to compile the frontend assets and install PHP dependencies.

This allows the build machine ("local") to be free of application
dependencies (PHP with all its extensions, node.js). It only needs Git
and Docker.

@gbirke gbirke requested a review from wiese March 14, 2018 17:57
Copy link
Contributor

@wiese wiese left a comment

Choose a reason for hiding this comment

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

PHP part is awesome. +2

Have doubts concerning the node part. We briefly talked about permission problems, to which this may be a solution, but I would hope there are others.

In dev, when developing cat17, my npm was
docker run --rm -v $PWD/skins/cat17:/data:delegated -v ~/.npm:/npm-cache -e "REDUX_LOG=on" digitallyseamless/nodejs-bower-grunt npm run build-assets - so no custom docker image. Unless we really need to change this I would try and stick with this (not having to build custom image).

Side note: In my dream world build steps in dev and on build are identical (safe for a .env file maybe) - and AFAIK none of us is using ansible in dev. Hence the make-approach (e.g. fundraising-memberships) as the lowest common denominator was the closest to a perfect solution in that regard I have seen. Would it not be worth striving for simply calling something akin to make js from ansible?

@JeroenDeDauw
Copy link
Contributor

Would it not be worth striving for simply calling something akin to make js from ansible?

Sounds like such a worthwhile to me

@gbirke
Copy link
Member Author

gbirke commented Mar 15, 2018

The reason why I opted for the custom build image was user permissions, especially for the npm install part. The desired outcome is that all files that are installed and generated belong to the user who ran the build command.

If you try to add --user $(id -u):$(id -g) to your command line, you'll get all kinds of errors during npm install, because the user with that id does not exist in the /etc/passwd file of the Docker image and has no writable home directory in the container. Both can be faked via volumes, but it complicates matters more than it simplifies, IMHO. Please have a look at the new commit and tell me what you think of it.

@wiese
Copy link
Contributor

wiese commented Mar 16, 2018

Nice findings and workaround. I may not look beautiful, but it clearly separates the tool's build time from our application's build time - what I missed in the custom image approach.

I agree that having to pass in all that information at run time feels cumbersome, OTOH you took away all the parts that could be taken away while keeping it functional. What remains is needed to work around current(?) limitations in node/npm. Would be great if we could identify open tickets (sure there are with docker gaining more and more popularity) to be able to regularly check them to see if our mitigation is still needed.

Questions

  • PR make creates <PROJECT>/tmp while ansible and blog post use /tmp - oversight or is there more to it? I'd prefer /tmp if possible.
  • ansible still does not invoke make and as such the building steps and their order are not defined in one place (which I imagined our Makefile to be) and could diverge in case of future oversights. Are there reasons not to go down this road?

@gbirke
Copy link
Member Author

gbirke commented Mar 16, 2018

PR make creates /tmp while ansible and blog post use /tmp - oversight or is there more to it? I'd prefer /tmp if possible.

One reason is that creating TMPFILE in a Makefile, without conflicting with other software that uses /tmp. was a bit complicated. When I finally googled the right invocation

TMPFILE := $(shell mktemp -d -t npm-install)

I ran into a peculiar Docker for Mac issue, where the tmp dir can't be selected as a folder that's accessible to the Docker environment, because it's a symlink that's resolved in the configuration GUI but not in the -v flag (or the other way around).

ansible still does not invoke make and as such the building steps and their order are not defined in one place (which I imagined our Makefile to be) and could diverge in case of future oversights. Are there reasons not to go down this road?

Just a vague notion of "I don't know if I want to have make as a dependency for deployment". But the more I think of it, the more sense it makes. Especially, when we finally come around to use #959

I'll do a followup.

@gbirke gbirke force-pushed the build-with-docker branch from 4982694 to a999b1a Compare March 19, 2018 17:56
@gbirke gbirke changed the title Build dependencies with docker [WIP] Build dependencies with docker Mar 19, 2018
@gbirke gbirke force-pushed the build-with-docker branch 2 times, most recently from c03c699 to 8eb720a Compare March 20, 2018 14:22
@gbirke gbirke changed the title [WIP] Build dependencies with docker Build dependencies with docker Mar 20, 2018
@gbirke gbirke force-pushed the build-with-docker branch from 8eb720a to 82b7925 Compare March 20, 2018 14:42
@gbirke
Copy link
Member Author

gbirke commented Mar 20, 2018

Now the Makefile can be used for Ansible deployment and local installation. To make it work, I had to parameterize it heavily, so I don't know if that's better than a separate playbook file that mimicks the Makefile, but with the right parameters.

gbirke added 3 commits March 20, 2018 16:46
Call "composer install" with a Docker image.

Add Docker image that builds the skin assets with node.js
Due to npm permission madness (npm needs to be run as root, but then all
its generated files are owned by root), we use a custom Dockerfile.

This allows the build machine ("local") to be free of application
dependencies (PHP with all its extensions, node.js). It only needs Git
and Docker.
The `include` task is deprecated, `include_tasks` needs to be used
instead.

The `working_dir` parameter for `docker_image` from the previous commit
is only available in Ansible 2.4
@gbirke gbirke force-pushed the build-with-docker branch from 82b7925 to ffc8905 Compare March 20, 2018 15:46
Add make targets that use Docker to install PHP and Node dependencies.
Use make targets in ansible build task to make deploy and local dev more
consistent.
@gbirke gbirke force-pushed the build-with-docker branch from ffc8905 to 07a0f15 Compare March 20, 2018 15:52
gbirke added a commit that referenced this pull request Apr 4, 2018
Temporary fix until #1200 is merged, otherwise Travis won't install npm
packages.
gbirke added a commit that referenced this pull request Apr 5, 2018
Temporary fix until #1200 is merged, otherwise Travis won't install npm
packages.
@gbirke
Copy link
Member Author

gbirke commented Apr 9, 2018

Ping @wiese : Could you have another look at my changes since the last review? Is this now more in the direction you imagined?

@gbirke
Copy link
Member Author

gbirke commented Apr 12, 2018

Self-merging for now to be able to deploy, but feel free to comment and request changes ...

@gbirke gbirke merged commit aac566e into master Apr 12, 2018
@gbirke gbirke deleted the build-with-docker branch April 12, 2018 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants