Description of problem: 1. Do not have a single file, but a directory. 2. Read all *.conf in the directory, last value wins. 3. Write a single file by setup, always, do not check, nor inform user or anything. 4. If users wants to override us, they should add their own file, not update ours. We can add a README and/or a comment saying that. This is followup of discussion under https://gerrit.ovirt.org/c/108333/
This is not a 4.4.0 blocker, but if not done then, will make upgrades from 4.4.0 to 4.4.1 harder and more risky (mainly having to handle exactly the cases this bug is meant to not have to handle). So if possible I think it should be handled in 4.4.0.
I think this makes sense if we add something that works like and vdsm and systemd (we copied systemd behavior in vdsm). See - https://github.com/oVirt/vdsm/blob/564946bc5f78082ed9b453d6c1988aa27b423ec5/lib/vdsm/common/config.py.in#L25 - https://github.com/oVirt/vdsm/blob/564946bc5f78082ed9b453d6c1988aa27b423ec5/lib/vdsm/common/config.py.in#L717 When we have this vdsm will install: /etc/ovirt-imageio/daemon.conf.d/50-vdsm.conf. Engine will install: /etc/ovirt-imageio/daemon.conf.d/50-engine.conf. Instead of /etc/ovirt-imageio/daemon.conf.
While we add support for drop-in configuration files, it would be nice to unify handling of logger.conf and daemon.conf. Since both files use same format, we can merge the files into the same daemon.conf.sample. ovirt-imageio-daemon package should stop installing logger.conf - we don't want to depend on external file that can be edited by a used for defaults. defaults should be builtin in the application so running the application without any configuration works: ./ovirt-imageio Even when /etc/ovirt-imageio/ does not exists - this simplify testing and development. When configuration directory is specified, it should be used to search for drop-in files (e.g. /etc/ovirt-imageio/conf.d/*.conf). Otherwise the normal search path will be used: - /etc/ovirt-imageio/conf.d/ - for admin drop-in conf files. - /usr/lib/ovirt-imageio/conf.d/ - for vendor drop-in configuration files. - /var/run/ovirt-imageio/conf.d/ - for admin temporary configuration. Engine will install: $PREFIX/etc/ovirt-imageio/conf.d/50-engine.conf With both daemon configuration needed by engine. In development mode, engine can add relevant logging options (log to stderr). Vdsm will install: /etc/ovirt-imageio/conf.d/50-vdsm.conf With vdsm related daemon configuration (default logging is fine). If a user wants to change the configuration (e.g. use different PKI), they can install: /etc/ovirt-imageio/conf.d/99-user.conf This will override other drop-in files. Engine and vdsm drop-in files should not be considered as configuration files by rpm - rpm should replace old files without creating .rpmnew/.rpmsave files. /etc/ovirt-imageio/daemon.conf.sample need to explain how to use drop-in files.
Didi, please check comment 3 - do you have any comments?
(In reply to Nir Soffer from comment #4) > Didi, please check comment 3 - do you have any comments? Makes sense to me. Not sure what /var/run/ovirt-imageio/conf.d is to be used for.
(In reply to Yedidyah Bar David from comment #5) > Not sure what /var/run/ovirt-imageio/conf.d is to be used for. This can be used for temporary changes until the next reboot. Was copied from vdsm, which copied it from systemd: https://www.freedesktop.org/software/systemd/man/systemd-system.conf.html Probably not useful for imageio since systemd creates /run/ovirt-imageio when starting the service and delete when stopping the service, and we don't support reloading configuration.
BTW, many tools that support such drop-in directories (or even more complex ways to split/condition/whatever their conf), also have a utility that shows the "eventually-parsed" configuration, to ease on the user debugging conf issues. Not sure we are already there, with imageio, but might be nice to add.
You can see a concrete example of using drop-in configuration here: https://bugzilla.redhat.com/show_bug.cgi?id=1761960#c7
(In reply to Yedidyah Bar David from comment #10) > BTW, many tools that support such drop-in directories (or even more complex > ways to split/condition/whatever their conf), also have a utility that shows > the "eventually-parsed" configuration, to ease on the user debugging conf > issues. Can you point me to such tool?
(In reply to Nir Soffer from comment #12) > (In reply to Yedidyah Bar David from comment #10) > > BTW, many tools that support such drop-in directories (or even more complex > > ways to split/condition/whatever their conf), also have a utility that shows > > the "eventually-parsed" configuration, to ease on the user debugging conf > > issues. > > Can you point me to such tool? e.g.: httpd -t (also with -D DUMP_VHOSTS, -D DUMP_MODULES) postconf (and also e.g. -F, several other options)
(In reply to Yedidyah Bar David from comment #13) > (In reply to Nir Soffer from comment #12) > > (In reply to Yedidyah Bar David from comment #10) > > > BTW, many tools that support such drop-in directories (or even more complex > > > ways to split/condition/whatever their conf), also have a utility that shows > > > the "eventually-parsed" configuration, to ease on the user debugging conf > > > issues. > > > > Can you point me to such tool? > > e.g.: > > httpd -t (also with -D DUMP_VHOSTS, -D DUMP_MODULES) > > postconf (and also e.g. -F, several other options) Thanks, posted https://gerrit.ovirt.org/c/109041/. Would you like to open a bug for this?
(In reply to Nir Soffer from comment #14) > Would you like to open a bug for this? Done, bug 1835719.
Vojtech Juranek, Two problems i encounter which makes it hard to verify the following scenario (see my verification steps which are based on your comment): 1. ovirt-imageio doesn have --show-config arg: [root@storage-ge-05 conf.d]# ovirt-imageio --show-config usage: ovirt-imageio [-h] [-c CONF_DIR] ovirt-imageio: error: unrecognized arguments: --show-config 2. There is no folder /usr/lib/ovirt-imageio/conf.d/ neither on engine, nor host. ovirt-imageio i am using as apart of rhv-4.4.1-2 : [root@storage-ge5-vdsm1 ~]# rpm -qa | grep ovirt-imageio ovirt-imageio-daemon-2.0.6-0.el8ev.x86_64 ovirt-imageio-client-2.0.6-0.el8ev.x86_64 ovirt-imageio-common-2.0.6-0.el8ev.x86_64 ================================================ Verification steps: 1. Write log level DEBUG to /etc/ovirt-imageio/conf.d/a.conf cat a.conf [logger_root] level = DEBUG 2. systemctl restart ovirt-imageio Expected: /var/log/ovirt-imageio/daemon.log writing log with DEBUG level AND / OR ovirt-imageio --show-config should show DEBUG value for appropriate setting. 3. Write log level INFO and TLS=false to /usr/lib/ovirt-imageio/conf.d/b.conf cat b.conf [logger_root] level = INFO [tls] enable = true 4. systemctl restart ovirt-imageio Expected: A. /var/log/ovirt-imageio/daemon.log writing log with INFO level. This is because setting in b.conf overwrites the setting in a.conf (The order of files is alphabetically) This can be verified by grepping the /var/log/ovirt-imageio/daemon.log AND/OR ovirt-imageio --show-config B. tls should be enabled. This can be verified by ovirt-imageio --show-config
> 1. ovirt-imageio doesn have --show-config arg: > [root@storage-ge-05 conf.d]# ovirt-imageio --show-config > usage: ovirt-imageio [-h] [-c CONF_DIR] > ovirt-imageio: error: unrecognized arguments: --show-config Looks like this is not yet in the relase so you would have to test by checking imageio behaviour (as you did e.g. by checking log level has been changed). > 2. There is no folder /usr/lib/ovirt-imageio/conf.d/ neither on engine, > nor host. this is expected, engine setup should create only /etc/ovirt-imageio/conf.d/50-engine.conf > ================================================ > > Verification steps: > > 1. Write log level DEBUG to /etc/ovirt-imageio/conf.d/a.conf > > cat a.conf > [logger_root] > level = DEBUG > > 2. systemctl restart ovirt-imageio > > Expected: > > /var/log/ovirt-imageio/daemon.log writing log with DEBUG level > AND / OR ovirt-imageio --show-config should show DEBUG value for > appropriate setting. > > 3. Write log level INFO and TLS=false to /usr/lib/ovirt-imageio/conf.d/b.conf > > cat b.conf > [logger_root] > level = INFO > [tls] > enable = true > > 4. systemctl restart ovirt-imageio > > Expected: > > A. /var/log/ovirt-imageio/daemon.log writing log with INFO level. > This is because setting in b.conf overwrites the setting in a.conf (The > order of files is alphabetically) > This can be verified by grepping the /var/log/ovirt-imageio/daemon.log > AND/OR ovirt-imageio --show-config > > B. tls should be enabled. This can be verified by ovirt-imageio > --show-config not sure if there is any question about verification steps, but it LGTM (with a note that --show-config option is unfortunately not available yet).
Verified according those steps (taking in consideration that ovirt-imageio --show-config isnt implemented in current build rhv-4.4.1-2). 1. Write log level DEBUG to /etc/ovirt-imageio/conf.d/a.conf 2. Stop ovirt-imageio service 3. Clear daemon.log 4. Start ovirt-imageio service EXPECTED: daemon.log having DEBUG level statements 5. Create custom conf path as some vendors may have on their envs: /usr/lib/ovirt-imageio/conf.d/ 6. Write log level INFO to /usr/lib/ovirt-imageio/conf.d/b.conf 7. Stop ovirt-imageio service 8. Clear daemon.log 9. Start ovirt-imageio service EXPECTED: daemon.log having INFO level statements This is because setting in b.conf overwrites the setting in a.conf (The order of files is alphabetically) 10. rename /etc/ovirt-imageio/conf.d/a.conf to /etc/ovirt-imageio/conf.d/b.conf 11. rename /usr/lib/ovirt-imageio/conf.d/b.conf to /usr/lib/ovirt-imageio/conf.d/a.conf 12. Stop ovirt-imageio service 13. Clear daemon.log 14. Start ovirt-imageio service EXPECTED: daemon.log having DEBUG level statements This is because setting in b.conf (from /etc/ovirt-imageio/conf.d) overwrites the setting in a.conf (from /usr/lib/ovirt-imageio/conf.d/)
This bugzilla is included in oVirt 4.4.1 release, published on July 8th 2020. Since the problem described in this bug report should be resolved in oVirt 4.4.1 release, it has been closed with a resolution of CURRENT RELEASE. If the solution does not work for you, please open a new bug report.