[Openstack-devel] Splitting the nova package (and possibly others)?

[Thomas Goirand]

Hi Roland,

I believe the main issue we are having here is that pgsql doesn't
support remote configuration of users, and that dbconfig-common doesn't
take this into account. But that's it. Please read further.

On 11/20/2012 12:24 AM, Roland Mas wrote:
>   For the rest, I did some fact-checking, and it appears that:
> - dbconfig-common is indeed able to create remote databases; however,
>   that is hidden behind a low-priority Debconf question, so most users
>   won't even see that;

Yeah! Which is why we explicitly wrote in all of our templates: "You can
change this setting later on by running "dpkg-reconfigure -plow <pkg-name>".

> - apparently it cannot be told to just *use* existing remote databases;
> - it always tries to create the db.

The important bit is that it wouldn't *fail* in the case of MySQL.
Attempting to create a db which already exists isn't a problem, but it
doesn't even do that. dbconfig-common sees the db already exists and
doesn't creates it.

>> But it does *not*! 
> 
>   It does.  Really, truly, giving the database server password to some
> app/host/person does mean giving them full access to the database
> server, even if only temporarily.  I know plenty of places where this
> isn't going to happen (and that includes my own admittedly very small
> network of servers and VMs).

In this case, feel free to contribute patches against dbconfig-common
itself. But IMO, this is not a sufficient reason to reimplement it in a
different way, with new debconf templates asking the same thing
dbconfig-common does.

>> dbconfig-common asks for the admin privileges for the time to create
>> the db. Once it is done, it flushes it and never remembers it.
> 
>   I'm not sure it asks for it on every package upgrade, but even if it
> only asks for it on the initial installation, that would be once too
> many.

Upon a package upgrade, it will ask to the user "do you want to upgrade
the db with dbconfig-common?".

> Note that there are many database servers (including the default
> PostgreSQL installation in Debian) where the database admin can't even
> connect remotely.

I never tried PostgreSQL. Almost everyone uses MySQL with Openstack,
anyway, so don't bother with PostgreSQL too much.

>>> So you need the remote db to be setup externally, and what I
>>> did is just a way to help the admin of the compute node in configuring
>>> the node by providing a UI to type the required parameters.
>>
>> Well, this UI exists already in dbconfig-common.
> 
>   Not really.  What exists in dbconfig-common is UI to create a remote
> database, not a UI to use an existing remote one.  It'll always try to
> create it with the database admin account, which means it needs the
> admin credentials.

But that's not a problem for us (apart from the fact that you need the
admin credentials). Attempting to create the db, even if it exists
already, will not destroy it.

>>> (That's exactly the same as what dbconfig-common policy says to do,
>>> by the way: if we're not setting up the DB, then assume the local
>>> admin will take care of that.) Now if you can coax dbconfig-common
>>> into providing this UI, then fine, my change can be rewritten to use
>>> this.  I believe this is going to need changes *in* dbconfig-common,
>>> but I may be wrong.  In any case, as long as the results still work,
>>> I don't really care.
>>
>> Well, the result does *not* work. 
> 
>   Actually, it does.  I checked before pushing last week, and I just
> checked again, and it does work.

What you told me what you've checked was your preseed scripts. That
isn't the thing which I thought was broken.

>> Try to set debconf in non-interactive
>> mode, and you'll have db_get ${PKG_NAME}/configure_db set to false, then
>> it will go in your:
>> db_input high ${PKG_NAME}/remote_db_* || true
>> things, which will have no value (because running in non-interactive
>> mode), and then fail.
> 
> Fail how?  The installation succeeds.  It doesn't configure a
> database, apart from creating an empty SQLite one.  In other words, it
> does exactly what it did before my changes ("let the admin handle the
> database part").

Well, your script is just calling db_input without even checking what's
stored in the configuration file. If there was something there, with the
info about how to connect to a remote PostgreSQL db, it would fail to do
what the user expect.

> Except we now also have the ability to use a remote
> database, either with interactive prompts or with preseeding, which we
> didn't have before.

I don't agree. We did have such option. The problem is this:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=476946

This bug has been opened since 2008. If there is something to change, it
is in dbconfig-common, *not* in our packaging which depends on it.

>> I'm sorry, but I don't know what upgrade problem you are talking
>> about.  If there is one, please care to explain it.
> 
> It is not the compute node's role to handle database upgrades.

So, it's the role of who? Obviously, if you upgrade one of the
components of your compute thing, you will need to upgrade the others as
well, no?

> We're
> lucky that the upgrade script is idempotent in this case, but I don't
> think it's wise to rely on that luck.

It's *not* luck. See Julien's post about it. There's a version thing,
and it's written this way *on purpose*. This will not change, and we can
rely on this feature. If you don't trust this, ask core developers in
the openstack project.

> It is also generally not a good idea for a leaf node to handle
> database upgrades.

When you do your upgrade, you are free to start by upgrading the server
that you like. Proxy or compute (eg: leaf, as you wrote) node. The order
doesn't mater much. Why do you want to impose any kind of workflow to
the person doing the upgrade? We don't need to do that, IMO.

> Such things should be run on the central node,
> because that prevents leaf nodes from breaking the whole cluster.

That prevents nothing. You are, IMO, speculating here.

> Think
> of it as an API: you don't want users of the API to change it, you want
> the library to change it (and then upgrade the users).

Everything will be unstable until you finish your upgrade anyway, so in
would be a good idea to shutdown the API servers before you upgrade
anything, and until *everything* is upgraded, including all compute
nodes. But IMO, this has nothing to do with the leaf node having the
upgrade code also, which is unrelated.

>   To summarize: contrary to what you claim, my changes:
> - work (yes, I do test);
> - made it possible to *use* a remote database (as opposed to creating
>   it), which wasn't possible before;
> - didn't remove anything;
> - didn't break anything.

Your code:
- needs new debconf templates which are otherwise also available in
dbconfig-common, and for which the l12n team will complain very loudly
when they will translate (believe me, they will! if you don't trust this
fact, ask them now, before attempting any reimplementation...).
- removes the ability to be able to just reads everything from the
config file (your code doesn't read it, but of course, this could be
implemented...).
- is attempting to re-invent something we have already.

I don't agree it wasn't possible to use a remote db before. It only
breaks (according to you, as I didn't try myself) with PGSQL, since you
can't remotely create users in this case. If it really wasn't working,
please show me how dbconfig-common *destroys* the existing db while
*re*-creating it. That's simply not the case, dbconfig-common checks if
the db exists, and if it does, then it doesn't creates it. It even shows
what it does on the console with nice echoes...

Now and again, if you think the current dbconfig-common is a problem
(which it is, apparently, with PGSQL), then fix it in the
dbconfig-common package.

>   I'm not saying the changeset is perfect, and maybe it can be made
> better.  But the current code is actually working and actually useful,
> so I don't understand your objections to it.

My objection is that it did work before. Also, if you think there's a
problem in dbconfig-common, then please file bugs against it, and
eventually fix it. We aren't targeting testing/Wheezy here, so it really
is possible to bring change to dbconfig-common so it does what you
expect. I do think that having the possibility to just *use* a db
without requesting for the root credential would be a nice improvement
to dbconfig-common, and I would support you as much as I can if you do
it! I'm serious. I might also try to help to move in this direction.

> I just noticed you actually went and reverted anyway, without
> waiting for the explanations you requested.  I'm not too happy
> about that, as you can guess.  But whatever, I'll find something
> else to work on.

Of course I did! You left last Thursday evening with a commit on
openstack-pkg-tools which requires to add new templates in at least 5
packages (glance, keystone, nova, quantum, cinder...), but you added
these templates only in nova. As a consequence, 4 packages were broken,
and I couldn't afford to leave them in this state, since Loic explicitly
asked me to go through the howto and stop doing anything else. I really
had no choice.

Now, please realize that I wasn't "too happy about that" either, when I
saw that you implemented something I was strongly opposed to. The first
time was with the apache configuration files for Horizon which you
insisted to store in a patch file, and which I had to later fix with
this commit:
http://anonscm.debian.org/gitweb/?p=openstack/horizon.git;a=commitdiff;h=f8ee35931c773ec79fb824403c13af24d07c47f9

I believe that this last patch will satisfy both of us (eg: it uses
debian/openstack-dashboard-apache.install, and not BSD install, but it's
not a quilt patch anymore). However, that's not the point. You
completely ignore my requests to revert the changes making the
configuration files a patch. I explicitly wrote on IRC "please revert",
yet you didn't do anything for that one.

Now, you are now accusing me of not taking your opinion into account?!?
I have the opposite feeling... :/
We shall try that this doesn't happen again and that we can both learn
how to communicate better.

Going back to this dbconfig-common, we discussed these changes, I told
you I strongly did not agree with them, and even strongly opposed to it,
and you still went into that direction, broke 5 packages doing so (see
just above), then went away for 4 days, saying that you had something
else to deal with (which I respect, but you shouldn't break things and
run...). In this kind of situation, you can expect that I would revert a
patch (at least temporarily).

Don't take me wrongly, I do believe there's a problem, and that it shall
be fixed. But not this way.

> In the meantime, since you seem to appropriate this piece of code,
> please provide a way to tell nova-common to use a remote database
> server with Debconf.

Yup. As written in our templates. For example (from
debian/nova-common.templates):

 You can change this setting later on by running "dpkg-reconfigure
 -plow nova-common".

It's not perfect, and that even can be considered a bug in
dbconfig-common that it doesn't ask the right question on the first run,
and doesn't support a remote PGSQL db correctly. If dbconfig-common has
issues, then we shouldn't try to go around its problems, but we shall
fix it.

I would very welcome some changes in dbconfig-common by the way. Being
able to use *standard* preseeding (and not be forced into write files in
/etc/dbconfig-common/<package>) for example, would be very good.

Probably, you should also try with MySQL, as the dbconfig-common might
only be broken in the PGSQL case (I never tried it with pgsql, so I
can't tell, and you said that with PGSQL, it wouldn't give root access
remotely...).

> Until you do that, those of us who actually try to run the
> packages on several hosts are stuck.

I don't agree with this statement. A dpkg-reconfigure -plow does work, I
did use it before, it does ask for the host, and configures the db
correctly. The problem we are having is that it doesn't work with pgsql,
because pgsql doesn't have the feature to be administered remotely. If
you wish to explicitly write a warning about this in our debconf
templates, and/or patch dbconfig-common so that it will not do what's
needed in this case, I support this idea!

Thomas Goirand (zigo)