Bug 20692 - koha-plack doesn't check for Include *-plack.conf line in /etc/apache2/sites-available/$INSTANCE.conf
Summary: koha-plack doesn't check for Include *-plack.conf line in /etc/apache2/sites-...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Command-line Utilities (show other bugs)
Version: master
Hardware: All Linux
: P5 - low minor (vote)
Assignee: Jonathan Druart
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-05-01 20:26 UTC by Piotr Wejman
Modified: 2019-10-14 20:02 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.05.00, 18.11.05


Attachments
Bug 20692: Return and display and error if koha-plack failed (2.37 KB, patch)
2018-05-03 20:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20692: Return and display and error if koha-plack failed (2.38 KB, patch)
2018-05-03 20:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20692: Return and display and error if koha-plack failed (2.38 KB, patch)
2018-05-23 18:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20692: Return and display and error if koha-plack failed (2.38 KB, patch)
2018-05-23 18:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20692: Return and display and error if koha-plack failed (2.92 KB, patch)
2018-05-28 14:57 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 20692: Extreme toggle checking for plack (7.37 KB, patch)
2018-05-28 17:51 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 20692: Extreme toggle checking for plack (7.41 KB, patch)
2019-04-16 15:42 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 20692: Extreme toggle checking for plack (7.46 KB, patch)
2019-04-25 07:30 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Piotr Wejman 2018-05-01 20:26:39 UTC
When enabling Plack, koha-plack uncomment plack related Include in instance's apache conf. Little problem occurs when old pre-plack configs are used, without required Include lines. In this case koha-plack still report success, with no change done.

So in the end we have running Plack, but it's doing nothing. plack.log is empty, but there is an entry in plack-error.log that can trick people into thinking they have Plack properly configured and running:

2018/05/01-21:06:27 Starman::Server (type Net::Server::PreFork) starting! pid(10257)
Binding to UNIX socket file "/var/run/koha/galway/plack.sock"
Setting gid to "1001 1001"
Setting uid to "1001"

koha-plack:enable_plack() should check if Plack lines exists before trying to uncomment them and either fail with warn message, or add them.
Comment 1 Jonathan Druart 2018-05-03 19:43:05 UTC
What says the about page (tab "system information")?
Comment 2 Jonathan Druart 2018-05-03 20:13:03 UTC
Created attachment 75038 [details] [review]
Bug 20692: Return and display and error if koha-plack failed

If koha-plack did not comment/uncomment the line it should fails and
display an error instead of success.

Useful for old apache config files
Comment 3 Jonathan Druart 2018-05-03 20:22:16 UTC
Created attachment 75040 [details] [review]
Bug 20692: Return and display and error if koha-plack failed

If koha-plack did not comment/uncomment the line it should fails and
display an error instead of success.

Useful for old apache config files
Comment 4 Jonathan Druart 2018-05-23 18:06:46 UTC
Created attachment 75510 [details] [review]
Bug 20692: Return and display and error if koha-plack failed

If koha-plack did not comment/uncomment the line it should fails and
display an error instead of success.

Useful for old apache config files
Comment 5 Jonathan Druart 2018-05-23 18:08:45 UTC
Created attachment 75511 [details] [review]
Bug 20692: Return and display and error if koha-plack failed

If koha-plack did not comment/uncomment the line it should fails and
display an error instead of success.

Useful for old apache config files
Comment 6 M. Tompsett 2018-05-23 18:48:27 UTC
The 'q' portion of the sed logic is faulty.
Comment 7 Jonathan Druart 2018-05-28 14:57:03 UTC
Created attachment 75594 [details] [review]
Bug 20692: Return and display and error if koha-plack failed

If koha-plack did not comment/uncomment the line it should fails and
display an error instead of success.

Useful for old apache config files
Comment 8 Jonathan Druart 2018-05-28 14:57:52 UTC
Well, this is really not error proof but should improve a bit the behavior (not the code!)
Comment 9 M. Tompsett 2018-05-28 15:05:30 UTC
(In reply to Jonathan Druart from comment #8)
> Well, this is really not error proof but should improve a bit the behavior
> (not the code!)

Hmmm... it gives me an idea... I'll see if I can create a better counter patch, if not, I'll sign this off.
Comment 10 M. Tompsett 2018-05-28 17:51:24 UTC
Created attachment 75597 [details] [review]
Bug 20692: Extreme toggle checking for plack

TEST PLAN
---------
sudo koha-plack --disable
sudo koha-plack --disable
sudo koha-plack --enable

sudo koha-plack --disable
echo $?
sudo koha-plack --disable
echo $?
sudo koha-list --noplack
echo $?
sudo koha-plack --enable
echo $?
sudo koha-plack --enable
echo $?
sudo koha-list --plack
echo $?

git bz apply 20692
sudo perl ~/misc4dev/cp_debian_files.pl
then repeat the test.

It should also be noted, because the enable
and disable are now split, if for some reason
they become out of sync, an enable or disable
should correct them.

Because these are bash scripts, the test tool
(/home/vagrant/qa-test-tools/koha-qa.pl)
does not apply.
Comment 11 Liz Rea 2019-04-16 15:42:31 UTC
Created attachment 88085 [details] [review]
Bug 20692: Extreme toggle checking for plack

TEST PLAN
---------
sudo koha-plack --disable
sudo koha-plack --disable
sudo koha-plack --enable

sudo koha-plack --disable
echo $?
sudo koha-plack --disable
echo $?
sudo koha-list --noplack
echo $?
sudo koha-plack --enable
echo $?
sudo koha-plack --enable
echo $?
sudo koha-list --plack
echo $?

git bz apply 20692
sudo perl ~/misc4dev/cp_debian_files.pl
then repeat the test.

It should also be noted, because the enable
and disable are now split, if for some reason
they become out of sync, an enable or disable
should correct them.

Because these are bash scripts, the test tool
(/home/vagrant/qa-test-tools/koha-qa.pl)
does not apply.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 12 Martin Renvoize 2019-04-25 07:30:49 UTC
Created attachment 88698 [details] [review]
Bug 20692: Extreme toggle checking for plack

TEST PLAN
---------
sudo koha-plack --disable
sudo koha-plack --disable
sudo koha-plack --enable

sudo koha-plack --disable
echo $?
sudo koha-plack --disable
echo $?
sudo koha-list --noplack
echo $?
sudo koha-plack --enable
echo $?
sudo koha-plack --enable
echo $?
sudo koha-list --plack
echo $?

git bz apply 20692
sudo perl ~/misc4dev/cp_debian_files.pl
then repeat the test.

It should also be noted, because the enable
and disable are now split, if for some reason
they become out of sync, an enable or disable
should correct them.

Because these are bash scripts, the test tool
(/home/vagrant/qa-test-tools/koha-qa.pl)
does not apply.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2019-04-25 07:31:24 UTC
Nice bit of code curling there guys, a solid improvement, thanks.

Passing QA
Comment 14 Nick Clemens 2019-04-25 11:52:12 UTC
Awesome work all!

Pushed to master for 19.05
Comment 15 Martin Renvoize 2019-04-26 15:28:08 UTC
Pushed to 18.11.x for 18.11.05
Comment 16 Lucas Gass 2019-05-24 15:46:34 UTC
backported to 18.05.x for 18.05.13