Bug 26363 - Provide a systemd unit file for background_jobs_worker
Summary: Provide a systemd unit file for background_jobs_worker
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P1 - high major (vote)
Assignee: David Cook
QA Contact: Testopia
URL:
Keywords:
Depends on: 22417
Blocks: 27844
  Show dependency treegraph
 
Reported: 2020-09-03 07:02 UTC by Jonathan Druart
Modified: 2021-12-13 21:08 UTC (History)
5 users (show)

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


Attachments
Bug 26363: Provide a systemd unit file for koha-worker for source installs (1.91 KB, patch)
2020-10-06 06:37 UTC, David Cook
Details | Diff | Splinter Review
Bug 26363: Provide a systemd unit file for koha-worker in package installs (1.51 KB, patch)
2020-10-06 06:37 UTC, David Cook
Details | Diff | Splinter Review
Bug 26363: Provide a systemd unit file for koha-worker for source installs (1.96 KB, patch)
2020-11-11 00:16 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 26363: Provide a systemd unit file for koha-worker in package installs (1.57 KB, patch)
2020-11-11 03:19 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 26363: Change SyslogIdentifier to match koha-worker script style (1.25 KB, patch)
2021-01-05 05:10 UTC, David Cook
Details | Diff | Splinter Review
Bug 26363: Provide a systemd unit file for koha-worker for source installs (2.03 KB, patch)
2021-02-02 11:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26363: Provide a systemd unit file for koha-worker in package installs (1.63 KB, patch)
2021-02-02 11:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26363: Change SyslogIdentifier to match koha-worker script style (1.32 KB, patch)
2021-02-02 11:09 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 26363: Remove tmp file during package build (follow-up) (707 bytes, patch)
2021-03-04 02:49 UTC, Mason James
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2020-09-03 07:02:45 UTC
We need a systemd service’s unit file to start/stop misc/background_jobs_worker.pl, like we do with the debian packages in debian/scripts/koha-worker
Comment 1 David Cook 2020-09-04 03:50:14 UTC
I'll add it to my TODO list :D
Comment 2 Jonathan Druart 2020-10-05 14:44:52 UTC
(In reply to David Cook from comment #1)
> I'll add it to my TODO list :D

Move it up please ;)
Comment 3 David Cook 2020-10-05 23:33:59 UTC
(In reply to Jonathan Druart from comment #2)
> Move it up please ;)

Noted! Exciting!
Comment 4 David Cook 2020-10-06 06:37:35 UTC
Created attachment 111262 [details] [review]
Bug 26363: Provide a systemd unit file for koha-worker for source installs

This patch adds a systemd unit file which gets rewritten according to
the configuration specified by Makefile.PL. This can be linked or
copied to systemd directories for running a systemd service for
background_jobs_worker.pl

Test plan:
1. perl Makefile.PL
1a. Choose "single" installation
1b. Choose "/opt/koha" for a file path
2. make
3. vi blib/KOHA_CONF_DIR/koha-worker.service
4. Verify that PERL5LIB, KOHA_CONF, ExecStart, and SyslogIdentifier
all appear to be correctly filled in
Comment 5 David Cook 2020-10-06 06:37:39 UTC
Created attachment 111263 [details] [review]
Bug 26363: Provide a systemd unit file for koha-worker in package installs

While the packages already have a koha-zebra script that creates a service
using the 'daemon' utility, this unit file template provides an alternative.

Note that currently you'd have to start the service using the following:

```systemctl start koha-worker@NAME.service```

Where NAME is the name of your Koha instance.

The systemctl enable/start commands could be automated more with helper
scripts in debian/scripts, but this is a first step.
Comment 6 David Cook 2020-10-08 22:32:42 UTC
Probably will need to do something like https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=20582&attachment=111374 to remove the systemd unit file for source installs when doing packaging...
Comment 7 David Cook 2020-11-10 23:11:58 UTC
(In reply to David Cook from comment #5)
> Created attachment 111263 [details] [review] [review]
> Bug 26363: Provide a systemd unit file for koha-worker in package installs
> 

To test this on koha-testing-docker, you should be able to do something like the following:

```cp debian/templates/koha-worker@.service /etc/systemd/system/.```
```systemctl start koha-worker@kohadev.service```
Comment 8 David Cook 2020-11-10 23:19:44 UTC
(In reply to David Cook from comment #7)
> (In reply to David Cook from comment #5)
> > Created attachment 111263 [details] [review] [review] [review]
> > Bug 26363: Provide a systemd unit file for koha-worker in package installs
> > 
> 
> To test this on koha-testing-docker, you should be able to do something like
> the following:
> 
> ```cp debian/templates/koha-worker@.service /etc/systemd/system/.```
> ```systemctl start koha-worker@kohadev.service```

Except that won't work because koha-testing-docker is a Docker container without the systemd init system.

You should be able to do this using kohadevbox though or some other Linux environment.
Comment 9 Victor Grousset/tuxayo 2020-11-11 00:16:55 UTC
Created attachment 113450 [details] [review]
Bug 26363: Provide a systemd unit file for koha-worker for source installs

This patch adds a systemd unit file which gets rewritten according to
the configuration specified by Makefile.PL. This can be linked or
copied to systemd directories for running a systemd service for
background_jobs_worker.pl

Test plan:
1. perl Makefile.PL
1a. Choose "single" installation
1b. Choose "/opt/koha" for a file path
2. make
3. vi blib/KOHA_CONF_DIR/koha-worker.service
4. Verify that PERL5LIB, KOHA_CONF, ExecStart, and SyslogIdentifier
all appear to be correctly filled in

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 10 Victor Grousset/tuxayo 2020-11-11 00:18:14 UTC
Here is one patch signed off at least ^^
Comment 11 Victor Grousset/tuxayo 2020-11-11 03:19:36 UTC
Created attachment 113461 [details] [review]
Bug 26363: Provide a systemd unit file for koha-worker in package installs

While the packages already have a koha-zebra script that creates a service
using the 'daemon' utility, this unit file template provides an alternative.

Note that currently you'd have to start the service using the following:

```systemctl start koha-worker@NAME.service```

Where NAME is the name of your Koha instance.

The systemctl enable/start commands could be automated more with helper
scripts in debian/scripts, but this is a first step.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 12 Victor Grousset/tuxayo 2020-11-11 03:19:57 UTC
It works!
Comment 13 David Cook 2020-11-11 03:47:16 UTC
Note: I think Victor needed to update his kohadevbox to use koha_use_custom_repo so that it would use the koha-staging apt repo that is used by default in koha-testing-docker
Comment 14 Victor Grousset/tuxayo 2020-11-11 13:21:00 UTC
Indeed, I forgot to post the info
Here was custom config for my devbox

koha_custom_repo_origin: debian.koha-community.org
koha_custom_repo: "[trusted=yes] http://{{ koha_custom_repo_origin }}/koha-staging dev"
koha_custom_repo_has_key: no
koha_custom_repo_key:
koha_custom_repo_distribution: stretch

koha_use_custom_repo: yes


There was a bit of guesswork, not sure if had to use koha_custom_repo. So likely not a clean way.

Issue reported: https://gitlab.com/koha-community/kohadevbox/-/issues/309
Comment 15 Jonathan Druart 2020-11-19 10:24:57 UTC
We really need this part of 20.11 IMO
Comment 16 David Cook 2020-11-20 03:22:50 UTC
(In reply to Jonathan Druart from comment #15)
> We really need this part of 20.11 IMO

Agreed.
Comment 17 Jonathan Druart 2020-11-25 10:29:50 UTC
1. Those 2 lines are not equivalent:
  debian/templates/koha-worker@.service:SyslogIdentifier=koha-worker_%i
  etc/koha-worker.service:SyslogIdentifier=koha-worker-__DB_NAME__

2. Should not it be $instance-koha-worker instead, to match what we have in:
debian/scripts/koha-functions.sh:    if daemon --name="$instancename-koha-worker" \
Comment 18 David Cook 2020-11-26 00:10:01 UTC
(In reply to Jonathan Druart from comment #17)
> 1. Those 2 lines are not equivalent:
>   debian/templates/koha-worker@.service:SyslogIdentifier=koha-worker_%i
>   etc/koha-worker.service:SyslogIdentifier=koha-worker-__DB_NAME__
> 

That's because koha-worker@.service is a template file. When invoked using ```systemctl start koha-worker@NAME.service```, %i will have NAME in it. This could be used by koha-create.

For source installs, I was thinking that people would just copy or symlink the built file from the Koha installation directory to /etc/systemd/system, so they'd want to have the name hard-coded into the unit file. We could use a systemd template for source installs too and then just given an instruction to copy the template to /etc/systemd/system and then use ```systemctl start koha-worker@NAME.service```. It would make things more consistent.

> 2. Should not it be $instance-koha-worker instead, to match what we have in:
> debian/scripts/koha-functions.sh:    if daemon
> --name="$instancename-koha-worker" \

I don't understand the question. What do you mean by "it" in your question? The SyslogIdentifier could be done like $instancename-koha-worker, but not the systemd unit. It needs to start with a set name. 

Here's an example that I use locally already:

systemctl status "koha-mq*"
● koha-mq-scheduler@EXAMPLE.service - Koha Message Queue Work Scheduler
   Loaded: loaded (/etc/systemd/system/koha-mq-scheduler@.service; indirect; vendor preset: enabled)
   Active: active (running) since Sun 2020-11-22 19:33:35 AEDT; 3 days ago
 Main PID: 23226 (mq_scheduler.pl)
    Tasks: 1 (limit: 4915)
   CGroup: /system.slice/system-koha\x2dmq\x2dscheduler.slice/koha-mq-scheduler@EXAMPLE.service
           └─23226 /usr/bin/perl /usr/share/koha/bin/prosentient/mq_scheduler.pl -v

Nov 22 19:33:35 koha1911 systemd[1]: Started Koha Message Queue Work Scheduler.

At the moment, I only have one koha-mq-scheduler unit running on that server, but if there were others, that systemctl command would've listed them all.
Comment 19 Jonathan Druart 2020-11-27 08:45:13 UTC
(In reply to David Cook from comment #18)
> (In reply to Jonathan Druart from comment #17)
> > 2. Should not it be $instance-koha-worker instead, to match what we have in:
> > debian/scripts/koha-functions.sh:    if daemon
> > --name="$instancename-koha-worker" \
> 
> I don't understand the question. What do you mean by "it" in your question?

$instancename-koha-worker vs koha-worker-__DB_NAME__
Comment 20 David Cook 2020-11-29 22:41:58 UTC
(In reply to Jonathan Druart from comment #19)
> (In reply to David Cook from comment #18)
> > (In reply to Jonathan Druart from comment #17)
> > > 2. Should not it be $instance-koha-worker instead, to match what we have in:
> > > debian/scripts/koha-functions.sh:    if daemon
> > > --name="$instancename-koha-worker" \
> > 
> > I don't understand the question. What do you mean by "it" in your question?
> 
> $instancename-koha-worker vs koha-worker-__DB_NAME__

But in what context? Is it just in terms of the following?

debian/templates/koha-worker@.service:SyslogIdentifier=koha-worker_%i
etc/koha-worker.service:SyslogIdentifier=koha-worker-__DB_NAME__

If so, it would be trivial to do "__DB_NAME__-koha-worker" and "%i_koha-worker" instead, if that's what you mean?
Comment 21 David Cook 2020-12-23 04:01:16 UTC
Should this be "In discussion" or "Failed QA" or...?
Comment 22 Jonathan Druart 2021-01-04 10:24:13 UTC
(In reply to David Cook from comment #20)
> (In reply to Jonathan Druart from comment #19)
> > (In reply to David Cook from comment #18)
> > > (In reply to Jonathan Druart from comment #17)
> > > > 2. Should not it be $instance-koha-worker instead, to match what we have in:
> > > > debian/scripts/koha-functions.sh:    if daemon
> > > > --name="$instancename-koha-worker" \
> > > 
> > > I don't understand the question. What do you mean by "it" in your question?
> > 
> > $instancename-koha-worker vs koha-worker-__DB_NAME__
> 
> But in what context? Is it just in terms of the following?
> 
> debian/templates/koha-worker@.service:SyslogIdentifier=koha-worker_%i
> etc/koha-worker.service:SyslogIdentifier=koha-worker-__DB_NAME__
> 
> If so, it would be trivial to do "__DB_NAME__-koha-worker" and
> "%i_koha-worker" instead, if that's what you mean?

Yes, it is what I meant. Do you agree?
Comment 23 David Cook 2021-01-05 04:10:19 UTC
(In reply to Jonathan Druart from comment #22)
> > If so, it would be trivial to do "__DB_NAME__-koha-worker" and
> > "%i_koha-worker" instead, if that's what you mean?
> 
> Yes, it is what I meant. Do you agree?

I don't really care, so that sounds fine to me.
Comment 24 David Cook 2021-01-05 04:59:00 UTC
(In reply to David Cook from comment #8)
> Except that won't work because koha-testing-docker is a Docker container
> without the systemd init system.
> 

Actually, if we start the docker container as a privileged container, we should be able to launch using /lib/systemd/systemd but then we'd need to change the koha-testing-docker command but that would also be a hassle.
Comment 25 David Cook 2021-01-05 05:08:40 UTC
(In reply to David Cook from comment #24)
> (In reply to David Cook from comment #8)
> > Except that won't work because koha-testing-docker is a Docker container
> > without the systemd init system.
> > 
> 
> Actually, if we start the docker container as a privileged container, we
> should be able to launch using /lib/systemd/systemd but then we'd need to
> change the koha-testing-docker command but that would also be a hassle.

Note that I've never actually used systemd in a Docker container except for when using Red Hat's ubi8 ubi-init image:

https://catalog.redhat.com/software/containers/ubi8/ubi-init/5c359b97d70cc534b3a378c8?container-tabs=dockerfile

There's more interesting reading here about trying to run systemd without a privileged container:
https://developers.redhat.com/blog/2016/09/13/running-systemd-in-a-non-privileged-container/
Comment 26 David Cook 2021-01-05 05:10:59 UTC
Created attachment 114831 [details] [review]
Bug 26363: Change SyslogIdentifier to match koha-worker script style

This patch changes the SyslogIdentifier so that it matches
the style used by koha-worker.
Comment 27 Martin Renvoize 2021-02-02 11:09:51 UTC
Created attachment 116203 [details] [review]
Bug 26363: Provide a systemd unit file for koha-worker for source installs

This patch adds a systemd unit file which gets rewritten according to
the configuration specified by Makefile.PL. This can be linked or
copied to systemd directories for running a systemd service for
background_jobs_worker.pl

Test plan:
1. perl Makefile.PL
1a. Choose "single" installation
1b. Choose "/opt/koha" for a file path
2. make
3. vi blib/KOHA_CONF_DIR/koha-worker.service
4. Verify that PERL5LIB, KOHA_CONF, ExecStart, and SyslogIdentifier
all appear to be correctly filled in

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize 2021-02-02 11:09:54 UTC
Created attachment 116204 [details] [review]
Bug 26363: Provide a systemd unit file for koha-worker in package installs

While the packages already have a koha-zebra script that creates a service
using the 'daemon' utility, this unit file template provides an alternative.

Note that currently you'd have to start the service using the following:

```systemctl start koha-worker@NAME.service```

Where NAME is the name of your Koha instance.

The systemctl enable/start commands could be automated more with helper
scripts in debian/scripts, but this is a first step.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 29 Martin Renvoize 2021-02-02 11:09:57 UTC
Created attachment 116205 [details] [review]
Bug 26363: Change SyslogIdentifier to match koha-worker script style

This patch changes the SyslogIdentifier so that it matches
the style used by koha-worker.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 30 Martin Renvoize 2021-02-02 11:10:47 UTC
I'll admit, I'm not an expert in this area.. However, the unit files look as I would expect and work as I would expect.. I think we will only uncover edge cases at this point if we push them... 

So.. Passing QA
Comment 31 Jonathan Druart 2021-03-01 10:28:51 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 32 Fridolin Somers 2021-03-02 09:00:57 UTC
koha-worker will run as root.
Shouldn't we run as koha user for package install, as www-data for dev install ?
Comment 33 Fridolin Somers 2021-03-02 09:07:19 UTC
Pushed to 20.11.x for 20.11.04
Comment 34 Martin Renvoize 2021-03-02 09:37:12 UTC
(In reply to Fridolin Somers from comment #32)
> koha-worker will run as root.
> Shouldn't we run as koha user for package install, as www-data for dev
> install ?

Good call.. can't believe I didn't spot that!
Comment 35 Fridolin Somers 2021-03-02 11:21:20 UTC
(In reply to Martin Renvoize from comment #34)
> (In reply to Fridolin Somers from comment #32)
> > koha-worker will run as root.
> > Shouldn't we run as koha user for package install, as www-data for dev
> > install ?
> 
> Good call.. can't believe I didn't spot that!

I added to koha-worker.service :

[Service]
User=www-data
Comment 36 Fridolin Somers 2021-03-02 15:37:58 UTC
Also maybe needs Type=forking ?

https://www.freedesktop.org/software/systemd/man/systemd.service.html#Type=
Comment 37 David Cook 2021-03-02 22:16:28 UTC
(In reply to Fridolin Somers from comment #36)
> Also maybe needs Type=forking ?
> 
> https://www.freedesktop.org/software/systemd/man/systemd.service.html#Type=

I don't think so since it doesn't fork when it starts.
Comment 38 David Cook 2021-03-02 22:21:09 UTC
(In reply to Martin Renvoize from comment #34)
> (In reply to Fridolin Somers from comment #32)
> > koha-worker will run as root.
> > Shouldn't we run as koha user for package install, as www-data for dev
> > install ?
> 
> Good call.. can't believe I didn't spot that!

Good catch, Frido. Yes, the package install should be "%i-koha". 

As for a source/dev install, I'm not 100% sure what it should be. I suppose it depends on what the background worker needs to do. In theory, www-data shouldn't be needed, but I suppose there might be something that the web server has written as www-data which the background worker needs to manage?
Comment 39 David Cook 2021-03-02 22:24:23 UTC
(In reply to David Cook from comment #38)
> As for a source/dev install, I'm not 100% sure what it should be. I suppose
> it depends on what the background worker needs to do. In theory, www-data
> shouldn't be needed, but I suppose there might be something that the web
> server has written as www-data which the background worker needs to manage?

Source/dev installs require extra work anyway so that one might be solved by documentation...
Comment 40 Fridolin Somers 2021-03-03 08:54:32 UTC
(In reply to David Cook from comment #39)
> (In reply to David Cook from comment #38)
> > As for a source/dev install, I'm not 100% sure what it should be. I suppose
> > it depends on what the background worker needs to do. In theory, www-data
> > shouldn't be needed, but I suppose there might be something that the web
> > server has written as www-data which the background worker needs to manage?
> 
> Source/dev installs require extra work anyway so that one might be solved by
> documentation...

I agree.

Keep in mind that some files will be used by both koha-worker and starman. Like tempate cache or sessions in temporary files.
Comment 41 Mason James 2021-03-04 02:49:49 UTC
Created attachment 117630 [details] [review]
Bug 26363: Remove tmp file during package build (follow-up)
Comment 42 Jonathan Druart 2021-03-04 08:53:28 UTC
follow-up pushed to master.
Comment 43 Mason James 2021-03-04 11:33:58 UTC
(In reply to Jonathan Druart from comment #42)
> follow-up pushed to master.

hi Frido, can you push to 20.11 too
Comment 44 Fridolin Somers 2021-03-04 12:27:56 UTC
(In reply to Mason James from comment #43)
> (In reply to Jonathan Druart from comment #42)
> > follow-up pushed to master.
> 
> hi Frido, can you push to 20.11 too

Bug 26363: Remove tmp file during package build (follow-up)
Pushed to 20.11.x for 20.11.04
Comment 45 Andrew Fuerste-Henry 2021-03-05 20:11:22 UTC
Missing dependency, not pushed to 20.05.