Bug 32791 - Log4perl configuration files need a tool for updating on package upgrade
Summary: Log4perl configuration files need a tool for updating on package upgrade
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-02-02 02:16 UTC by David Cook
Modified: 2023-02-13 02:14 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 32791: Add Debian tool for adding missing config to log4perl files (6.95 KB, patch)
2023-02-02 03:16 UTC, David Cook
Details | Diff | Splinter Review
Bug 32791: Update koha-common and koha-core packaging to include koha-log4perl (9.31 KB, patch)
2023-02-02 03:22 UTC, David Cook
Details | Diff | Splinter Review
Bug 32791: [Alternative] Use "ucf" to manage log4perl.conf (16.72 KB, patch)
2023-02-13 01:48 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description David Cook 2023-02-02 02:16:14 UTC
Typically, we update existing log4perl.conf files using debian/koha-common.postinst but it's making that file grow way too much and it's difficult to test.

Instead, let's provide a CLI tool that we can more easily test and update without touching debian/koha-common.postinst directly. 

It should hopefully simplify maintenance.
Comment 1 David Cook 2023-02-02 03:16:11 UTC
Created attachment 145985 [details] [review]
Bug 32791: Add Debian tool for adding missing config to log4perl files

This change adds a tool for adding missing config to log4perl files
for individual Koha instances created using Debian packages

Test plan:
0. Apply patch
1. Comment out a component in /etc/koha/sites/kohadev/log4perl.conf
    or delete some of the configuration
2. Run the following:
    ./debian/scripts/koha-log4perl kohadev
3. Note warning saying config has been updated
4. Review config to see that it was updated correctly
5. Run the following:
    ./debian/scripts/koha-log4perl kohadev
6. Note that no config changes are made
Comment 2 David Cook 2023-02-02 03:20:25 UTC
Note that this will also make it so that debian/koha-core.postinst doesn't need manual updates as well...
Comment 3 David Cook 2023-02-02 03:22:07 UTC
Created attachment 145986 [details] [review]
Bug 32791: Update koha-common and koha-core packaging to include koha-log4perl
Comment 4 David Cook 2023-02-02 03:25:32 UTC
This is a first draft so I'm happy to tweak the koha-log4perl tool. 

On one hand, it would be nice to have this code in a Koha Perl module so that it could be suitably unit tested.

On the other hand, it's Debian-specific, so we'll need to be abstract the snippets more, and we'll need to make the koha-log4perl tool aware of paths in PERL5LIB. Probably parsing it, splitting it, and then using "use lib $path". 

I'm not opposed to that. But it's a consideration. 

It probably would be better to put it into a Perl module and unit test it, so that we can fully automate the testing of this one. 

--

Anyway, I mostly wanted to do these patches as a proof of concept and then get feedback.

I think it's a big step forward from doing this all in debian/koha-common.postinst and debian/koha-core.postinst themselves.
Comment 5 David Cook 2023-02-02 03:26:57 UTC
Note also that there is technically an XML configuration option available for Log4perl with /usr/share/perl5/Log/Log4perl/Config/DOMConfigurator.pm but it requires an additional dependency, and it's rather ugly...
Comment 6 Jonathan Druart 2023-02-09 09:16:53 UTC
Please perltidy the new file ;)

Should not we sort the file to ease comparison?

Should we deal with extra values (not existing in koha community config) or could we simply overwrite the file?
Comment 7 David Cook 2023-02-09 23:29:54 UTC
(In reply to Jonathan Druart from comment #6)
> Please perltidy the new file ;)

I still can't get perltidy to work correctly in ktd. It creates code that violates https://wiki.koha-community.org/wiki/Coding_Guidelines

¯\_(ツ)_/¯

That said, I see some obvious whitespace issues now outside vim...

--

> Should not we sort the file to ease comparison?
> 
> Should we deal with extra values (not existing in koha community config) or
> could we simply overwrite the file?

I think that would be a much heavier touch. 

But... I figure in 99% of cases no one customizes their log4perl file, so maybe it does make sense to just overwrite it.

I'm thinking about ways where we could generate a new log4perl file, compare it with the existing instance log4perl file, and then let sysadmins decide in the same way they do for conffiles provided by the Debian package.

This is an interesting read:
https://wiki.debian.org/ConfigPackages

It might be worth looking more into the "ucf" utility.

Looking at foomatic-filters.postinst, I see the following line:

ucf --three-way --debconf-ok $tempfile $FILTERCONF

It seems to generate the new config, put it in a temporary file, and then test that against $FILTERCONF which is /etc/foomatic/filter.conf.

In our case, we'd need to do it for every instance in "koha-list", but it could be doable. 

Using "ucf" could be better than trying to re-invent the wheel.
Comment 8 David Cook 2023-02-10 00:36:33 UTC
On ktd, I'm noticing /var/lib/ucf/cache/:etc:perl:XML:SAX:ParserDetails.ini

ucfq --verbose /etc/perl/XML/SAX/ParserDetails.ini
Configuration file                            Package             Exists Changed
/etc/perl/XML/SAX/ParserDetails.ini           libxml-sax-perl     Yes     No

In the postinst for the libxml-sax-perl Debian package, it calls "update-perl-sax-parsers" which runs the following:

system("ucf", "--debconf-ok", "--sum-file",  "/var/lib/libxml-sax-perl/ParserDetails.ini.md5sum", $tmpfile, $file);
unlink $tmpfile or die("unlink $tmpfile: $!");

/usr/share/doc/ucf/examples/postinst also includes an example of using a temporary file with "ucf", so that sounds pretty good to me. 

--

I've done some hacking on "update-perl-sax-parsers" and the "ucf" data files to try to get the prompt I wanted. I used "sudo su -" to bypass "DEBIAN_FRONTEND=noninteractive" in the ktd env vars. (In ktd images, we should use ARG instead of ENV for setting this env var. I'll double-check whether or not I have an existing issue or PR for that...)

Hacked files:
- /usr/bin/update-perl-sax-parsers
- /var/lib/ucf/cache/\:etc\:perl\:XML\:SAX\:ParserDetails.ini
- /var/lib/ucf/hashfile


dpkg-reconfigure libxml-sax-perl
debconf: unable to initialize frontend: Dialog
debconf: (No usable dialog-like program is installed, so the dialog based frontend cannot be used. at /usr/share/perl5/Debconf/FrontEnd/Dialog.pm line 78.)
debconf: falling back to frontend: Readline
update-perl-sax-parsers: Registering Perl SAX parser XML::SAX::PurePerl with priority 10...
update-perl-sax-parsers: Updating overall Perl SAX parser modules info file...
debconf: unable to initialize frontend: Dialog
debconf: (No usable dialog-like program is installed, so the dialog based frontend cannot be used. at /usr/share/perl5/Debconf/FrontEnd/Dialog.pm line 78.)
debconf: falling back to frontend: Readline
Modified configuration file
---------------------------

A new version (/tmp/61U6DAa6Rj) of configuration file /etc/perl/XML/SAX/ParserDetails.ini is available, but the version installed currently has been locally modified.

  1. install the package maintainer's version    3. show the differences between the versions            5. show a 3-way difference between available versions  7. start a new shell to examine the situation
  2. keep the local version currently installed  4. show a side-by-side difference between the versions  6. do a 3-way merge between available versions
What do you want to do about modified configuration file ParserDetails.ini?

--

Basically, it checks if the installed version and the ucf cached version match. If they match, then the upstream version is installed. If they don't match, then the sysadmin is prompted as above. 

I think that sounds pretty ideal. 

We could actually use that for a variety of generated Koha config files.

--

Anyway, I think I better change to a different task now, but I'm curious to hear what people think!
Comment 9 David Cook 2023-02-10 00:37:30 UTC
Looks like I raised the DEBIAN_FRONTEND issue in ktd 2 years ago but haven't gotten around to the PR... what is time...

https://gitlab.com/koha-community/koha-testing-docker/-/issues/238
Comment 10 Marcel de Rooy 2023-02-10 07:04:56 UTC
Yes, you should use ARG with DEBIAN_FRONTEND instead of ENV. Only needed when building..
Comment 11 David Cook 2023-02-12 23:51:50 UTC
I'm going to have a little crack at an alternative using "ucf"...
Comment 12 David Cook 2023-02-13 00:01:16 UTC
(In reply to David Cook from comment #11)
> I'm going to have a little crack at an alternative using "ucf"...

Then again... the function for generating configuration files is in debian/scripts/koha-create so it wouldn't be a trivial change...
Comment 13 David Cook 2023-02-13 01:37:02 UTC
Interestingly, debian/koha.postrm uses "ucf".
Comment 14 David Cook 2023-02-13 01:48:07 UTC
Created attachment 146567 [details] [review]
Bug 32791: [Alternative] Use "ucf" to manage log4perl.conf

Test plan:
0. Apply patch
1a. ln -s /kohadevbox/koha/debian/scripts/koha-config /usr/sbin/koha-config
1b. koha-config --create-log4perl kohadev
1c. Note that the output looks like a correct log4perl.conf configuration
2. cp debian/koha-common.postinst /var/lib/dpkg/info/koha-common.postinst
3. vi /var/lib/ucf/hashfile
4. Change the first character in the hash for
/etc/koha/sites/kohadev/log4perl.conf to a different letter/number
5. vi /var/lib/ucf/cache/\:etc\:koha\:sites\:kohadev\:log4perl.conf
6. Add a comment to the bottom of the file like "#OLD"
7. vi /etc/koha/sites/kohadev/log4perl.conf
8. Add a commen to the bottom of the file like "#LOCAL"
9. dpkg-reconfigure koha-common
10. Play with the options - especially options 1 and 2 that
install the package version or retain the local version

11. ucfq --verbose /etc/koha/sites/kohadev/log4perl.conf
12. Note output like the following:
Configuration file                            Package             Exists Changed
/etc/koha/sites/kohadev/log4perl.conf         koha-common         Yes    Yes

NOTE: We need to think about "ucf" and "ucfr" commands for removing Koha instances,
and removing/purging the Koha package
Comment 15 David Cook 2023-02-13 02:11:40 UTC
I suppose koha-create could use "ucfr" to register the log4perl.conf file in the "ucf" registry. (I've done a little test and it looks like if there's no data in the hashfile and cache, it just compares the installed file versus the new file for "ucf")

Then koha-remove could use "ucf" and "ucfr" to remove the file from the ucf cache, hashfile, and registry. 

It looks like "apt-get remove koha-common" doesn't touch "/etc/koha/sites". It leaves behind /var/lib/dpkg/info/koha-common.list and /var/lib/dpkg/info/koha-common.postrm

Interestingly, when I do "apt-get purge koha-common", I get the following errors:

dpkg: warning: while removing koha-common, directory '/var/spool/koha' not empty so not removed
dpkg: warning: while removing koha-common, directory '/var/log/koha' not empty so not removed
dpkg: warning: while removing koha-common, directory '/var/lock/koha' not empty so not removed
dpkg: warning: while removing koha-common, directory '/var/lib/koha' not empty so not removed
dpkg: warning: while removing koha-common, directory '/var/cache/koha' not empty so not removed
dpkg: warning: while removing koha-common, directory '/etc/koha/sites' not empty so not removed

So it looks like koha-common already has a purge problem...
Comment 16 David Cook 2023-02-13 02:14:19 UTC
(In reply to David Cook from comment #15)
> I suppose koha-create could use "ucfr" to register the log4perl.conf file in
> the "ucf" registry. (I've done a little test and it looks like if there's no
> data in the hashfile and cache, it just compares the installed file versus
> the new file for "ucf")
> 
> Then koha-remove could use "ucf" and "ucfr" to remove the file from the ucf
> cache, hashfile, and registry. 
> 

Although the problem with that is that you don't know if you're using "koha-common" or "koha-core" and you need the package name when using "ucfr"...

Technically, we don't have to use "ucfr" to get the functionality of "ucf".