Robottelo: Run setUpClass uploading manifest once

[lpramuk]

As we discovered recently setUpClass is run multiple times upto number of workers (our default = 8).
If such setUpClass uploads a manifest then the results is upto 8 uploaded manifests.

Lets stop polluting SAT with manifests as it then behaves badly by ensuring that these classes are being run in one worker only by decorating such classes with @run_in_one_thread

[lpramuk]

Note: we don't care about setUp as it is run multiple times anyway.

[rochacbruno]

We discussed some ideas, my suggestion is on http://pastebin.test.redhat.com/464365 and needs experimentation.

@abalakh @rplevka @renzon @ldjebran if you guys have other notes and ideas lets keep in this issue to not forget!

[abalakh]

@rochacbruno few comments to your suggestion:

• setUpClass shouldn't be locked, import should be locked instead (each worker has it's own setUpClass so no worker will proceed to tests while import is not yet finished)
• caller is interesting for us only in case it's setUpClass. If caller is some test - we should not lock next manifest creation for him, just put him on queue. Basically, if test tries to upload manifest - it means test wants extra layer of isolation and don't want to share his org & manifest with any other test (as otherwise it would use setUpClass' org). Also there may be some negative tests which are trying to upload the manifest twice, we should not block them.
• orgs support. It's not only about manifest upload, as manifest is uploaded to specific organization. Different workers will create different orgs and will try to upload manifest in them. If we simply upload manifest to one of organizations - well, other workers will fail their tests :) I'm suggesting to extend import func to create organization if None was passed and to return org_id so setUpClass can be updated to use the id returned by import func. Also we should store not just boolean True in locker DB, but org_id so we can return it to other workers.

I was playing with it a bit, here's very early draft version of my suggestion (using pickle just to demo it, we may think of something better instead):

import inspect
import pickle

def import_manifest(org_id=None, interface='api'):
    caller = inspect.stack()[1]
    caller_name = caller[3]
    if caller_name == 'setUpClass':
        caller_name = caller[0].f_locals['cls'].__name__ + '.' + caller_name
        # TODO: init 'import.lock' first and basically reuse code from
        # func_locker.py (or update it accordingly)
        import_locks = pickle.load(open('import.lock', 'rb'))
        if import_locks.get(caller_name):
            return import_locks[caller_name]

    org_id = __import(org_id, interface)

    # TODO: not atomic
    if caller_name == 'setUpClass':
        import_locks = pickle.load(open('import.lock', 'rb'))
        import_locks[caller_name] = org_id
        pickle.dump(import_locks, open('import.lock', 'wb'))
    return org_id


@lock_function
def __import(org_id=None, interface='api'):
    with manifests.clone() as manifest:
        if interface == 'api':
            org_id = org_id or entities.Organization().create().id
            upload_manifest(org_id, manifest.content)
        elif interface == 'cli':
            upload_file(manifest.content, manifest.filename)
            org_id = org_id or make_org()['id']
            Subscription.upload({
                'file': manifest.filename,
                'organization-id': org_id,
            })
    return org_id

@rochacbruno what do you think?

[ldjebran]

My suggestion is to keep the things very simple and for general usage:

Create a decorator function named shared:

import functools
from unittest import TestCase

STATUS_RUNNING = 'running'
STATUS_READY = 'ready'
STATUS_FAILED = 'fail'


def shared(function=None, scope=None, context=None, inject_kwargs=False):

    def main_wrapper(func):

        @functools.wraps(func)
        def function_wrapper(*args, **kw):
            # cached
            cached_data = get_cached(function, scope=scope, context=context)
            if cached_data is None:

            if cached_data and cached_data['status'] == STATUS_RUNNING:
                # enter a while loop waiting to cashed_data['status'] == STATUS_READY
                # or failed
                pass

            elif cached_data and cached_data['status'] == STATUS_READY:
                data = cached_data['data']
                if inject_kwargs:
                    kw.update(data)
                    return func(*args, **kw)
                else:
                    return data

            elif cached_data is None:
                # no data exist have to create
                set_cached(function, scope=scope, context=context,
                           data={'status': STATUS_RUNNING, 'data': None})
                result = func(*args, **kw)
                set_cached(function, scope=scope, context=context,
                           data={'status': STATUS_READY, 'data': result})
                return result

        return function_wrapper

    def wait_function(func):
        return main_wrapper(func)

    if function:
        return main_wrapper(function)
    else:
        return wait_function

    return main_wrapper


class SomeTestCase(TestCase):

    @classmethod
    @shared(inject_kwargs=True)
    def setUpClass(cls, org_id=None, repo_id=None):

        if org_id is None:
            # create the org
            org = make_org()
            # upload manifest
            pass
        else:
            org = Org.info({'id': org_id})

        cls.org = org

        if repo_id is None:
            # create the org
            cls.repo = make_repository()
            pass
        else:
            cls.repo = Repository.info({'id': repo_id})

        return {'org_id': org['id'], 'repo_id': repo['id']}


class SomeTestCase2(TestCase):

    @shared
    def _shared_function(cls):
        # a shared function can be used at module level
        org = make_org()
        # upload manifest
        repo = make_repository()
        return {'org_id': org['id'], 'repo_id': repo['id']}

    @classmethod
    @shared
    def setUpClass(cls):

        data = cls._shared_function()

        cls.org = Org.info({'id': data['org_id']})
        cls.repo = Repository.info({'id': data['repo_id']})

        return

if we need only one process to access the manifest clone, we can lock that function by lock_function decorator

[ldjebran]

Note the shared decorator args must be json compatible and only dicts , to be able to pass them as kwargs

the shared function run only once with the first process that will trigger

• advantage: all the process will use the same setup data even tier1, tier2 ...
• disadvantage:
  
  
  
  1. (this apply to any suggestion with shared data between processes), we have to find a way to clear the cache data before the big start (as this is like the lock_function that delete the file when the lock finish, the shared does not that as may be needed by not launched yet tests ), perhaps make a cmd make clean-tmp-cache after a build
  
  
  2. if any random error on the first shared function call will a failure for all the processes, but we can make an option to recall the function if one of them failed, we can make a failure counter so if failed twice, we will consider that as a permanent failure
  
  

[ldjebran]

@abalakh @lpramuk @renzon @rochacbruno what do you think?

[rochacbruno]

we have to find a way to clear the cache data before the big start

@ldjebran we can create cache namespaces and use the Jenkins build number or generate a random identifier when running out of jenkins.

I am really interested in getting Redis available on the machine running jenkins.

[rochacbruno]

if any random error on the first shared function call will a failure for all the processes, but we can make an option to recall the function if one of them failed, we can make a failure counter so if failed twice, we will consider that as a permanent failure

I like that idea, we can pass a retries to the shared decorator. If fails many times so it should fail the build anyway.

[rochacbruno]

@lpramuk @ldjebran @renzon @abalakh @rplevka lets decide the approach of this on next auto meeting.

[lpramuk]

@ldjebran what about [6.3] implement shared function (PR #4483 equivalent for master) so that we can close this issue

[rplevka]

this is done.