Hi Caleb! Thanks for taking a crack at integrating these into Ubuntu.
This is really great and even though my review below is long, I have to
say that this is a great first attempt at a really difficult problem
space. :) I'm going to unsubscribe ubuntu-sponsors for now, but please
do re-subscribe sponsors when you've made the necessary adjustments.
My Feedback:
* start on (local-filesystems and net-device-up IFACE!=lo)
This does not do what you think it does. The first time there is a
network connection available, this will work fine. But if the network
goes away and comes back, this will not be started. That is because the
'and' will be waiting for local-filesystems to be emitted again. If you
remove local-filesystems, then you will start too soon on systems that
have NFS root. Also for systems with multiple network interfaces, this
may not actually be sufficient either, as it doesn't mean the one that
is needed for the service is up, just that "one of them" is up.
This also means if a system is brought into single user mode (runlevel
1) that the service will never be started back up again because you
haven't listed runlevel [2345] in the start on.
Realistically, what you need is
start on runlevel [2345]
This will wait for any static interfaces configured in
/etc/network/interfaces. For network-manager managed interfaces, you'll
still be racing with them, so you also need a script in /etc/network/if-
up.d to start the job any time a network interface comes up after
runlevel 2 is reached:
#!/bin/sh
start nslcd 2> /dev/null
* Upstart logs all job output now (as of upstart 1.5, released w/ Ubuntu
12.04) so there is no need to write to a file in /tmp. Simply echo what
you want to say and it will end up in /var/log/upstart/$jobname.log.
Also the user is shown upstart's starting events in the plymouth details
plugin (available when you hit 'down' in the gui startup, or always
shown on servers). So there's really no need to echo 'Starting'
anything.
* We are deprecating use of /etc/default files. I recommend replacing
that file with a message like this: 'This file has been deprecated,
please edit /etc/init/nslcd.conf. For users who do not want to edit
packaged files, 'env' lines can be changed in /etc/init/nslcd.override'.
Then remove all checking for its existence and loading of it, and move
the defaults into the upstart jobs as 'env XXXXX=YYY' lines. This makes
a tiny improvement on boot speed for this one job, but it that adds up
as we do it for all jobs.
* In your post-start loop, you should check to see what the state of the
job is between iterations. An administrator may have decided to give up,
and run 'stop nslcd' since the loop started waiting, and respawn also
may happen if the binary exitted abnormally. Mysql shows how this works:
post-start script
for i in `seq 1 30` ; do
/usr/bin/mysqladmin --defaults-file="${HOME}"/debian.cnf ping && {
exec "${HOME}"/debian-start
# should not reach this line
exit 2
}
statusnow=`status`
if echo $statusnow | grep -q 'stop/' ; then
exit 0
elif echo $statusnow | grep -q 'respawn/' ; then
exit 1
fi
sleep 1
done
exit 1
end script
You might also want to tweak respawn limit as the defaults don't really
take into account a daemon that might take more than 0.5s to fail.
* exit 1 does not really abort the script the way you have it done here.
Instead you will want to use 'stop; exit 0'. This means the job
intentionally does not want to start. Also a non-existant binary is not
an error, so do not print anything or exit 1 for that. If a package has
been 'removed', its upstart job will still be present and should handle
this gracefully.
* Some of the variables have been blindly copied in as 'env' statements.
K5START_DESC for instance is copied in, but is not used in the upstart
job.
* Instead of mkdir+chown just use install -d.
--
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
