Re: [PATCH] CPU module, fixes to disk module and service module

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



----- Original Message -----
> On Fri, 2011-02-04 at 01:16 +0000, Tomas Edwardsson wrote:
> > 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
> 
> this looks fine.
> 
> > disk.py modifications
> > * Add filesystem type returned by disk.usage()
> 
> also fine
> 
> >
> > 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.
> 
> > * 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.

> 
> Maybe just run the ones and catch the error if it lacks a 'status'?
> 
> What's the impact if we raise properly?
> 
> -sv
diff --git a/func/minion/modules/service.py b/func/minion/modules/service.py
index 38de51c..05b92af 100644
--- a/func/minion/modules/service.py
+++ b/func/minion/modules/service.py
@@ -28,9 +28,9 @@ class Service(func_module.FuncModule):
 
         service_name = service_name.strip() # remove useless spaces
 
-        filename = os.path.join("/etc/rc.d/init.d/",service_name)
+        filename = os.path.join("/etc/init.d/",service_name)
         if os.path.exists(filename):
-            return sub_process.call(["/sbin/service", service_name, command], close_fds=True)
+            return sub_process.call(["/sbin/service", service_name, command], close_fds=True, env={ 'LANG':'C' })
         else:
             raise codes.FuncException("Service not installed: %s" % service_name)
 
@@ -87,7 +87,7 @@ class Service(func_module.FuncModule):
         only provide whether or not they are running, not specific runlevel info.
         """
 
-        chkconfig = sub_process.Popen(["/sbin/chkconfig", "--list"], stdout=sub_process.PIPE, close_fds=True)
+        chkconfig = sub_process.Popen(["/sbin/chkconfig", "--list"], stdout=sub_process.PIPE, close_fds=True, env={ "LANG": "C" })
         data = chkconfig.communicate()[0]
         results = []
         for line in data.split("\n"):
@@ -106,13 +106,29 @@ class Service(func_module.FuncModule):
         """
         Get a list of which services are running, stopped, or disabled.
         """
-        chkconfig = sub_process.Popen(["/sbin/service", "--status-all"], stdout=sub_process.PIPE, close_fds=True)
-        data = chkconfig.communicate()[0]
         results = []
-        for line in data.split("\n"):
-            if line.find(" is ") != -1:
-                tokens = line.split()
-                results.append((tokens[0], tokens[-1].replace("...","")))
+
+        # Get services
+        services = self.get_enabled()
+
+        init_return_codes = { 0: 'running', 1: 'dead', 2:'locked', 3:'stopped' }
+
+        for service in services:
+            filename = os.path.join("/etc/init.d/",service[0])
+            # Run status for service
+            try:
+                init_exec = sub_process.Popen([filename, "status"], stdout=sub_process.PIPE, close_fds=True, env={ "LANG": "C" })
+            except Exception, e:
+                raise codes.FuncException("Service status error %s on initscript %s" % (e, filename))
+
+            # Get status output
+            data = init_exec.communicate()[0]
+
+            # Wait for command to complete
+            init_exec.wait()
+
+            # Append the result, service name, return status and status output
+            results.append((service[0], init_return_codes[init_exec.returncode], data))
         return results
 
     def register_method_args(self):
@@ -143,3 +159,5 @@ class Service(func_module.FuncModule):
 
 
                 }
+
+# vi: ts=4 expandtab
_______________________________________________
Func-list mailing list
Func-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/func-list

[Index of Archives]     [Fedora Users]     [Linux Networking]     [Fedora Legacy List]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux