[Fuego] [PATCH 01/16] Add fuego-release Functional test

Guilherme Camargo guicc at profusion.mobi
Tue Apr 3 03:18:17 UTC 2018


On Fri, Mar 30, 2018 at 10:37:21PM +0000, Tim.Bird at sony.com wrote:
> 
> 
> > -----Original Message-----
> > From: Tim Bird
> > My strategy is to apply all your patches, but ask for modifications,
> > that you can provide as patches on top of this set.
> > 
> > If I make these changes, they will break other patches in the
> > series, which will be obnoxious and time-consuming to fix.
> > 
> > See below inline for comments.
> >  -- Tim
> > 
> > > -----Original Message-----
> > > From: Guilherme Campos Camargo
> > > Allows Fuego to test new Fuego releases.
> > >
> > > The fuego and fuego-core repositores and branches to be tested may be
> > > configured on the specs.json file. Their defaults are:
> > >
> > > "FUEGOREPO":"https://bitbucket.org/tbird20d/fuego",
> > > "FUEGOBRANCH":"master",
> > > "FUEGOCOREREPO":"https://bitbucket.org/tbird20d/fuego-core",
> > > "FUEGOCOREBRANCH":"master"
> > >
> > Can you please add underscores to these names:
> > FUEGO_REPO, FUEGO_CORE_BRANCH, etc.
> > ?
> > 
> > It's very nice to be able to customize these in the spec file.
> > 
> > > Signed-off-by: Guilherme Campos Camargo <guicc at profusion.mobi>
> > > ---
> > >  engine/tests/Functional.fuegotest/fuego_test.sh |  29 ++
> > >  engine/tests/Functional.fuegotest/spec.json     |  11 +
> > >  engine/tests/Functional.fuegotest/test_run.py   | 427
> > > ++++++++++++++++++++++++
> > >  3 files changed, 467 insertions(+)
> > >  create mode 100755 engine/tests/Functional.fuegotest/fuego_test.sh
> > >  create mode 100644 engine/tests/Functional.fuegotest/spec.json
> > >  create mode 100755 engine/tests/Functional.fuegotest/test_run.py
> > >
> > > diff --git a/engine/tests/Functional.fuegotest/fuego_test.sh
> > > b/engine/tests/Functional.fuegotest/fuego_test.sh
> > > new file mode 100755
> > > index 0000000..192c15b
> > > --- /dev/null
> > > +++ b/engine/tests/Functional.fuegotest/fuego_test.sh
> > > @@ -0,0 +1,29 @@
> > > +#!/bin/bash
> > > +
> > > +set -e
> > > +
> > > +readonly fuego_release_dir=fuego-release
> > > +
> > > +function test_build {
> > > +    if [ -d ${fuego_release_dir} ]; then
> > > +        rm -r ${fuego_release_dir}
> > > +    fi
> > > +    git clone --quiet --depth 1 --single-branch \
> > > +        --branch "${FUNCTIONAL_FUEGOTEST_FUEGOBRANCH}" \
> > > +        "${FUNCTIONAL_FUEGOTEST_FUEGOREPO}" \
> 
> I realize now that my suggestion to change the test name and
> add underscores is going to make some really long variable names.
> 
> e.g. FUNCTIONAL_FUEGO_RELEASE_TEST_FUEGO_CORE_REPO.
> 
> Yikes.  Oh well, let me know what you think.  I still think the
> name has more clarity, but maybe it could be shortened inside
> the test?
> 

I agree that the names will become more readable specially on the specs
json by using the underlines and I like the change on the test name. I
don't think that having these huge variables is a big problem, because,
as you said, we can just create an alias for them in the fuego_test.sh

That's what I will do, then.

Also, do we really need the to have the test name as a prefix for specs
variables? If not, we might consider changing that in the future, by
exporting the variables with the same names that they have on the
spec.json.

> > > +        "${fuego_release_dir}/fuego"
> > > +    git clone --quiet --depth 1 --single-branch \
> > > +        --branch "${FUNCTIONAL_FUEGOTEST_FUEGOCOREBRANCH}" \
> > > +        "${FUNCTIONAL_FUEGOTEST_FUEGOCOREREPO}" \
> > > +        "${fuego_release_dir}/fuego-core"
> > > +    cd -
> > > +}
> > > +
> > > +function test_run {
> > > +    sudo -n ${TEST_HOME}/test_run.py "${fuego_release_dir}/fuego"
> > > +    report "echo ok 1 minimal test on target"
> > > +}
> > > +
> > > +function test_processing {
> > > +    log_compare "$TESTDIR" "1" "^ok" "p"
> > > +}
> > > diff --git a/engine/tests/Functional.fuegotest/spec.json
> > > b/engine/tests/Functional.fuegotest/spec.json
> > > new file mode 100644
> > > index 0000000..cc345ba
> > > --- /dev/null
> > > +++ b/engine/tests/Functional.fuegotest/spec.json
> > > @@ -0,0 +1,11 @@
> > > +{
> > > +    "testName": "Functional.fuegotest",
> > > +    "specs": {
> > > +        "default": {
> > > +            "FUEGOREPO":"https://bitbucket.org/tbird20d/fuego",
> > > +            "FUEGOBRANCH":"master",
> > > +            "FUEGOCOREREPO":"https://bitbucket.org/tbird20d/fuego-core",
> > > +            "FUEGOCOREBRANCH":"master"
> > > +        }
> > Can you add specs for my 'test' and 'next' branches as well?
> > 
> > > +    }
> > > +}
> > > diff --git a/engine/tests/Functional.fuegotest/test_run.py
> > > b/engine/tests/Functional.fuegotest/test_run.py
> > > new file mode 100755
> > > index 0000000..96fcfb7
> > > --- /dev/null
> > > +++ b/engine/tests/Functional.fuegotest/test_run.py
> > > @@ -0,0 +1,427 @@
> > > +#!/usr/bin/env python3
> > > +import argparse
> > > +import http
> > > +import http.client
> > > +import logging
> > > +import os
> > > +import re
> > > +import subprocess
> > > +import sys
> > > +import time
> > > +
> > > +import docker
> > > +import pexpect
> > > +import requests
> > > +from selenium import webdriver
> > > +
> > > +LOGGER = logging.getLogger('test_run')
> > > +STREAM_HANDLER = logging.StreamHandler()
> > > +STREAM_HANDLER.setFormatter(
> > > +    logging.Formatter('%(name)s:%(levelname)s: %(message)s'))
> > > +LOGGER.setLevel(logging.DEBUG)
> > > +LOGGER.addHandler(STREAM_HANDLER)
> > > +
> > > +
> > > +def loop_until_timeout(func, timeout=10, num_tries=5):
> > > +    LOGGER.debug('Running %s()', func.__name__)
> > > +
> > > +    for _ in range(num_tries):
> > > +        LOGGER.debug('  Try number %s...', _ + 1)
> > > +        if func():
> > > +            LOGGER.debug('  Success')
> > > +            return True
> > > +        time.sleep(timeout/num_tries)
> > > +    LOGGER.debug('  Failure')
> > > +
> > > +    return False
> > > +
> > > +
> > > +class SeleniumCommand:
> > > +    def exec(self, selenium_ctx):
> > > +        self.driver = selenium_ctx.driver
> > > +        self.driver.refresh()
> > > +        self.driver.implicitly_wait(3)
> > > +        LOGGER.debug('Executing Selenium Command \'%s\'',
> > > +                     self.__class__.__name__)
> > > +
> > > +
> > > +class Visit(SeleniumCommand):
> > > +    def __init__(self, url, timeout=10, expected_result=200):
> > > +        self.url = url
> > > +        self.timeout = timeout
> > > +        self.expected_result = expected_result
> > > +
> > > +    def exec(self, selenium_ctx):
> > > +        super().exec(selenium_ctx)
> > > +
> > > +        LOGGER.debug('  Visiting \'%s\'', self.url)
> > > +        self.driver.get(self.url)
> > > +
> > > +        r = requests.get(self.url)
> > > +        if r.status_code != self.expected_result:
> > > +            LOGGER.debug('  HTTP Status Code \'%s\' is different '
> > > +                         'from the expected \'%s\'', r.status_cod, self.url)
> > > +            return False
> > > +
> > > +        LOGGER.debug('  HTTP Status Code is same as expected \'%s\'',
> > > +                     r.status_code)
> > > +        return True
> > > +
> > > +
> > > +class CheckText(SeleniumCommand):
> > > +    def __init__(self, _id, text, expected_result=True):
> > > +        self._id = _id
> > > +        self.text = text
> > > +        self.expected_result = expected_result
> > > +
> > > +    def exec(self, selenium_ctx):
> > > +        super().exec(selenium_ctx)
> > > +
> > > +        try:
> > > +            text = self.driver.find_element_by_id(self._id).text
> > > +        except Exception:  # TODO: Use proper Exception
> > > +            return False
> > > +
> > > +        LOGGER.debug(
> > > +            '  Searching for \'%s\' in \'id:%s\'', self.text, self._id)
> > > +
> > > +        result = True
> > > +        if self.text not in text:
> > > +            LOGGER.error(
> > > +                '  \'%s\' not found in id \'%s\' with text \'%s\'', self.text,
> > > +                self._id, text)
> > > +            result = False
> > > +
> > > +        LOGGER.debug('  \'%s\' was found', self.text)
> > > +
> > > +        return result == self.expected_result
> > > +
> > > +
> > > +class ShExpect():
> > > +    BASH_PATTERN = 'test_run_pr1:#'
> > > +    COMMAND_RESULT_PATTERN = re.compile('^([0-9]+)', re.M)
> > > +    OUTPUT_VARIABLE = 'cmd_output'
> > > +    COMMAND_OUTPUT_DELIM = ':test_run_cmd_out:'
> > > +    COMMAND_OUTPUT_PATTERN = re.compile(
> > > +        r'^{0}(.*){0}\s+{1}'.format(
> > > +            COMMAND_OUTPUT_DELIM, BASH_PATTERN), re.M | re.S)
> > > +
> > > +    def __init__(self, cmd, expected_output=None, expected_result=0):
> > > +        self.cmd = cmd
> > > +        self.expected_result = expected_result
> > > +        self.expected_output = expected_output
> > > +
> > > +    def exec(self, pexpect_ctx):
> > > +        self.client = pexpect_ctx.client
> > > +
> > > +        LOGGER.debug('Executing command \'%s\'', self.cmd)
> > > +        try:
> > > +            self.client.sendline('{}=$({} 2>&1)'.format(
> > > +                self.OUTPUT_VARIABLE, self.cmd))
> > > +            self.client.expect(self.BASH_PATTERN)
> > > +
> > > +            self.client.sendline('echo $?')
> > > +            self.client.expect(self.COMMAND_RESULT_PATTERN)
> > > +            result = int(self.client.match.group(1))
> > > +
> > > +            self.client.sendline(
> > > +                'echo "{0}${{{1}}}{0}"'.format(
> > > +                    self.COMMAND_OUTPUT_DELIM,
> > > +                    self.OUTPUT_VARIABLE))
> > > +            self.client.expect(self.COMMAND_OUTPUT_PATTERN)
> > > +            out = self.client.match.group(1)
> > > +
> > > +            if result != self.expected_result:
> > > +                LOGGER.error('The command \'%s\' returned the code \'%d\', '
> > > +                             'but the expected code is \'%d\''
> > > +                             '\nCommand output: \'%s\'',
> > > +                             self.cmd, result, self.expected_result, out)
> > > +                return False
> > > +            if self.expected_output is not None and \
> > > +                    re.search(self.expected_output, out) is None:
> > > +                LOGGER.error('Wrong output for command \'%s\'. '
> > > +                             'Expected \'%s\'\nReceived \'%s\'',
> > > +                             self.cmd, self.expected_output, out)
> > > +                return False
> > > +        except pexpect.exceptions.TIMEOUT:
> > > +            LOGGER.error('Timeout for command \'%s\'', self.cmd)
> > > +            return False
> > > +        except pexpect.exceptions.EOF:
> > > +            LOGGER.error('Lost connection with docker. Aborting')
> > > +            return False
> > > +        return True
> > > +
> > > +
> > > +class FuegoContainer:
> > > +    def __init__(self, install_script, image_name, container_name,
> > > +                 jenkins_port):
> > > +        self.install_script = install_script
> > > +        self.image_name = image_name
> > > +        self.container_name = container_name
> > > +        self.jenkins_port = jenkins_port
> > > +
> > > +        self.docker_client = docker.APIClient()
> > > +        self.container = self.setup_docker()
> > > +
> > > +    def __del__(self):
> > > +        if self.container:
> > > +            LOGGER.debug('Removing Container')
> > > +            self.container.remove(force=True)
> > > +
> > > +    def stop(self):
> > > +        self.container.remove(force=True)
> > > +        self.container = None
> > > +
> > > +    def setup_docker(self):
> > > +        cmd = './{} {}'.format(self.install_script, self.image_name)
> > I'm not sure I like mixing string formatting styles.
> > I'm not sure which is more prevalent in this code, but the rest
> > of Fuego uses '%s'.  Please change this to:
> > cmd = "./%s %s" % (self.install_script, self.image_name)
> > 
> > > +        LOGGER.debug('Running \'%s\' to install the docker image. '
> > > +                     'This may take a while....', cmd)
> > > +        status = subprocess.call(cmd, shell=True)
> > > +        if status != 0:
> > > +            return None
> > > +        docker_client = docker.from_env()
> > > +        containers = docker_client.containers.list(
> > > +            all=True, filters={'name': self.container_name})
> > > +        if containers:
> > > +            LOGGER.debug(
> > > +                'Erasing the container \'%s\', so a new one can be created',
> > Please use " (double-quote) on the outside of this string, to avoid having
> > to escape the internal single-quotes.
> > 
> > > +                self.container_name)
> > > +            containers[0].remove(force=True)
> > > +
> > > +        container = docker_client.containers.create(
> > > +            self.image_name,
> > > +            stdin_open=True, tty=True, network_mode='bridge',
> > > +            name=self.container_name, command='/bin/bash')
> > > +        LOGGER.debug('Container \'%s\' created', self.container_name)
> > Please use " (double-quote) on the outside of this string, to avoid having
> > to escape the internal single-quotes.
> > 
> > > +        return container
> > > +
> > > +    def is_running(self):
> > > +        try:
> > > +            container_status = self.docker_client.\
> > > +                inspect_container(self.container_name)['State']['Running']
> > > +        except KeyError:
> > > +            return False
> > > +
> > > +        return container_status
> > > +
> > > +    def get_ip(self):
> > > +        container_addr = None
> > > +
> > > +        if not self.is_running():
> > > +            return None
> > > +
> > > +        def fetch_ip():
> > > +            nonlocal container_addr
> > > +            try:
> > > +                container_addr = self.docker_client.\
> > > +                    inspect_container(
> > > +                        self.container_name)['NetworkSettings']['IPAddress']
> > > +            except KeyError:
> > > +                return False
> > > +
> > > +            return False if container_addr is None else True
> > I'd rather this was:
> > if container_addr is None:
> > 	return False
> > else
> > 	return True
> > I'm not a big fan of reordering the conditional logic, even to make the
> > code less verbose.
> > 
> > > +
> > > +        if not loop_until_timeout(fetch_ip, timeout=10):
> > > +            LOGGER.error('Could not fetch the container IP address')
> > > +            return None
> > > +
> > > +        return container_addr
> > > +
> > > +    def get_url(self):
> > > +        container_addr = self.get_ip()
> > > +
> > > +        if container_addr:
> > > +            return 'http://{}:{}/fuego/'.\
> > > +                format(container_addr, self.jenkins_port)
> > Please use '%s" and the string format operator '%'.
> > 
> > > +        else:
> > > +            return None
> > > +
> > > +
> > > +class PexpectContainerSession():
> > > +    def __init__(self, container, start_script, timeout):
> > > +        self.container = container
> > > +        self.start_script = start_script
> > > +        self.timeout = timeout
> > > +
> > > +    def start(self):
> > > +        LOGGER.debug(
> > > +            'Starting container \'%s\'', self.container.container_name)
> > Please use " (double-quote) on the outside of this string, to avoid having
> > to escape the internal single-quotes.
> > 
> > > +        self.client = pexpect.spawnu(
> > > +            '{} {}'.format(
> > > +                self.start_script, self.container.container_name),
> > > +            echo=False, timeout=self.timeout)
> > > +
> > > +        PexpectContainerSession.set_ps1(self.client)
> > > +
> > > +        if not self.wait_for_jenkins():
> > > +            return False
> > > +
> > > +        return True
> > > +
> > > +    def __del__(self):
> > > +        self.client.terminate(force=True)
> > > +
> > > +    @staticmethod
> > > +    def set_ps1(client):
> > > +        client.sendline('export PS1="{}"'.format(ShExpect.BASH_PATTERN))
> > Please use '%s" and the string format operator '%'.
> > 
> > > +        client.expect(ShExpect.BASH_PATTERN)
> > > +
> > > +    def wait_for_jenkins(self):
> > > +        def ping_jenkins():
> > > +            try:
> > > +                conn = http.client.HTTPConnection(container_addr,
> > > +                                                  self.container.jenkins_port,
> > > +                                                  timeout=30)
> > > +                conn.request('HEAD', '/fuego/')
> > > +                resp = conn.getresponse()
> > > +                version = resp.getheader('X-Jenkins')
> > > +                status = resp.status
> > > +                conn.close()
> > > +                LOGGER.debug(
> > > +                    '  HTTP Response code: \'%d\' - Jenkins Version: \'%s\'',
> > > +                    status, version)
> > > +                if status == http.client.OK and version is not None:
> > > +                    return True
> > > +            except (ConnectionRefusedError, OSError):
> > > +                return False
> > > +
> > > +            return False
> > > +
> > > +        container_addr = self.container.get_ip()
> > > +        if container_addr is None:
> > > +            return False
> > > +        LOGGER.debug('Trying to reach jenkins at container \'%s\' via '
> > > +                     'the container\'s IP \'%s\' at port \'%d\'',
> > Please use " (double-quote) on the outside of this string, to avoid having
> > to escape the internal single-quotes.
> > 
> > > +                     self.container.container_name,
> > > +                     container_addr, self.container.jenkins_port)
> > > +        if not loop_until_timeout(ping_jenkins, 10):
> > > +            LOGGER.error('Could not connect to jenkins')
> > > +            return False
> > > +
> > > +        return True
> > > +
> > > +
> > > +class SeleniumContainerSession():
> > > +    def __init__(self, container):
> > > +        self.container = container
> > > +        self.driver = None
> > > +        self.root_url = container.get_url()
> > > +
> > > +    def start(self):
> > > +        options = webdriver.ChromeOptions()
> > > +        options.add_argument('headless')
> > > +        options.add_argument('no-sandbox')
> > > +        options.add_argument('window-size=1200x600')
> > > +
> > > +        self.driver = webdriver.Chrome(chrome_options=options)
> > > +        self.driver.get(self.root_url)
> > > +
> > > +        self.driver.get(self.root_url)
> > > +        LOGGER.debug('Started a Selenium Session on %s', self.root_url)
> > > +        return True
> > > +
> > > +
> > > +def main():
> > > +    DEFAULT_TIMEOUT = 120
> > > +    DEFAULT_IMAGE_NAME = 'fuego-release'
> > > +    DEFAULT_CONTAINER_NAME = 'fuego-release-container'
> > > +    DEFAULT_INSTALL_SCRIPT = 'install.sh'
> > > +    DEFAULT_START_SCRIPT = 'fuego-host-scripts/docker-start-
> > container.sh'
> > > +    DEFAULT_JENKINS_PORT = 8080
> > > +
> > > +    def execute_tests(timeout):
> > > +        LOGGER.debug('Starting tests')
> > > +
> > > +        ctx_mapper = {
> > > +            ShExpect: pexpect_session,
> > > +            SeleniumCommand: selenium_session
> > > +        }
> > > +
> > > +        tests_ok = True
> > > +        for cmd in COMMANDS_TO_TEST:
> > > +            for base_class, ctx in ctx_mapper.items():
> > > +                if isinstance(cmd, base_class):
> > > +                    if not cmd.exec(ctx):
> > > +                        tests_ok = False
> > > +                        break
> > > +
> > > +        if tests_ok:
> > > +            LOGGER.debug('Tests finished.')
> > > +
> > > +        return tests_ok
> > > +
> > > +    parser = argparse.ArgumentParser()
> > > +    parser.add_argument('working_dir', help='The working directory',
> > > type=str)
> > > +    parser.add_argument('-s', '--install-script',
> > > +                        help='The script that will be used to install the '
> > > +                        'docker image. Defaults to \'{}\''
> > > +                        .format(DEFAULT_INSTALL_SCRIPT),
> > Please use + for strings concatenated at the end.
> > 
> > > +                        default=DEFAULT_INSTALL_SCRIPT,
> > > +                        type=str)
> > > +    parser.add_argument('-a', '--start-script',
> > > +                        help='The script used to start the container. '
> > Please use " (double-quote) on the outside of this string, to avoid having
> > to escape the internal single-quotes.
> > 
> > > +                        'Defaults to \'{}\''
> > Please use + for strings concatenated at the end.
> > 
> > > +                        .format(DEFAULT_START_SCRIPT),
> > > +                        default=DEFAULT_START_SCRIPT,
> > > +                        type=str)
> > > +    parser.add_argument('-i', '--image-name',
> > > default=DEFAULT_IMAGE_NAME,
> > > +                        help='The image name that should be used. '
> > > +                        'Defaults to \'{}\''
> > > +                        .format(DEFAULT_IMAGE_NAME), type=str)
> > Please use + for strings concatenated at the end.
> > 
> > > +    parser.add_argument('-c', '--container-name',
> > > +                        default=DEFAULT_CONTAINER_NAME,
> > > +                        help='The container name that should be used for the '
> > > +                        'test. Defaults to \'{}\''
> > > +                        .format(DEFAULT_CONTAINER_NAME),
> > Please use + for strings concatenated at the end.
> > 
> > > +                        type=str)
> > > +    parser.add_argument('-t', '--timeout', help='The timeout value for '
> > > +                        'commands. Defaults to {}'
> > > +                        .format(DEFAULT_TIMEOUT),
> > Please use + for strings concatenated at the end.
> > 
> > > +                        default=DEFAULT_TIMEOUT, type=int)
> > > +    parser.add_argument('-j', '--jenkins-port',
> > > +                        help='The port where the jenkins is running on the '
> > > +                        'test container. Defaults to {}'
> > > +                        .format(DEFAULT_JENKINS_PORT),
> > Please use + for strings concatenated at the end.
> > 
> > > +                        default=DEFAULT_JENKINS_PORT, type=int)
> > > +    args = parser.parse_args()
> > > +
> > > +    LOGGER.debug('Changing working dir to \'%s\'', args.working_dir)
> > Please use " (double-quote) on the outside of this string, to avoid having
> > to escape the internal single-quotes.
> > 
> > > +    os.chdir(args.working_dir)
> > > +
> > > +    container = FuegoContainer(args.install_script, args.image_name,
> > > +                               args.container_name, args.jenkins_port)
> > > +
> > > +    pexpect_session = PexpectContainerSession(container,
> > args.start_script,
> > > +                                              args.timeout)
> > > +    if not pexpect_session.start():
> > > +        return 1
> > > +
> > > +    selenium_session = SeleniumContainerSession(container)
> > > +    if not selenium_session.start():
> > > +        return 1
> > > +
> > > +    COMMANDS_TO_TEST = [
> > > +        ShExpect(
> > > +            'echo $\'hello\n\n\nfrom\n\n\ncontainer\'',
> > Please use " (double-quote) on the outside of this string, to avoid having
> > to escape the internal single-quotes.  (or vice-versa)
> > 
> > > +            r'hello\s+from\s+container'),
> > > +        ShExpect(
> > > +            'cat -ThisOptionDoesNotExists', expected_result=1),
> > > +        ShExpect('ftc add-nodes docker'),
> > > +        ShExpect(
> > > +            'ftc list-nodes', r'Jenkins nodes in this system:\s*docker\s*.*'),
> > > +        ShExpect('ftc add-jobs -b docker -p testplan_docker'),
> > > +        ShExpect(
> > > +            'ftc list-jobs', r'Jenkins jobs in this system:(\s*docker\..*)+'),
> > > +        Visit(url=container.get_url()),
> > > +        CheckText(_id='executors', text='docker'),
> > > +        CheckText(_id='executors', text='master')
> > > +    ]
> > > +
> > > +    if not execute_tests(args.timeout):
> > > +        return 1
> > > +
> > > +    return 0
> > > +
> > > +
> > > +if __name__ == '__main__':
> > > +    sys.exit(main())
> > > --
> > > 2.16.2
> > 
> > Thanks.
> >  -- Tim
> > _______________________________________________
> > Fuego mailing list
> > Fuego at lists.linuxfoundation.org
> > https://lists.linuxfoundation.org/mailman/listinfo/fuego


More information about the Fuego mailing list