puppet-openldap
puppet-openldap copied to clipboard
Create olcDbDirectory before its database and the start of slapd
Rehash of #397
Pull Request (PR) description
There is an ordering in manifests/server/database.pp:
Class['openldap::server::service']
-> Openldap::Server::Database[$title]
This is subtly bad. The service (slapd) must be spun up before a database can be created, and that makes sense. However, it means the service happens before the whole defined resource Openldap::Server::Database ... and there is more going on in the defined resource of manifests/server/database.pp than just the openldap_database creation: there is also the creation of File[$manage_directory]. In most folks' cases, using a vendor-made package, this directory will be something like /var/lib/ldap, which happens to be installed by the RPM/dpkg package, so "you get it for free" / it already exists. Thus the file creation doesn't need to be done by puppet and ordering doesn't matter. However, if you set the directory to something else (that doesn't exist), you have a circular dependency problem. slapd (the service) needs the database's directory to exist before slapd starts up -> slapd is ordered before the database manifest -> the database manifest creates the database directory -> the database directory has to happen before the service.
Ultimately, the ordering is in error. The service has to happen before openldap_database BUT NOT all of the ridealong items in openldap::server::database. That breaks out of the dependency loop, and allows the directory creation to be marked as required before the Service is started.
Very likely, most folks are running one-DB-only in /var/lib/ldap (which matches most examples) and haven't tickled this issue. That said, OpenLDAP maintainers are advising folks to use subdirectories which puts this into the realm of needing to make a directory upon install, particularly when you want a second database.
Second commit, after failing acceptance tests initially.
This is a simple change that belies a lot.
The edit is Class['openldap::server::slapdconf'] is no longer forced to come after Class['openldap::server::service'] (and, test cleanup related to that).
Looking at openldap::server::slapdconf, there are calls to openldap::server::globalconf (which themselves correctly police "do this only after the service starts" and are thus safe and boring). And then slapdconf invokes create_resources(openldap::server::database) which is where the break happens. In the current world, Class['openldap::server::service'] has to finish before we start Class['openldap::server::slapdconf'], which is the trigger into making databases. Which means the service has already tried to come up (and failed) before openldap::server::slapdconf can come in and try to make things right, by creating a necessary empty directory.
slapdconf seems to have sufficient ordering on its own and its invokees, so, removing this forced class-ordering seems to be a good path forward.