Before you start, ensure you’ve read:
Run the unit tests¶
Extra dependencies are required to run the tests. On Debian based
distributions, you can install
To run the tests, use the
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.
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.
Source code formatting¶
black and isort should be applied to all LAVA source code files. Merge requests will fail CI if a change breaks the formatting.
isort should be run with –profile black option to ensure compatibility with black.
When changing files formatted by black, make your changes and then run
black on all modified Python files before pushing the branch to
GitLab. In some situations,
pylint can disagree on
continuation of long lines, particularly when using multiple operators
and bracketing. In case of conflict, black is always correct. If
you disagree with how black has formatted your change, consider
expanding list comprehensions and other syntax until you and black can
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:
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.
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.
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.
Whenever new functionality is added to
a new Strategy class, there must be
some new unit tests added to allow some assurance that the new classes
continue to operate as expected as the rest of the codebase continues
to develop. There are a lot of examples in the current unit tests.
Start with a sample test job which is known to work. Copy that into
lava_dispatcher/tests/sample_jobs. The URLs in that sample job will need to be valid URLs but do not need to be working files. (This sample_job is not being submitted to run on a device, it is only being used to check that the construction of the pipeline is valid.) If you need files which other sample jobs do not use then we can help with that by putting files onto images.validation.linaro.org.
Use the updated
Factorysupport to generate the device configuration directly from the
If a suitable device dictionary does not already exist in
lava_scheduler_app/tests/devices, a new one can be added to support the unit tests.
Add a function to a suitable Factory class to use the device config file to create a device and use the parser to create a Job instance by following the examples in the existing unit tests
Create the pipeline ref by following the
pipeline_refdirectory. The simplest way to create a single new pipeline reference file is to add one line to the new unit test function:
self.update_ref = True
Run the unit test and the pipeline reference will be created. Remove the line before committing for review or the
./ci-runcheck will fail.
This file acts as a description of the classes involved in the pipeline which has been constructed from the supplied test job and device configuration. Validating it in the unit tests ensures that later development does not invalidate the new code by accidentally removing or adding unexpected actions.
In the new function, use the
pipeline_refsREADME to add a check that the pipeline reference continues to reflect the pipeline which has been constructed by the parser.
unit tests do not typically check any of the
code. Do as much checking as is practical in the
functions of all the new classes. For example, if
run relies on
a parameter being set, check for that parameter in
check that the value of that parameter is correct based on the
sample job and the supplied device configuration.
Some parts of lava_scheduler_app are easier to test than others. New
device-type templates need to have specific unit tests added to
lava_scheduler_app/tests/test_templates or one of the relevant
specialist template unit test files. Follow the examples and make sure
that if the new template adds new items then those items are checked
for existence and validity in the new function which tests the new
$ 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
If you are adding or modifying documentation in
lava-server, make sure that
the documentation builds cleanly:
$ make -C doc/v2 clean $ make -C doc/v2 html
For other parts of
lava-server, follow the examples of the existing unit
tests and talk to us.
Re-run the unit tests¶
Make sure that your changes do not cause any failures in the unit tests:
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:
Ensure that your local branch is clean - check for left over debug code.
Ensure that your local branch has been rebased against current
Build and install a package from the
masterbranch. If you have added any new files in your local change, make sure these have been removed. Reproduce the original bug or problem.
Build and install a package from your local branch and repeat the tests.
Changes to most files in
lava_dispatcher can be symlinked or copied
into the packaged locations. e.g.:
$ PYTHONDIR=/usr/lib/python3/dist-packages/ $ sudo cp <path_to_file> $PYTHONDIR/<path_to_file>
The path used for
PYTHONDIR has changed with the LAVA
runtime support moving to Python3 in 2018.4.
There is no need to copy files used solely by the unit tests.
Changes to files in
./etc/ will require restarting the relevant
Changes to files in
./lava/dispatcher/ will need the
service to be restarted but changes to
./lava_dispatcher/ will not.
When adding or modifying
cleanupfunctions, always ensure that
superis called appropriately, for example:
super().validate() connection = super().run(connection, max_end_time)
When adding or modifying
runfunctions in subclasses of
Action, always ensure that each return point out of the
runfunction returns the
When adding new classes, use hyphens,
-, as separators in
self.name, not underscores,
_. The function will fail if underscore or whitespace is used. Action names need to all be lowercase and describe something about what the action does at runtime. More information then needs to be added to the
self.summaryand an extended sentence in
self.name = 'do-something-at-runtime'
Use namespaces for all dynamic data. Parameters of actions are immutable. Use the namespace functions when an action needs to store dynamic data, for example the location of files which have been downloaded to temporary directories, Do not access
self.datadirectly (except for use in iterators). Use the get and set primitives, for example:
set_namespace_data(action='boot', label='shared', key='boot-result', value=res) image_arg = self.get_namespace_data(action='download-action', label=label, key='image_arg')
Changes to device-type templates and device dictionaries take effect
immediately, so simply submitting a test job will pick up the latest
version of the code in
/etc/lava-server/dispatcher-config/device-types/. Make changes to
the templates in
lava_scheduler_app/tests/device-types/. Check them
test_all_templates test, and only then copy the updates
/etc/lava-server/dispatcher-config/device-types/ when the
Changes to django templates can be applied immediately by copying the
template into the packaged path, e.g. html files in
lava_scheduler_app/templates/lava_scheduler_app/ can be copied or
The path changed when the LAVA runtime support moved to Python3 with the 2018.4 release.
Changes to python code generally require copying the files and
lava-server-gunicorn service before the changes will
sudo service lava-server-gunicorn restart
require restarting the
sudo service lava-master restart
Changes to files in
./etc/ will require restarting the relevant
service. If multiple services are affected, it is normally best to
build and install a new package.
Database migrations are a complex area - read up on the django
documentation for migrations. Instead of
python ./manage.py, use
sudo lava-server manage.
Documentation files in
doc/v2 can be built locally in the git
make -C doc/v2 clean make -C doc/v2 html
Files can then be checked in a web browser using the
scheme and the
_build/html/ subdirectory. For example:
Some documentation changes can add images, example test jobs, test definitions and other files. Depending on the type of file, it may be necessary to make changes to the packaging, so talk to us before making such changes.
Documentation is written in RST, so the RST Primer is essential reading when modifying the documentation.
Keep all documentation paragraphs wrapped to 80 columns.
Strip trailing whitespace from all modified files.
When you build your changes from clean, make sure there are no warning or error messages from the build.
en_USin both code and documentation.
Use syntax highlighting for code and check the rendered page. For example,
code-block:: shellrelates to the contents of shell scripts, not the output of commands or scripts in a shell (those should use
Wherever possible, pull in code samples from working example files so that these can be checked for accuracy on staging before future releases.
Debugging lava-dispatcher with pdb, the Python debugger¶
Due to the nature of how
lava-run is executed by
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
lava-worker is started with the
--debug command line option, then it
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
Send your commits for review¶
From each topic branch, just run:
each merge request is reviewed and approved individually and
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.
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.
Fixes from comments or unit test failures in one review are not acceptable as separate merge requests.
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.
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.
All merge requests must pass all CI tests.
Therefore the recommendations are:
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.
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.
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.
Check gitlab periodically and ensure that you address all comments on the review.
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
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.
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
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
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')
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
On Debian Jessie and later:
$ sudo lava-server manage makemigrations lava_scheduler_app
The migration file will be created in
(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.
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.
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-server unit tests
against python 3.x and changes must pass all unit tests.
./ci-run script for
run the unit tests using Python3:
Some additional Python3 dependencies will be required. In particular,
python3-django-auth-ldap will need to be installed.
Django will be dropping python2.7 support with the 2.2LTS release, frozen instances of LAVA will not be able to use django updates after that point.
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):
docstringmust include the full user-facing documentation of each function exposed through the API.
Authentication should be supported using the base class support:
Catch exceptions for all errors,
DoesNotExistand others, then re-raise as
Move as much of the work into the relevant app as possible, either in
dbutils.py. Wherever possible, re-use existing functions with wrappers for error handling in the API code.
/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
Most settings for the instance are handled inside django using
/etc/lava-server/settings.conf. (For historical reasons, this file
uses JSON syntax.)
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
Docstrings should still be added wherever a docstring would be useful.
Many developers use a
~/.pylintrc file which already includes a
sample list of warnings to disable. Other warnings frequently disabled
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
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
codebase, it still has a high level of false indications, particularly
when extending an existing model.
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
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-dispatcher, lavacli have unit tests.
Extra dependencies are required to run the tests. On Debian based
distributions, you need to install
To run the tests, use the ci-run / ci-build scripts:
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
$ 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
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
settings.conf is JSON syntax, so ensure that the previous
line ends with a comma and that the resulting file validates as
JSON. Use JSONLINT
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).