Bug 1025794 - Add osinfo configuration instruction and warning not to override defaults
Summary: Add osinfo configuration instruction and warning not to override defaults
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat Enterprise Virtualization Manager
Classification: Red Hat
Component: ovirt-engine-setup
Version: 3.3.0
Hardware: Unspecified
OS: Unspecified
unspecified
high
Target Milestone: ---
: 3.3.0
Assignee: Alon Bar-Lev
QA Contact: Jiri Belka
URL:
Whiteboard: integration
Depends On:
Blocks: 3.3snap2
TreeView+ depends on / blocked
 
Reported: 2013-11-01 15:27 UTC by David Jaša
Modified: 2014-01-21 22:25 UTC (History)
10 users (show)

Fixed In Version: is23
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2014-01-21 22:19:44 UTC
oVirt Team: ---
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)


Links
System ID Private Priority Status Summary Last Updated
oVirt gerrit 20806 0 None None None Never
oVirt gerrit 21121 0 None None None Never

Description David Jaša 2013-11-01 15:27:41 UTC
Description of problem:
/etc/ovirt-engine/osinfo.conf.d/00-defaults.properties is meant for modifications by hand as opposed to /usr/share/ovirt-engine/conf/osinfo-defaults.properties which contains default configuration provided by package. Therefore, the file in /etc should be marked as a configuration file in rpm package and its modifications should not be overridden on package upgrades.

Version-Release number of selected component (if applicable):
is20.2 / rhevm-backend-3.3.0-0.30.beta1.el6ev.noarch

How reproducible:
always

Steps to Reproduce:
1. modify /etc/ovirt-engine/osinfo.conf.d/00-defaults.properties
2. upgrade rhev-m
3.

Actual results:
the file gets reverted to its stock content

Expected results:
the user modifications get preserved; a new version gets installed as .rpmnew file

Additional info:

Comment 1 Jiri Belka 2013-11-01 15:48:27 UTC
The question is why this file exists if it is 100% equal to /usr/share/ovirt-engine/conf/osinfo-defaults.properties. If it should be as sample file for later configuration then all its values should be commented and still... never touched by upgrade of rpm.

Comment 2 Alon Bar-Lev 2013-11-01 16:01:50 UTC
(In reply to Jiri Belka from comment #1)
> The question is why this file exists if it is 100% equal to
> /usr/share/ovirt-engine/conf/osinfo-defaults.properties. If it should be as
> sample file for later configuration then all its values should be commented
> and still... never touched by upgrade of rpm.

it is symlink:
---
# ls -la /etc/ovirt-engine/osinfo.conf.d/00-defaults.properties
/etc/ovirt-engine/osinfo.conf.d/00-defaults.properties -> /usr/share/ovirt-engine/conf/osinfo-defaults.properties
---

The /etc/ovirt-engine/osinfo.conf.d is dropdir, users who which to add values should create their own configuration file that override base product or add whatever they need.

The product defaults 00-defaults.properties should not be modified, it is not reside in /etc, hence not %config.

This issue is related to the argument of usage conf.d within product at bug#1020675.

Configuration directory at /etc which ends with .d is configuration directory / drop dir, that's mean that you do not override configuration but apply your own file. This is standard approach in *NIX, it allows to avoid the rpmsave/rpmnew hell.

We use conf.d all over in 3.3, this is a great improvement over what we had.

Comment 3 David Jaša 2013-11-01 18:29:06 UTC
(In reply to Alon Bar-Lev from comment #2)
> it is symlink:

It should not be. Files in /usr are generally part of program, files in /etc are generally user configuration. Symlink from /etc to /usr is a recipe for confusion.

> Configuration directory at /etc which ends with .d is configuration
> directory / drop dir, that's mean that you do not override configuration but
> apply your own file. 

No, the standard is to have non-overridable file in /usr (if the default configuration is separated at all) and have overrides in /etc. Non-overridable /etc files are generally legacy stuff that gets gradually cleaned up in order to have /etc

> This is standard approach in *NIX, it allows to avoid
> the rpmsave/rpmnew hell.
> 
> We use conf.d all over in 3.3, 

I have no problem with a desire to avoid rpmsave/rpmnew but then there another approach should be taken: osinfo.conf.d/ should not contain any .properties file by default, it should instead have a brief README pointing to a skeleton file somewhere in /usr/share/doc/$PKGNAME (that could actually be a symlink to the /usr/share/o-e/... file).

FWIW I actually like .rpmnew approach because administrator knows about existence of updates to default configuration and their time when she looks into /etc without new configuration being forcefully applied - but that's just my taste.

> this is a great improvement over what we had.

No question about that, 3.3 is a big leap in terms of packaging & associated stuff. BZ is not however a good place for giving praise over general concepts. This particular tidbit is just wrong and as such it should be fixed.

Comment 4 David Jaša 2013-11-01 18:32:17 UTC
> in order to have /etc

... filled just with user-configurable stuff. Systemd & friends is one big example but they were predated by xorg configuration clean ups, and similar stuff years before.

Comment 5 David Jaša 2013-11-01 18:34:52 UTC
One more note: if 00-defaults.properties symlink stays in /etc, it should contain *BIG PHAT WARNING* on top that it gets overriden on updates, as other polite generated configs do.

Comment 6 Alon Bar-Lev 2013-11-01 18:36:22 UTC
(In reply to David Jaša from comment #3)
> (In reply to Alon Bar-Lev from comment #2)
> > it is symlink:
> 
> It should not be. Files in /usr are generally part of program, files in /etc
> are generally user configuration. Symlink from /etc to /usr is a recipe for
> confusion.

It is also receipt to simple algorithm, so user who refer to the drop dir can view the entire configuration based on simple logic of application - all configuration is taken from etc.

> > Configuration directory at /etc which ends with .d is configuration
> > directory / drop dir, that's mean that you do not override configuration but
> > apply your own file. 
> 
> No, the standard is to have non-overridable file in /usr (if the default
> configuration is separated at all) and have overrides in /etc.
> Non-overridable /etc files are generally legacy stuff that gets gradually
> cleaned up in order to have /etc

In this case I preferred to have simple scheme without exception (including own application default). The osinfo is going to be more complex, more similar to the branding interface, in which every profile is actually a profile with images and other files. As far as I know it has not implemented yet because of resource lack.

> 
> > This is standard approach in *NIX, it allows to avoid
> > the rpmsave/rpmnew hell.
> > 
> > We use conf.d all over in 3.3, 
> 
> I have no problem with a desire to avoid rpmsave/rpmnew but then there
> another approach should be taken: osinfo.conf.d/ should not contain any
> .properties file by default, it should instead have a brief README pointing
> to a skeleton file somewhere in /usr/share/doc/$PKGNAME (that could actually
> be a symlink to the /usr/share/o-e/... file).
> 
> FWIW I actually like .rpmnew approach because administrator knows about
> existence of updates to default configuration and their time when she looks
> into /etc without new configuration being forcefully applied - but that's
> just my taste.

And you get non workable application if you do not modify the original file... not sure how it can be accepted.

> > this is a great improvement over what we had.
> 
> No question about that, 3.3 is a big leap in terms of packaging & associated
> stuff. BZ is not however a good place for giving praise over general
> concepts. This particular tidbit is just wrong and as such it should be
> fixed.

I disagree.

Comment 7 Alon Bar-Lev 2013-11-01 18:36:43 UTC
(In reply to David Jaša from comment #5)
> One more note: if 00-defaults.properties symlink stays in /etc, it should
> contain *BIG PHAT WARNING* on top that it gets overriden on updates, as
> other polite generated configs do.

This can be done.

Comment 8 Alon Bar-Lev 2013-11-01 19:00:23 UTC
commit fe6594e10bd99f32afe7a423b0d748a5fc3cadeb
Author: Alon Bar-Lev <alonbl>
Date:   Fri Nov 1 20:48:08 2013 +0200

    packaging: osinfo: add README and overwrite warning for defaults
    
    Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1025794
    Change-Id: I6ac7a780de042d57775762f72a1cc378b0dd51b1
    Signed-off-by: Alon Bar-Lev <alonbl>

Comment 9 Jiri Belka 2013-11-14 15:03:51 UTC
ok, is23.

Comment 10 Itamar Heim 2014-01-21 22:19:44 UTC
Closing - RHEV 3.3 Released

Comment 11 Itamar Heim 2014-01-21 22:25:45 UTC
Closing - RHEV 3.3 Released


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