#614569 slapd: Dist-Upgrade Lenny->Squeeze: syncreplication w/ incomplete objects fails

Package:
slapd
Source:
openldap
Description:
OpenLDAP server (slapd)
Submitter:
Rainer Ruprechtsberger
Date:
2014-12-04 13:09:10 UTC
Severity:
important
#614569#5
Date:
2011-02-22 10:43:26 UTC
From:
To:
*** Please type your report below this line ***

Hallo again,

dist-upgrading ldap syncreplication slaves with incomplete object fails.
My setting uses 'schemachecking=off' and the replication user is not
allowed to read all attributes on the master server. The upgrade scripts
make a backup of the existing ldap database and tries to restore it with
slapadd. This fails and leave the the slapd package half configured.
For example:

Loading the database from the LDIF dump failed with the following
error while running slapadd:
    Entry (uid=foobar,ou=people,dc=volkshilfe-ooe,dc=at): object class
'inetOrgPerson' requires attribute 'sn'
    slapadd: dn="uid=foobar,ou=people,dc=volkshilfe-ooe,dc=at"
(line=24): (65) object class 'inetOrgPerson' requires attribute 'sn'
dpkg: error processing slapd (--configure):
 subprocess installed post-installation script returned error exit status 1

Exchanging the backup with a fake (only containing the top object) was
the fastest way to resolve this problem i was able to find.

/r

#614569#10
Date:
2011-03-28 13:47:58 UTC
From:
To:
Hi all,

Cc'ing my fellow maintainers.

To fix this bug I can add simply -s to the slapadd in the function load_databases.

The -s option disables the schemachecking. But I'm not sure if this is a good solution
to fix the upgrade procedure.

Maybe it's better to look at the configuration and check if it's a slave and then add this
option.

Any comments?

Regards,

Matthijs Möhlmann

#614569#15
Date:
2011-03-28 16:51:27 UTC
From:
To:
--On Monday, March 28, 2011 3:47 PM +0200 Matthijs Möhlmann 
<matthijs@cacholong.nl> wrote:

It is never wise to disable schema checking.

#614569#20
Date:
2011-03-28 17:11:36 UTC
From:
To:
Well, the bugsubmitter has slaves with partial replication (not all attributes are
replicated) and the replication user is not allowed to read all attributes on the
master.

If you say it's never wise to disable schemachecking, why is there an option to
disable the schemachecking in the replication?

Regards,

Matthijs Möhlmann

#614569#25
Date:
2011-03-28 17:31:16 UTC
From:
To:
--On Monday, March 28, 2011 7:11 PM +0200 Matthijs Möhlmann 
<matthijs@cacholong.nl> wrote:

For that very reason (partial replication). :/

So the problem is that if you use -s, people who have invalid databases
that shouldn't get imported may then get imported.  But if you don't, you
hit this issue.  I guess you need to decide which is the lesser evil. ;)

#614569#30
Date:
2011-03-28 20:08:48 UTC
From:
To:
Than it's easy, I'll stay with the current behaviour, I won't disable schemachecking for now.

The default for schemachecking is off in olcSyncrepl, so the solution here is to check if
the directory is a slave and based on the schemachecking use the -s flag.

Well I'll move that one a bit down on my priority list.

Regards,

Matthijs Möhlmann

#614569#35
Date:
2011-03-30 14:51:41 UTC
From:
To:
slave. In the end its what I did anyway.
A clean way to get to this point is whats missing :)

/r

#614569#40
Date:
2011-03-30 16:49:20 UTC
From:
To:
--On Wednesday, March 30, 2011 4:51 PM +0200 Rainer Ruprechtsberger 
<rainer.ruprechtsberger@volkshilfe-ooe.at> wrote:

That is not a very worthwhile suggestion for anyone with a sizeable
database.  It would take hours to days to weeks.

#614569#49
Date:
2014-12-02 17:03:58 UTC
From:
To:
Control: clone 770827 -2
Control: retitle -2 slapd: tries to reload on upgrade even with dumping disabled
Control: severity 614569 important

Just to be clear, we're talking about (at least) three separate bugs:

#614569 - fails to reload a partial replica
#770827 - dpkg-reconfigure doesn't ask the dumping question
#??? - attempts database reload even with dumping set to 'never'

#770827 I think is clearly not RC. #614569 would have been good to fix
for jessie, but I personally have basically no time available at the
moment. I can't provide a well-tested fix before the 5th, sorry.

Other facts about #614569 are that it has been around for two upgrade
cycles already, and that it can be worked around by editing the postinst
to add -s to the slapadd invocation and running 'dpkg --configure -a' to
retry the upgrade.

Unless someone else wants to prepare and upload a fix in the next couple
of days, I'll fix it in unstable later and then propose it for a point
release of jessie. I'm also monitoring some lmdb issues being worked on
upstream, in case some of the patches turn out to be suitable for a
point release.

I doubt this was the answer you hoped for; sorry for that.

thanks,
Ryan

#614569#56
Date:
2014-12-02 18:45:26 UTC
From:
To:
Ryan Tandy <ryan@nardis.ca> writes:

Precisely.  Thanks for setting this up!

Well, you can work around it by debconf-set-selections I guess...  If
#771823 didn't spoil it, this could be the recommended workaround for
upgrades of partial MDB replicas.

I'm inclined to simply add that -s option.  If the database was schema-
correct, it will stay so, and it it wasn't, then upgrade isn't the best
time to point that out.  There isn't much to test on such a change.  To
do any better, you'd need separate dumping decisions on each configured
database, which would be nice (ATM only BDB and HDB need this, after
all) but that may be too much for a last-minute change.  I'm willing to
work on this if we can agree on the direction.  I'll have to upgrade my
servers somehow, doing it offically can't hurt...

That's what I did.  Not exactly the expected Debian upgrade
experience. :(

That's good to hear.

Sure it was, really!  Unfortunately, I can't upload, so a sponsor would
be needed to conclude this.  Also, do you think some pre-agreement from
the release team would be needed before the upload?

#614569#61
Date:
2014-12-02 20:32:34 UTC
From:
To:
[dropping #770827 from Cc, keeping #614569, changing subject]

I don't like interrupting upgrades either, but if we don't check
correctness during reloads, I don't know what other opportunity we have.
I suppose you could argue it's the admin's concern and not ours.

Anyway, I'm not the one you have to convince, rather someone who
actually has the ability to sponsor your NMU. ;) But I don't have
concrete objections to the change, only a general fear of "unknown
unknowns", as some call them.

For stretch, I plan to implement Matthijs' idea:

Eventually (2.5) that will be needed for mdb too. It should be a
relatively small change.
reportbug release.debian.org, attach a source debdiff) before uploading
(but after securing a sponsor).

thanks,
Ryan

#614569#66
Date:
2014-12-03 17:14:39 UTC
From:
To:
Ryan Tandy <ryan@nardis.ca> writes:

Yes, that's my point.  The upgrade does not change schema validity, and
should not crash in either case.

I pushed my proposed change at a separate branch:
http://anonscm.debian.org/cgit/users/wferi-guest/openldap.git/log/?h=bug/614569
Reproducing it here:

commit 8a769204be90c1eaf7d5070c4e8a2d1eb18a82dc
Author: Ferenc Wágner <wferi@niif.hu>
Date:   Wed Dec 3 15:51:58 2014 +0100

    Disable schema checking for reloading the dumped data during upgrades

    This avoids interrupting the upgrade procedure of partial replicas (#614569).

diff --git a/debian/changelog b/debian/changelog
index 1fcc7f3..012df5d 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,12 @@
+openldap (2.4.40-3.1) unstable; urgency=medium
+
+  * Non-maintainer upload.
+  * Disable schema checking for reloading the dumped data during upgrades,
+    to avoid interrupting the upgrade procedure of partial replicas.
+    (Closes: #614569).
+
+ -- Ferenc Wágner <wferi@niif.hu>  Wed, 03 Dec 2014 15:35:44 +0100
+
 openldap (2.4.40-3) unstable; urgency=medium

   * Remove trailing spaces from slapd.templates.
diff --git a/debian/slapd.scripts-common b/debian/slapd.scripts-common
index f56b3b1..8cf7f7b 100644
--- a/debian/slapd.scripts-common
+++ b/debian/slapd.scripts-common
@@ -220,8 +220,11 @@ load_databases() {                                                 # {{{
                else
                        slapadd_opts="-g -F ${SLAPD_CONF}"
                fi
+               # Disable schema checking for the reload of the dumped data.
+               # Otherwise, reloading partial replicas fails, breaking the
+               # upgrade process.
                capture_diagnostics slapadd ${slapadd_opts} \
-                       -q -b "$suffix" -l "$file" || failed=1
+                       -q -b "$suffix" -l "$file" -s || failed=1
                if [ "$failed" ]; then
                        rm -f "$dbdir"/*
                        echo "failed." >&2

It behaved as expected in my tests.  Now I'm doing a full clean sbuild
with tests (which take LONG), but let me first try to get some review
here:

Would anybody please sponsor an NMU with the above change added to
2.4.40-3, if I happen to get the change pre-approved by the release
team?

#614569#71
Date:
2014-12-03 22:40:24 UTC
From:
To:
Ferenc Wagner <wferi@niif.hu> writes:

I got pre-approval on #771962: the upload will be unblocked, provided
it's in unstable by Monday the 8th of December.  People with upload
rights, if you can spare a minute please review the above change and
consider sponsoring the upload!  Actually, review is welcome from
anybody. :)

The package is available at
http://mentors.debian.net/package/openldap

The dsc file can be downloaded from
http://mentors.debian.net/debian/pool/main/o/openldap/openldap_2.4.40-3.1.dsc

Ryan, please note that my upload somehow ended up under your name on
mentors.debian.net.  Thus I can't flip the "Needs a sponsor" flag.

Also, QA information says "Bug #614569 does not belong to this package",
and I only wonder why...

#614569#76
Date:
2014-12-04 02:21:08 UTC
From:
To:
Thanks very much for working on this, and the debdiff looks fine (but I
haven't actually built or tested it). I hope you can find a sponsor in
time. Luca, are you reading?

Er... oops. Just before reading this I logged into my account there and
deleted the openldap package, thinking I'd clear out the rejected
wheezy-backports package...  I think it actually took out your upload,
sorry! You might want to re-upload that.

Oh, yeah. My prior 2.4.40 uploads had a bunch of those. According to
pabs, just a bug in the web interface. It doesn't match up source and
binary packages properly, or something like that.

thanks,
Ryan

#614569#81
Date:
2014-12-04 10:13:03 UTC
From:
To:
comfortable in uploading this NMU at this point of the release cycle.
So NACK on my side.

My main concerns are:
 * I am not sure that I grok all the implications of that. I suspect that
   most of our (overly) complex pre/postinst scripts makes some assumption on
   schema/db consistency, at least implicitly.
 * We are changing the default behavior to fix a single-case situation, by
   removing a safety check. Mmmh...
 * This bug has been open since some time, never marked as RC, put on
   low-priority by the maintainer and upstream discouraged dropping the "-s".
 * This is not even the proper solution, just a quick-hack patch.

Moreover, I don't consider myself an LDAP expert, but I have some comments on
the issue:
 * I would say that requiring/checking schema integrity across upgrade is a
   good general measure, and we should NOT work around that. Fail early, fail
   loudly.
 * IIRC disabling schemachecking is heavily discouraged. We don't offer that
   option in debconf. I assume the admin are really aware of the setup, and
   know its quirk.
 * Workarounds for the specific corner case exists, but maybe are a bit
   undocumented.

So my alternative suggestion is: let's make it explicit that we value schema
integrity, and we rely on it across upgrades; let's put a point in the release
notes to document how to workaround this partial replica scenario; let's try
to address this better in the next point release.

Cheers, Luca

#614569#86
Date:
2014-12-04 13:05:48 UTC
From:
To:
Luca Bruno <lucab@debian.org> writes:

I went through all the maintainer scripts in the package, and did not
find any dependency on DB consistency, apart from the config DB itself,
which we touch on the filesystem (which is unsupported).  And of course
apart from the current issue, which is assuming that slapadd will work
on the result of slapcat.  This is false without the -s option.

As you point out below, debconf does not offer switching off schema
checking.  Just like it does not offer configuring replication.  Thus if
a database is not schema-correct, that must be the result of manual
configuration, not some default behavior.  From this point of view, we
do not change the default behavior, we extend it to other cases.  If the
database was schema-correct, it will stay so.  If the admin uses a
config where it wasn't, then it will stay so as well.

I don't understand why.  This bug breaks upgrades, and the only
workaround I know of is manually editing the postinst script.

(I guess you mean "using").  Yes, but I think their reasoning about
importing invalid databases does not apply in our case, see above and
below.

The "proper" solution would require separate dump/restore decisions for
each database, and we haven't yet got the infrastructure for that.  And
it would only achieve not using the -s flag when it wouldn't have any
effect anyway (maybe apart from slowing down the import).  After all,
we're loading dumps we ourselves made a couple of moments earlier, not
some synthetic data.

My point here is that schema-correctness has nothing to do with the
upgrade, as the schema is just a part of the local configuration, which
is not changed by the upgrade.

As far as I know, it's not only discouraged, but is actually impossible
for some time now.  The only way to switch off schema checking is using
replication, making your database a read-only shadow.

I don't consider editing the postinst script a viable workaround.  Maybe
I miss something here?  Disabling dumping does not cut it, see #770827
and #771823.

OpenLDAP also values schema-correctness (see above), and that's why
artifically relying on it does not really help, only needlessly breaks
upgrades on partial replicas.

I honestly don't think there's much to gain in that direction.  Unless
there really is a good workaround (not involving hand-editing maintainer
scripts during the upgrade), I find it an unneeded burden on the user.