Hi Arther,

Thanks for taking the time to review the patch. :)

The motive for Upstart scripts instead of a init.d script is simple:
with the init.d script, I couldn't find a simple way to guarantee nslcd
started _after_ a network connection was available. At the time I logged
the feature request (I haven't tested this recently--I probably should),
nslcd would simply terminate if no network connections were available,
so there was a race condition between nslcd and the networking
configuration scripts. The Upstart script is triggered by an event
that's emitted when a network interface is fully configured (net-device-
up), so that race condition is avoided.

I removed the init.d script for a couple reasons. First, running
`service nslcd restart` was preferring the init.d script to the Upstart
script if the init.d script was present, leading to unexpected behavior.
Second, it seemed best to have a single method for starting the daemon.
Multiple, independent methods seemed like a good way to confuse users.
When I built a test package with debuild in Ubuntu, wrappers for the
Upstart scripts were placed in /etc/init.d/. I'd assume the same is true
for Debian packages, but I haven't tested it.

Point taken about the status 1 exit when sasl_mech or krb5_ccname isn't
available--those two checks should definitely exit with status 0.

I setup logging to the /tmp directory because AFAIK there's no good way
to leverage existing log facilities with Upstart, and I understand that
/tmp is usually available earlier in the boot process than /var. The log
files contain diagnostic information about the startup process only--no
secrets are shared. So, it seemed safe to have a log file, even though
it wouldn't necessarily survive a reboot.

Can you point out the duplicate checks? There's a lot of annoying
redundancy in the variable initialization because Upstart's env
directive doesn't support expansion (see
https://bugs.launchpad.net/upstart/+bug/328366), but I tried to
eliminate duplicate checks wherever possible. The one duplicate check I
see is the initialization of the state directory, which happens in both
nslcd and nslcd-kerberos scripts. That's there because we require a
state directory for both daemons, but we can't assume one has run before
the other.

Resolving the race condition with networking configuration is the
primary motive for moving to the Upstart script. Extracting the
Kerberos-related initialization into a separate file doesn't improve the
functionality at all; I did it so I could take advantage of Upstart's
management features instead of manually killing the daemon. Also, it
makes the core nslcd script much more readable, IMO.

Hopefully that answers your questions. Just let me know if anything
needs clarification.

-- 
You received this bug notification because you are a member of Ubuntu
Bugs, which is subscribed to Ubuntu.
https://bugs.launchpad.net/bugs/806761

Title:
  Feature Request: Upstart scripts for nslcd

To manage notifications about this bug go to:
https://bugs.launchpad.net/ubuntu/+source/nss-pam-ldapd/+bug/806761/+subscriptions

-- 
ubuntu-bugs mailing list
ubuntu-bugs@lists.ubuntu.com
https://lists.ubuntu.com/mailman/listinfo/ubuntu-bugs

Reply via email to