Docker Container Discussion

[slominskir]

Hi,
Some thoughts on the Docker container "dscontainer":

1. Would be nice if there was a "stop" command. If you want to customize an image you can't easily stop the container after starting it. Currently I'm resorting to kill command with ps then sleep for a bit and hope. Seems like simply running dscontainer takes longer the first time so useful to pay that cost at image build time (🎉 Instance setup complete logs once).
2. Would be nice if there was support for initialing database contents. Many "official" database Docker images and some defacto ones rely on a convention of a directory named /docker-entrypoint-initdb.d (mariadb, mysql, oracle (defacto), postgres). See Example dirsrv.
3. Would be nice if there was a clean way to programmatically know when the container is ready for configuration. It appears the dscontainer -H healthcheck indicates healthy before the dirsrv is really completely ready. Well, unless you start dscontainer during the build to initialize it and get to the 🎉 Instance setup complete. After that it seems the healthcheck is then reliable.

Thoughts? Let me know if this is wrong place to discuss this (discussion 1!).

Regards,

Ryan

[vashirov]

Hi @slominskir,

this is the right place to discuss, and congrats on opening the first discussion here 🥇!

1. dscontainer handles SIGTERM signal to shutdown gracefully:

   389-ds-base/src/lib389/cli/dscontainer

   Lines 346 to 361 in f26ac01

   	# Setup the process and shutdown handler in an init-esque fashion.
   	def kill_ds():
   	if ds_proc is None:
   	pass
   	else:
   	try:
   	ds_proc.terminate()
   	log.info("STOPPING: Sent SIGTERM to ns-slapd ...")
   	except:
   	# It's already gone ...
   	pass
   	log.info("STOPPING: Shutting down 389-ds-container ...")
   	# To make sure we really do shutdown, we actually re-block on the proc
   	# again here to be sure it's done.
   	ds_proc.wait()
   	log.info("STOPPED: Shut down 389-ds-container")

   
   Do you want something like dscontainer stop that would do the heavy lifting of finding the PID and sending the signal?
2. I agree, unfortunately we're not heavily investing in the DS container right now, so if you want to help us implement this, it would be greatly appreciated! In the meantime, you can check out my approach here https://github.com/389ds/ds-container/blob/2102e7e5c184f049ac839139dd9ee20bdf868181/fedora-sample_entries/init.sh#L26
3. IIRC, healthcheck is a bit racy and I have this workaround that works for me pretty well https://github.com/389ds/ds-container/blob/2102e7e5c184f049ac839139dd9ee20bdf868181/fedora-sample_entries/init.sh#L10-L24

[slominskir]

A stop function for dscontainer would be great. It can be sneaky complicated with PID files, signals, and UNIX process hierarchies. I started a draft pull for this as I was poking around the code, but it will take more effort to move it out of draft form: #6062. Feel free to make edits, comments, or take this over.

Sounds like you are open to a conventional init directory and enhancements to the healthcheck. I'll investigate as time allows.

[slominskir]

After poking around a little more I noticed a few interesting things:

1. There appears to be a fairly large existing Python API. I haven't located any Sphinx docs or other developer API docs yet though. There may be a stop() method or equivalent already in the exiting lib.
2. The dscontainer app sets a random password first, then later sets the password to whatever was passed in by the user, and does so after the health check is already saying the server is ready (thus the race condition mentioned above).

[vashirov]

1. In a traditional installs we rely on the init system to stop the server. And existing method to stop the server in a container is kill_ds(), but it works only in the context of the existing dscontainer instance.
2. Healthcheck works over ldapi socket, so the password doesn't matter.

[slominskir]

I think it's a reasonable expectation that setting the Directory Manager password via environment variable would take effect before the healthcheck says the server is ready. Ignoring the Directory Manager account and only using ldapi would avoid the race condition, but won't work for other containers (remote users) that want to interact (presumably waiting via depends_on "healthy" condition). Further ldapi assumes the container is run as root, which as discussed below may be non-optimal.

[slominskir]

stop() function issue: #6068

[slominskir]

As an aside, it seems PID 1 responsibilities (signal forwarding and zombie reaping) add complexity to Docker containers and adds a burden for all container developers to understand. The dscontainer init probably shouldn't rely on the runtime engine init process for this since it is optional (opt-in) and non-standard, but at the same time, re-inventing the wheel in every Docker container project out there seems odd. I think dscontainer may already handle this, though it requires some effort to verify it is being done fully. Docker run --init and docker compose init: true are the Docker runtime options. Kubernetes doesn't appear to have a bare init option and it's a controversial topic. PID 1 responsibilities are often handled by systemd when running on Linux VM hosts, but in container it's historically been tini.

[slominskir]

Also worth mentioning dscontainer assumes default of running as root. Best practice is to use a limited, non-root service account user. Not really an issue for me as I'm interested in dscontainer for development and test purposes only. In production we use RHEL yum installed IdM (freeipa, powered by 389).

[vashirov]

DS is stateful application. ns-slapd (the main DS binary) doesn't handle the state inside the container, i.e. it can't initialize a new instance, if there is no data. Cloud native apps have this functionality inside the main binary, so they can be started as PID 1 and handle their own state. But DS is not cloud native. You have to have this complexity somewhere. So a wrapper dscontainer exists to handle that and forward signals from the container runtime.

Also worth mentioning dscontainer assumes default of running as root.

This file is outdated and will be removed #5733

Best practice is to use a limited, non-root service account user.

Modern container engines (Podman, CRI-O) leverage user namespaces, which provide much greater flexibility. See for example https://opensource.com/article/18/12/podman-and-user-namespaces and https://www.redhat.com/sysadmin/rootless-podman-user-namespace-modes.

[slominskir]

Remapping user namespace is great (Docker does it too), but just building your container image to not use root in the first place might be easier for end users.

I noticed the new Dockerfile indirectly mentioned above installs client tools. I'm extending 389ds/dirsrv:2.4 and I must install client tools in my own image to get them. How does the versioning work? Is latest and most recent version (2.4) not matching? Using latest tag is probably more trouble than it's worth (though it's a common convention). Versioning doesn't appear to follow semver. Would be nice if versions pushed to DockerHub are fixed and don't change (create a new version on every change - I've noticed pull on fixed version results periodically in a new/different image). Also looks like quay.io is starting to complicate things with alternate option that is completely unique.

[vashirov]

Please see #5436 regarding container versioning. As I mentioned somewhere else, we're not investing in DS containerization at this time due to other priorities, so things are a bit unmaintained right now.

[slominskir]

Thanks. Sounds like versioning/packaging has been discussed with good ideas, just nothing has changed yet. Poking around I think I found the version that has client tools: it's probably: https://hub.docker.com/r/389ds/dirsrv_debug (as opposed to https://hub.docker.com/r/389ds/dirsrv)

[Firstyear]

1. pay that cost at image build time (🎉 Instance setup complete logs once).

You can't do this. We have to generate some internal id's and set a new directory manager password at startup, as well as being able to create user requested state. So this has to stay at "first run".

[slominskir]

It appears there is code in dscontainer that only runs once. Perhaps due to this check:

389-ds-base/src/lib389/cli/dscontainer

Line 223 in 1f95b57

if not os.path.exists(DSRC_CONTAINER):

. It's nice to pre-run this code. I don't think I'm understanding your comment, what is it exactly we can't do because of internal IDs? It's possible the official image could actually pre-run this during the original build even as far as I can tell. Perhaps you are concerned with the case when a user wants to mount the /data directory to a volume? In that case the mount would erase whatever is inside the container, true, and it would need to re-generate. But in the case when a user does not mount the /data directory to a volume I don't see why it can't be pre-generated.

The Directory Manager password handling is odd. I'm not sure we absolutely must be setting the Directory Manager password to random value first then setting it to user environment supplied value later on (though this only happens on first run, later runs it just overwrites). I haven't dug into why it's doing that though. It may make sense to reset it each run to allow users to pass the value in each time, but the timing of this with relation to the health check could be improved.

[Firstyear]

It's because a DM password has to be set iirc else the container won't start. So if we hardcoded this at build time we just introduced a remote root to anyone who "forgets" to reset or change this.

[progier389]

If it is the only reason we could have done things more simply by hardcoding an hash value that is impossible to match (by having characters that are not base64 like nsslapd-rootpw: {PBKDF2-SHA512}*** INVALID *** INVALID *** INVALID *** INVALID
***) that provides a value but no one can use it to bind to the server. And these days we can use ldapi to reset rootpw

[slominskir]

A hard-coded Directory Manager password that satisfies the initialization requirement, but isn't useable (can't bind with it) sounds ideal. Alternatively, or in addition, we could refuse to start the server unless a new Directory Manager password is passed in (not as flexible, but might be ok). I've created an issue ticket to track the dscontainer Directory Manager handling in general, though specifically the race condition: #6069