Volumes are not correct

[jbehrends]

I'm commenting specifically on the unifi5 docker file.

I docker exec'd into the running container and reviewed the declared volume folders and found the following:

/var/lib/unifi - This volume seems ok, contains MogoDB Data AND Unifi Server Logs + Settings
/var/log/unifi - This folder is empty.. Logs are contained in the previous mount.
/var/run/unifi - This folder is empty. Not sure what should/would have been here?
/usr/lib/unifi/work - This folder contains an empty folder called ROOT.. nothing else.

What's missing are the MongoDB logs... They can be found here: /usr/lib/unifi/logs/mongod.log

So I purpose we REMOVE /var/log/unifi /var/run/unifi /usr/lib/unifi/work and ADD /usr/lib/unifi/logs

So end result would be:
VOLUME ["/var/lib/unifi", "/usr/lib/unifi/logs"]

Thoughts? I'd be more than happy to submit a PR too...

[jacobalberty]

I believe the /var/run is or was a requirement for running unifi under
docker with a read only root Fs, alas that broke with the new tags and I
haven't spent the time investigating. The extraneous log volumes are due to
the changes in unifi. They move stuff around from release to release.

On Wed, Nov 9, 2016, 10:35 PM Josh Behrends notifications@github.com
wrote:

I'm commenting specifically on the unifi5 docker file.

I docker exec'd into the running container and reviewed the declared
volume folders and found the following:

/var/lib/unifi - This volume seems ok, contains MogoDB Data AND Unifi
Server Logs + Settings
/var/log/unifi - This folder is empty.. Logs are contained in the previous
mount.
/var/run/unifi - This folder is empty. Not sure what should/would have
been here?
/usr/lib/unifi/work - This folder contains an empty folder called ROOT..
nothing else.

What's missing are the MongoDB logs... They can be found here:
/usr/lib/unifi/logs/mongod.log

So I purpose we REMOVE /var/log/unifi /var/run/unifi /usr/lib/unifi/work
and ADD /usr/lib/unifi/logs

So end result would be:
VOLUME ["/var/lib/unifi", "/usr/lib/unifi/logs"]

Thoughts? I'd be more than happy to submit a PR too...

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#12, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AB42gijye4FVp4AnO7SAna4X2LHhKS92ks5q8p8kgaJpZM4KuRaU
.

[JPvRiel]

/var/lib/unifi - This volume seems ok, contains MogoDB Data AND Unifi Server Logs + Settings
/var/log/unifi - This folder is empty.. Logs are contained in the previous mount.
/var/run/unifi - This folder is empty. Not sure what should/would have been here?
/usr/lib/unifi/work - This folder contains an empty folder called ROOT.. nothing else.

What's missing are the MongoDB logs... They can be found here: /usr/lib/unifi/logs/mongod.log

Writing to "/usr/..." is not good form because it conflicts with the Filesystem Hierarchy Standard which intends it for read-only user data. logs/mongod.log and work should have been placed elsewhere...

VOLUME ["/var/lib/unifi", "/usr/lib/unifi/logs"]

Again, /usr/lib/unifi/logs implies writing data in the wrong place.

After downloading the latest .deb package (also observing what the package scripts do), I noticed that Ubiquiti target /var/lib/unifi, /var/log/unifi, /var/run/unifi as the directories to be removed when using --purge if one wanted to remove the application. So that's clearly where persistent data should be going.

Reading the init script provides good details about how and from where the software normally runs...

At a guess, odd behaviour is seen because unifi controller is not being run via the init script (or systemd). It simply runs ENTRYPOINT ["/usr/bin/java", "-Xmx1024M", "-jar", "/usr/lib/unifi/lib/ace.jar"] for this docker image.

When trying to shoehorn in applications not intentionally coded to be run as a micro-service, one might miss a whole bunch of things the init script or service type files do... For example, the init script does symbolic linking (via a dir_symlink_fix function) as follows:

• /usr/lib/unifi/data -> /var/lib/unifi
• /usr/lib/unifi/logs -> /var/log/unifi
• /usr/lib/unifi/run -> /var/run/unifi

Another key observation is that the process is run with /usr/lib/unif as the base directory.

cd ${BASEDIR}

So the application probably expects ./data, ./log and ./run run directories from it's current working directory. Ubiquity were trying to use the init script as a quick bit of sticky tape and wire to make the app follow the Filesystem Hierarchy Standard by using symlinks...

Arguably, perhaps they should've rather gone with /opt, which is typically more accommodating for apps that want to execute and use data from within their one sub-directory and not split across...

Also, JSVC was used to wrap the java process. Note all the Java JSVC options for running the java app as a service in unix. It uses the pidfile and output to SYSLOG. So ordinarily, it'd have logged to syslog, not files.

JSVC_OPTS="${JSVC_OPTS}\
 -home ${JAVA_HOME} \
 -cp /usr/share/java/commons-daemon.jar:${BASEDIR}/lib/ace.jar \
 -pidfile ${PIDFILE} \
 -procname ${NAME} \
 -outfile SYSLOG \
 -errfile SYSLOG \
 ${JSVC_EXTRA_OPTS} \
 ${JVM_OPTS}"

Also, a main class was specified MAINCLASS="com.ubnt.ace.Launcher". Not sure how important that was.

Hope the above info helps provide pointers to try refactor the docker image.

I might try submit a PR the replicates the symlinking and a perhaps uses a minimal init parent (in case of orphaned / defunct processes / zombie reaping problem). But this is a good example of how trying to retrofit a full blown Unix style app into docker is a pain.

[jacobalberty]

Thank you @JPvRiel for the pull request I went ahead and merged it, this issue should be closed now.

[JPvRiel]

@jacobalberty well, I created a new issue where the java jsvc stop command had a typo. Fixed that with a 2nd PR. Apologies, should've have double checked ending the process worked cleanly. I've checked the logs, things seem to proceed normally.

E.g. I see was automatically readopted working when I restart the container, so data dir seems in order.

I did see just one WARN. Hopefully that's normal?

[2017-01-09 03:05:00,321] <db-server> WARN  system - process is interupted: bin/mongod
[2017-01-09 03:05:00,321] <db-server> INFO  db     - DbServer stopped

[JPvRiel]

I did see just one WARN. Hopefully that's normal?

Tested vanilla package in a VM. Indeed, that warning message seems normal.

[jacobalberty]

Had to back out the previous pull request. unifi.sh was missing, need to do
git add unifi.sh
and then push that to your repository.

[JPvRiel]

Sorry, whoops, fixed - that's why one shouldn't commit at ~3AM (it GMT +2 my side)!

[jacobalberty]

I'm going to go through in a minute and straighten out the commits to get everything resolved and back up to speed. I think it would be better to install dumb-init from the upstream package (possibly pinned to a specific version) though rather than including it in the repository for unifi-docker directly to provide more information on the source for the paranoid. I've forgotten the specifics of version pinning in Debian ATM but I can look at it later this evening (I'm GMT -6)

…

On Jan 9, 2017 2:56 PM, "JPvRiel" ***@***.***> wrote: Sorry, whoops, fixed - that's why one shouldn't commit at ~3AM (it GMT +2 my side)! — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#12 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB42ghdicC-474zUqnBJJbDdqNf0ydpHks5rQp7zgaJpZM4KuRaU> .

[JPvRiel]

Sure, makes sense! The PR went from something simple (fix volume mounts) to trying to address more (safeguard against Unix orphaned process handling, graceful shutdown, etc). So some caution is warranted.

I had a look, but unfortunately debian only has dump-init in it's testing and unstable repos: https://packages.debian.org/search?searchon=names&keywords=dumb-init

If you'd like to decouple this, to simply fix the original issue #12, the key changes needed in the Dockerfile are:

ENV BASEDIR=/usr/lib/unifi \
  DATADIR=/var/lib/unifi \
  RUNDIR=/var/run/unifi \
  LOGDIR=/var/log/unifi

RUN ln -s ${BASEDIR}/data ${DATADIR} && \
  ln -s ${BASEDIR}/run ${RUNDIR} && \
  ln -s ${BASEDIR}/logs ${LOGDIR}

In retrospect, I should have rather split this as two PRs, one fixing #12, the other being quite a major refactor.

[JPvRiel]

Approaches that can be taken:

1. Use an alternate lightweight init already in the jessie repo (I haven't found one yet, and bungling systemd into the container seems like a mission/bloat).
2. Or forgo it and risk using the signal trap I added to bash unifi.sh.
3. Or stick with getting the .deb from yelp's upstream github account.
4. Investigate and validate that jsvc plus controller software code behaves correctly when getting the TERM signal as the container is stopped. If the unifi package depends on a good version of jsvc and handles SIGTERM well, then jsvc itself with with right arguments can be the ENTRYPOINT

Maybe we can get away without needing an init process and just rely on unifi.sh? Think on it, let me know what you prefer, and I'd be happy to make the changes and submit another PR.

# trap SIGTERM (or SIGINT or SIGHUP) and send `-stop`
trap "echo 'Stopping unifi controller service (TERM signal caught).'; ${JSVC} -nodetach -pidfile ${PIDFILE} -stop ${MAINCLASS} stop; exit 0" 1 2 15

# keep attached to shell so we can wait on it
echo 'Starting unifi controller service.'
${JSVC} -nodetach ${JSVC_OPTS} ${MAINCLASS} start &

wait

echo "WARN: unifi service process ended without being singaled? Check for errors in ${LOGDIR}." >&2
exit 1

AFAIK, the above will:

• Setup the trap for signals and only run the jsvc -stop command when SIGHUP(1), SIGINT(2) and SIGTERM(15) are seen (maybe I should've also included SIGQUIT(3)), but SIGTERM is the standard signal passed down by docker.
• It will wait indefinitely for the jsvc with -nodetach process to return (as a child of the sh process). It will not usually get past this wait, because the signal trap above it will exit (with success command 0).
  • Actually, I made a mistake there, that should have passed the exit status of the previous command run for jsvc. So exit $? instead. E.g. trap "echo 'Stopping unifi controller service (TERM signal caught).'; ${JSVC} -nodetach -pidfile ${PIDFILE} -stop ${MAINCLASS} stop; exit $?" 1 2 15.
• If for some reason, the jsvc process did exit at some point, that's considered unexpected, the wait command returns, and then exit with a failure code.

My rough understanding is that it might work okay, provided exec style ENTRYPOINT or CMD:

Trapping signals in Docker containers

use exec form of ENTRYPOINT or RUN commands. Otherwise the application will be started as a subcommand of /bin/sh -c, which does not pass signals

Simplest approach could be to trust jsvc assuming the ubiquity controller software gracefully handles the TERM signal. To be sure, might require reverse compiling the java classes, etc (going too far)? That avoids the bash unifi.sh wrapper. However, because com.ubnt.ace.Launcher MAINCLASS seems to want an explicit stop arugment passed to it, I don't trust the assumption that SIGTERM will be properly handled.

Clean shutdown? Can sessions survive a jsvc stop/start ?

They do if you're not using the broken version of jsvc that ships with
Tomcat. The non-buggy jsvc program catches SIGTERM and performs a clean
shutdown of Tomcat.

[fflo]

Hi,

since Revert "fixed issue #12 and refactored how unifi processes are run" the Stripe payment api does no longer work correctly. To be able to use the Stripe payment Api, Java need to support TLS 1.2. Java7 does not support it, but Java8 (debian jessie backports) does.

Is it possible to re-add openjdk-8-jre-headless from debian jessie backports to fix this issue?

[jacobalberty]

Was stripe payment api working before that revert?

[fflo]

Was stripe payment api working before that revert?

I did not try it out yet, but manually installing openjdk-8-jre-headless and purging the old java 7 version within the running container does fix the issue.

[fflo]

was there any technical reason for downgrading from openjdk-8-jre back to ...7-jre? Did it break something else?

[jacobalberty]

We never used openjdk 8 in a working build. That pull request was missing a file which resulted in a broken build and I haven't had a chance to sort it out and check the changes back in properly fixed

…

On Jan 18, 2017 4:00 PM, "fflo" ***@***.***> wrote: was there any technical reason for downgrading from openjdk-8-jre back to ...7-jre? Did it break something else? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#12 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB42gqn5i8HyucqxSbpYvmmiTOJ3oFn0ks5rTouSgaJpZM4KuRaU> .

[fflo]

It really took me some time to debug why the Stripe payment api was not working with your image; the error message trying to finish the payment is misleading.
It has nothing to do with the real cause: Java 7 does not support TLS v1.2 which is required to complete any Stripe payment.

[JPvRiel]

By default, the debian package from Ubiquity pulls in Java 7 on Jessie, unless one explicitly pulls in Java 8. When I attempted re-factoring it, I decided to pull in Java 8 as part of improving the the image, given Java 7 is getting fairly old and has some limitations.

[jacobalberty]

This in theory should be fixed now, sorry took so long. @fflo, it probably should be a separate issue but could you confirm TLS v1.2 is working?

[fflo]

This in theory should be fixed now, sorry took so long. @fflo, it probably should be a separate issue but could you confirm TLS v1.2 is working?

Thank you!
Everything is working fine using :latest 👍

[JPvRiel]

Everything is working fine using :latest

Probably because the Dockerfile now uses OpenJDK 8 which should have better support for TLS 1.2 cipher suites.

apt-get install -qy --no-install-recommends openjdk-8-jre-headless