deprecated parameter nova::database_connection is non-functional

[Matt Fischer]

I have some odd behavior for nova::database_connection that I cannot explain and is probably a bug. The commit 30b6a9c4c7d9b662eb8e70b58a1ad63d1c4e89aa moved the database connection stuff into the db class. As a part of this a pick was added that looks at

nova::db:database_connection and nova::database_connection (the old param).

When we moved to puppet-nova on master though the pick is returning false. The code only works if nova::db::database is set. So I think you're deprecating people here without them knowing it.

A test case would be to set nova::database_connection to something and then notify on it and see that the pick returns an empty string.

I'm pretty stumped on this one, so I'd appreciate if someone could try to repro.

We are running puppet 3.7.3 if that makes a difference

[Emilien Macchi (emilienm)]

So you're talking about this patch I did: https://github.com/stackforge/puppet-nova/commit/30b6a9c4c7d9b662eb8e70b58a1ad63d1c4e89aa

Though if you look at the code, I tried to keep backward compatibility (rspec tests).
Can you paste the Puppet logs and your manifests ?

[Matt Fischer (mfisch)]

I'll see how I can repro it tonight. It was quite strange. When I used notify to dump the old setting it was empty when inside nova::db, and since I didnt have the new one set, the pick was returning false. None of our code includes nova::db directly and the way you reference the old variable looks right to me.

I'll see if I can repro it here in a bit.

[Matt Fischer (mfisch)]

This is what's in my hiera:

sql_idle_timeout: &sql_idle_timeout 30
nova::database_idle_timeout: *sql_idle_timeout
nova::database_connection: "%{hiera('db_type')}://nova:%{hiera('nova_db_password')}@{hiera('controller_internal_address')}/nova"

This of course worked fine before.

Now any generated nova.conf files are completely missing the database section.

I've re-run after adding the following debug patch:

diff --git a/manifests/db.pp b/manifests/db.pp
index 50f9798..51e08d6 100644
--- a/manifests/db.pp
+++ b/manifests/db.pp
@@ -36,6 +36,9 @@ class nova::db (
   $database_connection_real = pick($database_connection, $::nova::database_connection, false)
   $database_idle_timeout_real = pick($database_idle_timeout, $::nova::database_idle_timeout, false)

+ notify { "mfisch: db_conn_real: $database_connection_real": }
+ notify { "mfisch: db_timeout_real: $database_idle_timeout_real": }
+
   if $database_connection_real {
     if($database_connection_real =~ /mysql:\/\/\S+:\S+@\S+\/\S+/) {
       require 'mysql::bindings'
diff --git a/manifests/init.pp b/manifests/init.pp
index 9558360..555bf0e 100644
--- a/manifests/init.pp
+++ b/manifests/init.pp
@@ -286,6 +286,9 @@ class nova(
   # maintain backward compatibility
   include nova::db

+ notify { "mfisch: db_conn: $database_connection": }
+ notify { "mfisch: db_timeout: $database_idle_timeout": }

Results are below.

[Matt Fischer (mfisch)]

It looks like that the _real stuff is being evaluated first, maybe thats the problem?

Notice: mfisch: db_timeout_real: false
Notice: mfisch: db_conn_real: false
Notice: mfisch: db_conn: mysql://nova:<email address hidden>/nova
Notice: mfisch: db_timeout: 30

[Matt Fischer (mfisch)]

I should note that using the new settings is fine.

[Matt Fischer (mfisch)]

UPDATE:

The issue here is my manifests, I'm not including ::nova first which is needed now.

Our manifests did this:

  include ::nova::api
  include ::nova::scheduler
  include ::nova::scheduler::filter
  include ::nova::objectstore
  include ::nova::cert
  include ::nova::consoleauth
  include ::nova::conductor
  include ::nova::vncproxy
  include ::nova

Which breaks this behavior since nova::api includes the db class before ::nova is evaluated

[Richard Raseley (richard-raseley)]

Matt,

Based upon your previous update, am I correct in understanding that this isn't actually an issue in puppet-nova?

Regards,

Richard

[Matt Fischer (mfisch)]

Well its probably still a bug unless we intend to force people to include classes in a specific order. Do we want to make that a policy?

[Emilien Macchi (emilienm)]

I think we should have this orchestration layer in puppet-nova. The bug is still there.

[Matt Fischer (mfisch)]

For the record I thnk the fix is to deprecate database_connection in init.pp and remove it.

[Takashi Kajinami (kajinamit)]

We've deprecated/removed the database_* parameters from the base nova class so I'll close this bug now.