17467 – Introduce a single koha-zebra script to handle Zebra daemons for instances

Bug 17467 - Introduce a single koha-zebra script to handle Zebra daemons for instances

Summary: Introduce a single koha-zebra script to handle Zebra daemons for instances

Status:	CLOSED FIXED

Alias:	None

Product:	Koha
Classification:	Unclassified
Component:	Command-line Utilities (show other bugs)
Version:	Main
Hardware:	All All

Importance:	P5 - low enhancement
Assignee:	Tomás Cohen Arazi (tcohen)
QA Contact:	Testopia

URL:
Keywords:

Depends on:
Blocks:	17468 18114 20234
	Show dependency tree / graph

Reported:	2016-10-19 12:44 UTC by Tomás Cohen Arazi (tcohen)
Modified:	2018-12-03 20:03 UTC (History)
CC List:	9 users (show)

See Also:
Change sponsored?:	---
Patch complexity:	---
Documentation contact:
Documentation submission:
Text to go in the release notes:	To ease multi-tenant sites maintenance, several handy scripts were introduced. For handling Zebra, 4 scripts were introduced: koha-start-zebra, koha-stop-zebra, koha-restart-zebra and koha-rebuild-zebra. This patch introduces a new script, koha-zebra, that unifies those actions regarding Zebra daemons on a per instance base, through the use of option switches.
Version(s) released in:
Circulation function:

Attachments
Bug 17467: Add koha-zebra script to handle Zebra daemons (7.91 KB, patch) 2016-10-19 13:27 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 17467: (followup) Make koha-zebra handle legacy script names (2.09 KB, patch) 2016-10-19 13:27 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 17467: Add koha-zebra documentation (3.09 KB, patch) 2016-10-19 13:27 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 17467: (followup) Make the install script install koha-zebra (933 bytes, patch) 2016-10-19 14:26 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 17467: Add tab-completion for koha-zebra (2.27 KB, patch) 2016-10-19 19:00 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 17467: Add missing --status switch handling (738 bytes, patch) 2016-10-20 13:01 UTC, Tomás Cohen Arazi (tcohen)	Details \| Diff \| Splinter Review
Bug 17467: Restart failing, missing instance parameter (1.41 KB, patch) 2017-01-01 07:22 UTC, Mark Tompsett	Details \| Diff \| Splinter Review
[SIGNED-OFF] Bug 17467: Add koha-zebra script to handle Zebra daemons (7.98 KB, patch) 2017-08-24 14:15 UTC, Josef Moravec	Details \| Diff \| Splinter Review
[SIGNED-OFF] Bug 17467: (followup) Make koha-zebra handle legacy script names (2.16 KB, patch) 2017-08-24 14:15 UTC, Josef Moravec	Details \| Diff \| Splinter Review
[SIGNED-OFF] Bug 17467: Add koha-zebra documentation (3.15 KB, patch) 2017-08-24 14:15 UTC, Josef Moravec	Details \| Diff \| Splinter Review
[SIGNED-OFF] Bug 17467: (followup) Make the install script install koha-zebra (1001 bytes, patch) 2017-08-24 14:15 UTC, Josef Moravec	Details \| Diff \| Splinter Review
[SIGNED-OFF] Bug 17467: Add tab-completion for koha-zebra (2.34 KB, patch) 2017-08-24 14:15 UTC, Josef Moravec	Details \| Diff \| Splinter Review
[SIGNED-OFF] Bug 17467: Add missing --status switch handling (806 bytes, patch) 2017-08-24 14:16 UTC, Josef Moravec	Details \| Diff \| Splinter Review
[SIGNED-OFF] Bug 17467: Restart failing, missing instance parameter (1.47 KB, patch) 2017-08-24 14:16 UTC, Josef Moravec	Details \| Diff \| Splinter Review
Bug 17467: Add koha-zebra script to handle Zebra daemons (8.02 KB, patch) 2017-10-26 15:28 UTC, Nick Clemens (kidclamp)	Details \| Diff \| Splinter Review
Bug 17467: (followup) Make koha-zebra handle legacy script names (2.21 KB, patch) 2017-10-26 15:28 UTC, Nick Clemens (kidclamp)	Details \| Diff \| Splinter Review
Bug 17467: Add koha-zebra documentation (3.19 KB, patch) 2017-10-26 15:28 UTC, Nick Clemens (kidclamp)	Details \| Diff \| Splinter Review
Bug 17467: (followup) Make the install script install koha-zebra (1.02 KB, patch) 2017-10-26 15:28 UTC, Nick Clemens (kidclamp)	Details \| Diff \| Splinter Review
Bug 17467: Add tab-completion for koha-zebra (2.38 KB, patch) 2017-10-26 15:28 UTC, Nick Clemens (kidclamp)	Details \| Diff \| Splinter Review
Bug 17467: Add missing --status switch handling (850 bytes, patch) 2017-10-26 15:28 UTC, Nick Clemens (kidclamp)	Details \| Diff \| Splinter Review
Bug 17467: Restart failing, missing instance parameter (1.52 KB, patch) 2017-10-26 15:28 UTC, Nick Clemens (kidclamp)	Details \| Diff \| Splinter Review
Show Obsolete (14) View All Add an attachment (proposed patch, testcase, etc.)

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

Description Tomás Cohen Arazi (tcohen) 2016-10-19 12:44:28 UTC

There's really no need for one script per action.

Comment 1 Tomás Cohen Arazi (tcohen) 2016-10-19 13:27:13 UTC

Created attachment 56662 [details] [review]
Bug 17467: Add koha-zebra script to handle Zebra daemons

This script is intended to replace the following currently used scripts
on packages setups:
- koha-start-zebra
- koha-stop-zebra
- koha-restart-zebra

It also introduces a --status option switch, for asking for daemon statuses

To test

- Apply the patch
- Run:
  $ sudo debian/scripts/koha-zebra --start kohadev
=> SUCCESS: same behaviour than koha-start-zebra kohadev
- Run:
  $ sudo debian/scripts/koha-zebra --stop kohadev
=> SUCCESS: same behaviour as koha-stop-zebra kohadev
- Run:
  $ sudo debian/scripts/koha-zebra --restart kohadev
=> SUCCESS: same behaviour than koha-restart-zebra kohadev
- Run:
  $ sudo debian/scripts/koha-zebra --status kohadev
=> SUCCESS: It correctly shows the status for the running (or not) process

Play with different combinations of this commands

Comment 2 Tomás Cohen Arazi (tcohen) 2016-10-19 13:27:17 UTC

Created attachment 56663 [details] [review]
Bug 17467: (followup) Make koha-zebra handle legacy script names

This patch adds the new koha-zebra script the ability to handle being
called by other script names, for backwards compatibility with the legacy
koha-*-zebra scripts.

It is intended to be used in conjunction with a patch that replaces the old
koha-*-zebra scripts for symbolic links, pointing to the new koha-zebra script.

To test:
- Apply this patch
- Create symbolic links to the new script:
  vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-start-zebra
  vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-stop-zebra
  vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-restart-zebra
- Run:
  $ sudo ./koha-start-zebra kohadev
=> SUCCESS: Same behaviour than the koha-start-zebra script
- Run:
  sudo ./koha-stop-zebra kohadev
=> SUCCESS: Same behaviour than the koha-stop-zebra script
- Run:
  sudo ./koha-restart-zebra kohadev
=> SUCCESS: Same behaviour than the koha-restart-zebra script

Comment 3 Tomás Cohen Arazi (tcohen) 2016-10-19 13:27:22 UTC

Created attachment 56664 [details] [review]
Bug 17467: Add koha-zebra documentation

Comment 4 Tomás Cohen Arazi (tcohen) 2016-10-19 14:26:22 UTC

Created attachment 56665 [details] [review]
Bug 17467: (followup) Make the install script install koha-zebra

Comment 5 Tomás Cohen Arazi (tcohen) 2016-10-19 19:00:42 UTC

Created attachment 56669 [details] [review]
Bug 17467: Add tab-completion for koha-zebra

This patch adds tab-completion for bash, for the koha-zebra script.

To test:
- Run (on the vagrant user):
  $ source kohaclone/debian/koha-common.bash-completion
- Run:
  $ sudo koha-zebra -<TAB>
  (<TAB> means press the tab key)
=> SUCCESS: Tab completion works as expected for the koha-zebra command.
- Sign off :-D

Comment 6 Josef Moravec 2016-10-20 11:40:09 UTC

I started to test these patches:

after run 
sudo debian/scripts/koha-zebra --status kohadev
i get:
Error: invalid option switch (--status)

everything other works as expected

Comment 7 Tomás Cohen Arazi (tcohen) 2016-10-20 12:56:26 UTC

(In reply to Josef Moravec from comment #6)
> I started to test these patches:
> 
> after run 
> sudo debian/scripts/koha-zebra --status kohadev
> i get:
> Error: invalid option switch (--status)
> 
> everything other works as expected

Thanks, Josef. It seems that I forgot to add it

Comment 8 Tomás Cohen Arazi (tcohen) 2016-10-20 13:01:05 UTC

Created attachment 56693 [details] [review]
Bug 17467: Add missing --status switch handling

Comment 9 Mark Tompsett 2017-01-01 04:23:10 UTC

Comment on attachment 56662 [details] [review]
Bug 17467: Add koha-zebra script to handle Zebra daemons

Review of attachment 56662 [details] [review]:
-----------------------------------------------------------------

::: debian/scripts/koha-zebra
@@ +135,5 @@
> +
> +    if is_zebra_running ${name}; then
> +
> +        stop_zebra ${name} && \
> +        start_zebra ${name}

This logic is failing. The stop always runs, but the start never does.
Remember 0 success, but 0 && foo is short-circuited logic.

Also, missing verbose check to print a nice informational message about Restarting Zebra.

@@ +148,5 @@
> +zebra_status()
> +{
> +    local name=$1
> +
> +    if is_zebra_running; then

missing $name.

Comment 10 Mark Tompsett 2017-01-01 04:27:42 UTC

Also, while I understand the change of logic makes sense in terms of the verbose switch, the script before defaulted to verbose, so perhaps the switch should be silent? That's not a Failed QA issue for me, but the ones in comment #9 are.

Comment 11 Mark Tompsett 2017-01-01 07:22:06 UTC

Created attachment 58550 [details] [review]
Bug 17467: Restart failing, missing instance parameter

This follow up should make it work. Perhaps there is a more
elegant way, but wanted to generate something that works.

Before this patch, the --restart stops Zebra, but does not
restart it. After, it does restart.

Before this patch, the --status gives nasty long help.
After this patch, reasonable output.

Comment 12 Marcel de Rooy 2017-02-20 09:13:11 UTC

What about adding dev_install here now, Tomas?

Comment 13 Josef Moravec 2017-08-24 14:15:43 UTC

Created attachment 66420 [details] [review]
[SIGNED-OFF] Bug 17467: Add koha-zebra script to handle Zebra daemons

This script is intended to replace the following currently used scripts
on packages setups:
- koha-start-zebra
- koha-stop-zebra
- koha-restart-zebra

It also introduces a --status option switch, for asking for daemon statuses

To test

- Apply the patch
- Run:
  $ sudo debian/scripts/koha-zebra --start kohadev
=> SUCCESS: same behaviour than koha-start-zebra kohadev
- Run:
  $ sudo debian/scripts/koha-zebra --stop kohadev
=> SUCCESS: same behaviour as koha-stop-zebra kohadev
- Run:
  $ sudo debian/scripts/koha-zebra --restart kohadev
=> SUCCESS: same behaviour than koha-restart-zebra kohadev
- Run:
  $ sudo debian/scripts/koha-zebra --status kohadev
=> SUCCESS: It correctly shows the status for the running (or not) process

Play with different combinations of this commands

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Comment 14 Josef Moravec 2017-08-24 14:15:47 UTC

Created attachment 66421 [details] [review]
[SIGNED-OFF] Bug 17467: (followup) Make koha-zebra handle legacy script names

This patch adds the new koha-zebra script the ability to handle being
called by other script names, for backwards compatibility with the legacy
koha-*-zebra scripts.

It is intended to be used in conjunction with a patch that replaces the old
koha-*-zebra scripts for symbolic links, pointing to the new koha-zebra script.

To test:
- Apply this patch
- Create symbolic links to the new script:
  vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-start-zebra
  vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-stop-zebra
  vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-restart-zebra
- Run:
  $ sudo ./koha-start-zebra kohadev
=> SUCCESS: Same behaviour than the koha-start-zebra script
- Run:
  sudo ./koha-stop-zebra kohadev
=> SUCCESS: Same behaviour than the koha-stop-zebra script
- Run:
  sudo ./koha-restart-zebra kohadev
=> SUCCESS: Same behaviour than the koha-restart-zebra script

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Comment 15 Josef Moravec 2017-08-24 14:15:51 UTC

Created attachment 66422 [details] [review]
[SIGNED-OFF] Bug 17467: Add koha-zebra documentation

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Comment 16 Josef Moravec 2017-08-24 14:15:54 UTC

Created attachment 66423 [details] [review]
[SIGNED-OFF] Bug 17467: (followup) Make the install script install koha-zebra

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Comment 17 Josef Moravec 2017-08-24 14:15:57 UTC

Created attachment 66424 [details] [review]
[SIGNED-OFF] Bug 17467: Add tab-completion for koha-zebra

This patch adds tab-completion for bash, for the koha-zebra script.

To test:
- Run (on the vagrant user):
  $ source kohaclone/debian/koha-common.bash-completion
- Run:
  $ sudo koha-zebra -<TAB>
  (<TAB> means press the tab key)
=> SUCCESS: Tab completion works as expected for the koha-zebra command.
- Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Comment 18 Josef Moravec 2017-08-24 14:16:01 UTC

Created attachment 66425 [details] [review]
[SIGNED-OFF] Bug 17467: Add missing --status switch handling

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Comment 19 Josef Moravec 2017-08-24 14:16:05 UTC

Created attachment 66426 [details] [review]
[SIGNED-OFF] Bug 17467: Restart failing, missing instance parameter

This follow up should make it work. Perhaps there is a more
elegant way, but wanted to generate something that works.

Before this patch, the --restart stops Zebra, but does not
restart it. After, it does restart.

Before this patch, the --status gives nasty long help.
After this patch, reasonable output.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Comment 20 Marcel de Rooy 2017-08-28 07:36:37 UTC

Tomas, this patch does (indeed) not need dev_install/adjust_paths_dev_install since it does not refer to KOHA_HOME or PERL5LIB.

Comment 21 Nick Clemens (kidclamp) 2017-10-26 15:28:07 UTC

Created attachment 68623 [details] [review]
Bug 17467: Add koha-zebra script to handle Zebra daemons

This script is intended to replace the following currently used scripts
on packages setups:
- koha-start-zebra
- koha-stop-zebra
- koha-restart-zebra

It also introduces a --status option switch, for asking for daemon statuses

To test

- Apply the patch
- Run:
  $ sudo debian/scripts/koha-zebra --start kohadev
=> SUCCESS: same behaviour than koha-start-zebra kohadev
- Run:
  $ sudo debian/scripts/koha-zebra --stop kohadev
=> SUCCESS: same behaviour as koha-stop-zebra kohadev
- Run:
  $ sudo debian/scripts/koha-zebra --restart kohadev
=> SUCCESS: same behaviour than koha-restart-zebra kohadev
- Run:
  $ sudo debian/scripts/koha-zebra --status kohadev
=> SUCCESS: It correctly shows the status for the running (or not) process

Play with different combinations of this commands

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Comment 22 Nick Clemens (kidclamp) 2017-10-26 15:28:11 UTC

Created attachment 68624 [details] [review]
Bug 17467: (followup) Make koha-zebra handle legacy script names

This patch adds the new koha-zebra script the ability to handle being
called by other script names, for backwards compatibility with the legacy
koha-*-zebra scripts.

It is intended to be used in conjunction with a patch that replaces the old
koha-*-zebra scripts for symbolic links, pointing to the new koha-zebra script.

To test:
- Apply this patch
- Create symbolic links to the new script:
  vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-start-zebra
  vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-stop-zebra
  vagrant@kohadevbox:~$ ln -s kohaclone/debian/scripts/koha-zebra koha-restart-zebra
- Run:
  $ sudo ./koha-start-zebra kohadev
=> SUCCESS: Same behaviour than the koha-start-zebra script
- Run:
  sudo ./koha-stop-zebra kohadev
=> SUCCESS: Same behaviour than the koha-stop-zebra script
- Run:
  sudo ./koha-restart-zebra kohadev
=> SUCCESS: Same behaviour than the koha-restart-zebra script

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Comment 23 Nick Clemens (kidclamp) 2017-10-26 15:28:14 UTC

Created attachment 68625 [details] [review]
Bug 17467: Add koha-zebra documentation

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Comment 24 Nick Clemens (kidclamp) 2017-10-26 15:28:17 UTC

Created attachment 68626 [details] [review]
Bug 17467: (followup) Make the install script install koha-zebra

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Comment 25 Nick Clemens (kidclamp) 2017-10-26 15:28:20 UTC

Created attachment 68627 [details] [review]
Bug 17467: Add tab-completion for koha-zebra

This patch adds tab-completion for bash, for the koha-zebra script.

To test:
- Run (on the vagrant user):
  $ source kohaclone/debian/koha-common.bash-completion
- Run:
  $ sudo koha-zebra -<TAB>
  (<TAB> means press the tab key)
=> SUCCESS: Tab completion works as expected for the koha-zebra command.
- Sign off :-D

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Comment 26 Nick Clemens (kidclamp) 2017-10-26 15:28:24 UTC

Created attachment 68628 [details] [review]
Bug 17467: Add missing --status switch handling

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Comment 27 Nick Clemens (kidclamp) 2017-10-26 15:28:27 UTC

Created attachment 68629 [details] [review]
Bug 17467: Restart failing, missing instance parameter

This follow up should make it work. Perhaps there is a more
elegant way, but wanted to generate something that works.

Before this patch, the --restart stops Zebra, but does not
restart it. After, it does restart.

Before this patch, the --status gives nasty long help.
After this patch, reasonable output.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Comment 28 Jonathan Druart 2017-10-27 17:13:37 UTC

Pushed to master for 17.11, thanks to everybody involved!

Comment 29 Jonathan Druart 2017-11-26 14:26:11 UTC

Tomas, please add a text to go to the release notes.

Comment 30 Jonathan Druart 2018-01-19 18:56:02 UTC

(In reply to Nick Clemens from comment #26)
> Created attachment 68628 [details] [review] [review]
> Bug 17467: Add missing --status switch handling
> 
> Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
> 
> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

I missed that one! Pushed to master for 18.05

Comment 31 Nick Clemens (kidclamp) 2018-01-23 04:10:38 UTC

Awesome work all!

Pushed missed patch to Stable for 17.11.02

Comment 32 Fridolin Somers 2018-01-31 07:41:24 UTC

Enhancement not pushed to 17.05.x