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

New, slimmer image and updated Dockerfile #4934

Merged
merged 2 commits into from
Oct 11, 2018

Conversation

zwolf
Copy link
Member

@zwolf zwolf commented Sep 26, 2018

There were a few issues early this week with PFE deployment Jenkins jobs failing to complete. The issue was node crashing with an error that indicated it was out of memory. Taking a look at the Dockerfile, it seemed that it was building a larger than necessary image (full ubuntu) and loading a bunch of stuff it didn't need (i.e. phantomjs) just to run the npm build.

This PR switches to a slim alpine image straight from npm and only installs git because npm needs it (and bash, but that's probably extraneous). The extra Dockerfile.deps was removed with a piece or two collapsed into the main one. This should keep total image size way down and eliminate the need for two separate images.

This is WIP because while I tested a few docker run commands, I'm not sure how to test short of straight up deploying master to staging and I'd like another pair of PFE-familiar and/or devops eyeballs looking when that happens.

@zwolf zwolf changed the title New, slimmer image and updated Dockerfile [WIP] New, slimmer image and updated Dockerfile Sep 26, 2018
@coveralls
Copy link

coveralls commented Sep 26, 2018

Coverage Status

Coverage increased (+0.6%) to 44.046% when pulling ba0c10a on zwolf:dockerfile-update into e178662 on zooniverse:master.

Copy link
Contributor

@srallen srallen left a comment

Choose a reason for hiding this comment

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

Phantom.js maybe was needed for the really old build process that used bash scripts. I'm not sure. Regardless, it's a deprecated project and shouldn't be used anymore for that reason alone.

Looks good, but I'm not sure how to test this all exactly either. I'm gonna take a pass at writing some docs on the deployment process to add to the readme in a separate PR.

@rogerhutchings
Copy link
Contributor

We could create another folder in the preview S3 bucket, duplicate the Jenkins job, tweak the stage target folder in package.json and try doing a dry-run deploy to that rather than the original staging folder?

Dockerfile Outdated

WORKDIR /src

RUN apk update && apk upgrade && apk add --no-cache bash git openssh
Copy link
Contributor

Choose a reason for hiding this comment

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

It's a minor thing, but there's no need to update/upgrade here. The --no-cache option loads the package index automatically.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good to know. I'm gonna pare this down to just adding git since bash isn't needed and ssh is a dep of git.

@adammcmaster
Copy link
Contributor

It should be safe to just run the normal stage command to test this. docker-compose run dev npm run stage

@zwolf zwolf changed the title [WIP] New, slimmer image and updated Dockerfile New, slimmer image and updated Dockerfile Oct 4, 2018
@zwolf
Copy link
Member Author

zwolf commented Oct 4, 2018

Seems to work fine, I was able to successfully stage a branch here with docker-compose run dev npm run stage: https://dockerfile-update.pfe-preview.zooniverse.org/

@eatyourgreens eatyourgreens merged commit edac5a9 into zooniverse:master Oct 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants