Bug 840942

Summary: atop systemd unit doesn't reference /etc/sysconfig/atop or write to $LOGPATH
Product: [Fedora] Fedora Reporter: Robert Kennedy <rt>
Component: atopAssignee: Gwyn Ciesla <gwync>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: unspecified    
Version: 17CC: gwync
Target Milestone: ---   
Target Release: ---   
Hardware: x86_64   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2012-08-01 22:25:30 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:

Description Robert Kennedy 2012-07-17 16:43:37 UTC
Description of problem:

It looks like with the systemd-ification of atop that it's not trying to read /etc/sysconfig/atop. It just forks off '/usr/bin/atop', which appears to simply shove the every 10s interactive content into /var/log/messages. Ugh.

The systemd answer, I guess, is for me to write /etc/systemd/system/atop.service the way I want it. But the rpm still contains /etc/sysconfig/atop which is kind of misleading. Adding an EnvironmentFile= line doesn't help since atop doesn't actually look in the env for those variables anyway, and systemd doesn't run the sysconfig file through a shell to handle stuff like CURDAY=`date +%Y%m%d`.

Or I could be crazy.

Version-Release number of selected component (if applicable):
atop-1.26-6.fc17.x86_64
atop-1.26-6.fc16.x86_64

How reproducible:
Always

Steps to Reproduce:
1. Install atop 1.26-6 in Fedora 16 or 17.
2. Change something in /etc/sysconfig/atop expecting it to do something.
3. systemctl restart atop.service
  
Actual results:
No change.

Expected results:
A change.

Additional info:

Comment 1 Robert Kennedy 2012-07-17 16:51:37 UTC
Ahhhh. I see /usr/bin/atopd now. I'm guessing that's what needs to go in ExecStart.

Comment 2 Robert Kennedy 2012-07-17 17:35:18 UTC
[Service]
Type=forking
PIDFile=/var/run/atop.pid
ExecStart=/usr/bin/atopd
ExecReload=/usr/bin/atopd
ExecStopPost=/bin/rm -f /var/run/atop.pid

Seems to work a bit better for me. I am no systemd expert though. PID cleanup should probably go in atopd as well with a stop parameter or somesuch. PIDFile= and $PIDFILE could get out of whack with this setup. Ugh.

Comment 3 Robert Kennedy 2012-07-17 17:41:14 UTC
Nevermind. We are try-restart'ing so the reload is not needed. and ExecStopPost needs to be ExecStop for try-restart to work.

[Service]
Type=forking
PIDFile=/var/run/atop.pid
ExecStart=/usr/bin/atopd
ExecStop=/bin/rm -f /var/run/atop.pid

Comment 4 Gwyn Ciesla 2012-07-23 19:56:29 UTC
So you'd ideally like to see:

Description=advanced interactive monitor
After=syslog.target

[Service]
ExecStart=/usr/bin/atop

[Install]
WantedBy=multi-user.target


Become:

Description=advanced interactive monitor
After=syslog.target

[Service]
Type=forking
PIDFile=/var/run/atop.pid
EnvironmentFile=-/etc/sysconfig/atop
ExecStart=/usr/bin/atopd $CURDAY $LOGPATH $BINPATH $PIDFILE $INTERVAL
ExecStop=/bin/rm -f /var/run/atop.pid

[Install]
WantedBy=multi-user.target

Comment 5 Robert Kennedy 2012-07-24 17:48:59 UTC
Since the atopd script reads /etc/sysconfig/atop itself, I don't think the EnvironmentFile is necessary, nor is trying to pass the variables into atopd like that.

I'm not sure what the packager wants to do for the future - get rid of the atopd wrapper script completely?

Comment 6 Gwyn Ciesla 2012-07-24 18:10:49 UTC
Of course, that was silly.  Sorry, I've done several systemd migrations since this and had forgotten about the script.

So:

Description=advanced interactive monitor
After=syslog.target

[Service]
Type=forking
PIDFile=/var/run/atop.pid
ExecStart=/usr/bin/atopd
ExecStop=/bin/rm -f /var/run/atop.pid

[Install]
WantedBy=multi-user.target

. . .like you originally posted.  Right?

Comment 7 Robert Kennedy 2012-07-24 18:35:30 UTC
Works for me...

It could be possible to ditch atopd completely and use EnvironmentFile and a more complicated ExecStop (to send the USR1 signal, then TERM, then remove PID). But I would leave that for another iteration.

Comment 8 Gwyn Ciesla 2012-07-24 18:46:47 UTC
Ok, I'll get this into rawhide and get an update out for f17, you'll see a link here for testing/karma etc.  Thanks!

Comment 9 Fedora Update System 2012-07-24 18:57:31 UTC
atop-1.26-8.fc17 has been submitted as an update for Fedora 17.
https://admin.fedoraproject.org/updates/atop-1.26-8.fc17

Comment 10 Fedora Update System 2012-07-26 03:59:45 UTC
Package atop-1.26-8.fc17:
* should fix your issue,
* was pushed to the Fedora 17 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing atop-1.26-8.fc17'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2012-11108/atop-1.26-8.fc17
then log in and leave karma (feedback).

Comment 11 Fedora Update System 2012-08-01 22:25:30 UTC
atop-1.26-8.fc17 has been pushed to the Fedora 17 stable repository.  If problems still persist, please make note of it in this bug report.