OpenStack has a general set of code review guidelines: https://docs.openstack.org/infra/manual/developers.html#peer-review
What follows is a very terse set of points for reviewers to consider when looking at nova code. These are things that are important for the continued smooth operation of Nova, but that tend to be carried as “tribal knowledge” instead of being written down. It is an attempt to boil down some of those things into nearly checklist format. Further explanation about why some of these things are important belongs elsewhere and should be linked from here.
When making a change to the nova API, we should always follow the API WG guidelines rather than going for “local” consistency. Developers and reviewers should read all of the guidelines, but they are very long. So here are some key points:
project
should be used in the REST API instead of tenant
.server
should be used in the REST API instead of instance
.compute
should be used in the REST API instead of nova
.201 Created
202 Accepted
204 No Content
200 OK
The central place where all config options should reside is the /nova/conf/
package. Options that are in named sections of nova.conf
, such as
[serial_console]
, should be in their own module. Options that are in the
[DEFAULT]
section should be placed in modules that represent a natural
grouping. For example, all of the options that affect the scheduler would be
in the scheduler.py
file, and all the networking options would be moved
to network.py
.
A config option should be checked for:
Previously used sections which explained which services consume a specific config option and which options are related to each other got dropped because they are too hard to maintain: http://lists.openstack.org/pipermail/openstack-dev/2016-May/095538.html
Any change that is not tested well by the Jenkins check jobs must have a recent +1 vote from an appropriate third party test (or tests) on the latest patchset, before a core reviewer is allowed to make a +2 vote.
At a minimum, we must ensure that any technology specific code has a +1 from the relevant third party test, on the latest patchset, before a +2 vote can be applied. Specifically, changes to nova/virt/driver/<NNNN> need a +1 vote from the respective third party CI. For example, if you change something in the Hyper-V virt driver, you must wait for a +1 from the Hyper-V CI on the latest patchset, before you can give that patch set a +2 vote.
This is important to ensure:
Please note:
Because we can’t run all CI jobs in the check and gate pipelines, some jobs can be executed on demand, thanks to the experimental pipeline. To run the experimental jobs, you need to comment your Gerrit review with “check experimental”.
The experimental jobs aim to test specific features, such as LXC containers or DVR with multiple nodes. Also, it might be useful to run them when we want to test backward compatibility with tools that deploy OpenStack outside Devstack (e.g. TripleO, etc). They can produce a non-voting feedback of whether the system continues to work when we deprecate or remove some options or features in Nova.
The experimental queue can also be used to test that new CI jobs are correct before making them voting.
utf8
charset only where necessary. Some string fields, such as
hex-stringified UUID values, MD5 fingerprints, SHA1 hashes or base64-encoded
data, are always interpreted using ASCII encoding. A hex-stringified UUID
value in latin1
is 1/3 the size of the same field in utf8
, impacting
performance without bringing any benefit. If there are no string type columns
in the table, or the string type columns contain only the data described
above, then stick with latin1
.If a new microversion API is added, the following needs to happen:
A release note is required on changes that have upgrade impact, security impact, introduce a new feature, fix Critical bugs, or fix long-standing bugs with high importance. See Release Notes for details on how to create a release note, each available section and the type of content required.
Except where otherwise noted, this document is licensed under Creative Commons Attribution 3.0 License. See all OpenStack Legal Documents.