New issue
Advanced search Search tips

Issue 771827 link

Starred by 0 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

control file getter is very dumb

Project Member Reported by ayatane@chromium.org, Oct 5 2017

Issue description

From a cursory analysis, the control file getter iterates through every directory under /usr/local/autotest.  That includes all of the results and containers.

It causes errors and is slow, even when it's not breaking: https://bugs.chromium.org/p/chromium/issues/detail?id=771823
 
Components: -Infra>Client>ChromeOS Infra>Client>ChromeOS>Test
Owner: xixuan@chromium.org
Status: Assigned (was: Untriaged)
Might be relevant to dynamic_suite_v2
> From a cursory analysis, the control file getter iterates through every directory under /usr/local/autotest.

What's really happening is that FileSystemGetter (by design) iterates
through whatever directory hierarchies it's told to search.  Some
callers are respectful, and only search hierarchies that actually
contain control files.  Other callers are ... not respectful.

There are two plausible fixes:
 1) Change the interface to FileSystemGetter so that the input paths
    aren't specified by the caller, or otherwise make it impossible to
    request a search of everything.
 2) Change the problem callers not to search /usr/local/autotest.

Option 2) is probably easier, for now.

These are the problem call sites:

==== frontend/afe/rpc_interface.py
def _get_control_file_by_suite(suite_name):
    # ...
    getter = control_file_getter.FileSystemGetter(
            [_CONFIG.get_config_value('SCHEDULER',
                                      'drone_installation_directory')])
    return getter.get_control_file_contents_by_name(suite_name)

==== server/hosts/servo_host.py
    def schedule_synchronized_reboot(self, dut_list, afe, force_reboot=False):
        # ...
        getter = control_file_getter.FileSystemGetter([AUTOTEST_BASE])
        # ...

==== server/sequence.py
    def child_control_file(self): 
        # ...
            cntl_file_getter = control_file_getter.FileSystemGetter(
                    [os.path.join(os.path.dirname(os.path.realpath(__file__)),
                                  '..')])
        # ...

> There are two plausible fixes:

Either of the fixes presupposes that the list of directories that
can hold test control files is known without searching the file system.
However, that doesn't account for how we install "site_utils/autotest_private",
which is installed in that location via 'git clone'.  That hierarchy
_also_ contains tests, and since no source code names it, the only way to
find the tests there is by searching from /usr/local/autotest.

Oh, what a tangled web...

Comment 4 by ihf@chromium.org, Apr 27 2018

Cc: ihf@chromium.org
Do we have to follow symlink at all? 
in def _get_control_file_list(self, suite_name=''):,

instead of  
elif os.path.isdir(fullpath):
 directories.append(fullpath)

what about
elif os.path.isdir(fullpath) and not os.path.islink(fullpath)  :
 directories.append(fullpath)

Sign in to add a comment