Patchworkβ [RFC] : Make some aspects of client behavior configurable

login
register
about
Submitter Lucas Meneghel Rodrigues
Date 2009-11-04 17:11:52
Message ID <1257354712-5724-1-git-send-email-lmr@redhat.com>
Download mbox | patch
Permalink /patch/57648/
State New
Headers show

Comments

Lucas Meneghel Rodrigues - 2009-11-04 17:11:52
Right now, by default autotest does:

 * Drop the caches of the client machine
 * Queue the package manager for all packages installed

On every test execution, which takes a long time for
tests to complete, which is good in many scenarios, but
could be configurable with minimal fuss for other test
needs. This test moves some of those decisions to the
global_config.ini file, adding the [CLIENT] session.

By default, the options are set to what the client
currenty does, but makes it a lot easier and less
hacky to change the behavior if neeeded. The 3 new
configuration keys are:

[CLIENT]
drop_caches: False
drop_caches_between_iterations: False
verify_packages: False

Looking at client/bin/base_sysinfo.py I believe we could also make
the sets of information collected from client machines configurable,
making things convenient for users, but I'd like to hear more
opinions about it.

Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/bin/autotest        |    7 ++++++-
 client/bin/base_sysinfo.py |   34 +++++++++++++++++++---------------
 client/bin/job.py          |   10 +++++++---
 client/bin/package.py      |    1 -
 global_config.ini          |    9 ++++++++-
 5 files changed, 40 insertions(+), 21 deletions(-)
Lucas Meneghel Rodrigues - 2009-11-05 19:44:32
On Thu, 2009-11-05 at 09:01 -0800, John Admanski wrote:
> Well, the drop_caches configs I definitely like. As a general rule I
> think it's reasonable to translate most global defaults into a global
> config setting.

Ok, great, so 'drop_caches' will be kept as is.

> 
> The sysinfo changes I'm a little less certain about. I do like the
> idea of the sysinfo being more configurable in a way that doesn't
> require making code changes, it's just that your proposed change just
> targets one specific piece of sysinfo collection, and then wraps some
> if blocks around it; I wish we could do something a little more
> general.
> 
> 
> I'm just picturing something more along the lines of breaking the
> existing collection up into multiple functions (or classes) and then
> letting you control which classes/functions actually get run with the
> global config. Right now we just pile everything into one big class so
> the only way to customize it at config time is to add a bunch of ifs,
> like in your patch. It's not so bad for just enabling/disabling one
> thing, but if the intention is to add more switches in the future it
> doesn't really scale.

Absolutely, I agree with you. I've spin this quick patch and put it as
an RFC more to get some discussion about the subject. I am going to
separate the drop_caches thing on its own patch (so it can be
incorporated quickly) and will work on the sysinfo stuff, taking a more
general approach.

Thanks for your insight on this!

Lucas

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/client/bin/autotest b/client/bin/autotest
index fe8f2c7..2d47843 100755
--- a/client/bin/autotest
+++ b/client/bin/autotest
@@ -6,6 +6,7 @@  import os, sys, shutil
 import common
 from optparse import OptionParser
 from autotest_lib.client.bin import job
+from autotest_lib.client.common_lib import global_config
 
 
 # Use the name of the binary to find the real installation directory
@@ -58,5 +59,9 @@  options, args = parser.parse_args()
 if len(args) != 1:
     usage()
 
+drop_caches = global_config.global_config.get_config_value('CLIENT',
+                                                           'drop_caches',
+                                                           type=bool)
+
 # JOB: run the specified job control file.
-job.runjob(os.path.realpath(args[0]), options)
+job.runjob(os.path.realpath(args[0]), drop_caches, options)
diff --git a/client/bin/base_sysinfo.py b/client/bin/base_sysinfo.py
index 2cd2f45..dd48bb1 100644
--- a/client/bin/base_sysinfo.py
+++ b/client/bin/base_sysinfo.py
@@ -1,6 +1,6 @@ 
 import os, shutil, re, glob, subprocess, logging
 
-from autotest_lib.client.common_lib import log
+from autotest_lib.client.common_lib import log, global_config
 from autotest_lib.client.bin import utils, package
 
 
@@ -23,7 +23,9 @@  _DEFAULT_FILES_TO_LOG_BEFORE_ITERATION = [
 _DEFAULT_FILES_TO_LOG_AFTER_ITERATION = [
     "/proc/schedstat", "/proc/meminfo", "/proc/slabinfo", "/proc/interrupts"
     ]
-
+_LIST_PACKAGES = global_config.global_config.get_config_value('CLIENT',
+                                                              'verify_packages',
+                                                              type=bool)
 
 class loggable(object):
     """ Abstract class for representing all things "loggable" by sysinfo. """
@@ -225,15 +227,17 @@  class base_sysinfo(object):
             log.run(logdir)
 
         # also log any installed packages
-        installed_path = os.path.join(logdir, "installed_packages")
-        installed_packages = "\n".join(package.list_all()) + "\n"
-        utils.open_write_close(installed_path, installed_packages)
+        if _LIST_PACKAGES:
+            installed_path = os.path.join(logdir, "installed_packages")
+            installed_packages = "\n".join(package.list_all()) + "\n"
+            utils.open_write_close(installed_path, installed_packages)
 
 
     @log.log_and_ignore_errors("pre-test sysinfo error:")
     def log_before_each_test(self, test):
         """ Logging hook called before a test starts. """
-        self._installed_packages = package.list_all()
+        if _LIST_PACKAGES:
+            self._installed_packages = package.list_all()
         if os.path.exists("/var/log/messages"):
             stat = os.stat("/var/log/messages")
             self._messages_size = stat.st_size
@@ -266,17 +270,17 @@  class base_sysinfo(object):
         test.write_test_keyval(keyval)
 
         # log any changes to installed packages
-        old_packages = set(self._installed_packages)
-        new_packages = set(package.list_all())
-        added_path = os.path.join(test_sysinfodir, "added_packages")
-        added_packages = "\n".join(new_packages - old_packages) + "\n"
-        utils.open_write_close(added_path, added_packages)
-        removed_path = os.path.join(test_sysinfodir, "removed_packages")
-        removed_packages = "\n".join(old_packages - new_packages) + "\n"
-        utils.open_write_close(removed_path, removed_packages)
+        if _LIST_PACKAGES:
+            old_packages = set(self._installed_packages)
+            new_packages = set(package.list_all())
+            added_path = os.path.join(test_sysinfodir, "added_packages")
+            added_packages = "\n".join(new_packages - old_packages) + "\n"
+            utils.open_write_close(added_path, added_packages)
+            removed_path = os.path.join(test_sysinfodir, "removed_packages")
+            removed_packages = "\n".join(old_packages - new_packages) + "\n"
+            utils.open_write_close(removed_path, removed_packages)
 
 
-    @log.log_and_ignore_errors("pre-test siteration sysinfo error:")
     def log_before_each_iteration(self, test, iteration=None):
         """ Logging hook called before a test iteration."""
         if not iteration:
diff --git a/client/bin/job.py b/client/bin/job.py
index ebfb3a3..45f8b4d 100755
--- a/client/bin/job.py
+++ b/client/bin/job.py
@@ -15,6 +15,7 @@  from autotest_lib.client.bin import config, sysinfo, test, local_host
 from autotest_lib.client.bin import partition as partition_lib
 from autotest_lib.client.common_lib import error, barrier, log, logging_manager
 from autotest_lib.client.common_lib import base_packages, packages
+from autotest_lib.client.common_lib import global_config
 
 LAST_BOOT_TAG = object()
 NO_DEFAULT = object()
@@ -251,7 +252,10 @@  class base_job(object):
         """
         Perform the drop caches initialization.
         """
-        self.drop_caches_between_iterations = True
+        self.drop_caches_between_iterations = (
+                       global_config.global_config.get_config_value('CLIENT',
+                                            'drop_caches_between_iterations',
+                                            type=bool))
         self.drop_caches = drop_caches
         if self.drop_caches:
             logging.debug("Dropping caches")
@@ -1339,7 +1343,7 @@  class disk_usage_monitor:
         return decorator
 
 
-def runjob(control, options):
+def runjob(control, drop_caches, options):
     """
     Run a job using the given control file.
 
@@ -1367,7 +1371,7 @@  def runjob(control, options):
         if options.cont and not os.path.exists(state):
             raise error.JobComplete("all done")
 
-        myjob = job(control, options)
+        myjob = job(control=control, drop_caches=drop_caches, options=options)
 
         # Load in the users control file, may do any one of:
         #  1) execute in toto
diff --git a/client/bin/package.py b/client/bin/package.py
index 9a836b6..422fb31 100644
--- a/client/bin/package.py
+++ b/client/bin/package.py
@@ -13,7 +13,6 @@  from autotest_lib.client.common_lib import error
 # As more package methods are implemented, this list grows up
 KNOWN_PACKAGE_MANAGERS = ['rpm', 'dpkg']
 
-
 def _rpm_info(rpm_package):
     """\
     Private function that returns a dictionary with information about an
diff --git a/global_config.ini b/global_config.ini
index cc20a96..d06e6d7 100644
--- a/global_config.ini
+++ b/global_config.ini
@@ -28,7 +28,6 @@  parse_failed_repair_default: 0
 # Autotest potential install paths
 client_autodir_paths: /usr/local/autotest,/home/autotest
 
-
 [SERVER]
 hostname: autotest
 # Turn on RPC Logging
@@ -48,6 +47,14 @@  smtp_port:
 smtp_user:
 smtp_password:
 
+[CLIENT]
+# Drop test client caches between every test execution
+drop_caches: False
+# Drop test client caches between every test iteration execution
+drop_caches_between_iterations: False
+# Check all installed packages installed on the system between test executions
+verify_packages: False
+
 [SCHEDULER]
 die_on_orphans: False
 enable_scheduler: True