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 the project #23

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

gautamjajoo
Copy link
Member

@gautamjajoo gautamjajoo commented Dec 3, 2018

Added Docker files to run the project easily using Docker. Screencast
@fristonio @RishabhJain2018 @techytushar

Dockerfile.txt Outdated Show resolved Hide resolved
Dockerfile.txt Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@fristonio fristonio left a comment

Choose a reason for hiding this comment

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

Also, this shouldn't be working, since we don't have docker inside docker. Please also mount the host docker socket in the origamid container so that it should be able to launch containers.

Copy link
Member

@fristonio fristonio left a comment

Choose a reason for hiding this comment

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

There are a lot of things missing in this

  • Celery needs a message broker, so we also need to setup that in docker-compose.yml
  • Change name Dockerfile.txt to Dockerfile
  • This won't work until origamid is able to create new container which is only possibly by mounting host docker unix domain socket. Configure this in the Dockerfile.

@gautamjajoo
Copy link
Member Author

@fristonio I haven't yet completed it. I am working on it. 😄

@fristonio fristonio added the WIP label Dec 4, 2018
@fristonio
Copy link
Member

Ok cool, just putting a WIP label so no one gets confused. 😅

@gautamjajoo
Copy link
Member Author

@fristonio When I try locally, the error is Unable to find image 'origami-daemon:latest' locally. Can you guide me how should I proceed?

@fristonio
Copy link
Member

Ok, I think the problem here is that the image is not tagged properly. Run docker images to see if there is a image named origami-daemon with tag latest.
So how the build should work is first you will need to build the image using docker build . --tag origami-daemon:latest and then run it using docker run -v /var/run/docker.sock:/var/run/docker.sock origami-daemon. Also update the docker-compose.yml file too, it wont work and I should not require to build the origami-daemon docker container manually, do this using docker-compose too.
I hope this helps.

@fristonio
Copy link
Member

Also, since origami-daemon exposes an API server to work with, you should also open the port when building the docker container. Google on how to expose ports in docker containers.

@gautamjajoo
Copy link
Member Author

@fristonio @RishabhJain2018 Done the changes! Added wrapper for easy installation too. Can you review this?

@techytushar
Copy link

please rename the Dockerifle to Dockerfile

@gautamjajoo
Copy link
Member Author

@fristonio @RishabhJain2018 @techytushar Please review the PR!

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@fristonio
Copy link
Member

@gautamjajoo is the docker-compose setup working properly on your local machine?

@techytushar
Copy link

  • The worker service is redundant, just the rabbitmq and celery services are enough.
  • The command celery -A origamid worker -l info can be run in the Dockerfile and origamid run_server must be run in the compose file.
    Just these changes otherwise this LGTM.

@gautamjajoo
Copy link
Member Author

gautamjajoo commented Dec 11, 2018

@techytushar Can you explain this part origamid run_server must be run in the compose file. What I understood is that I should add command : ["origamid" , "run_server"]
Also, since worker is redundant but we have to expose the port so should I add it to Dockerfile.
Rest all the changes which were requested I have done!

@gautamjajoo
Copy link
Member Author

@techytushar Done the required changes. @fristonio Can you review?

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