Added VOLUME definitions, updated version

OpenShiftDemos / gogs-openshift-docker

A Docker setup for gogs.io with OpenShift in mind

55 stars 162 forks source link

Added VOLUME definitions, updated version #12

Closed wkulhanek closed 7 years ago

wkulhanek commented 7 years ago

This Dockerfile was missing two VOLUME statements. Besides PostgreSQL we need two data directories: /data and /home/gogs/gogs-repositories. The actual repositories don't seem to be put into the PostgreSQL database - but into the latter directory.

thoraxe commented 7 years ago

Other than the semantic correctness of the volume definitions, what do they do in an OpenShift context?

wkulhanek commented 7 years ago

Funny (well...) story: I had a few repos and after a few restarts Gogs still knew about the repos. It threw an error 500 when trying to access them. Upon further investigation I discovered that it was looking for the actual repos in /home/fogs/gods-repositories. Which was empty. So I figured we will need a volume mounted there. Which is a bit confusing since I had been under the impression that everything except for the configuration would have been stored in the PostgreSQL database...

thoraxe commented 7 years ago

The need for the volume is true. However adding the VOLUME definition to the Dockerfile does not change OpenShift's behavior, nor does it automatically make the storage persistent.

So, again, the question remains - other than the semantic correctness of the VOLUME statement in the Dockerfile, is there any reason for it?

wkulhanek commented 7 years ago

Proper documentation? And OpenShift creating default volume entries pointing to an EmptyDir

thoraxe commented 7 years ago

Does OpenShift automatically create volume definitions for emptydirs for volumes defined in the dockerfile?

Looking at the OpenShift persistent template: https://github.com/OpenShiftDemos/gogs-openshift-docker/blob/master/openshift/gogs-persistent-template.yaml

There is potentially a bug in the template that I'd like this PR to additionally address. Can you please ensure that we are:

only creating VOLUME definitions for volumes that are required
creating persistent storage requests and volume mounts in the template for all required volumes

siamaksade commented 7 years ago

As this PR ages, there will be more conflicts. Can this be merged without the additional issues it needs to address?

@thoraxe @wkulhanek

wkulhanek commented 7 years ago

Oh. I had already given up on this PR. :-( I ended up building a new Gogs image from the latest Gogs version from scratch.

https://github.com/wkulhanek/docker-openshift-gogs

Learned quite a bit about nss_wrapper etc. :-) And also learned more about how Gogs stores data - so that image uses only one PVC.

Guess what I'm saying is: it's OK to close/reject this PR.

siamaksade commented 7 years ago

I understand but that's a pity. Would have been great to improve the same image since we keep pushing new versions to docker hub.

I'll look into what we need to align them.

wkulhanek commented 7 years ago

Siamak, still happy to merge efforts. Check out if what I did makes sense and we can talk how we can merge things next time we chat. :-)

siamaksade commented 7 years ago

@wkulhanek the build scripts seems to be specific for your Docker Hub account :)