Re: [Autotest] [PATCH 2/3] Make a standalone client to be able to use global_config.ini

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

 



FYI, this broke server/autotest_unittest:

showard@gluon:~/opensrc/autotest$ server/autotest_unittest.py
ERROR:root:Could not install autotest from repos
......E
======================================================================
ERROR: test_run (__main__.TestBaseAutotest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "server/autotest_unittest.py", line 246, in test_run
    self.base_autotest.run(control, timeout=30)
  File "/home/showard/opensrc/autotest/server/autotest.py", line 321, in run
    client_disconnect_timeout, job_tag)
  File "/home/showard/opensrc/autotest/server/autotest.py", line 396, in _do_run
    client_config_file = self._create_client_config_file(host.job)
  File "/home/showard/opensrc/autotest/server/autotest.py", line 437,
in _create_client_config_file
    return self._create_aux_file(job, client_config.write)
  File "/home/showard/opensrc/autotest/server/autotest.py", line 452,
in _create_aux_file
    fd, path = tempfile.mkstemp(dir=job.tmpdir)
  File "/home/showard/opensrc/autotest/client/common_lib/test_utils/mock.py",
line 207, in __call__
    return self.playback(self.symbol, *args, **dargs)
  File "/home/showard/opensrc/autotest/client/common_lib/test_utils/mock.py",
line 468, in __method_playback
    self._append_error(msg)
  File "/home/showard/opensrc/autotest/client/common_lib/test_utils/mock.py",
line 499, in _append_error
    raise CheckPlaybackError(error)
CheckPlaybackError: Unexpected call: mkstemp(dir='/job/tmp')
Expected: sysinfo.serialize()

----------------------------------------------------------------------
Ran 7 tests in 0.022s

FAILED (errors=1)

Please try to remember to run unit tests before mailing patches (and
preferably, in your case, before committing, at least the basic unit
tests -- it might be helpful to make a commit script for yourself that
automatically runs them :)).

I'll have a patch along momentarily.

Thanks,
Steve

PS Granted, the test that was broken is a pretty poorly-written and
extremely brittle test.  Still, we need to keep the unit tests passing
or they're entirely useless to us.  But I'd love to see a rewrite to
make that test (or others like it) less brittle.


On Wed, Nov 18, 2009 at 11:06 AM, Gregory P. Smith <gps@xxxxxxxxxx> wrote:
> looks good.
>
> On Wed, Nov 18, 2009 at 8:45 AM, Lucas Meneghel Rodrigues <lmr@xxxxxxxxxx>
> wrote:
>>
>> Greg, would you mind giving a last review on the patchset before I
>> check this in?
>>
>> On Wed, Nov 18, 2009 at 2:09 PM, Lucas Meneghel Rodrigues
>> <lmr@xxxxxxxxxx> wrote:
>> > In order to make it possible the autotest client to use the
>> > global_config.ini configuration files:
>> >
>> >  * Modified global_config.py to support verifying if the
>> >   configuration file is under autotest's root directory,
>> >   or the client directory
>> >  * Extended the autotest run method to copy over
>> >   the configuration files to the client.
>> >
>> > Notes for revisors (changes from previous patchset and other stuff):
>> >
>> >  * Latest comments from John adressed
>> >
>> > Signed-off-by: Lucas Meneghel Rodrigues <lmr@xxxxxxxxxx>
>> > ---
>> >  client/common_lib/global_config.py    |   61
>> > +++++++++++++++++++++++++++++----
>> >  client/common_lib/host_protections.py |   26 ++++++++------
>> >  global_config.ini                     |    3 ++
>> >  server/autotest.py                    |   52
>> > +++++++++++++++++++++++++---
>> >  4 files changed, 119 insertions(+), 23 deletions(-)
>> >
>> > diff --git a/client/common_lib/global_config.py
>> > b/client/common_lib/global_config.py
>> > index 2bbeca0..04ab7ff 100644
>> > --- a/client/common_lib/global_config.py
>> > +++ b/client/common_lib/global_config.py
>> > @@ -8,12 +8,6 @@ __author__ = 'raphtee@xxxxxxxxxx (Travis Miller)'
>> >  import os, sys, ConfigParser
>> >  from autotest_lib.client.common_lib import error
>> >
>> > -dirname = os.path.dirname(sys.modules[__name__].__file__)
>> > -DEFAULT_CONFIG_FILE = os.path.abspath(os.path.join(dirname,
>> > -
>> >  "../../global_config.ini"))
>> > -DEFAULT_SHADOW_FILE = os.path.abspath(os.path.join(dirname,
>> > -
>> >  "../../shadow_config.ini"))
>> > -
>> >
>> >  class ConfigError(error.AutotestError):
>> >     pass
>> > @@ -23,12 +17,50 @@ class ConfigValueError(ConfigError):
>> >     pass
>> >
>> >
>> > +
>> > +common_lib_dir = os.path.dirname(sys.modules[__name__].__file__)
>> > +client_dir = os.path.dirname(common_lib_dir)
>> > +root_dir = os.path.dirname(client_dir)
>> > +
>> > +# Check if the config files are at autotest's root dir
>> > +# This will happen if client is executing inside a full autotest tree,
>> > or if
>> > +# other entry points are being executed
>> > +global_config_path_root = os.path.join(root_dir, 'global_config.ini')
>> > +shadow_config_path_root = os.path.join(root_dir, 'shadow_config.ini')
>> > +config_in_root = (os.path.exists(global_config_path_root) and
>> > +                  os.path.exists(shadow_config_path_root))
>> > +
>> > +# Check if the config files are at autotest's client dir
>> > +# This will happen if a client stand alone execution is happening
>> > +global_config_path_client = os.path.join(client_dir,
>> > 'global_config.ini')
>> > +config_in_client = os.path.exists(global_config_path_client)
>> > +
>> > +if config_in_root:
>> > +    DEFAULT_CONFIG_FILE = global_config_path_root
>> > +    DEFAULT_SHADOW_FILE = shadow_config_path_root
>> > +    RUNNING_STAND_ALONE_CLIENT = False
>> > +elif config_in_client:
>> > +    DEFAULT_CONFIG_FILE = global_config_path_client
>> > +    DEFAULT_SHADOW_FILE = None
>> > +    RUNNING_STAND_ALONE_CLIENT = True
>> > +else:
>> > +    raise ConfigError("Could not find configuration files "
>> > +                      "needed for this program to function. Please
>> > refer to "
>> > +                      "http://autotest.kernel.org/wiki/GlobalConfig "
>> > +                      "for more info.")
>> > +
>> > +
>> >  class global_config(object):
>> >     _NO_DEFAULT_SPECIFIED = object()
>> >
>> >     config = None
>> >     config_file = DEFAULT_CONFIG_FILE
>> >     shadow_file = DEFAULT_SHADOW_FILE
>> > +    running_stand_alone_client = RUNNING_STAND_ALONE_CLIENT
>> > +
>> > +
>> > +    def check_stand_alone_client_run(self):
>> > +        return self.running_stand_alone_client
>> >
>> >
>> >     def set_config_files(self, config_file=DEFAULT_CONFIG_FILE,
>> > @@ -47,6 +79,21 @@ class global_config(object):
>> >             return default
>> >
>> >
>> > +    def get_section_values(self, section):
>> > +        """
>> > +        Return a config parser object containing a single section of
>> > the
>> > +        global configuration, that can be later written to a file
>> > object.
>> > +
>> > +        @param section: Section we want to turn into a config parser
>> > object.
>> > +        @return: ConfigParser() object containing all the contents of
>> > section.
>> > +        """
>> > +        cfgparser = ConfigParser.ConfigParser()
>> > +        cfgparser.add_section(section)
>> > +        for option, value in self.config.items(section):
>> > +            cfgparser.set(section, option, value)
>> > +        return cfgparser
>> > +
>> > +
>> >     def get_config_value(self, section, key, type=str,
>> >                          default=_NO_DEFAULT_SPECIFIED,
>> > allow_blank=False):
>> >         self._ensure_config_parsed()
>> > @@ -106,7 +153,7 @@ class global_config(object):
>> >         # now also read the shadow file if there is one
>> >         # this will overwrite anything that is found in the
>> >         # other config
>> > -        if os.path.exists(self.shadow_file):
>> > +        if self.shadow_file and os.path.exists(self.shadow_file):
>> >             shadow_config = ConfigParser.ConfigParser()
>> >             shadow_config.read(self.shadow_file)
>> >             # now we merge shadow into global
>> > diff --git a/client/common_lib/host_protections.py
>> > b/client/common_lib/host_protections.py
>> > index 9457114..7c9e6a0 100644
>> > --- a/client/common_lib/host_protections.py
>> > +++ b/client/common_lib/host_protections.py
>> > @@ -22,19 +22,23 @@ Protection = enum.Enum('No protection',          #
>> > Repair can do anything to
>> >                                                  # this host
>> >                        )
>> >
>> > +running_client =
>> > global_config.global_config.check_stand_alone_client_run()
>> > +
>> >  try:
>> >     _bad_value = object()
>> > -    default = Protection.get_value(
>> > -        global_config.global_config.get_config_value(
>> > -            'HOSTS', 'default_protection', default=_bad_value))
>> > -    if default == _bad_value:
>> > -        raise global_config.ConfigError(
>> > -            'No HOSTS.default_protection defined in global_config.ini')
>> > +    default_protection = global_config.global_config.get_config_value(
>> > +                            'HOSTS', 'default_protection',
>> > default=_bad_value)
>> > +    if default_protection == _bad_value:
>> > +        if running_client:
>> > +            logging.debug('Client stand alone run detected. '
>> > +                          'host_protection.default will not be set.')
>> > +        else:
>> > +            raise global_config.ConfigError(
>> > +                'No HOSTS.default_protection defined in
>> > global_config.ini')
>> > +    else:
>> > +        default = Protection.get_value(default_protection)
>> > +
>> >  except global_config.ConfigError:
>> > -    # can happen if no global_config.ini exists at all, but this can
>> > actually
>> > -    # happen in reasonable cases (e.g. on a standalone clinet) where
>> > it's
>> > -    # safe to ignore
>> > -    logging.debug('No global_config.ini exists so
>> > host_protection.default '
>> > -                  'will not be defined')
>> > +    raise global_config.ConfigError('No global_config.ini exists,
>> > aborting')
>> >
>> >  choices = Protection.choices()
>> > diff --git a/global_config.ini b/global_config.ini
>> > index 1cd3b4f..ac6baab 100644
>> > --- a/global_config.ini
>> > +++ b/global_config.ini
>> > @@ -28,6 +28,9 @@ parse_failed_repair_default: 0
>> >  # Autotest potential install paths
>> >  client_autodir_paths: /usr/local/autotest,/home/autotest
>> >
>> > +[CLIENT]
>> > +drop_caches: True
>> > +drop_caches_between_iterations: True
>> >
>> >  [SERVER]
>> >  hostname: autotest
>> > diff --git a/server/autotest.py b/server/autotest.py
>> > index 3d307a9..9270437 100644
>> > --- a/server/autotest.py
>> > +++ b/server/autotest.py
>> > @@ -391,6 +391,12 @@ class
>> > BaseAutotest(installable_object.InstallableObject):
>> >         cfile += open(tmppath).read()
>> >         open(tmppath, "w").write(cfile)
>> >
>> > +        # Create and copy configuration file based on the state of the
>> > +        # client configuration
>> > +        client_config_file = self._create_client_config_file(host.job)
>> > +        host.send_file(client_config_file, atrun.config_file)
>> > +        os.remove(client_config_file)
>> > +
>> >         # Create and copy state file to remote_control_file + '.state'
>> >         sysinfo_state = {"__sysinfo": host.job.sysinfo.serialize()}
>> >         state_file = self._create_state_file(host.job, sysinfo_state)
>> > @@ -408,12 +414,47 @@ class
>> > BaseAutotest(installable_object.InstallableObject):
>> >
>> >
>> >     def _create_state_file(self, job, state_dict):
>> > -        """ Create a state file from a dictionary. Returns the path of
>> > the
>> > -        state file. """
>> > +        """
>> > +        Create a temporary file with the state described by state_dict.
>> > +
>> > +        @param job: Autotest job.
>> > +        @param state_dict: Dictionary containing the state we want to
>> > write to
>> > +                the temporary file
>> > +        @return: Path of the temporary file generated.
>> > +        """
>> > +        return self._create_aux_file(job, pickle.dump, state_dict)
>> > +
>> > +
>> > +    def _create_client_config_file(self, job):
>> > +        """
>> > +        Create a temporary file with the [CLIENT] section configuration
>> > values
>> > +        taken from the server global_config.ini.
>> > +
>> > +        @param job: Autotest job.
>> > +        @return: Path of the temporary file generated.
>> > +        """
>> > +        client_config =
>> > global_config.global_config.get_section_values("CLIENT")
>> > +        return self._create_aux_file(job, client_config.write)
>> > +
>> > +
>> > +    def _create_aux_file(self, job, func, *args):
>> > +        """
>> > +        Creates a temporary file and writes content to it according to
>> > a content
>> > +        creation function. The file object is appended to *args, which
>> > is then
>> > +        passed to the content creation function
>> > +
>> > +        @param job: Autotest job instance.
>> > +        @param func: Function that will be used to write content to the
>> > +                temporary file.
>> > +        @param *args: List of parameters that func takes.
>> > +        @return: Path to the temporary file that was created.
>> > +        """
>> >         fd, path = tempfile.mkstemp(dir=job.tmpdir)
>> > -        state_file = os.fdopen(fd, "w")
>> > -        pickle.dump(state_dict, state_file)
>> > -        state_file.close()
>> > +        aux_file = os.fdopen(fd, "w")
>> > +        list_args = list(args)
>> > +        list_args.append(aux_file)
>> > +        func(*list_args)
>> > +        aux_file.close()
>> >         return path
>> >
>> >
>> > @@ -459,6 +500,7 @@ class _Run(object):
>> >             control += '.' + tag
>> >         self.manual_control_file = control
>> >         self.remote_control_file = control + '.autoserv'
>> > +        self.config_file = os.path.join(self.autodir,
>> > 'global_config.ini')
>> >
>> >
>> >     def verify_machine(self):
>> > --
>> > 1.6.2.5
>> >
>> > _______________________________________________
>> > Autotest mailing list
>> > Autotest@xxxxxxxxxxxxxxx
>> > http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>> >
>>
>>
>>
>> --
>> Lucas
>
>
> _______________________________________________
> Autotest mailing list
> Autotest@xxxxxxxxxxxxxxx
> http://test.kernel.org/cgi-bin/mailman/listinfo/autotest
>
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux