-
Notifications
You must be signed in to change notification settings - Fork 60
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
Clean restart #12
Clean restart #12
Conversation
Currently looking into
See also: |
Bump for testing ome/apacheds-docker#12
Bump for testing ome/apacheds-docker#12
Dockerfile
Outdated
ENV APACHEDS_USER apacheds | ||
ENV APACHEDS_GROUP apacheds | ||
|
||
RUN ln -s ${APACHEDS_DATA}-${APACHEDS_VERSION} ${APACHDS_DATA} |
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.
typo: APACHDS_DATA
so there appear to be two data directories:
root@63b43aeb4f88:/var/lib# ls -l apacheds* -d
drwxr-xr-x 3 root root 4096 Mar 16 16:43 apacheds
drwxr-xr-x 3 apacheds apacheds 4096 Mar 15 11:43 apacheds-2.0.0-M24
root@63b43aeb4f88:/var/lib# ls apacheds*
apacheds:
default
apacheds-2.0.0-M24:
default
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.
Cheers.
README.md
Outdated
@@ -13,7 +13,7 @@ The project sources can be found on [GitHub](https://github.com/openmicroscopy/a | |||
|
|||
## Installation | |||
|
|||
The folder */var/lib/apacheds-${APACHEDS_VERSION}* contains the runtime data and thus has been defined as a volume. A [volume container](https://docs.docker.com/userguide/dockervolumes/) could be used for that. The image uses exactly the file system structure defined by the [ApacheDS documentation](https://directory.apache.org/apacheds/advanced-ug/2.2.1-debian-instance-layout.html). | |||
The folder */var/lib/apacheds* contains the runtime data and thus has been defined as a volume. A [volume container](https://docs.docker.com/userguide/dockervolumes/) could be used for that. The image uses exactly the file system structure defined by the [ApacheDS documentation](https://directory.apache.org/apacheds/advanced-ug/2.2.1-debian-instance-layout.html). |
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.
https://docs.docker.com/userguide/dockervolumes/
is broken. In any case volume containers aren't needed anymore.
############################################# | ||
# ApacheDS wrapper command | ||
############################################# | ||
|
||
CMD ${APACHEDS_CMD} |
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.
Since this is removed do you still need the configurability provided by ENV APACHEDS_CMD
ENV APACHEDS_SCRIPT
: https://github.com/openmicroscopy/apacheds-docker/pull/12/files#diff-3254677a7917c6c01f55212f86c57fbfL41 ?
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 couldn't get the syntax right to put the environment variable as the entrypoint (at least not without reverting to bash -c
. I'll drop the rest.
scripts/run.sh
Outdated
} | ||
|
||
trap shutdown INT TERM | ||
tail --pid=$(cat $PIDFILE) -f /dev/null |
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.
There's also
/var/lib/apacheds/default/log/apacheds.log
/var/lib/apacheds/default/log/wrapper.log
Is one of these more useful?
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.
Neither is good, but that's likely a log4j.properties issue. I'll use the more specific apacheds.log.
Pushed. |
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 was asked to review here too, but did not get to it for IDR-reasons. Also, I am just not sure aobut the workflow:
Is that right ? @joshmoore is the workflow correct and, after we have clarified the steps (see above) I can run it as a part of some trello card task (which one ?) ? |
No worries, @pwalczysko. Though I am looking forward to your showing me how this change could be better (which I'll happily implement and release incrementally) I was equally if not more interested in how you found the usability of the deployed service. (Can give you pointers to that off-PR)
Roughly yes. What this work tries to enable (using your bullet list) is:
This would be my hope that the mentioned service that I spun up will become a persistent way for us to maintain users, including multiple groups and perhaps even different styles of LDAP layouts. |
This branch attempts a couple of things:
/var/lib/apacheds
) as a volumedocker stop
fixes gh-11