Bug 840942 - atop systemd unit doesn't reference /etc/sysconfig/atop or write to $LOGPATH
atop systemd unit doesn't reference /etc/sysconfig/atop or write to $LOGPATH
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: atop (Show other bugs)
17
x86_64 Linux
unspecified Severity medium
: ---
: ---
Assigned To: Gwyn Ciesla
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-17 12:43 EDT by Robert Kennedy
Modified: 2012-08-01 18:25 EDT (History)
1 user (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2012-08-01 18:25:30 EDT
Type: Bug
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)

  None (edit)
Description Robert Kennedy 2012-07-17 12:43:37 EDT
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 12:51:37 EDT
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 13:35:18 EDT
[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 13:41:14 EDT
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 15:56:29 EDT
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 13:48:59 EDT
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 14:10:49 EDT
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 14:35:30 EDT
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 14:46:47 EDT
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 14:57:31 EDT
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-25 23:59:45 EDT
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 18:25:30 EDT
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.

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