Bug 1285904 - During upgrades vmdb configurations are not updated
During upgrades vmdb configurations are not updated
Status: CLOSED DUPLICATE of bug 1082155
Product: Red Hat CloudForms Management Engine
Classification: Red Hat
Component: Appliance (Show other bugs)
5.5.0
Unspecified Unspecified
unspecified Severity high
: GA
: 5.6.0
Assigned To: Gregg Tanzillo
Dave Johnson
: Reopened
Depends On:
Blocks: 1288185
  Show dependency treegraph
 
Reported: 2015-11-26 18:16 EST by Federico Simoncelli
Modified: 2015-12-04 11:08 EST (History)
7 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
: 1288185 (view as bug list)
Environment:
Last Closed: 2015-12-04 11:08:00 EST
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Federico Simoncelli 2015-11-26 18:16:49 EST
Description of problem:
During upgrades the vmdb configurations (stored in the database) are not updated with new defaults added to vmdb.tmpl.yml.
This results generic failures with the code that assumes the new keys being available.

How reproducible:
100%

Steps to Reproduce:
1. upgrade to a newer version

Actual results:
The new defaults in vmdb.tmpl.yml are not added in the configuration in the database.

Expected results:
The new defaults in vmdb.tmpl.yml should be added in the configuration in the database.
Comment 1 Joe Rafaniello 2015-11-30 10:00:55 EST
Hi Federico, 

Thanks for the bug report.
This, unfortunately, is working as designed.

We currently don't have a good separation of user settings vs "suggested" defaults vs "required" defaults in the configuration.  

If a user made a change to a setting, in most cases, we want to allow that setting to remain after upgrading.  In some case, we need to overwrite the user settings regardless of what they configured.  These two requirements contradict each other.  This is why we don't automatically migrate their configuration to the new vmdb.tmpl.yml.  The developer who adds sections or keys to the configuration needs to explicitly migrate via "merge_from_template" (see below) or provide intelligent defaults in the code.

We do have plans and some code for a general rewrite of the configuration that would allow a better story for the problems above but it's not yet done.

For now, I can recommend you take a look at how we do it elsewhere.

The key methods are:
VMDB::Config#merge_from_template_if_missing
and 
VMDB::Config#merge_from_template

The first one will take a VMDB::Config object and merge from the template specific sections that you want ONLY if they're missing entirely from the user's settings.

The second one will a VMDB::Config object and merge from the template specific sections that you want and OVERWRITE what may already be there.

The general flow is something like:

cfg = VMDB::Config.new("vmdb")
cfg.merge_from_template_if_missing(*database_metrics_collection_schedule)
# more code
cfg.save

I don't know if Jason Frey has a bug open for the general "rewrite" of the configuration that would make this problem easier.  If not, we can re-open this bug and use it.

If I missed something, please don't hesitate or ask as we an easily reopen if need be.

Thanks!
Joe
Comment 2 Joe Rafaniello 2015-11-30 10:10:10 EST
Oh and if you need help migrating anything using merge_from_template_if_missing or merge_from_template, please ping me on gitter/irc/github.

Thanks,
Joe
Comment 3 Federico Simoncelli 2015-12-01 12:04:01 EST
(In reply to Joe Rafaniello from comment #1)
> Hi Federico, 
> 
> Thanks for the bug report.
> This, unfortunately, is working as designed.
> 
> We currently don't have a good separation of user settings vs "suggested"
> defaults vs "required" defaults in the configuration.  
> 
> If a user made a change to a setting, in most cases, we want to allow that
> setting to remain after upgrading.  In some case, we need to overwrite the
> user settings regardless of what they configured.  These two requirements
> contradict each other.  This is why we don't automatically migrate their
> configuration to the new vmdb.tmpl.yml.  The developer who adds sections or
> keys to the configuration needs to explicitly migrate via
> "merge_from_template" (see below) or provide intelligent defaults in the
> code.
> 
> We do have plans and some code for a general rewrite of the configuration
> that would allow a better story for the problems above but it's not yet done.
> 
> For now, I can recommend you take a look at how we do it elsewhere.
> 
> The key methods are:
> VMDB::Config#merge_from_template_if_missing
> and 
> VMDB::Config#merge_from_template
> 
> The first one will take a VMDB::Config object and merge from the template
> specific sections that you want ONLY if they're missing entirely from the
> user's settings.
> 
> The second one will a VMDB::Config object and merge from the template
> specific sections that you want and OVERWRITE what may already be there.
> 
> The general flow is something like:
> 
> cfg = VMDB::Config.new("vmdb")
> cfg.merge_from_template_if_missing(*database_metrics_collection_schedule)
> # more code
> cfg.save
> 
> I don't know if Jason Frey has a bug open for the general "rewrite" of the
> configuration that would make this problem easier.  If not, we can re-open
> this bug and use it.

The gist of this BZ was to plan for something better than what exists today.

It is clear that somehow you dealt with this in the past, but the issues of the current solution are:

1. it generates inconsistent configs between an upgraded system and a fresh installed one (beside user's changes). In fact if you don't use merge_from_template_if_missing by default the upgraded system will have missing configs.

2. code needs a special handling for missing configs that is never used/tested except for upgraded systems (which means that you require the developer to write extra logic and it's not tested)

3. it's not clear when/where you should run merge_from_template_if_missing (for example it could be enough when you update the appliance and not every time you run a specific method as today)

4. current use of merge_from_template_if_missing is scattered all over the code and it's not clear when it should run (it's obviously specific to the case but too much is left to the reader, while as mentioned above it could be done just once at the end of the db migration)


As first step wouldn't it be enough to chain all the merge_from_template_if_missing after db:migrate?

I am reopening this because I strongly believe that we can do better than what we have today.
Comment 4 Joe Rafaniello 2015-12-01 16:21:23 EST
Federico, can you review Jason's bug below.

The general problem of having ways for settings to come from region, zone, server, development, template, user with some form of inheritance is the gist of Jason's bug https://bugzilla.redhat.com/show_bug.cgi?id=1082155

If there is anything in this bug that's missing in Jason's design document or his bug, we should discuss it there.

If I don't hear from you or bug 1082155 addresses the issues you are seeing, I'll close this bug as a duplicate.

Thanks!
Comment 5 Federico Simoncelli 2015-12-04 11:08:00 EST
Closing in favor of bug #1082155

Thanks Joe!

*** This bug has been marked as a duplicate of bug 1082155 ***

Note You need to log in before you can comment on or make changes to this bug.