[Openstack-devel] Bug#687311: Bug#687311: Bug#687311: Bug#687311: Diff for keystone 2012.1.1-7, trying to fix RC bug #687311

[Thomas Goirand]

Hi,

Thanks *A LOT* for reviewing my work, this was much needed as we are 
approaching the release date for Wheezy, and that I asked for such review.

On 09/19/2012 10:35 AM, YunQiang Su wrote:
> Two questions about postinst:
>
> 1.  if [ $RET = "sqlite3" ]
> 	then
> 	    dbc_name="keystone.sqlite"
> 	    db_set keystone/db/dbname $dbc_name
> 	fi
>    If user modified /etc/keystone.conf manually to change the path of
> sqlite database file,
>    while this paragraph will change it to the default.

First, this was there before my changes. So it's not me who did it, I 
just didn't correct it.

Currently, if you change the path in /etc/keystone/keystone.conf (and 
not /etc/keystone.conf), then it will stay. The only thing that is 
forced is the filename of the db.

Anyway, I think you are right, so I removed that part.

> 2. if [ "$dbc_dbtype" = "mysql" ] || [ "$dbc_dbtype" = "pgsql" ] ; then
> 	        [ -n "$dbc_dbport" ]&&  dbport=:$dbc_dbport
> 	        SQL_CONNECTION="$dbc_dbtype://$dbc_dbuser:$dbc_dbpass@${dbc_dbserver:-localhost}$dbport/$dbc_dbname"
> 	    else
> 	        SQL_CONNECTION="sqlite:///$dbc_basepath/$dbc_dbname"
>     fi
>
>    The problem similar with above one, if user changed the line
> "connection=" to mssql or oracle or db2 etc,
>    this will override the users config.

Keystone only supports sqlite3, mysql or pgsql. Unless I'm mistaking, 
these are the only valid values. So I believe the above is ok.

>    Maybe, now dbc or something else cannot support msssql, oracle or
> db2 etc, but this is still a bad idea to
>    override the users config.

The idea is to overwrite anything that is a wrong configuration. The 
most easy way is to do it using sqlite. I don't think there's any wrong 
doing here (correct me if you still think there is).

On 09/19/2012 10:44 AM, YunQiang Su wrote:
 > Maybe what we should do is to:
 >
 > Init /etc/keystone/keystone.conf from example, and debconf only when
 > the first time,
 > etc if /etc/keystone/keystone.conf doesn't exist.
 >
 > Do nothing if /etc/keystone/keystone.conf is there.
 >
 > If user want to reinit the configuration with debconf, then ask them 
to delete
 > /etc/keystone/keystone.conf first, and reinstall this package, or,
 > give him/her an option in debconf.

We can't do that. This isn't what the policy say about debconf handling. 
It has to read what the user wrote, and deal with it. I think anyway 
that deleting the keystone.conf just because you want to reconfigure the 
db thing is a radical move, and not convenient (you may well have 
changed other things in this configuration file which you want to keep).

Thomas