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

Simplify docker invoke tasks #3673

Merged
merged 5 commits into from Sep 23, 2019

Conversation

@patjouk
Copy link
Collaborator

commented Sep 19, 2019

As pointed by Pomax in #3610 and #3536, if you run inv docker-setup multiple times, you will run into various unexpected issues. This is because the previous volumes are still there and need to be deleted before we can run the setup steps again.
To do that, I added a docker-compose down --volumes to delete all volumes (DB + nodes modules) and do a docker-compose build --no-cache to force rebuilding images in the setup step.

I also realized that switching branch and setting things up for the first time have a lot of similarities and constrains: I removed the docker-switch-branch command and renamed the docker-first-setup to docker-new-env. It should be more robust and less confusing that way.

The double env vars were another source of confusion: since the issue between pipenv and docker-compose around quotes is fixed, I reverted to having one .env file only. I'll make sure everyone knows on Slack but you will need to manually update your .env or run the inv docker-new-env: your .env won't be overridden, only the DATABASE_URL and ALLOWED_HOSTS will be modified.

Last but not least, I added a config file for invoke with echo at true. I think it will help people understand more how docker and pipenv work if I don't hide it in the back. Every command line run by invoke will know echo by default.

@patjouk patjouk requested review from Pomax and cadecairos Sep 19, 2019
@patjouk patjouk temporarily deployed to foundation-mofostaging-pr-3673 Sep 19, 2019 Inactive
@Pomax

This comment has been minimized.

Copy link
Collaborator

commented Sep 19, 2019

Let me find a machine without docker already set up (or one I can fully remove it from and then reinstall) but looking at the code it seems like the original issue I ran into (docker crashing on the database delete, because it tries to delete a volume that doesn't exist, rather than trying to do something and finding that already existing) is still there, so we probably still need a try/except around that specific operation.

tasks.py Outdated Show resolved Hide resolved
@patjouk

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 19, 2019

Which commands were you running and can you share logs?

tasks.py Show resolved Hide resolved
@Pomax

This comment has been minimized.

Copy link
Collaborator

commented Sep 19, 2019

Looks like that's no longer an issue anymore - I did a full removal of docker (using Nektonic's App Cleaner & Uninstaller), deleted the foundation dir, recloned foundation, reinstalled docker, ran inv docker-new-env, and managed to make it to the end without issue, so: hurray!

@Pomax
Pomax approved these changes Sep 19, 2019
Copy link
Collaborator

left a comment

works great! Question about whether there's a way to "reuse" containers but that shouldn't hold up this PR

typo

Co-Authored-By: Pomax <pomax@nihongoresources.com>
@patjouk patjouk merged commit 0a595d9 into master Sep 23, 2019
3 checks passed
3 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 75.514%
Details
percy/foundation.mozilla.org Visual review automatically approved, no visual changes found.
Details
@patjouk patjouk deleted the revisit-invoke-tasks branch Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.