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 Dockerfile config #441

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

Add Dockerfile config #441

wants to merge 7 commits into from

Conversation

lkraider
Copy link

This adds the ability to create docker images for this project.

You probably want to change the maintainer names in the Makefile and the build/Dockerfile to your dockerhub user.

Use make test to run a local dev build of the project while developing and testing the image.

When ready, tag the current version using git and run make prod to create a production image ready to be pushed to the docker registry.

Originally based on the work done by: https://github.com/KebinuChiousu/docker-nsupdate

Use `make test` to run a local dev build of the project, and `make prod`
to create a production image ready to be pushed to a docker registry.
@ThomasWaldmann
Copy link
Member

Not sure I want that because I can't maintain it (I don't use docker).

@elnappo what do you think?

@elnappo
Copy link
Member

elnappo commented Dec 27, 2019

We could move it into a /contrib folder?

@ThomasWaldmann
Copy link
Member

Could this live inside a misc/contrib/docker/ directory?

* upstream/master:
  Add missing migration files
  remove tr and cn translations from repo (see transifex)
  update year
  Update fontawesome, bootstrap and jquery, fix nsupdate-info#444
  Create FUNDING.yml
  Add GitHub social preview image
  Remove Python 2.7 support, add Python 3.8 support
  Update Django to v2.2, see nsupdate-info#386
  Add a link to user in host admin, fix nsupdate-info#440
  Add docs on how to disable user registration, see nsupdate-info#438
  Set HTTPONLY to CSRF cookies
  Add Referrer-Policy HTTP Header, nsupdate-info#281
  Add X-XSS-Protection and X-Content-Type-Option HTTP Header
@lkraider
Copy link
Author

Done, PR updated!

@lkraider
Copy link
Author

lkraider commented Dec 31, 2019

Quick instructions on how to use:

  1. install docker
  2. cd misc/contrib/docker
  3. make test -> build and run a dev version
  4. docker ps -> service will be running on a random port, check the output and try to open on localhost to see if it works
  5. git tag -a 0.12.0 then make -> build a prod version (make sure to tag first to get a nice name)
  6. docker login then make release -> release on docker registry

Copy link
Member

@ThomasWaldmann ThomasWaldmann left a comment

Choose a reason for hiding this comment

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

some feedback / questions.

i don't use docker, so this needs review from some docker user also.

misc/contrib/docker/.dockerignore Outdated Show resolved Hide resolved
@@ -0,0 +1,31 @@
MAINTAINER=nkeyapps
TAG=nsupdate.info
VER=$(shell git describe)
Copy link
Member

Choose a reason for hiding this comment

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

Is that the nsupdate.info version?

We use setuptools_scm - maybe that can help with versioning here also?

Copy link
Author

@lkraider lkraider Jan 2, 2020

Choose a reason for hiding this comment

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

Yes, it uses the git tag from the project for versioning the generated docker image. This way there are no dependencies, using setuptools would require the release manager to create a virtualenv/pip install things first on the host machine. (one of the usefulness of docker is that all dependencies are installed into the container, not on the host)

Copy link
Author

Choose a reason for hiding this comment

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


release: prod release_latest
docker push $(MAINTAINER)/$(TAG):$(VER)
@echo "*** Don't forget to create a tag by creating an official GitHub release."
Copy link
Member

Choose a reason for hiding this comment

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

usually tagging happens before a release is made from that tag.

Copy link
Author

Choose a reason for hiding this comment

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

Not sure I follow the implication. Let me know if I misunderstood your point, but this is the process to release a docker image, which will be run after a git tag is made in the nsupdate.info project on the master branch. So the docker release manager runs from a checkout of that tag something like: git checkout 0.12.0 && make release. This will build the image and tag it accordingly, then upload it to the docker registry.

Copy link
Author

Choose a reason for hiding this comment

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

Oh ok, I see the comment is misleading. This is an artifact of the previous makefile. I'll clean it up.

Copy link
Author

Choose a reason for hiding this comment

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

I have added an explicit requirement for a git tag when invoking the release target.

misc/contrib/docker/build/Dockerfile Show resolved Hide resolved
misc/contrib/docker/build/setup.sh Outdated Show resolved Hide resolved
Other small fixes for code review.
@ThomasWaldmann
Copy link
Member

@elnappo can you review also?

@elnappo elnappo self-requested a review January 7, 2020 15:00
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