Discussion:
[PATCH] CPU module, fixes to disk module and service module
Tomas Edwardsson
2011-02-04 01:16:03 UTC
Permalink
This patch introduces a new module, cpu.py and includes fixes for disk.py and service.py.

cpu.py exposes cpu information. It has two methods:
* cpu_sample_percentage - gets the average cpu usage, returns per cpu percentage for user, nice, system, idle, iowait and irq.
* jiffies - fetches cpu statistics from /proc/stat and returns them

disk.py modifications
* Add filesystem type returned by disk.usage()

service.py modifications
* chkconfig and /sbin/service now run with environment variable LANG=C, tools were returning internationalized strings which wouldn't parse correctly.
* Rewrote get_running(), now lists scripts from /etc/init.d and runs status itself instead of running /sbin/service --status-all. /sbin/service was returning internationalized strings and ignoring LANG variable.
--
Tomas Edwardsson
seth vidal
2011-02-04 05:03:23 UTC
Permalink
Post by Tomas Edwardsson
This patch introduces a new module, cpu.py and includes fixes for disk.py and service.py.
* cpu_sample_percentage - gets the average cpu usage, returns per cpu
percentage for user, nice, system, idle, iowait and irq.
* jiffies - fetches cpu statistics from /proc/stat and returns them
this looks fine.
Post by Tomas Edwardsson
disk.py modifications
* Add filesystem type returned by disk.usage()
also fine
Post by Tomas Edwardsson
service.py modifications
* chkconfig and /sbin/service now run with environment variable
LANG=C, tools were returning internationalized strings which wouldn't
parse correctly.
this looks like a good idea.
Post by Tomas Edwardsson
* Rewrote get_running(), now lists scripts from /etc/init.d and runs
status itself instead of running /sbin/service
--status-all. /sbin/service was returning internationalized strings
and ignoring LANG variable.
A little worried here. Using /etc/init.d directly is kinda contrary to
the whole point of chkconfig and /sbin/service. I'm also not in love
with the hardcoded list of things to skip. Feels like it is going to be
annoying to maintain.

Maybe just run the ones and catch the error if it lacks a 'status'?

What's the impact if we raise properly?

-sv
Tomas Edwardsson
2011-02-04 10:27:08 UTC
Permalink
----- Original Message -----
Post by seth vidal
Post by Tomas Edwardsson
This patch introduces a new module, cpu.py and includes fixes for
disk.py and service.py.
* cpu_sample_percentage - gets the average cpu usage, returns per cpu
percentage for user, nice, system, idle, iowait and irq.
* jiffies - fetches cpu statistics from /proc/stat and returns them
this looks fine.
Post by Tomas Edwardsson
disk.py modifications
* Add filesystem type returned by disk.usage()
also fine
Post by Tomas Edwardsson
service.py modifications
* chkconfig and /sbin/service now run with environment variable
LANG=C, tools were returning internationalized strings which
wouldn't
parse correctly.
this looks like a good idea.
Post by Tomas Edwardsson
* Rewrote get_running(), now lists scripts from /etc/init.d and runs
status itself instead of running /sbin/service
--status-all. /sbin/service was returning internationalized strings
and ignoring LANG variable.
A little worried here. Using /etc/init.d directly is kinda contrary to
the whole point of chkconfig and /sbin/service. I'm also not in love
with the hardcoded list of things to skip. Feels like it is going to be
annoying to maintain.
I agree that using /sbin/service would be cleanest, but since /sbin/service does not honor the LANG variable and that seems to be the intended (see https://bugzilla.redhat.com/show_bug.cgi?id=422141) I see no other way if the intent is to have the service module work with non US machines.

# func lnxmgmt call service get_running
{'lnxmgmt.ok.is': [['certmaster', 'running'],
['/usr/bin/funcd', 'running'],
['Monthly', 'disabled.'],
['sysvik-data', 'running'],
['vmware-guestd', 'running']]}
# service --status-all|tail -3
xinetd (pid 2037) er í gangi...
ypbind hefur verið stöðvað
yum-updatesd (pid 2518) er í gangi...

# LANG=C service --status-all|tail -3
wpa_supplicant (pid 1583) er í gangi...
ypbind hefur verið stöðvað
zvbid hefur verið stöðvað

As you see this does not work at all on non-US locale machines. An alternative way which I can implement this is by running get_enabled() and then running /etc/init.d/<service> status from the output. That way we do not need the hardcoded list.

Another thing, instead of relying on the string output we could catch the return code of the service.

New patch attached which implements this.
Post by seth vidal
Maybe just run the ones and catch the error if it lacks a 'status'?
What's the impact if we raise properly?
-sv
Greg Swift
2011-03-04 17:08:26 UTC
Permalink
Post by Tomas Edwardsson
This patch introduces a new module, cpu.py and includes fixes for disk.py and service.py.
* cpu_sample_percentage - gets the average cpu usage, returns per cpu
percentage for user, nice, system, idle, iowait and irq.
* jiffies - fetches cpu statistics from /proc/stat and returns them
disk.py modifications
* Add filesystem type returned by disk.usage()
service.py modifications
* chkconfig and /sbin/service now run with environment variable LANG=C,
tools were returning internationalized strings which wouldn't parse
correctly.
* Rewrote get_running(), now lists scripts from /etc/init.d and runs status
itself instead of running /sbin/service --status-all. /sbin/service was
returning internationalized strings and ignoring LANG variable.
Tomas,

so i'm testing these for inclusion right now. I'm curious how you feel
about the concept of adjusting the "cpu_sample_percentage" method to a bit
more cli friendly name like "usage" or "sample". I'm more inclined
towards 'usage' myself, but am open to suggestion. I realize "cpu sample
percentage" is more descriptive, but documentation should cover that, such
as the description in register and the ModulesList/CPUModule page (you gonna
make one?).

The primary issue with this, as skvidal pointed out, is if you are already
using this extensively in in house scripts, ecspecially if anyone else is
also using it.

thoughts?

xaeth
Tomas Edwardsson
2011-03-07 00:01:47 UTC
Permalink
----- Original Message -----





On Thu, Feb 3, 2011 at 19:16, Tomas Edwardsson < ***@tommi..org > wrote:

<blockquote>


This patch introduces a new module, cpu.py and includes fixes for disk.py and service.py.

cpu.py exposes cpu information. It has two methods:
* cpu_sample_percentage - gets the average cpu usage, returns per cpu percentage for user, nice, system, idle, iowait and irq.
* jiffies - fetches cpu statistics from /proc/stat and returns them

disk.py modifications
* Add filesystem type returned by disk.usage()

service.py modifications
* chkconfig and /sbin/service now run with environment variable LANG=C, tools were returning internationalized strings which wouldn't parse correctly.
* Rewrote get_running(), now lists scripts from /etc/init.d and runs status itself instead of running /sbin/service --status-all. /sbin/service was returning internationalized strings and ignoring LANG variable.






Tomas,

so i'm testing these for inclusion right now. I'm curious how you feel about the concept of adjusting the "cpu_sample_percentage" method to a bit more cli friendly name like "usage" or "sample". I'm more inclined towards 'usage' myself, but am open to suggestion. I realize "cpu sample percentage" is more descriptive, but documentation should cover that, such as the description in register and the ModulesList/CPUModule page (you gonna make one?).

The primary issue with this, as skvidal pointed out, is if you are already using this extensively in in house scripts, ecspecially if anyone else is also using it.

thoughts?

xaeth
</blockquote>

Main reason for not using something like 'usage' is the thought of having a better way of implementing this in the future. The minion internally storing something like 1/5/15 minute averages and storing them in mem is one idea. So, to boil down, not to have conflict with the "better" implemented function in the future which would then be named 'usage'.

There is no inconvenience for me to update my scripts to use 'usage' instead of 'cpu_sample_percentage'.

Sure, I'll put together the CPUModule page.
--
Tommi
Loading...