Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

installer.pl does not check for changed file Kernel/Config.pm #1575

Closed
bschmalhofer opened this issue Jan 27, 2022 · 5 comments
Closed

installer.pl does not check for changed file Kernel/Config.pm #1575

bschmalhofer opened this issue Jan 27, 2022 · 5 comments
Assignees
Labels
bug Something isn't working as intended
Milestone

Comments

@bschmalhofer
Copy link
Contributor

During testing for OTOBO 10.1.1 Beta2 @svenoe noticed database connection failures while running installer.pl . This was almost certainly caused by the web server process having still loaded an unchanged version of Kernel::Config. This is relevant as Kernel/Modules/Installer.pm writes the new database credentials into that file.

The big mystery is why this behavior was noticed only now. Doing a test installation with OTOBO 10.1.1 Beta2 showed the same error.

A possible solution is to check for every request whether Kernel/Config.pm has changed. This is basically the same check as done in Kernel::Config::Defaults:;new() for possibly changed ZZZ*.pm files.

While we are at it, we should also check for explicit use Kernel::Config; statements. These statements should be avoided unless they are really necessary. But they are not really the culprit as the object manager calls under the hood also require Kernel::Config.

@bschmalhofer bschmalhofer added the bug Something isn't working as intended label Jan 27, 2022
@bschmalhofer bschmalhofer added this to the OTOBO 10.1.1 milestone Jan 27, 2022
@bschmalhofer bschmalhofer self-assigned this Jan 27, 2022
bschmalhofer added a commit that referenced this issue Jan 28, 2022
bschmalhofer added a commit that referenced this issue Jan 28, 2022
as the config module is loaded via the object manager
bschmalhofer added a commit that referenced this issue Jan 28, 2022
as the config module is loaded via the object manager
bschmalhofer added a commit that referenced this issue Jan 28, 2022
bschmalhofer added a commit that referenced this issue Jan 28, 2022
by using a join instead of concatenation
bschmalhofer added a commit that referenced this issue Jan 28, 2022
used for debuggings, commented out per default
bschmalhofer added a commit that referenced this issue Jan 28, 2022
bschmalhofer added a commit that referenced this issue Jan 28, 2022
also using map and join for generating a message
bschmalhofer added a commit that referenced this issue Jan 28, 2022
Avoiding that the module is modified before the first check.
bschmalhofer added a commit that referenced this issue Jan 28, 2022
@bschmalhofer
Copy link
Contributor Author

No new insights turned up, why this wasn't an issue before.

For fixing the bug I added the refresh of Kernel/Config.pm in otobo.psgi . Care had to be taken that Kernel/Config.pm is in the refresh cache before the first check. Otherwise the following situation could arise:

  1. Kernel::Config.pm is loaded in process A, but not in refresh cache
  2. Kernel/Config.pm is modified in process B
  3. Kernel/Config.pm is checked in process A, but not reloaded is this is the first check

@bschmalhofer
Copy link
Contributor Author

I tested installer.pl with a local Apache and the apache2-httpd-cgi.include.conf configuration. Installing was without hassle. But there were errors after the installation was completed. Looks like we need a bit more testing and maybe fixing.

bschmalhofer added a commit that referenced this issue Jan 30, 2022
Adding a sprinkling of 'our $Self;' while we are at it.
bschmalhofer added a commit that referenced this issue Jan 30, 2022
Because this is called before otobo.psgi kicks in
bschmalhofer added a commit that referenced this issue Jan 30, 2022
because the file is explicitly checked above
@bschmalhofer
Copy link
Contributor Author

Looked again what Module::Refresh::refresh_module_if_modified() is actually doing. There is no refreshing when the module has been loaded and changed after the last time it has been encountered by Module::Refresh. So the current approach is;

  1. require $PMFile; load when it wasn't already loaded
  2. Module::Refresh->refresh_module_if_modified('Kernel/Config.pm'); either remember the timestamp, do nothing, or reload

Made the changes and did quick tests of installer.pl.

  • Apache2 with apache2-httpd.include.conf
  • Apache2 with apache2-httpd-cgi.include.conf
  • Gazelle running in Docker

The quick tests looked fine and the test suite looked fine too. Except #1587.
Thus I merged the changes into rel-10_1.

bschmalhofer added a commit that referenced this issue Jan 30, 2022
…fig_another_attempt

Issue #1575 update kernel config another attempt
@svenoe
Copy link
Contributor

svenoe commented Feb 7, 2022

Can this be closed? We are already one step further anyways, I guess, aren't we?

@bschmalhofer
Copy link
Contributor Author

The reloading of Kernel/Config.pm appears to be more sane now. Note that there was yet another iteration handled in issue #1594. Closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working as intended
Projects
None yet
Development

No branches or pull requests

2 participants