Bug 719371 - Provide native systemd unit file
Summary: Provide native systemd unit file
Keywords:
Status: CLOSED RAWHIDE
Alias: None
Product: Fedora
Classification: Fedora
Component: mailman
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Jan Kaluža
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: SysVtoSystemd
TreeView+ depends on / blocked
 
Reported: 2011-07-06 15:40 UTC by Jóhann B. Guðmundsson
Modified: 2011-07-14 09:37 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2011-07-14 09:37:39 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
Native systemd service file mailman (309 bytes, text/plain)
2011-07-06 15:44 UTC, Jóhann B. Guðmundsson
no flags Details
native mailman.service (388 bytes, text/plain)
2011-07-07 11:50 UTC, Tomasz Torcz
no flags Details
service with cron support (662 bytes, text/plain)
2011-07-13 10:37 UTC, Jan Kaluža
no flags Details
mailman.spec patch (4.45 KB, patch)
2011-07-13 11:38 UTC, Jan Kaluža
no flags Details | Diff

Description Jóhann B. Guðmundsson 2011-07-06 15:40:11 UTC
Description of problem:

https://fedoraproject.org/wiki/Features/SysVtoSystemd

Version-Release number of selected component (if applicable):


How reproducible:


Steps to Reproduce:
1.
2.
3.
  
Actual results:


Expected results:


Additional info:

Comment 1 Jóhann B. Guðmundsson 2011-07-06 15:44:08 UTC
Created attachment 511522 [details]
Native systemd service file mailman

Note that the cronjob part should be in the rpm package and happen at install time if you need valid sysadmin arguements it's simple you would not install mailman if you had not intention of running it and if the cron job bothers you then uninstall mailman... 

https://fedoraproject.org/wiki/Packaging:Guidelines:Systemd
https://fedoraproject.org/wiki/Packaging:ScriptletSnippets#Systemd
http://fedoraproject.org/wiki/Packaging:Tmpfiles.d

Comment 2 Tomasz Torcz 2011-07-07 11:50:02 UTC
Created attachment 511679 [details]
native mailman.service

I've tested and slightly improved provided unit file:
- added Type=forking
- added reload command
- redirected stderr to syslog (optional, but nice)

With above changes mailman correctly starts, stops and reacts to reload. You still have to install cron job from RPM, like it was done before.

Comment 3 Jan Kaluža 2011-07-11 11:46:18 UTC
Thanks for your job, I will test it and if it will work correctly for me too, I will commit it.

Comment 4 Jan Kaluža 2011-07-13 06:38:02 UTC
Well, I don't think the cronjob should be active when mailman is stopped. Mailman can be installed and stopped and admin should not for example receive emails from it when it's stopped. That would be unexpected behaviour.

You're right that it's logical to have mailman installed in order to use it, but since there's a way to have it installed but turned off, we have to fully support it.

Comment 5 Jóhann B. Guðmundsson 2011-07-13 09:46:26 UTC
Do you need update the cron job then as in first check if mailman is running then do $foo if not then do nothing ?

Comment 6 Jan Kaluža 2011-07-13 10:36:52 UTC
Well, I've changed my new mailman.service and it works as expected for me. Attaching it here.

Comment 7 Jan Kaluža 2011-07-13 10:37:43 UTC
Created attachment 512628 [details]
service with cron support

Comment 8 Tomasz Torcz 2011-07-13 10:47:49 UTC
Maybe create second unit, mailman-cron-control, with

---
[Unit]
Description=Mailman cron job
BindTo=mailman.service

[Service]
ExecStart=/usr/bin/install -m644 -o root -g root /usr/lib/mailman/cron/crontab.in /etc/cron.d/mailman
ExecStop=/bin/sh -c 'echo -e "# DO NOT EDIT THIS FILE!\n#\n# Contents of this file managed by /lib/systemd/system/mailman-cron-control.service\n# Master copy is /usr/lib/mailman/cron/crontab.in" > /etc/cron.d/mailman
Type=oneshot
RemainAfterExit=yes
---

And then, in mailman.service use Requires=mailman-cron-control.service in [Unit] and Also=mailman-cron-control.service in [Install].

Those are defined in systemd.unit man page, but I'm not sure if I got BindTo= correctly.

Comment 9 Jóhann B. Guðmundsson 2011-07-13 10:58:20 UTC
I'm still on the view that this should not be done by the unit file and
replaced with something like this on top of my head 

Crontab file would call 

0 8 * * * mailman /usr/lib/mailman/cron/checkdbs.sh

checkdbs.sh would look something like.. 

#!/bin/sh


if [ -s /var/run/mailman.pid ]; 
        then
              /usr/lib/mailman/cron/checkdbs
        else 
              exit 1
fi

exit 0

Anyway let's just package what Jan had the rest can be fixed later via update

Comment 10 Jan Kaluža 2011-07-13 11:04:05 UTC
(In reply to comment #8)
> Maybe create second unit, mailman-cron-control, with
> 
> ---
> [Unit]
> Description=Mailman cron job
> BindTo=mailman.service
> 
> [Service]
> ExecStart=/usr/bin/install -m644 -o root -g root
> /usr/lib/mailman/cron/crontab.in /etc/cron.d/mailman
> ExecStop=/bin/sh -c 'echo -e "# DO NOT EDIT THIS FILE!\n#\n# Contents of this
> file managed by /lib/systemd/system/mailman-cron-control.service\n# Master copy
> is /usr/lib/mailman/cron/crontab.in" > /etc/cron.d/mailman
> Type=oneshot
> RemainAfterExit=yes
> ---
> 
> And then, in mailman.service use Requires=mailman-cron-control.service in
> [Unit] and Also=mailman-cron-control.service in [Install].
> 
> Those are defined in systemd.unit man page, but I'm not sure if I got BindTo=
> correctly.

I can, but are there any benefits? I could do the same with "/usr/bin/mailman-update-cfg" and call it mailman-config-control.

Comment 11 Jan Kaluža 2011-07-13 11:09:34 UTC
(In reply to comment #9)
> I'm still on the view that this should not be done by the unit file and
> replaced with something like this on top of my head 
> 
> Crontab file would call 
> 
> 0 8 * * * mailman /usr/lib/mailman/cron/checkdbs.sh
> 
> checkdbs.sh would look something like.. 
> 
> #!/bin/sh
> 
> 
> if [ -s /var/run/mailman.pid ]; 
>         then
>               /usr/lib/mailman/cron/checkdbs
>         else 
>               exit 1
> fi
> 
> exit 0
> 
> Anyway let's just package what Jan had the rest can be fixed later via update

I'm not against something like that probably, but we would have to change it in all scripts or write one wrapper script and call it with the original scripts are arguments.

Comment 12 Jan Kaluža 2011-07-13 11:38:16 UTC
Created attachment 512638 [details]
mailman.spec patch

Comment 13 Jan Kaluža 2011-07-13 11:39:16 UTC
This is mailman.spec patch which works for me and which I'm going to commit. If you see something bad there, please tell me.

Comment 14 Jan Kaluža 2011-07-14 09:37:39 UTC
Well, I've committed those changes into rawhide. Closing this bug to not block SysVtoSystemd. Fill another one for to further changes.


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