Bug#801065: Section 6.4 - discourage failing install or upgrade when service fails to start

[Marvin Renich]

* Henrique de Moraes Holschuh  [151007 09:42]:
> On Mon, Oct 5, 2015, at 22:13, Marvin Renich wrote:
> > The discussion started on d-devel; should it be moved back there?  The
> > overwhelming majority of opinion seems to be in favor of the change.
> 
> We have supported per-package behavior on this for a very long time,
> maybe since forever (this is no joke).  We have had packages that
> restart after the upgrade instead of stopping before, or that ignore
> service start failures during upgrades for *years*.

Agreed.

> All it takes is that the package maintainer actually stop to think about
> it, consider which behavior is more appropriate to that specific
> package, and adjust things appropriately.

I enthusiastically agree!

> There is a damn good reason
> why policy does not use "must" to mandate service start/stop/restart
> behavior across package updates.

Again, agreed.  I was not proposing to use "must", and would not support
such a proposal.

> The reason for this thread, an "undesired" behavior of stopping a
> package upgrade if the service fails to start, which is the current
> default, is both employed by the (likely few) packages that absolutely
> depend on it to avoid worse damage down the service/package dependency
> chain, as well as by packages that the maintainer did not even think
> about the issue (and therefore might or might not need this behavior).

Right.  And the second case, which is at least perceived by me (and
apparently others) to be the vast majority of services, is what this
proposal is about.

> IMO, we should not just "change this default" in the *implementation*
> (debhelper, etc), at least not without actually acessing the real risk
> of the change.  It does not look like this kind of risk accessment was
> done by anyone yet in the d-devel thread.  Just because we expect it to
> be low, doesn't mean it doesn't exist or that it won't have a high
> impact on the user if it happens.

Actually, I was not aware that dh had a helper for this that defaulted
to "fail upgrade if service fails", and so was not intending to propose
an automatic change in behavior based on a change to dh.  This part of
it certainly requires more thought and a thorough risk assessment.

My proposal, and what I still think we should do regardless of any
change or lack of change to dh, is to document, in a place that package
maintainers will see, that the "best practice" is to not fail the
install just because the service fails to start.

> If the need for this kind of provision in a possible policy text update
> (possibly as a foot-note) is contentious, IMHO the matter needs to go
> back to d-devel for further discussion.

I don't see this as a contentious change; at least the part about
changing what is considered "best practice".  Changing dh hasn't, as far
as I can see, been discussed enough to know whether it is contentious or
not.  But, as you point out, a risk assessment followed by some
discussion on d-devel would be the first two steps.

I don't think a documentation change to "best practices" should be held
up by the dh issue.  Perhaps someone wants to clone this bug to the
debhelper package so the documentation change to developers-reference
and the potential implementation change to debhelper are tracked
independently.  If you would like me to do so, let me know.

...Marvin

Re: Bug#801065: Section 6.4 - discourage failing install or upgrade when service fails to start

[Henrique de Moraes Holschuh]

On Mon, Oct 5, 2015, at 22:13, Marvin Renich wrote:
> The discussion started on d-devel; should it be moved back there?  The
> overwhelming majority of opinion seems to be in favor of the change.

We have supported per-package behavior on this for a very long time,
maybe since forever (this is no joke).  We have had packages that
restart after the upgrade instead of stopping before, or that ignore
service start failures during upgrades for *years*.

All it takes is that the package maintainer actually stop to think about
it, consider which behavior is more appropriate to that specific
package, and adjust things appropriately.  There is a damn good reason
why policy does not use "must" to mandate service start/stop/restart
behavior across package updates.

The reason for this thread, an "undesired" behavior of stopping a
package upgrade if the service fails to start, which is the current
default, is both employed by the (likely few) packages that absolutely
depend on it to avoid worse damage down the service/package dependency
chain, as well as by packages that the maintainer did not even think
about the issue (and therefore might or might not need this behavior).

IMO, we should not just "change this default" in the *implementation*
(debhelper, etc), at least not without actually acessing the real risk
of the change.  It does not look like this kind of risk accessment was
done by anyone yet in the d-devel thread.  Just because we expect it to
be low, doesn't mean it doesn't exist or that it won't have a high
impact on the user if it happens.

If the need for this kind of provision in a possible policy text update
(possibly as a foot-note) is contentious, IMHO the matter needs to go
back to d-devel for further discussion.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique de Moraes Holschuh 

Bug#801065: Section 6.4 - discourage failing install or upgrade when service fails to start

[Marvin Renich]

Package: developers-reference
Version: 3.4.16
Severity: wishlist

As discussed on debian-devel starting at [1], I would like a comment
added to Section 6.4 "Best practices for maintainer scripts" that
recommends preventing the postinst script from returning failure when a
service fails to start.

A general rule of thumb is that if the corrective action would not
otherwise require re-running dpkg, then the postinst should behave as if
the installation succeeded.

An example of a case where postinst should succeed is if the admin,
prior to an upgrade, modified a configuration file which resulted in the
service being unable to restart during the upgrade.

Another example is if the service requires another resource to be
available that might be on another machine (e.g. a database).

The rational is that the failure has nothing to do with the installation
process or the contents of the .deb.  The service might just as well
have failed on reboot even though it was correctly installed.

I will send a follow-up message that contains a condensed digest of the
thread beginning at [1].

...Marvin

[1] https://lists.debian.org/debian-devel/2015/09/msg00496.html

Bug#801065: Section 6.4 - discourage failing install or upgrade when service fails to start

[Marvin Renich]

* Russ Allbery  [151005 18:24]:
> I'm also in favor.  However, this is a very substantial change to Debian
> practice, and I'm not sure what process should be used for making this
> kind of decision.  This wasn't a gap in our specification; rather, the
> previous standard was explicitly chosen (by at least some folks).  Failure
> to install was intentional and believed better since it didn't hide
> service failures.

I understand that this was intentional, but it is not the only way to
make sure the user (admin) is informed of the failure.

It was suggested in the discussion on d-devel that some other
notification mechanism could be implemented instead of installation
failure.

I was thinking about a special dpkg trigger that would be handled
internally by dpkg.  Packages would, in their postinst, place a file
containing non-fatal but important failure information in a directory
owned by dpkg.  This info would be printed out by dpkg at the end of the
run, and would also be passed to front ends that asked.

Front ends such as aptitude could ask dpkg for these notifications.  If
a large installation needed to be split into multiple calls to dpkg, the
front end can aggregate the notifications and present them all at the
very end, in whatever way is native to the front end.

One of the other notifications that I, personally, would like to see
this used for is when a configuration file has changes that cannot be
handled automatically.  Currently you are asked in the middle of the
installation; this would not change at all.  But it would be nice to
have a summary at the end of all the config files that had incompatible
changes, regardless of how I answered the dpkg or ucf prompt.

> I feel like this would benefit from a broader discussion than just the
> Policy list, but I'm not sure how to go about that, or what teams in
> particular should weigh in.

The discussion started on d-devel; should it be moved back there?  The
overwhelming majority of opinion seems to be in favor of the change.

...Marvin

Bug#801065: Section 6.4 - discourage failing install or upgrade when service fails to start

[Sam Hartman]

> "Marvin" == Marvin Renich  writes:

Marvin> As discussed on debian-devel starting at [1], I would like a
Marvin> comment added to Section 6.4 "Best practices for maintainer
Marvin> scripts" that recommends preventing the postinst script from
Marvin> returning failure when a service fails to start.

I strongly support this practice.

Bug#801065: Section 6.4 - discourage failing install or upgrade when service fails to start

[Bill Allombert]

On Mon, Oct 05, 2015 at 03:20:34PM -0700, Russ Allbery wrote:
> Sam Hartman  writes:
> >> "Marvin" == Marvin Renich  writes:
> 
> > Marvin> As discussed on debian-devel starting at [1], I would like a
> > Marvin> comment added to Section 6.4 "Best practices for maintainer
> > Marvin> scripts" that recommends preventing the postinst script from
> > Marvin> returning failure when a service fails to start.
> 
> > I strongly support this practice.
> 
> I'm also in favor.  However, this is a very substantial change to Debian
> practice, and I'm not sure what process should be used for making this
> kind of decision.  This wasn't a gap in our specification; rather, the
> previous standard was explicitly chosen (by at least some folks).  Failure
> to install was intentional and believed better since it didn't hide
> service failures.

But it had the major drawback that you could not fix the issue when the fix
required to install more packages, which is common.

So I am also in favor.

Cheers,
-- 
Bill. 

Imagine a large red swirl here. 

Bug#801065: Section 6.4 - discourage failing install or upgrade when service fails to start

[Russ Allbery]

Sam Hartman  writes:
>> "Marvin" == Marvin Renich  writes:

> Marvin> As discussed on debian-devel starting at [1], I would like a
> Marvin> comment added to Section 6.4 "Best practices for maintainer
> Marvin> scripts" that recommends preventing the postinst script from
> Marvin> returning failure when a service fails to start.

> I strongly support this practice.

I'm also in favor.  However, this is a very substantial change to Debian
practice, and I'm not sure what process should be used for making this
kind of decision.  This wasn't a gap in our specification; rather, the
previous standard was explicitly chosen (by at least some folks).  Failure
to install was intentional and believed better since it didn't hide
service failures.

I feel like this would benefit from a broader discussion than just the
Policy list, but I'm not sure how to go about that, or what teams in
particular should weigh in.

-- 
Russ Allbery (r...@debian.org)   

Bug#801065: Section 6.4 - discourage failing install or upgrade when service fails to start

[Sam Hartman]

> "Russ" == Russ Allbery  writes:

Yeah, but there's a significant factor that reduces things somewhat.
In the past, /etc/init.d/foo failing would often cause postinst to
break.
However, in the past, there were a lot of failures that were not
detected by the init.d script.
We have two intentional decisions conflicting:

1) systemd tries to be a lot better about reporting service status than
our previous infrastructure

2) We had the decision on a number of people to not hide failures by
causing installation to fail.

I actually think the folks in category 2 weren't typically hiding a lot
of service failures, because  it was fairly common for the init script
to mask that, but it did tend to hide failures like missing
configuration files etc.

I certainly know that when deploying units for krb5 I had to mask a
bunch more failures to get behavior consistent with the previous
packages.

Given the above, I think a discussion on -devel (which has happened) and
a discussion on-policy is sufficient.

--Sam