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

Include Docker Compose #62

Merged
merged 5 commits into from
May 2, 2017
Merged

Include Docker Compose #62

merged 5 commits into from
May 2, 2017

Conversation

akath20
Copy link
Member

@akath20 akath20 commented Apr 17, 2017

Working on dockerizing the application with compose. Currently facing ~90% cpu from the application imagine when using docker-compose, need to check in with @7imbrook how to resolve before merging. #57

Copy link
Member

@7imbrook 7imbrook left a comment

Choose a reason for hiding this comment

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

Left some comments, mainly to move it to a prod config from a dev config.

@@ -0,0 +1,18 @@
version: "2"
services:
mongo:
Copy link
Member

Choose a reason for hiding this comment

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

will need

deploy:
    constraints:
        - node.labels.db=true

if planning on running our own

Copy link
Member

Choose a reason for hiding this comment

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

its something like OneRepos config

Copy link
Member

Choose a reason for hiding this comment

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

cause you'll also have to add

volumes:
    volumeName:

- "27017:27017"
web:
build:
context: .
Copy link
Member

Choose a reason for hiding this comment

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

Additionally this is good for dev, but we'll also need an image only one for prod deploys.

links:
- mongo
volumes:
- .:/usr/src/app
Copy link
Member

Choose a reason for hiding this comment

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

Same as above

Copy link
Member

Choose a reason for hiding this comment

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

I understand that this is a dev config

@@ -0,0 +1,18 @@
version: "2"
Copy link
Member

Choose a reason for hiding this comment

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

Update to version 3.1

Dockerfile.dev Outdated

ADD start.sh .

RUN chmod +x ./start.sh
Copy link
Member

Choose a reason for hiding this comment

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

Permissions are preserved and should be maintained in source. So change this outside the build.

@akath20
Copy link
Member Author

akath20 commented Apr 19, 2017

Can't believe it, but everything is working with this push. The first workaround that needs to be addressed though, which I would need help by likely both @7imbrook and @khanny17 on figuring out where I'm going wrong is the config.js is not being added to the container correctly. I believe this is because I am working with the local file as context, and not dist/. It's trying to access config.js, but it's not actually there/not being updated correctly. The second issue is high CPU usage...

@akath20
Copy link
Member Author

akath20 commented Apr 19, 2017

I fixed the above issue of the config file. This is fully working, I just need to investigate the CPU issue...

@akath20
Copy link
Member Author

akath20 commented Apr 20, 2017

@7imbrook have you ever encountered com.docker.hyperkit acting up?
docker/for-mac#1094
https://forums.docker.com/t/com-docker-hyperkit-up-cpu-to-340/13057/68
I'm not sure if this might be fixed in the beta channel you're on? It's working, I'm just concerned as I mentioned today it might cause issues on the server, even though we can mask the issue. It seems like a bug in Docker beyond our control at the moment...

@khanny17
Copy link
Collaborator

Did you confirm that the high cpu issue is due to docker and not due to gulp devrun?

@akath20
Copy link
Member Author

akath20 commented May 2, 2017

This looks good to go, and with this fix should fix the production issue. Once this is merged to master, I can redeploy and test.

@khanny17 khanny17 dismissed 7imbrook’s stale review May 2, 2017 00:25

Probably addressed

@khanny17 khanny17 merged commit d4df2f2 into master May 2, 2017
@khanny17 khanny17 deleted the dockerize branch May 2, 2017 00:26
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.

3 participants