dh_installinit upstart support start a job in postinst script regardless of previous status

This bug was fixed in the package debhelper - 8.0.0ubuntu2

---------------
debhelper (8.0.0ubuntu2) natty; urgency=low

  * autoscripts postinst-upstart, prerm-upstart, prerm-upstart-norestart
    need to use invoke-rc.d instead of calling start/stop directly, so that
    they're well-behaved in chroots and other places policy-rc.d may be used.
    LP: #690640.
 -- Steve Langasek <email address hidden> Sun, 23 Jan 2011 12:38:00 -0800

Hi Steve

Is it possible to backport this fix to Lucid?
I tried your patch on lucid's debhelper and packages compiled with this patched version has correct scripts

thanks

On Tue, Jan 25, 2011 at 03:00:32PM -0000, Alessandro Bono wrote:

> Is it possible to backport this fix to Lucid?
> I tried your patch on lucid's debhelper and packages compiled with this patched version has correct scripts

I think a backport to lucid is of questionable benefit, unfortunately, since
we would then also need to rebuild all of the affected packages that ship
upstart jobs and I don't think we have a committment to do that work. I'll
subscribe the ubuntu server team to this bug however, for their input - if
there's interest in doing the work to rebuild the affected server packages,
I'm ok with backporting this.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>

By my estimate, there are a bit over 50 pending, unassigned SRU bugs for the server team in lucid. Many of these are small but the time just to get them into the pipeline has been scarce and we are under-staffed as it is. Also the SRU team isn't exactly bursting at the seams. So while I do think this is important, I want to caution any suggestion that we add a giant heap of packages to this list without thinking clearly about it first.

On this bug specifically, one thing I see is that we're really running in to the upstart bug which makes it hard to definitively disable a service.

With sysv scripts, invoke-rc.d looks in the rc#.d dirs and makes sure that this service is supposed to be started, and if it is, then it starts the service, otherwise, the action is ignored.

With upstart jobs, its not really possible to know if a job should be running by looking at the filesystem. So this seems to be missing a fix that would, as Alessandro said, query and save the status when the stop action is run during preinst, and return the job to the goal it was in pre-upgrade. The policy-rc.d stuff would work too, but that is less discoverable and won't actually disable the upstart job at boot time, so I think before we discuss lucid, we should look at whether or not whats been done now is a complete solution.

On Tue, Jan 25, 2011 at 08:04:15PM -0000, Clint Byrum wrote:

> With upstart jobs, its not really possible to know if a job should be
> running by looking at the filesystem. So this seems to be missing a fix
> that would, as Alessandro said, query and save the status when the stop
> action is run during preinst, and return the job to the goal it was in
> pre-upgrade.

That's not a policy-correct solution for service handling. There needs to
be a method for distinguishing between jobs that have been administratively
disabled or that aren't supposed to run because of the current policy (e.g.
runlevel), and those that are not running for some other reason (such as a
prior package upgrade failure).

Implementing support for such information is on the upstart roadmap, but it
won't be reasonably backportable to lucid.

> The policy-rc.d stuff would work too, but that is less discoverable and
> won't actually disable the upstart job at boot time, so I think before we
> discuss lucid, we should look at whether or not whats been done now is a
> complete solution.

Yes, the fix I implemented has two main purposes:

 - to make upstart services respect policy-rc.d which, awkward as it is, is
   the interface Debian policy gives us to work with for setting service
   policies
 - to fix the use of '|| :' which causes maintainer scripts to ignore *all*
   causes of job start failures, instead of just those caused by trying to
   start an already-running job

This doesn't fix all the problems... just those that we can fix, right now,
in debhelper. It's also the *only* change we should need to make in
debhelper; the rest of the solution needs to be implemented in upstart +
invoke-rc.d.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>

Alright, I see where we do need this information encoded.

The current recommendation for natty is to add the word 'manual' to the job configuration file to disable it. For 10.10 and earlier, commenting out the start on line is the preferred method, or moving the file somewhere else.

What if we added

if grep -q '^\s*start\s*on' /etc/init/${JOB}.conf && ! grep -q '^\s*manual' /etc/init/${JOB}.conf ; then
  $ACTION $JOB
fi

To upstart-job?

Since we already have to do a --no-start for special case event based tasks, this would reliably catch the known ways to disable upstart jobs, and otherwise doesn't change the behavior.

Lucid backport is a good thing to do because also if we don't rebuild all affected package now at first update we resolve a bug for free, lucid is a LTS and probability of update package in future is high

On Tue, Jan 25, 2011 at 09:15:25PM -0000, Clint Byrum wrote:
> Alright, I see where we do need this information encoded.

> The current recommendation for natty is to add the word 'manual' to the
> job configuration file to disable it. For 10.10 and earlier, commenting
> out the start on line is the preferred method, or moving the file
> somewhere else.

> What if we added

> if grep -q '^\s*start\s*on' /etc/init/${JOB}.conf && ! grep -q '^\s*manual' /etc/init/${JOB}.conf ; then
> $ACTION $JOB
> fi

> To upstart-job?

> Since we already have to do a --no-start for special case event based
> tasks, this would reliably catch the known ways to disable upstart jobs,
> and otherwise doesn't change the behavior.

Not very elegant to have upstart-job parsing the job files manually. If
upstart now knows about jobs that should only be started manually (this new
'manual' keyword), could we not instead extend the control interface so that
/sbin/start can take a new option ('--system'? '--maintainer-script'?) that
tells upstart to skip the operation for 'manual' jobs?

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>

I like that idea quite a bit for natty and later, thats probably the right way to go, especially since we'll also be getting "override" files at some point. I'll submit it to upstart as a feature request.

I doubt we'll backport manual to maverick and lucid's upstart, so we're still left with the problem of updating peoples' systems and potentially starting services that are meant to stay stopped.

On Wed, Jan 26, 2011 at 09:28:00AM -0000, Clint Byrum wrote:
> I like that idea quite a bit for natty and later, thats probably the
> right way to go, especially since we'll also be getting "override" files
> at some point. I'll submit it to upstart as a feature request.

Ok.

> I doubt we'll backport manual to maverick and lucid's upstart, so we're
> still left with the problem of updating peoples' systems and potentially
> starting services that are meant to stay stopped.

Right; I'm not going to try to fix that issue in lucid/maverick. It's
unfortunate, but that's a significant semantic change that I don't think is
backportable.

BTW, a prerequisite to backporting this debhelper change is to fix bug
#707971 in upstart-job, which is causing much havoc for people right now in
natty.

--
Steve Langasek Give me a lever long enough and a Free OS
Debian Developer to set it on, and I can move the world.
Ubuntu Developer http://www.debian.org/
<email address hidden> <email address hidden>