At the Boston 2017 Summit I had mentioned that the pattern of using non voting/experimental jobs was not working for getting new features into Devstack. It is slow and leads people to being too conservative when it comes to pushing new things in. Instead I suggested that since Devstack changes are self testing we add the features, have change that enables the feature, and if that changes passes we move forward with merging (assuming code review is fine and necessary communication is done). Document this process in the HACKING file so that we have something we can point to when people want to add a new experimental job for every new little thing (ipv6, tls, systemd, etc). Change-Id: I5190cc3d3de4e81d52748347306133b5034d5531
16 KiB
Contributing to DevStack
General
DevStack is written in UNIX shell script. It uses a number of bash-isms and so is limited to Bash (version 4 and up) and compatible shells. Shell script was chosen because it best illustrates the steps used to set up and interact with OpenStack components.
DevStack's official repository is located on git.openstack.org at https://git.openstack.org/openstack-dev/devstack. Besides the master branch that tracks the OpenStack trunk branches a separate branch is maintained for all OpenStack releases starting with Diablo (stable/diablo).
Contributing code to DevStack follows the usual OpenStack process as described in How To Contribute in the OpenStack wiki. DevStack's LaunchPad project contains the usual links for blueprints, bugs, etc.
The Gerrit review queue is used for all commits.
The primary script in DevStack is stack.sh
, which
performs the bulk of the work for DevStack's use cases. There is a
subscript functions
that contains generally useful shell
functions and is used by a number of the scripts in DevStack.
A number of additional scripts can be found in the tools
directory that may be useful in supporting DevStack installations. Of
particular note are info.sh
to collect and report
information about the installed system, and
install_prereqs.sh
that handles installation of the
prerequisite packages for DevStack. It is suitable, for example, to
pre-load a system for making a snapshot.
Repo Layout
The DevStack repo generally keeps all of the primary scripts at the root level.
doc
- Contains the Sphinx source for the documentation.
tools/build_docs.sh
is used to generate the HTML versions
of the DevStack scripts. A complete doc build can be run with
tox -edocs
.
exercises
- Contains the test scripts used to
sanity-check and demonstrate some OpenStack functions. These scripts
know how to exit early or skip services that are not enabled.
extras.d
- Contains the dispatch scripts called by the
hooks in stack.sh
, unstack.sh
and
clean.sh
. See the plugins
docs <plugins>
for more information.
files
- Contains a variety of otherwise lost files used
in configuring and operating DevStack. This includes templates for
configuration files and the system dependency information. This is also
where image files are downloaded and expanded if necessary.
lib
- Contains the sub-scripts specific to each project.
This is where the work of managing a project's services is located. Each
top-level project (Keystone, Nova, etc) has a file here. Additionally
there are some for system services and project plugins. These variables
and functions are also used by related projects, such as Grenade, to
manage a DevStack installation.
samples
- Contains a sample of the local files not
included in the DevStack repo.
tests
- the DevStack test suite is rather sparse, mostly
consisting of test of specific fragile functions in the
functions
and functions-common
files.
tools
- Contains a collection of stand-alone scripts.
While these may reference the top-level DevStack configuration they can
generally be run alone. There are also some sub-directories to support
specific environments such as XenServer.
Scripts
DevStack scripts should generally begin by calling
env(1)
in the shebang line:
#!/usr/bin/env bash
Sometimes the script needs to know the location of the DevStack
install directory. TOP_DIR
should always point there, even
if the script itself is located in a subdirectory:
# Keep track of the current DevStack directory.
TOP_DIR=$(cd $(dirname "$0") && pwd)
Many scripts will utilize shared functions from the
functions
file. There are also rc files
(stackrc
and openrc
) that are often included
to set the primary configuration of the user environment:
# Keep track of the current DevStack directory.
TOP_DIR=$(cd $(dirname "$0") && pwd)
# Import common functions
source $TOP_DIR/functions
# Import configuration
source $TOP_DIR/openrc
stack.sh
is a rather large monolithic script that flows
through from beginning to end. It has been broken down into
project-specific subscripts (as noted above) located in lib
to make stack.sh
more manageable and to promote code
reuse.
These library sub-scripts have a number of fixed entry points, some
of which may just be stubs. These entry points will be called by
stack.sh
in the following order:
install_XXXX
configure_XXXX
init_XXXX
start_XXXX
stop_XXXX
cleanup_XXXX
There is a sub-script template in lib/templates
to be
used in creating new service sub-scripts. The comments in
<>
are meta comments describing how to use the
template and should be removed.
In order to show the dependencies and conditions under which project
functions are executed the top-level conditional testing for things like
is_service_enabled
should be done in stack.sh
.
There may be nested conditionals that need to be in the sub-script, such
as testing for keystone being enabled in
configure_swift()
.
stackrc
stackrc
is the global configuration file for DevStack.
It is responsible for calling local.conf
(or
localrc
if it exists) so local user configuration is
recognized.
The criteria for what belongs in stackrc
can be vaguely
summarized as follows:
- All project repositories and branches handled directly in
stack.sh
- Global configuration that may be referenced in
local.conf
, i.e.DEST
,DATA_DIR
- Global service configuration like
ENABLED_SERVICES
- Variables used by multiple services that do not have a clear owner,
i.e.
VOLUME_BACKING_FILE_SIZE
(nova-compute, nova-volumes and cinder) orPUBLIC_NETWORK_NAME
(nova-network and neutron) - Variables that can not be cleanly declared in a project file due to dependency ordering, i.e. the order of sourcing the project files can not be changed for other reasons but the earlier file needs to dereference a variable set in the later file. This should be rare.
Also, variable declarations in stackrc
before
local.conf
is sourced do NOT allow overriding (the form
FOO=${FOO:-baz}
); if they did then they can already be
changed in local.conf
and can stay in the project file.
Documentation
The DevStack repo now contains all of the static pages of
devstack.org in the doc/source
directory. The OpenStack CI
system rebuilds the docs after every commit and updates devstack.org
(now a redirect to docs.openstack.org/developer/devstack).
All of the scripts are processed with shocco to
render them with the comments as text describing the script below. For
this reason we tend to be a little verbose in the comments _ABOVE the code they pertain to. Shocco also supports
Markdown formatting in the comments; use it sparingly. Specifically,
stack.sh
uses Markdown headers to divide the script into
logical sections.
The script used to drive <code>shocco</code> is <code>tools/build_docs.sh</code>. The complete docs build is also handled with <code>tox -edocs</code> per the OpenStack project standard.
Exercises
The scripts in the exercises directory are meant to 1) perform basic operational checks on certain aspects of OpenStack; and b) document the use of the OpenStack command-line clients.
In addition to the guidelines above, exercise scripts MUST follow the
structure outlined here. swift.sh
is perhaps the clearest
example of these guidelines. These scripts are executed serially by
exercise.sh
in testing situations.
Begin and end with a banner that stands out in a sea of script logs to aid in debugging failures, particularly in automated testing situations. If the end banner is not displayed, the script ended prematurely and can be assumed to have failed.
echo "**************************************************" echo "Begin DevStack Exercise: $0" echo "**************************************************" ... set +o xtrace echo "**************************************************" echo "End DevStack Exercise: $0" echo "**************************************************"
The scripts will generally have the shell
xtrace
attribute set to display the actual commands being executed, and theerrexit
attribute set to exit the script on non-zero exit codes:# This script exits on an error so that errors don't compound and you see # only the first error that occurred. set -o errexit # Print the commands being run so that we can see the command that triggers # an error. It is also useful for following as the install occurs. set -o xtrace
Settings and configuration are stored in
exerciserc
, which must be sourced afteropenrc
orstackrc
:# Import exercise configuration source $TOP_DIR/exerciserc
There are a couple of helper functions in the common
functions
sub-script that will check for non-zero exit codes and unset environment variables and print a message and exit the script. These should be called after most client commands that are not otherwise checked to short-circuit long timeouts (instance boot failure, for example):swift post $CONTAINER die_if_error "Failure creating container $CONTAINER" FLOATING_IP=`euca-allocate-address | cut -f2` die_if_not_set FLOATING_IP "Failure allocating floating IP"
If you want an exercise to be skipped when for example a service wasn't enabled for the exercise to be run, you can exit your exercise with the special exitcode 55 and it will be detected as skipped.
The exercise scripts should only use the various OpenStack client binaries to interact with OpenStack. This specifically excludes any
*-manage
tools as those assume direct access to configuration and databases, as well as direct database access from the exercise itself.If specific configuration needs to be present for the exercise to complete, it should be staged in
stack.sh
, or called fromstack.sh
.The
OS_*
environment variables should be the only ones used for all authentication to OpenStack clients as documented in the CLIAuth wiki page.The exercise MUST clean up after itself if successful. If it is not successful, it is assumed that state will be left behind; this allows a chance for developers to look around and attempt to debug the problem. The exercise SHOULD clean up or graciously handle possible artifacts left over from previous runs if executed again. It is acceptable to require a reboot or even a re-install of DevStack to restore a clean test environment.
Bash Style Guidelines
DevStack defines a bash set of best practices for maintaining large collections of bash scripts. These should be considered as part of the review process.
DevStack uses the bashate style checker to enforce basic guidelines, similar to pep8 and flake8 tools for Python. The list below is not complete for what bashate checks, nor is it all checked by bashate. So many lines of code, so little time.
Whitespace Rules
- lines should not include trailing whitespace
- there should be no hard tabs in the file
- indents are 4 spaces, and all indentation should be some multiple of them
Control Structure Rules
- then should be on the same line as the if
- do should be on the same line as the for
Example:
if [[ -r $TOP_DIR/local.conf ]]; then
LRC=$(get_meta_section_files $TOP_DIR/local.conf local)
for lfile in $LRC; do
if [[ "$lfile" == "localrc" ]]; then
if [[ -r $TOP_DIR/localrc ]]; then
warn $LINENO "localrc and local.conf:[[local]] both exist, using localrc"
else
echo "# Generated file, do not edit" >$TOP_DIR/.localrc.auto
get_meta_section $TOP_DIR/local.conf local $lfile >>$TOP_DIR/.localrc.auto
fi
fi
done
fi
Variables and Functions
- functions should be used whenever possible for clarity
- functions should use
local
variables as much as possible to ensure they are isolated from the rest of the environment - local variables should be lower case, global variables should be upper case
- function names should_have_underscores, NotCamelCase.
- functions should be declared as per the regex ^function foo {$ with code starting on the next line
Review Criteria
There are some broad criteria that will be followed when reviewing your change
Is it passing tests -- your change will not be reviewed thoroughly unless the official CI has run successfully against it.
Does this belong in DevStack -- DevStack reviewers have a default position of "no" but are ready to be convinced by your change.
For very large changes, you should consider
the plugins system <plugins>
to see if your code is better abstracted from the main repository.For smaller changes, you should always consider if the change can be encapsulated by per-user settings in
local.conf
. A common example is adding a simple config-option to anini
file. Specific flags are not usually required for this, although adding documentation about how to achieve a larger goal (which might include turning on various settings, etc) is always welcome.Work-arounds -- often things get broken and DevStack can be in a position to fix them. Work-arounds are fine, but should be presented in the context of fixing the root-cause of the problem. This means it is well-commented in the code and the change-log and mostly likely includes links to changes or bugs that fix the underlying problem.
Should this be upstream -- DevStack generally does not override default choices provided by projects and attempts to not unexpectedly modify behavior.
Context in commit messages -- DevStack touches many different areas and reviewers need context around changes to make good decisions. We also always want it to be clear to someone -- perhaps even years from now -- why we were motivated to make a change at the time.
Reviewers -- please see
MAINTAINERS.rst
for a list of people that should be added to reviews of various sub-systems.
Making Changes, Testing, and CI
Changes to Devstack are tested by automated continuous integration jobs that run on a variety of Linux Distros using a handful of common configurations. What this means is that every change to Devstack is self testing. One major benefit of this is that developers do not typically need to add new non voting test jobs to add features to Devstack. Instead the features can be added, then if testing passes with the feature enabled the change is ready to merge (pending code review).
A concrete example of this was the switch from screen based service management to systemd based service management. No new jobs were created for this. Instead the features were added to devstack, tested locally and in CI using a change that enabled the feature, then once the enabling change was passing and the new behavior communicated and documented it was merged.
Using this process has been proven to be effective and leads to quicker implementation of desired features.