Bug 719371

Summary: Provide native systemd unit file
Product: [Fedora] Fedora Reporter: Jóhann B. Guðmundsson <johannbg>
Component: mailmanAssignee: Jan Kaluža <jkaluza>
Status: CLOSED RAWHIDE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: jkaluza, tomek
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2011-07-14 05:37:39 EDT Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---
Bug Depends On:    
Bug Blocks: 713562    
Attachments:
Description Flags
Native systemd service file mailman
none
native mailman.service
none
service with cron support
none
mailman.spec patch none

Description Jóhann B. Guðmundsson 2011-07-06 11:40:11 EDT
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 11:44:08 EDT
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 07:50:02 EDT
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 07:46:18 EDT
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 02:38:02 EDT
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 05:46:26 EDT
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 06:36:52 EDT
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 06:37:43 EDT
Created attachment 512628 [details]
service with cron support
Comment 8 Tomasz Torcz 2011-07-13 06:47:49 EDT
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 06:58:20 EDT
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 07:04:05 EDT
(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 07:09:34 EDT
(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 07:38:16 EDT
Created attachment 512638 [details]
mailman.spec patch
Comment 13 Jan Kaluža 2011-07-13 07:39:16 EDT
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 05:37:39 EDT
Well, I've committed those changes into rawhide. Closing this bug to not block SysVtoSystemd. Fill another one for to further changes.