Fix for issue #50 - ports remapping on server/docker restart

[scourou]

This fix ensures the postgresql commands inspect docker for the correct port, rather than stashing the port number in a file. If the server reboots, docker won't necessarily reassign the same port to the container.

Also includes a slight refactor for getting the ID of a container.

[rvalyi]

Hum in fact this patch doesn't seem complete: here the code is still looking for a $DOKKUROOT/.postgresql/port$APP https://github.com/Kloadut/dokku-pg-plugin/blob/master/pre-deploy#L30

[scourou]

This will be my relative inexperience with dokku showing through. I'll fix and have a hunt around for more.

[scourou]

Right. I think this is best opened as a new bug - I think the fix would be to stop the reliance on PG_PORT completely and just let docker pick a port (as the port is now dynamically worked out). So to launch the volume as:

docker run -v $PG_VOLUME -p 5432 -d $PG_APP_IMAGE /usr/bin/start_pgsql.sh $DB_PASSWORD

However I'm really wary of introducing more bugs, so someone more familiar with the dokku flow/code should probably take a look at this.

[ndelage]

Looks like this patch was merged, but the following line in pre-deploy prevents a successful restart of the pg container:

PG_PORT="`cat $DOKKU_ROOT/.postgresql/port_$APP`"

Since the file $DOKKU_ROOT/.postgresql/port_$APP doesn't exist, the pre-deploy script fails. This is a show stopper, preventing postgres containers from restarting successfully.

[scourou]

Nate - I think I have this fixed, can you test this fork please:

https://github.com/scourou/dokku-pg-plugin

If this has fixed the issue for you, I'll issue a pull request

[ndelage]

Yep, this approach works. I tried the same thing on my fork earlier. Seems we should create a shared function for starting the container, so we have fewer edge cases to track down in the future.

I'm still struggling to get a clean restart though. This might not be related to this plugin though. I suspect the app & db container need to be re-linked as part of a restart (since the port has changed).

[ndelage]

Yep, when a new port is assigned, it's necessary to relink the db container with the app container (setting DATABASE_URL and restarting the app.

Seems appropriate that this be part of a new post-deploy script. Any thoughts?

[lngsx]

@scourou. Your fix is not works for me. It succeeded to keep PG container to run after rebooting but DATABASE_URL was not updated by the new assigned port.

The relink command is fails to lookup the currently running container port because get_postgresql_id() returns all PG container ids which match the app name and don't care if they were junk (exited status). In my case, it returns a list of 3 containers.

Manually setting the port into DATABASE_URL by config:set can fixes this issue.

In my fork I've done these

• Use this fix f4532e12f83a72dc7300f3d4dd282ac99db609c4 from @scourou.
• removed -a arg in get_postgresql_id() to ensure it gets only running container.
• added postgresql:link command to the end of the pre-deployfile.

All functions work fine. So I've made a pull request, please review.