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

Dockerize featureService #24

Merged
merged 13 commits into from Feb 16, 2018

Conversation

Projects
None yet
2 participants
@c-w
Copy link
Member

commented Feb 15, 2018

As part of the Fortis security review, we've established that the featureService should be deployed inside of the Fortis kubernetes cluster so that we don't have a single point of failure. More details on this discussion can be found in CatalystCode/project-fortis-pipeline#222 (comment)

In order to enable the featureService to be deployable to kubernetes we must dockerize it. As such, this pull request implements a Dockerfile and entry-point for the featureService that sets up the Node service, sets up the database schema, populates the database from a dump, etc. When starting the service via docker run, we now get a fully-functional deployment.

Additionally, this pull request also implements CD via Travis for the featureService to make it easy for us to publish new Docker images by simply authoring a release on Github. To facilitate the CD, the pull request also implements CI via Travis. Currently the CI just runs eslint so the pull request also includes assorted lint fixes.

I realize that the pull request is quite long, so here are pointers to the core of the new functionality:

@c-w c-w requested review from timfpark and jcjimenez Feb 15, 2018

Dockerfile Outdated
CMD /app/docker-entrypoint.sh

ENV PORT=80
EXPOSE 80

This comment has been minimized.

Copy link
@jcjimenez

jcjimenez Feb 15, 2018

Contributor

Do you know if it's possible to do a EXPOSE $PORT?

This comment has been minimized.

Copy link
@c-w

c-w Feb 16, 2018

Author Member

Not sure. From the docs/spec it seems like only constants can be exposed. I'll give it a try though and report back.

This comment has been minimized.

Copy link
@c-w

c-w Feb 16, 2018

Author Member

Yes, seems to work. Good catch. See 453cccc

@jcjimenez
Copy link
Contributor

left a comment

LGTM w/minor question.

@c-w c-w merged commit db88a67 into master Feb 16, 2018

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

@c-w c-w deleted the dockerize branch Feb 16, 2018

@c-w c-w referenced this pull request Feb 16, 2018

Closed

Reminder: Security Review #222

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.