feat(docker): add docker support with circle-ci #1692

seanmonstar · 2017-03-02T00:36:41Z

This takes over the circle.yml file to be used for our docker publishing, to be similar with our other repos.

@philbooth I'm not certain exactly how the fxa-test repo updates itself, but perhaps instead of using a complete mirror, what if we just made a small repo in the fxa-test org that essentially just has the old circle.yml, and let that run?

philbooth · 2017-03-02T07:16:10Z

I'm not certain exactly how the fxa-test repo updates itself...

There's a cron job running every 10 minutes on an EC2 instance, which does:

git fetch -p origin
git push --mirror

...instead of using a complete mirror, what if we just made a small repo in the fxa-test org that essentially just has the old circle.yml, and let that run?

At the moment, I don't understand how that would work. But if we can make it work, obviously it's fine. Are you volunteering to make it work? 😄

Some of the things I don't understand yet:

How will we ensure the tests run automatically for every branch?
How will we ensure that the committer gets emailed when the tests fail?

Would we need to shift the responsibility for those things into the cron script, or can we still lean on Circle to do it for us?

vladikoff · 2017-03-02T15:11:02Z

@philbooth can we do a conditional, such that IF org is fxa-test THEN run extra_tests.sh ELSE docker

philbooth · 2017-03-02T15:18:22Z

can we do a conditional, such that IF org is fxa-test THEN run extra_tests.sh ELSE docker

What a capital idea! Circle lets you set env vars for builds right? We could set one of those?

vladikoff · 2017-03-02T23:35:01Z

What a capital idea! Circle lets you set env vars for builds right? We could set one of those?

https://circleci.com/docs/1.0/environment-variables/#build-details CIRCLE_PROJECT_USERNAME should work, it might make sense to move all the fxa-test stuff into its own script, but up to you all

seanmonstar · 2017-03-03T00:56:47Z

can we do a conditional, such that IF org is fxa-test THEN run extra_tests.sh ELSE docker

I looked into doing this, and it may not be easily possible. The problem is that we need different directives in the circle.yml file. Unless the docs don't include some critical part (I wouldn't be surprised, Circle's docs have already let me down earlier), there doesn't seem to be a way to conditionally configure the machine or cache_dependencies.

We could either:

Just fill in all the directives for machine and cache_directories for both build modes, and perhaps it's fine? For our docker build, we only need the docker service. Our integration tests require including node on the machine, and some cached directories.

Besides those, it looks like we can check environment variables in any of the pre, post, and override directives.
Maybe we keep the integration yml as something like circle.integration.yml, and in the cron job, override circle.yml before updating fxa-test?

seanmonstar · 2017-03-04T02:09:16Z

Well heyyy! It's working! Some notes I've been keeping:

@jbuck I had to copy over a couple of scripts to run after the npm install step, which involve generating keys for signing assertions and push notifications. However, I started to wonder if auto generating those keys in our image was a good idea, or if they should only be built in the test image.
@philbooth I've modified to the circle.yml to make use of scripts/test-integrations.sh, such that it may still work with fxa-test. Care a look over?
As I had mentioned, we now declare cached directories for dependencies, but since our docker build doesn't use them, Circle is placing a warning about them. Is that harmless?
Lastly, to make sure that Circle only tries to publish to docker when using the docker build, I added owner: mozilla to the deployment directives. @jbuck is that sufficient?

seanmonstar · 2017-03-04T02:12:31Z

Dockerfile-build

+RUN node scripts/gen_vapid_keys.js
+
+USER root
+RUN chown root:root /app/config


I didn't like the idea of leaving the config directory forever writable by the app, cause a mistake on our part could mean changing a config file out from under us?

I don't think you need to worry about changing the permissions here - we'll just ignore the config file

For completeness, this is just setting the permissions of the directory back to what they were because of the COPY.

seanmonstar · 2017-03-04T02:14:28Z

Dockerfile-test

@@ -0,0 +1,3 @@
+FROM fxa-auth-server:build
+ENV FXA_L10N_SKIP 1


This is already downloaded in the Dockerfile-build npm install step, but by the time we get here, we no longer have permission to remove files from node_modules, and the download_l10n.sh script causes npm to unconditionally remove a directory to try to replace it.

So, I settled on skipping it here.

seanmonstar · 2017-03-04T02:15:11Z

circle.yml

    - |
-      if [ -e fxa-js-client ]; then


The changes to this file are probably a lot easier to grok if using the split diff view.

seanmonstar · 2017-03-04T02:16:13Z

scripts/test-integrations.sh

+
+    cd fxa-content-server && CONFIG_FILES=server/config/local.json,server/config/production.json node_modules/.bin/grunt serverproc:dist 2>&1 | tee $HOME/fxa-content-server.log &
+
+    cd fxa-oauth-server && LOG_LEVEL=error NODE_ENV=dev node ./bin/server 2>&1 | tee $HOME/fxa-oauth-server.log &


I wasn't certain if the & suffix was enough to mimic the background directive for Circle...

It's not unfortunately, the circle docs suggest that we probably need to use nohup, but I haven't found the correct incantation to get it all working yet. Most of the servers are up and running when I SSH into the box post-build, but the content server is always dead. Haven't managed to make it dump any useful error information yet either.

seanmonstar · 2017-03-04T02:17:07Z

scripts/test-integrations.sh

+  test)
+    cd fxa-js-client && npm run test-local
+
+    cd fxa-content-server && retry -n 1 -- node_modules/.bin/intern-runner config=tests/intern_functional_circle firefoxBinary=$HOME/fxa-auth-server/firefox/firefox fxaProduction=true


There was a parallel directive for this command in the circle.yml, but I wasn't certain why, and what exactly it did, so this line currently just executes serially.

It only costs us a bit more time to wait for the build, not the end of the world if we lose it.

philbooth · 2017-03-06T13:57:57Z

@seanmonstar, I had to fix a few things to get the cross-repo testing working, you can see my changes in phil/dockerize.

Of course, I say "working", but really I mean "working-ish" because the content server isn't running so the tests all fail still. See my comment about nohup inline, if you have any good ideas for making it work feel free to suggest / or try them out and check the build results in Circle.

jbuck · 2017-03-06T18:17:17Z

@jbuck I had to copy over a couple of scripts to run after the npm install step, which involve generating keys for signing assertions and push notifications. However, I started to wonder if auto generating those keys in our image was a good idea, or if they should only be built in the test image.

Yeah, this is kinda annoying but it's the easiest way to ensure tests run

Lastly, to make sure that Circle only tries to publish to docker when using the docker build, I added owner: mozilla to the deployment directives. @jbuck is that sufficient?

Yep!

jbuck · 2017-03-06T18:21:24Z

Dockerfile-build

@@ -0,0 +1,30 @@
+FROM mhart/alpine-node:4.8.0
+
+RUN apk update && apk upgrade && apk add git make gcc g++ linux-headers openssl python


Damn, sucks that we need all these dependencies.

Could this be reduced to apk add --no-cache git make gcc g++ linux-headers openssl python ?

vladikoff · 2017-03-06T20:53:11Z

from mtg: content server needs a fix for nohup.
Once we figure that out we should be able to merge this...

philbooth · 2017-03-07T20:52:33Z

Just so I know where we are, did the content server issue get fixed or should I open an issue for it?

vladikoff · 2017-03-07T20:54:29Z

Ugh the circle thing didn't run at all, the 2 listed checks were travis

seanmonstar · 2017-03-07T20:55:14Z

I merged your branch into this, but I think it was still failing, right?

philbooth · 2017-03-07T20:55:23Z

Ugh the circle thing didn't run at all, the 2 listed checks were travis

No dramas, I'm happy to try and get it fixed up asynchronously. And yeah, I switched off circle again after our conversation yesterday, was that the wrong thing to do?

seanmonstar · 2017-03-07T20:55:30Z

Wat

vladikoff · 2017-03-07T20:56:12Z

@philbooth nah, no worries. I checked the code and the thing showed all green, but it was 2 instead of 3. This is running Circle now: #1703

seanmonstar added the waffle:active label Mar 2, 2017

seanmonstar force-pushed the dockerize branch 8 times, most recently from 827123f to 99aa4c0 Mar 2, 2017

philbooth added waffle:review and removed waffle:active labels Mar 2, 2017

seanmonstar force-pushed the dockerize branch 2 times, most recently from 2f45c65 to 9c66e23 Mar 2, 2017

seanmonstar force-pushed the dockerize branch 12 times, most recently from a4503e3 to 37cdf01 Mar 3, 2017

seanmonstar force-pushed the dockerize branch from fa0c173 to bb2fee4 Mar 4, 2017

seanmonstar reviewed Mar 4, 2017

View changes

circle.yml

- |

if [ -e fxa-js-client ]; then

This comment has been minimized.

Sign in to view

seanmonstar Mar 4, 2017
Author Member

The changes to this file are probably a lot easier to grok if using the split diff view.

seanmonstar reviewed Mar 4, 2017

View changes

jbuck reviewed Mar 6, 2017

View changes

vladikoff assigned philbooth, jbuck, seanmonstar and vladikoff Mar 6, 2017

feat(docker): add docker support with circle-ci

3c04db2

seanmonstar force-pushed the dockerize branch from bb2fee4 to 3c04db2 Mar 7, 2017

vladikoff merged commit 4fbc25f into master Mar 7, 2017
2 checks passed

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

continuous-integration/travis-ci/push The Travis CI build passed
Details

vladikoff removed the waffle:review label Mar 7, 2017

vladikoff deleted the dockerize branch Mar 7, 2017

philbooth mentioned this pull request Mar 7, 2017

Cross-repo testing is broken #1704

Closed

seanmonstar mentioned this pull request Mar 10, 2017

Add dockerflow build logic #1660

Closed

rfk added this to the FxA-67: dockerflow everywhere milestone Apr 12, 2017

mozilla / fxa-auth-server Archived

feat(docker): add docker support with circle-ci #1692

feat(docker): add docker support with circle-ci #1692

seanmonstar commented Mar 2, 2017 •

edited by vladikoff

philbooth commented Mar 2, 2017 •

edited

vladikoff commented Mar 2, 2017

philbooth commented Mar 2, 2017

vladikoff commented Mar 2, 2017

seanmonstar commented Mar 3, 2017

seanmonstar commented Mar 4, 2017

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

philbooth commented Mar 6, 2017 •

edited

jbuck commented Mar 6, 2017

This comment has been minimized.

vladikoff commented Mar 6, 2017

philbooth commented Mar 7, 2017 •

edited

vladikoff commented Mar 7, 2017

seanmonstar commented Mar 7, 2017

philbooth commented Mar 7, 2017 •

edited

seanmonstar commented Mar 7, 2017

vladikoff commented Mar 7, 2017

		@@ -0,0 +1,3 @@
		FROM fxa-auth-server:build
		ENV FXA_L10N_SKIP 1


		cd fxa-content-server && CONFIG_FILES=server/config/local.json,server/config/production.json node_modules/.bin/grunt serverproc:dist 2>&1 \| tee $HOME/fxa-content-server.log &

		cd fxa-oauth-server && LOG_LEVEL=error NODE_ENV=dev node ./bin/server 2>&1 \| tee $HOME/fxa-oauth-server.log &

		@@ -0,0 +1,30 @@
		FROM mhart/alpine-node:4.8.0

		RUN apk update && apk upgrade && apk add git make gcc g++ linux-headers openssl python

mozilla / fxa-auth-server Archived

feat(docker): add docker support with circle-ci #1692

feat(docker): add docker support with circle-ci #1692

Conversation

seanmonstar commented Mar 2, 2017 • edited by vladikoff

philbooth commented Mar 2, 2017 • edited

vladikoff commented Mar 2, 2017

philbooth commented Mar 2, 2017

vladikoff commented Mar 2, 2017

seanmonstar commented Mar 3, 2017

seanmonstar commented Mar 4, 2017

This comment has been minimized.

seanmonstar Mar 4, 2017 Author Member

This comment has been minimized.

jbuck Mar 6, 2017 Member

This comment has been minimized.

seanmonstar Mar 7, 2017 Author Member

This comment has been minimized.

seanmonstar Mar 4, 2017 Author Member

This comment has been minimized.

seanmonstar Mar 4, 2017 Author Member

This comment has been minimized.

seanmonstar Mar 4, 2017 Author Member

This comment has been minimized.

philbooth Mar 6, 2017 Contributor

This comment has been minimized.

seanmonstar Mar 4, 2017 Author Member

This comment has been minimized.

philbooth Mar 6, 2017 Contributor

philbooth commented Mar 6, 2017 • edited

jbuck commented Mar 6, 2017

This comment has been minimized.

jbuck Mar 6, 2017 Member

vladikoff commented Mar 6, 2017

philbooth commented Mar 7, 2017 • edited

vladikoff commented Mar 7, 2017

seanmonstar commented Mar 7, 2017

philbooth commented Mar 7, 2017 • edited

seanmonstar commented Mar 7, 2017

vladikoff commented Mar 7, 2017

seanmonstar commented Mar 2, 2017 •

edited by vladikoff

philbooth commented Mar 2, 2017 •

edited

seanmonstar Mar 4, 2017
Author Member

jbuck Mar 6, 2017
Member

seanmonstar Mar 7, 2017
Author Member

seanmonstar Mar 4, 2017
Author Member

seanmonstar Mar 4, 2017
Author Member

seanmonstar Mar 4, 2017
Author Member

philbooth Mar 6, 2017
Contributor

seanmonstar Mar 4, 2017
Author Member

philbooth Mar 6, 2017
Contributor

philbooth commented Mar 6, 2017 •

edited

jbuck Mar 6, 2017
Member

philbooth commented Mar 7, 2017 •

edited

philbooth commented Mar 7, 2017 •

edited