-
-
Notifications
You must be signed in to change notification settings - Fork 120
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
base: master
Are you sure you want to change the base?
Conversation
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.
Not sure I want that because I can't maintain it (I don't use docker). @elnappo what do you think? |
We could move it into a |
Could this live inside a |
* 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
Done, PR updated! |
Quick instructions on how to use:
|
There was a problem hiding this 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.
@@ -0,0 +1,31 @@ | |||
MAINTAINER=nkeyapps | |||
TAG=nsupdate.info | |||
VER=$(shell git describe) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: setuptools_scm basically does the same command behind the scenes (https://github.com/pypa/setuptools_scm/blob/fbaeaa947e403d78964654d3ab68cbc75caccfec/src/setuptools_scm/git.py#L15)
misc/contrib/docker/Makefile
Outdated
|
||
release: prod release_latest | ||
docker push $(MAINTAINER)/$(TAG):$(VER) | ||
@echo "*** Don't forget to create a tag by creating an official GitHub release." |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Other small fixes for code review.
@elnappo can you review also? |
This adds the ability to create docker images for this project.
You probably want to change the maintainer names in the
Makefile
and thebuild/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