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.

Sign up for GitHub

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

Jump to bottom

First stab at full build in docker #4482

Closed
wants to merge 2 commits into from
Closed

First stab at full build in docker #4482

wants to merge 2 commits into from

Conversation

wpears
Copy link
Member

@wpears wpears commented Aug 31, 2018

Closes #4468
Closes #4462

This is my first stab at running the full build in the backend. From a fresh repo, with the cfgov-refresh_python image removed, you should be able to simply docker-compose up to end up with working dockerized cfgov. (no front-end stuff, the .env_SAMPLE is copied over by default). The only thing you'd need to do afterwards to check it out at localhost:8000, is run ./shell.sh and ./refresh-data.sh with a dump once inside. This process could be done automatically (say, based on the existence of a data.tar.gz file or something) but I've elided that for now.

Satellite apps are currently unsupported as there are some decisions to be made around what is optional/what should be baked into the image that I didn't want to cloud this PR. That is, should optional-public be installed at build time? The argument for is to move all the painful installation stuff into one place and to then have everything available by default, the argument against is to limit the pain of that first image build. I'm more in the for camp, but reasonable people may disagree (and please do with comments on this PR).

If installation of the wheels is done at image build time, then, on container startup, we'd walk develop-apps, pip uninstall as needed, and pip install -e each app there. This preserves the ability to locally edit satellites without any mucking around inside the container, while also not affecting anyone who isn't editing satellites.

I'll be out for two weeks starting tomorrow 😅 , but I'll likely peep in on this issue to address questions/comments in the evenings over the next fews days.

@rosskarchner
Copy link
Member

I gave this a try, and it crashed on some of the gulp stuff:

[15:18:26] 'scripts:apps' errored after 66 ms
[15:18:26] Error: Cannot find module 'workbox-build'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:581:15)
    at Function.Module._load (internal/modules/cjs/loader.js:507:25)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at Object.<anonymous> (/src/cfgov/unprocessed/apps/regulations3k/webpack-config.js:10:22)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Module.require (internal/modules/cjs/loader.js:637:17)
    at require (internal/modules/cjs/helpers.js:20:18)
    at apps.forEach.app (/src/gulp/tasks/scripts.js:204:26)
    at Array.forEach (<anonymous>)
    at scriptsApps (/src/gulp/tasks/scripts.js:191:8)
    at taskWrapper (/src/node_modules/undertaker/lib/set-task.js:13:15)
[15:18:26] 'scripts' errored after 73 ms
[15:18:26] 'build' errored after 77 ms
[15:18:26] The following tasks did not complete: styles, images, styles:apps, styles:featureFlags, styles:ie, styles:knowledgebaseSpanish, styles:modern, styles:nemo, styles:ondemand, scripts:polyfill, scripts:modern, scripts:external, scripts:nemo, scripts:spanish, scripts:ondemand, images:apps, images:general, styles:knowledgebaseSpanishProd, styles:knowledgebaseSpanishIE8, styles:nemoProd, styles:nemoIE8
[15:18:26] Did you forget to signal async completion?
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! cfgov-refresh@ gulp: `gulp "build"`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the cfgov-refresh@ gulp script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2018-09-11T15_18_26_256Z-debug.log
ERROR: Service 'python' failed to build: The command '/bin/sh -c python /src/get-pip.py &&   /src/setup.sh docker' returned a non-zero code: 1

@rosskarchner
Copy link
Member

(that was after a complete removal of all containers and images)

@wpears
Copy link
Member Author

wpears commented Sep 19, 2018

@rosskarchner, should be fixed.

@wpears wpears mentioned this pull request Sep 24, 2018
Open
@rosskarchner
Copy link
Member

Now it comes up with:

settings.DATABASES is improperly configured. Please supply the ENGINE value. Check settings documentation for more details.

Do I need to set the database in my .env file?

@chosak
Copy link
Member

chosak commented Oct 9, 2018

@rosskarchner yes, see https://github.com/cfpb/cfgov-refresh/blob/master/.env_SAMPLE#L53.

rosskarchner
rosskarchner approved these changes Oct 9, 2018
View reviewed changes
Copy link
Member

@rosskarchner rosskarchner left a comment

Choose a reason for hiding this comment

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

It worked! I was fond of the extend-environment approach, but understand that it was maybe a little too magical.

@chosak
Copy link
Member

chosak commented Oct 9, 2018

we'd walk develop-apps, pip uninstall as needed, and pip install -e each app there

This feels a little bit too magical to me. I'm also in the for camp above and would vote to make optional-public.txt non-optional by including it in requirements/local.txt (and possibly also in tox.ini). If and when developers want to develop the apps locally, they could manually pip install -e as needed, the same as they would any other Python package.

@wpears
Copy link
Member Author

wpears commented Oct 9, 2018

they could manually pip install -e as needed, the same as they would any other Python package

I'm fine with this approach, especially since this should once have to be done once per built image.

As a side note, I have a very secret branch, that moves the docker builds to alpine-based images and uses a multi-stage build to keep the image size small (and incidentally, has yarn stuff working as well).

Would there be interest in me bringing the alpine docker work into this PR (or a separate PR and closing this one)?

@rosskarchner
Copy link
Member

@wpears good branch 11/10

@willbarton
Copy link
Member

@wpears is this PR deprecated by #5051?

@wpears
Copy link
Member Author

wpears commented Jul 8, 2019 •
edited
Loading

@willbarton yeah, I think in most respects it is. Closing!
(i'll keep the branch for reference)

@wpears wpears closed this Jul 8, 2019
@cwdavies cwdavies deleted the dev-dockerfile branch April 7, 2020 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rosskarchner rosskarchner rosskarchner approved these changes

@willbarton willbarton Awaiting requested review from willbarton

@chosak chosak Awaiting requested review from chosak

@anselmbradford anselmbradford Awaiting requested review from anselmbradford

@contolini contolini Awaiting requested review from contolini

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Incorporate front-end build tools into the docker image Add git into python image
4 participants
@wpears @rosskarchner @chosak @willbarton