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

Add option to dockerize netbox #107

Merged
merged 1 commit into from
Jul 1, 2016
Merged

Add option to dockerize netbox #107

merged 1 commit into from
Jul 1, 2016

Conversation

koep
Copy link
Contributor

@koep koep commented Jun 29, 2016

Hi guys,

this patch enables users to build and run netbox inside a docker container. I also added the option to run netbox via docker-compose (docker-compose up -d magic).

The implementation can probably be improved, but I think a lot of people like to use docker to try applications out these days. This is a good start.

Let me know what you think!

@jeremystretch
Copy link
Member

Looks great! Can we get a few people to test it out and see if they run into any problems?

@pitkley
Copy link
Contributor

pitkley commented Jun 29, 2016

I have a separate implementation available on Docker Hub (see also source on GitHub) which I think is overall a cleaner implementation in terms of the Dockerfile and entrypoint, but one thing I am a big fan of in your solution is the availability of the environment variables.

Some overall notes (which are obviously influenced by my implementation):

  • I would not use debian as the base-image, but rather python since we are dealing with a Python app anyway.

  • Using sleep to wait for the database is -- as you write yourself -- dirty. Cleaner options are to depend on the database-service in docker-compose.yml and/or to check for actual availability of the database.

  • I'm not sure how I feel about creating the superuser silently with default data. Maybe one could use additional environment variables to specify the username and password, or just have the end-user add it like with bare-metal (also very easy to do if docker-compose is used).

  • Gunicorn and nginx: having both in the same container is for one thing against the Docker mantra "Run only one process per container" -- but I personally think that is a bit too strict. Still, there is a big problem with having more than one process running: process reaping. In my opinion, nginx should not be part of the container. If a user wants to have nginx or Apache (or any other webserver) in front of NetBox it should be their decision.

    Splitting nginx (or any other webserver) into a separate container poses a slight issue in regards to the static files, but they could either be shared using a volume both containers mount, or using the --insecure option of runserver (although I don't know how that would translate to the .wsgi file).

In my opinion, adding a first-party Dockerfile is something that should not be rushed since there are a lot of fine details as for what makes a good, user-friendly Docker image.

Regarding next steps, I would suggest to open a separate PR to have NetBox configurable through environment variables, since that is more or less a prerequisite for the use with Docker (so that we don't have to mess around with sed...). One immediate issue I can spot with your implementation is ALLOWED_HOSTS, since it only allows for a single host. Maybe something like the following could work?

ALLOWED_HOSTS = os.environ['ALLOWED_HOSTS'].split(' ') if 'ALLOWED_HOSTS' in os.environ else []

Another issue is: if the file is called configuration.example.py, the environment variables obviously will do nothing. Should the file be renamed to configuration.py?

After that issue/PR is solved, one could go on with the Docker image.


FWIW, I have been more or less in charge for adding a first-party Docker image to the digital paper archive Paperless through issue #2 and eventually PR #39. There are definitely some important points that can be extracted from the discussions over there.

@mdlayher
Copy link
Contributor

Works great here! Not sure what the WARNING is about though.

[zsh|matt@nerr-2]:~/git/netbox 0 (064d088) ± docker-compose up -d
WARNING: The rClfWNj variable is not set. Defaulting to a blank string.
Creating postgres
Creating netbox
[zsh|matt@nerr-2]:~/git/netbox 0 (064d088) ± docker ps
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS                NAMES
c38a18a1e950        netbox_netbox       "/docker-entrypoint.s"   2 seconds ago       Up 2 seconds        0.0.0.0:80->80/tcp   netbox
c267da30f2e7        postgres:9.6        "/docker-entrypoint.s"   3 seconds ago       Up 2 seconds        5432/tcp             postgres
[zsh|matt@nerr-2]:~/git/netbox 0 (064d088) ± curl -i http://localhost/api/ipam/vlans/
HTTP/1.1 200 OK
Server: nginx
Date: Wed, 29 Jun 2016 19:40:43 GMT
Content-Type: application/json
Transfer-Encoding: chunked
Connection: keep-alive
Vary: Accept, Cookie
X-Frame-Options: SAMEORIGIN
Allow: GET, HEAD, OPTIONS
P3P: CP="ALL DSP COR PSAa PSDa OUR NOR ONL UNI COM NAV"

@koep
Copy link
Contributor Author

koep commented Jun 29, 2016

Thanks for your feedback. I might be able to implement some of your suggestions tomorrow. Others may take a little longer.

@koep
Copy link
Contributor Author

koep commented Jun 30, 2016

So I just fixed some of the issues

  • WARNING: The rClfWNj variable is not set. Defaulting to a blank string. (docker-compose assumed the '$' character in the SECRET_KEY environment variable was an actual variable. So I changed the example key slightly)
  • I updated docker-compose.yml to version 2 and added the "depends_on" feature. I also updated the documentation to make sure that people use the latest version of docker-compose.
  • The superuser creation is now configurable via environment variables
  • Moved nginx out of the netbox image and put it in a seperate container (static files live in a docker volume)
  • Made sure that the container does not exit until the db migrations worked (the db is up and running)
  • And I actually changed the base image to ubuntu 14.04 to match the manual installation process even more. (I tried using the official python image, but this ended up increasing the image size to about 800 MB)

I think the current state of the Dockerfile lets people quickly try NetBox out. I agree that there is some work to be done in order to make this "production ready". (Maybe the digitalocean team should decide if they actually want it to be prod ready before merging it).

@mdlayher
Copy link
Contributor

mdlayher commented Jun 30, 2016

Pulled latest changes and am now getting a 502 from nginx:

$ curl http://localhost/api/ipam/vlans/                                        
<html>
<head><title>502 Bad Gateway</title></head>
<body bgcolor="white">
<center><h1>502 Bad Gateway</h1></center>
<hr><center>nginx</center>
</body>
</html>

Also looks like my netbox container stops running after about ten seconds or so.

Any ideas @koep ?

@mdlayher
Copy link
Contributor

Running docker-compose build to rebuild the images fixed my issue! Looks good here!

@jeremystretch
Copy link
Member

This is awesome! I just have one request: Instead of modifying configuration.example.py, could you provide an alternate file with the content you've proposed, perhaps named configuration.docker.py?

My reason for asking is that using os.environ.get() to retrieve settings could potentially be a security vulnerability when used outside a container.

@koep
Copy link
Contributor Author

koep commented Jun 30, 2016

@jeremystretch sure! I just updated this PR.

@mdlayher mdlayher added the status: accepted This issue has been accepted for implementation label Jul 1, 2016
@mdlayher
Copy link
Contributor

mdlayher commented Jul 1, 2016

Looks good to me. Will let @jeremystretch give the final approval.

@linuxsimba
Copy link
Contributor

installed netbox using this PR. no problems so far.

@jeremystretch jeremystretch merged commit aabe641 into netbox-community:develop Jul 1, 2016
@ChristianKniep
Copy link

Hey there, nice tool. If I may I would reckon that you change the docker-compose file to use image instead of build and put this docker image on dockerhub under the same name as the github repository digitalocean/netbox.
By doing so, one does not have to build it, but just fire up docker-compose up -d and off we go.

@koep
Copy link
Contributor Author

koep commented Jul 4, 2016

I think this would be really cool. The DO team has to create an account on docker hub and implement an automated build to accomplish this. (I can't do this).

ping @mdlayher, @jeremystretch

@mdlayher
Copy link
Contributor

mdlayher commented Jul 4, 2016

Created #189 to track that issue.

@jeremystretch jeremystretch mentioned this pull request Jul 7, 2016
if-fi pushed a commit to if-fi/netbox that referenced this pull request Oct 1, 2016
@lock lock bot locked as resolved and limited conversation to collaborators Jan 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: accepted This issue has been accepted for implementation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants