Bug 840942 - atop systemd unit doesn't reference /etc/sysconfig/atop or write to $LOGPATH
Summary: atop systemd unit doesn't reference /etc/sysconfig/atop or write to $LOGPATH
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: atop
Version: 17
Hardware: x86_64
OS: Linux
unspecified
medium
Target Milestone: ---
Assignee: Gwyn Ciesla
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2012-07-17 16:43 UTC by Robert Kennedy
Modified: 2012-08-01 22:25 UTC (History)
1 user (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2012-08-01 22:25:30 UTC
Type: Bug
Embargoed:


Attachments (Terms of Use)

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.


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