LAVA development¶

Run the unit tests¶

Extra dependencies are required to run the tests. On Debian based distributions, you can install lava-dev.

To run the tests, use the ci-run script:

$ ./ci-run

There is never any need to use sudo for unit tests, it causes lots of complications by changing file permissions in your local git clone.

Functional testing¶

Unit tests cannot replicate all tests required on LAVA code, some tests will need to be run with real devices under test. On Debian based distributions, see Developer package build. See Writing Tests for information on writing LAVA test jobs to test particular device functionality.

Make your changes¶

• Follow PEP8 style for Python code.
• Use one topic branch for each logical change.
• Include new unit tests in the proposed merge request.
• Write good commit messages.
  • Describe why the change was made, not what files were changed. The commit message should reflect your intention, not the contents of the commit.
  • Avoid putting documentation into the commit message. Keep the commit message to a reasonable length (about 10 to 12 lines at most). Include changes to the existing documentation in your commit.
  • Usage examples need to go into the documentation, not the commit message. Everything which is intended to help users to add this support to their own test jobs must be in the documentation.
  • Avoid duplicating or summarizing the documentation in the commit message, reviewers will be reading the documentation as well.
  • Use comments in the code in preference to detailed commit messages.

Add some unit tests¶

Some changes will always need additional unit tests and reviews will not be merged without this support. The purpose is to ensure that future changes in the codebase have some assurance that existing support has not been affected. The intent is that as much as possible of the test job and device configuration is covered by at least one unit test. Some examples include:

1. Changes to an existing jinja2 device-type template which change the output YAML of the device configuration need a unit test to show that the change is being included.
2. Adding a new deployment or boot method needs unit tests (including sample test jobs) which check that all validate() functions work correctly and particular tests checking for the specific details of the new method.
3. Adding a change to an existing deployment or boot method which changes the construction of the pipeline based on test job or device configuration. Unit tests will be required to show that the change is being made.

Reviewers may ask for unit test support for any change, so talk to us during development. You can also use an WIP: prefix in your git commit to indicate that the change is not ready for merging but is ready for comments.

$ python3 -m unittest -vcf lava_scheduler_app.tests.test_fastboot_templates

$ python3 -m unittest -vcf lava_scheduler_app.tests.test_qemu_templates

$ python3 -m unittest -vcf lava_scheduler_app.tests.test_uboot_templates

$ make -C doc/v2 clean
$ make -C doc/v2 html

Re-run the unit tests¶

Make sure that your changes do not cause any failures in the unit tests:

$ ./ci-run

Wherever possible, always add new unit tests for new code.

Testing local changes¶

For any sufficiently large change, building and installing a new package on a local instance is recommended. Ensure that the test instance is already running the most recent production release.

If the test instance has a separate worker, ensure that the master and the worker always have precisely the same code applied. For some changes, it may be necessary to have a test instance which is a clone of a production instance, complete with devices. Never make live changes to a production instance. (This is why integrating new device types into LAVA requires multiple devices.)

Once your change is working successfully:

1. Ensure that your local branch is clean - check for left over debug code.
2. Ensure that your local branch has been rebased against current master
3. Build and install a package from the master branch. If you have added any new files in your local change, make sure these have been removed. Reproduce the original bug or problem.
4. Build and install a package from your local branch and repeat the tests.

$ PYTHONDIR=/usr/lib/python3/dist-packages/
$ sudo cp <path_to_file> $PYTHONDIR/<path_to_file>

sudo service lava-server-gunicorn restart

sudo service lava-master restart

make -C doc/v2 clean
make -C doc/v2 html

Debugging lava-dispatcher with pdb, the Python debugger¶

Due to the nature of how lava-run is executed by lava-worker, it’s tricky to debug lava-dispatcher directly. However, one can use the remote-pdb package and do remote debugging.

You need to have the remote-pdb python package installed, and a telnet client.

If lava-worker is started with the --debug command line option, then it will make lava-run stop right before running the test job for debugging. You will see a message on the console where lava-run is running that is similar to this:

RemotePdb session open at 127.0.0.1:37865, waiting for connection ...

Note the address where the debuggin server is listening. Then, to access the debugger, you point telnet to the provided address and port:

telnet 127.0.0.1 37865
Connected to 127.0.0.1.
Escape character is '^]'.
> /path/to/lava/dispatcher/lava-run(274)main()
-> job.run()
(Pdb)

From that point on, you have a normal pdb session, and can debug the execution of lava-dispatcher.

Send your commits for review¶

From each topic branch, just run:

git push

1. each merge request is reviewed and approved individually and
2. later commits will depend on earlier commits, so if a later commit is approved and the one before it is not, the later commit will not be merged until the earlier one is approved.
3. you are responsible for rebasing your branch(es) against updates on master and this can become more difficult when there are multiple commits on one local branch.
4. Fixes from comments or unit test failures in one review are not acceptable as separate merge requests.
5. It is common for merge requests to go through repeated cycles of comments and updates. This is not a reflection on the usefulness of the change or on any particular contributors, it is a natural evolution of the code. Comments may reflect changes being made in other parallel reviews or reviews merged whilst this change was being reviewed. Contributors may be added to other reviews where the team consider this to be useful for feedback or where the documentation is being updated in areas which relate to your change. The number of comments per review is no indication of the quality of that review and does not affect when the review would be merged.
6. It is common for changes to develop merge conflicts during the review process as other reviews are merged. You are responsible for fixing all merge conflicts in your merge requests.
7. All merge requests must pass all CI tests.

Therefore the recommendations are:

1. Always use a separate local branch per change and a new commit for changes on that branch each time branch gets pushed until it is merged.
2. Think carefully about whether to base one local branch on another local branch. This is recommended when one change logically extends an earlier change and makes it a lot easier than having multiple commits on a single branch.
3. Keep all your branches up to date with master regularly. It is much better to resolve merge conflicts one change at a time instead of having multiple merge commits all in the one rebase operation when the merge request is finally ready to be merged. GitLab will show a message if a rebase is required but you can also simply rebase your local branch before pushing any new changes.
4. Check gitlab periodically and ensure that you address all comments on the review.

Adding reviewers¶

The lava group is automatically added as approver for every merge request.

Optionally, you can put WIP: at the start of your git commit message and then amend the message when the request is ready to merge.

Handling your local branches¶

After placing a few reviews, there will be a number of local branches. To keep the list of local branches under control, the local branches can be easily deleted after the merge. Note: git will warn if the branch has not already been merged when used with the lower case -d option. This is a useful check that you are deleting a merged branch and not an unmerged one, so work with git to help your workflow.

$ git checkout bugfix
$ git rebase master
$ git checkout master
$ git branch -d bugfix

If the final command fails, check the status of the review of the branch. If you are completely sure the branch should still be deleted or if the review of this branch was abandoned, use the -D option instead of -d and repeat the command.

Future proofing¶

All developers are encouraged to write code with future changes in mind, so that it is easy to do a technology upgrade. This includes watching for errors and warnings generated by dependency packages, as well as upgrading and migrating to newer APIs as a normal part of development.

This is particularly true for Django where the lava-server package needs to retain support for multiple django versions as well as monitoring for deprecation warnings in the newest django version. Where necessary, write code for different versions and separate with:

import django
if django.VERSION > (1, 8):
    pass  # newer code
else:
    pass  # older compatibility code

Use templates to generate device configuration¶

One of the technical reasons to merge the lava-dispatcher and lava-server source trees into a single source is to allow lava-dispatcher to use the output of the lava-server templates in development. Further changes are being made in this area to provide a common module but it is already possible to build a lava_dispatcher unit test which pulls device configuration directly from the templates in lava_scheduler_app. This removes the problem of static YAML files in lava_dispatcher/devices getting out of date compared to the actual YAML created by changes in the templates.

The YAML device configuration is generated from a device dictionary in lava_scheduler_app which extends a template in lava_scheduler_app - the same template which is used at runtime on LAVA instances. Any change to the template or device dictionary is immediately reflected in the YAML sent to the lava_dispatcher unit test.

import unittest
from lava_dispatcher.tests.test_basic import Factory, StdoutTestCase
from lava_dispatcher.tests.utils import infrastructure_error_multi_paths

class TestFastbootDeploy(StdoutTestCase):  # pylint: disable=too-many-public-methods

    def setUp(self):
        super().setUp()
        self.factory = Factory()

    @unittest.skipIf(infrastructure_error_multi_paths(
        ['lxc-info', 'img2simg', 'simg2img']),
        "lxc or img2simg or simg2img not installed")
    def test_lxc_api(self):
        job = self.factory.create_job('d02-01.jinja2', 'sample_jobs/grub-ramdisk.yaml')

Database migrations¶

The LAVA team recommend using Debian stable but also support testing and unstable which have a newer version of python-django.

Database migrations on Debian Jessie and later are managed within django. Support for python-django-south has been dropped. Only django migration types should be included in any reviews which involve a database migration.

Once modified, the updated models.py file needs to be copied into the system location for the relevant extension, e.g. lava_scheduler_app. This is a step which needs to be done by the developer - developer packages cannot be installed cleanly and unit tests will likely fail until the migration has been created and applied.

On Debian Jessie and later:

$ sudo lava-server manage makemigrations lava_scheduler_app

The migration file will be created in /usr/lib/python3/dist-packages/lava_scheduler_app/migrations/ (which is why sudo is required) and will need to be copied into your git working copy and added to the review.

The migration is applied using:

$ sudo lava-server manage migrate lava_scheduler_app

See django docs for more information.

Python 3.x¶

Python3 support in LAVA is related to a number of factors:

• Forthcoming LTS releases of django which will remove support for python2.7
• Transition within Debian to full python3 support.

https://lists.lavasoftware.org/pipermail/lava-announce/2017-June/000032.html

https://lists.lavasoftware.org/pipermail/lava-announce/2018-January/000046.html

lava-dispatcher and lava-server now fully support python3, runtime and testing. Code changes to either codebase must be Python3 compatible.

All reviews run the lava-dispatcher and lava-server unit tests against python 3.x and changes must pass all unit tests.

The ./ci-run script for lava-dispatcher and lava-server can run the unit tests using Python3:

./ci-run -a

Some additional Python3 dependencies will be required. In particular, python3-django-auth-ldap will need to be installed.

XML-RPC changes¶

Each of the installed django apps in lava-server are able to expose functionality using XML-RPC.

from linaro_django_xmlrpc.models import ExposedAPI

class SomeAPI(ExposedAPI):

1. The docstring must include the full user-facing documentation of each function exposed through the API.

2. Authentication should be supported using the base class support:

   self._authenticate()
   

3. Catch exceptions for all errors, SubmissionException, DoesNotExist and others, then re-raise as xmlrpc.client.Fault.

4. Move as much of the work into the relevant app as possible, either in models.py or in dbutils.py. Wherever possible, re-use existing functions with wrappers for error handling in the API code.

Instance settings¶

/etc/lava-server/instance.conf is principally for V1 configuration. V2 uses this file only for the database connection settings on the master, instance name and the lavaserver user.

Most settings for the instance are handled inside django using /etc/lava-server/settings.conf. (For historical reasons, this file uses JSON syntax.)

Pylint3¶

Pylint is a tool that checks for errors in Python code, tries to enforce a coding standard and looks for bad code smells. We encourage developers to run LAVA code through pylint and fix warnings or errors shown by pylint to maintain a good score. For more information about code smells, refer to Martin Fowler’s refactoring book. LAVA developers stick on to PEP 008 (aka Guido’s style guide) across all the LAVA component code.

pylint3 does need to be used with some caution, the messages produced should not be followed blindly. It can be very useful for spotting unused imports, unused variables and other issues. To simplify the pylint output, some warnings are recommended to be disabled:

$ pylint3 -d line-too-long -d missing-docstring

Many developers use a ~/.pylintrc file which already includes a sample list of warnings to disable. Other warnings frequently disabled in ~/.pylintrc include:

too-many-locals,
too-many-ancestors,
too-many-arguments,
too-many-instance-attributes,
too-many-nested-blocks,
too-many-return-statements,
too-many-branches,
too-many-statements,
too-few-public-methods,
wrong-import-order,
ungrouped-imports,

pylint also supports local disabling of warnings and there are many examples of:

variable = func_call()  # pylint: disable=

There is a pylint-django plugin available in unstable and testing and whilst it improves the pylint output for the lava-server codebase, it still has a high level of false indications, particularly when extending an existing model.

pep8¶

In order to check for PEP 008 compliance the following command is recommended:

$ pep8 --ignore E501

pep8 can be installed in Debian based systems as follows:

$ apt install pep8

Unit-tests¶

LAVA has set of unit tests which the developers can run on a regular basis for each change they make in order to check for regressions if any. Most of the LAVA components such as lava-server, lava-dispatcher, lavacli have unit tests.

Extra dependencies are required to run the tests. On Debian based distributions, you need to install lava-dev.

To run the tests, use the ci-run / ci-build scripts:

$ ./ci-run

LAVA database model visualization¶

LAVA database models can be visualized with the help of django_extensions along with tools such as pydot. In Debian based systems install the following packages to get the visualization of LAVA database models:

$ apt install python-django-extensions python-pydot

Once the above packages are installed successfully, use the following command to get the visualization of lava-server models in PNG format:

$ sudo lava-server manage graph_models --pydot -a -g -o lava-server-model.png

More documentation about graph models is available in https://django-extensions.readthedocs.io/en/latest/graph_models.html

Other useful features from django_extensions are as follows:

• shell_plus - similar to the built-in “shell” but autoloads all models

• validate_templates - check templates for rendering errors:

  $ sudo lava-server manage validate_templates
  

• runscript - run arbitrary scripts inside lava-server environment:

  $ sudo lava-server manage runscript fix_user_names --script-args=all
  

Developer access to django shell¶

Default configurations use a side-effect of the logging behavior to restrict access to the lava-server manage operations which typical Django apps expose through the manage.py interface. This is because lava-server manage shell provides read-write access to the database, so the command requires sudo.

On developer machines, this can be unnecessary. Set the location of the django log to a new location to allow easier access to the management commands to simplify debugging and to be able to run a Django Python Console inside a development environment. In /etc/lava-server/settings.conf add:

"DJANGO_LOGFILE": "/tmp/django.log"

The new location needs to be writable by the lavaserver user (for use by localhost) and by the developer user (but would typically be writeable by anyone).