| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 1 | Contributing to DevStack | 
 | 2 | ======================== | 
 | 3 |  | 
 | 4 |  | 
 | 5 | General | 
 | 6 | ------- | 
 | 7 |  | 
| Dean Troyer | b8dd27b | 2013-10-17 12:03:55 -0500 | [diff] [blame] | 8 | DevStack is written in UNIX shell script.  It uses a number of bash-isms | 
| Dean Troyer | d97ee30 | 2015-02-04 12:35:39 -0600 | [diff] [blame] | 9 | and so is limited to Bash (version 4 and up) and compatible shells. | 
| Dean Troyer | b8dd27b | 2013-10-17 12:03:55 -0500 | [diff] [blame] | 10 | Shell script was chosen because it best illustrates the steps used to | 
 | 11 | set up and interact with OpenStack components. | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 12 |  | 
| Adrien Cunin | eaff3e1 | 2014-10-21 13:46:54 +0200 | [diff] [blame] | 13 | DevStack's official repository is located on git.openstack.org at | 
 | 14 | https://git.openstack.org/openstack-dev/devstack.  Besides the master branch that | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 15 | tracks the OpenStack trunk branches a separate branch is maintained for all | 
 | 16 | OpenStack releases starting with Diablo (stable/diablo). | 
 | 17 |  | 
| Dean Troyer | 6d04fd7 | 2012-12-21 11:03:37 -0600 | [diff] [blame] | 18 | Contributing code to DevStack follows the usual OpenStack process as described | 
 | 19 | in `How To Contribute`__ in the OpenStack wiki.  `DevStack's LaunchPad project`__ | 
| Roger Luethi | c21cfc9 | 2014-07-22 15:35:02 +0200 | [diff] [blame] | 20 | contains the usual links for blueprints, bugs, etc. | 
| Dean Troyer | 6d04fd7 | 2012-12-21 11:03:37 -0600 | [diff] [blame] | 21 |  | 
 | 22 | __ contribute_ | 
| Jeremy Stanley | 3eeed23 | 2014-12-05 03:30:41 +0000 | [diff] [blame] | 23 | .. _contribute: http://docs.openstack.org/infra/manual/developers.html | 
| Dean Troyer | 6d04fd7 | 2012-12-21 11:03:37 -0600 | [diff] [blame] | 24 |  | 
 | 25 | __ lp_ | 
 | 26 | .. _lp: https://launchpad.net/~devstack | 
 | 27 |  | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 28 | The primary script in DevStack is ``stack.sh``, which performs the bulk of the | 
 | 29 | work for DevStack's use cases.  There is a subscript ``functions`` that contains | 
 | 30 | generally useful shell functions and is used by a number of the scripts in | 
 | 31 | DevStack. | 
 | 32 |  | 
| Dean Troyer | cc6b443 | 2013-04-08 15:38:03 -0500 | [diff] [blame] | 33 | The ``lib`` directory contains sub-scripts for projects or packages that ``stack.sh`` | 
 | 34 | sources to perform much of the work related to those projects.  These sub-scripts | 
 | 35 | contain configuration defaults and functions to configure, start and stop the project | 
 | 36 | or package.  These variables and functions are also used by related projects, | 
 | 37 | such as Grenade, to manage a DevStack installation. | 
 | 38 |  | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 39 | A number of additional scripts can be found in the ``tools`` directory that may | 
| Dean Troyer | cc6b443 | 2013-04-08 15:38:03 -0500 | [diff] [blame] | 40 | be useful in supporting DevStack installations.  Of particular note are ``info.sh`` | 
| Adam Spiers | d903476 | 2013-10-04 23:20:24 +0100 | [diff] [blame] | 41 | to collect and report information about the installed system, and ``install_prereqs.sh`` | 
| Dean Troyer | cc6b443 | 2013-04-08 15:38:03 -0500 | [diff] [blame] | 42 | that handles installation of the prerequisite packages for DevStack.  It is | 
 | 43 | suitable, for example, to pre-load a system for making a snapshot. | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 44 |  | 
 | 45 |  | 
 | 46 | Scripts | 
 | 47 | ------- | 
 | 48 |  | 
 | 49 | DevStack scripts should generally begin by calling ``env(1)`` in the shebang line:: | 
 | 50 |  | 
 | 51 |     #!/usr/bin/env bash | 
 | 52 |  | 
 | 53 | Sometimes the script needs to know the location of the DevStack install directory. | 
 | 54 | ``TOP_DIR`` should always point there, even if the script itself is located in | 
 | 55 | a subdirectory:: | 
 | 56 |  | 
| Dean Troyer | b8dd27b | 2013-10-17 12:03:55 -0500 | [diff] [blame] | 57 |     # Keep track of the current DevStack directory. | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 58 |     TOP_DIR=$(cd $(dirname "$0") && pwd) | 
 | 59 |  | 
 | 60 | Many scripts will utilize shared functions from the ``functions`` file.  There are | 
 | 61 | also rc files (``stackrc`` and ``openrc``) that are often included to set the primary | 
 | 62 | configuration of the user environment:: | 
 | 63 |  | 
| Dean Troyer | b8dd27b | 2013-10-17 12:03:55 -0500 | [diff] [blame] | 64 |     # Keep track of the current DevStack directory. | 
| Dean Troyer | 51fb454 | 2012-03-09 22:21:59 -0600 | [diff] [blame] | 65 |     TOP_DIR=$(cd $(dirname "$0") && pwd) | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 66 |  | 
 | 67 |     # Import common functions | 
| Dean Troyer | 51fb454 | 2012-03-09 22:21:59 -0600 | [diff] [blame] | 68 |     source $TOP_DIR/functions | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 69 |  | 
 | 70 |     # Import configuration | 
| Dean Troyer | 51fb454 | 2012-03-09 22:21:59 -0600 | [diff] [blame] | 71 |     source $TOP_DIR/openrc | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 72 |  | 
 | 73 | ``stack.sh`` is a rather large monolithic script that flows through from beginning | 
| Dean Troyer | cc6b443 | 2013-04-08 15:38:03 -0500 | [diff] [blame] | 74 | to end.  It has been broken down into project-specific subscripts (as noted above) | 
 | 75 | located in ``lib`` to make ``stack.sh`` more manageable and to promote code reuse. | 
| Dean Troyer | 05f2365 | 2012-08-29 15:20:21 -0500 | [diff] [blame] | 76 |  | 
 | 77 | These library sub-scripts have a number of fixed entry points, some of which may | 
 | 78 | just be stubs.  These entry points will be called by ``stack.sh`` in the | 
 | 79 | following order:: | 
 | 80 |  | 
 | 81 |     install_XXXX | 
 | 82 |     configure_XXXX | 
 | 83 |     init_XXXX | 
 | 84 |     start_XXXX | 
 | 85 |     stop_XXXX | 
 | 86 |     cleanup_XXXX | 
 | 87 |  | 
 | 88 | There is a sub-script template in ``lib/templates`` to be used in creating new | 
 | 89 | service sub-scripts.  The comments in ``<>`` are meta comments describing | 
 | 90 | how to use the template and should be removed. | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 91 |  | 
| Dean Troyer | 6d04fd7 | 2012-12-21 11:03:37 -0600 | [diff] [blame] | 92 | In order to show the dependencies and conditions under which project functions | 
 | 93 | are executed the top-level conditional testing for things like ``is_service_enabled`` | 
 | 94 | should be done in ``stack.sh``.  There may be nested conditionals that need | 
 | 95 | to be in the sub-script, such as testing for keystone being enabled in | 
 | 96 | ``configure_swift()``. | 
 | 97 |  | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 98 |  | 
| Dean Troyer | 2b7ce5a | 2013-01-10 13:22:45 -0600 | [diff] [blame] | 99 | stackrc | 
 | 100 | ------- | 
 | 101 |  | 
 | 102 | ``stackrc`` is the global configuration file for DevStack.  It is responsible for | 
| Dean Troyer | b8dd27b | 2013-10-17 12:03:55 -0500 | [diff] [blame] | 103 | calling ``local.conf`` (or ``localrc`` if it exists) so local user configuration | 
 | 104 | is recognized. | 
| Dean Troyer | 2b7ce5a | 2013-01-10 13:22:45 -0600 | [diff] [blame] | 105 |  | 
 | 106 | The criteria for what belongs in ``stackrc`` can be vaguely summarized as | 
 | 107 | follows: | 
 | 108 |  | 
| Dean Troyer | b8dd27b | 2013-10-17 12:03:55 -0500 | [diff] [blame] | 109 | * All project repositories and branches handled directly in ``stack.sh`` | 
 | 110 | * Global configuration that may be referenced in ``local.conf``, i.e. ``DEST``, ``DATA_DIR`` | 
| Dean Troyer | 2b7ce5a | 2013-01-10 13:22:45 -0600 | [diff] [blame] | 111 | * Global service configuration like ``ENABLED_SERVICES`` | 
 | 112 | * Variables used by multiple services that do not have a clear owner, i.e. | 
| Daniel Genin | d470867 | 2014-10-31 15:01:29 -0400 | [diff] [blame] | 113 |   ``VOLUME_BACKING_FILE_SIZE`` (nova-compute, nova-volumes and cinder) or | 
 | 114 |   ``PUBLIC_NETWORK_NAME`` (nova-network and neutron) | 
| Dean Troyer | 2b7ce5a | 2013-01-10 13:22:45 -0600 | [diff] [blame] | 115 | * Variables that can not be cleanly declared in a project file due to | 
 | 116 |   dependency ordering, i.e. the order of sourcing the project files can | 
 | 117 |   not be changed for other reasons but the earlier file needs to dereference a | 
 | 118 |   variable set in the later file.  This should be rare. | 
 | 119 |  | 
| Dean Troyer | b8dd27b | 2013-10-17 12:03:55 -0500 | [diff] [blame] | 120 | Also, variable declarations in ``stackrc`` before ``local.conf`` is sourced | 
 | 121 | do NOT allow overriding (the form | 
 | 122 | ``FOO=${FOO:-baz}``); if they did then they can already be changed in ``local.conf`` | 
| Dean Troyer | 2b7ce5a | 2013-01-10 13:22:45 -0600 | [diff] [blame] | 123 | and can stay in the project file. | 
 | 124 |  | 
| Dean Troyer | cc6b443 | 2013-04-08 15:38:03 -0500 | [diff] [blame] | 125 |  | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 126 | Documentation | 
 | 127 | ------------- | 
 | 128 |  | 
| Dean Troyer | f5cb1ce | 2014-10-21 11:16:58 -0500 | [diff] [blame] | 129 | The DevStack repo now contains all of the static pages of devstack.org in | 
 | 130 | the ``doc/source`` directory. The OpenStack CI system rebuilds the docs after every | 
 | 131 | commit and updates devstack.org (now a redirect to docs.openstack.org/developer/devstack). | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 132 |  | 
 | 133 | All of the scripts are processed with shocco_ to render them with the comments | 
 | 134 | as text describing the script below.  For this reason we tend to be a little | 
 | 135 | verbose in the comments _ABOVE_ the code they pertain to.  Shocco also supports | 
 | 136 | Markdown formatting in the comments; use it sparingly.  Specifically, ``stack.sh`` | 
 | 137 | uses Markdown headers to divide the script into logical sections. | 
 | 138 |  | 
| Dean Troyer | b8dd27b | 2013-10-17 12:03:55 -0500 | [diff] [blame] | 139 | .. _shocco: https://github.com/dtroyer/shocco/tree/rst_support | 
 | 140 |  | 
 | 141 | The script used to drive <code>shocco</code> is <code>tools/build_docs.sh</code>. | 
| Dean Troyer | f5cb1ce | 2014-10-21 11:16:58 -0500 | [diff] [blame] | 142 | The complete docs build is also handled with <code>tox -edocs</code> per the | 
 | 143 | OpenStack project standard. | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 144 |  | 
 | 145 |  | 
 | 146 | Exercises | 
 | 147 | --------- | 
 | 148 |  | 
 | 149 | The scripts in the exercises directory are meant to 1) perform basic operational | 
 | 150 | checks on certain aspects of OpenStack; and b) document the use of the | 
 | 151 | OpenStack command-line clients. | 
 | 152 |  | 
 | 153 | In addition to the guidelines above, exercise scripts MUST follow the structure | 
 | 154 | outlined here.  ``swift.sh`` is perhaps the clearest example of these guidelines. | 
 | 155 | These scripts are executed serially by ``exercise.sh`` in testing situations. | 
 | 156 |  | 
 | 157 | * Begin and end with a banner that stands out in a sea of script logs to aid | 
 | 158 |   in debugging failures, particularly in automated testing situations.  If the | 
 | 159 |   end banner is not displayed, the script ended prematurely and can be assumed | 
 | 160 |   to have failed. | 
 | 161 |  | 
 | 162 |   :: | 
 | 163 |  | 
 | 164 |     echo "**************************************************" | 
 | 165 |     echo "Begin DevStack Exercise: $0" | 
 | 166 |     echo "**************************************************" | 
 | 167 |     ... | 
 | 168 |     set +o xtrace | 
 | 169 |     echo "**************************************************" | 
 | 170 |     echo "End DevStack Exercise: $0" | 
 | 171 |     echo "**************************************************" | 
 | 172 |  | 
 | 173 | * The scripts will generally have the shell ``xtrace`` attribute set to display | 
 | 174 |   the actual commands being executed, and the ``errexit`` attribute set to exit | 
 | 175 |   the script on non-zero exit codes:: | 
 | 176 |  | 
 | 177 |     # This script exits on an error so that errors don't compound and you see | 
| Joe Gordon | 4640026 | 2013-06-30 04:32:27 -0700 | [diff] [blame] | 178 |     # only the first error that occurred. | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 179 |     set -o errexit | 
 | 180 |  | 
 | 181 |     # Print the commands being run so that we can see the command that triggers | 
 | 182 |     # an error.  It is also useful for following allowing as the install occurs. | 
 | 183 |     set -o xtrace | 
 | 184 |  | 
| Dean Troyer | 51fb454 | 2012-03-09 22:21:59 -0600 | [diff] [blame] | 185 | * Settings and configuration are stored in ``exerciserc``, which must be | 
 | 186 |   sourced after ``openrc`` or ``stackrc``:: | 
 | 187 |  | 
 | 188 |     # Import exercise configuration | 
 | 189 |     source $TOP_DIR/exerciserc | 
 | 190 |  | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 191 | * There are a couple of helper functions in the common ``functions`` sub-script | 
 | 192 |   that will check for non-zero exit codes and unset environment variables and | 
 | 193 |   print a message and exit the script.  These should be called after most client | 
 | 194 |   commands that are not otherwise checked to short-circuit long timeouts | 
 | 195 |   (instance boot failure, for example):: | 
 | 196 |  | 
 | 197 |     swift post $CONTAINER | 
 | 198 |     die_if_error "Failure creating container $CONTAINER" | 
 | 199 |  | 
 | 200 |     FLOATING_IP=`euca-allocate-address | cut -f2` | 
 | 201 |     die_if_not_set FLOATING_IP "Failure allocating floating IP" | 
 | 202 |  | 
| Chmouel Boudjnah | 408b009 | 2012-03-15 23:21:55 +0000 | [diff] [blame] | 203 | * If you want an exercise to be skipped when for example a service wasn't | 
 | 204 |   enabled for the exercise to be run, you can exit your exercise with the | 
 | 205 |   special exitcode 55 and it will be detected as skipped. | 
 | 206 |  | 
| Dean Troyer | 07c3557 | 2012-03-05 07:15:30 -0600 | [diff] [blame] | 207 | * The exercise scripts should only use the various OpenStack client binaries to | 
 | 208 |   interact with OpenStack.  This specifically excludes any ``*-manage`` tools | 
 | 209 |   as those assume direct access to configuration and databases, as well as direct | 
 | 210 |   database access from the exercise itself. | 
 | 211 |  | 
 | 212 | * If specific configuration needs to be present for the exercise to complete, | 
 | 213 |   it should be staged in ``stack.sh``, or called from ``stack.sh`` (see | 
 | 214 |   ``files/keystone_data.sh`` for an example of this). | 
 | 215 |  | 
 | 216 | * The ``OS_*`` environment variables should be the only ones used for all | 
 | 217 |   authentication to OpenStack clients as documented in the CLIAuth_ wiki page. | 
 | 218 |  | 
 | 219 | .. _CLIAuth: http://wiki.openstack.org/CLIAuth | 
 | 220 |  | 
 | 221 | * The exercise MUST clean up after itself if successful.  If it is not successful, | 
 | 222 |   it is assumed that state will be left behind; this allows a chance for developers | 
 | 223 |   to look around and attempt to debug the problem.  The exercise SHOULD clean up | 
 | 224 |   or graciously handle possible artifacts left over from previous runs if executed | 
 | 225 |   again.  It is acceptable to require a reboot or even a re-install of DevStack | 
 | 226 |   to restore a clean test environment. | 
| Sean Dague | 6db2892 | 2013-11-22 12:16:02 -0500 | [diff] [blame] | 227 |  | 
 | 228 |  | 
 | 229 | Bash Style Guidelines | 
 | 230 | ~~~~~~~~~~~~~~~~~~~~~ | 
| Roger Luethi | 473b628 | 2014-04-13 13:47:42 +0200 | [diff] [blame] | 231 | DevStack defines a bash set of best practices for maintaining large | 
| Sean Dague | 6db2892 | 2013-11-22 12:16:02 -0500 | [diff] [blame] | 232 | collections of bash scripts. These should be considered as part of the | 
 | 233 | review process. | 
 | 234 |  | 
| Dean Troyer | f5cb1ce | 2014-10-21 11:16:58 -0500 | [diff] [blame] | 235 | DevStack uses the bashate_ style checker | 
 | 236 | to enforce basic guidelines, similar to pep8 and flake8 tools for Python. The | 
 | 237 | list below is not complete for what bashate checks, nor is it all checked | 
 | 238 | by bashate.  So many lines of code, so little time. | 
 | 239 |  | 
 | 240 | .. _bashate: https://pypi.python.org/pypi/bashate | 
| Sean Dague | 6db2892 | 2013-11-22 12:16:02 -0500 | [diff] [blame] | 241 |  | 
 | 242 | Whitespace Rules | 
 | 243 | ---------------- | 
 | 244 |  | 
 | 245 | - lines should not include trailing whitespace | 
 | 246 | - there should be no hard tabs in the file | 
 | 247 | - indents are 4 spaces, and all indentation should be some multiple of | 
 | 248 |   them | 
 | 249 |  | 
 | 250 | Control Structure Rules | 
 | 251 | ----------------------- | 
 | 252 | - then should be on the same line as the if | 
 | 253 | - do should be on the same line as the for | 
 | 254 |  | 
 | 255 | Example:: | 
 | 256 |  | 
 | 257 |   if [[ -r $TOP_DIR/local.conf ]]; then | 
 | 258 |       LRC=$(get_meta_section_files $TOP_DIR/local.conf local) | 
 | 259 |       for lfile in $LRC; do | 
 | 260 |           if [[ "$lfile" == "localrc" ]]; then | 
 | 261 |               if [[ -r $TOP_DIR/localrc ]]; then | 
 | 262 |                   warn $LINENO "localrc and local.conf:[[local]] both exist, using localrc" | 
 | 263 |               else | 
 | 264 |                   echo "# Generated file, do not edit" >$TOP_DIR/.localrc.auto | 
 | 265 |                   get_meta_section $TOP_DIR/local.conf local $lfile >>$TOP_DIR/.localrc.auto | 
 | 266 |               fi | 
 | 267 |           fi | 
 | 268 |       done | 
 | 269 |   fi | 
 | 270 |  | 
 | 271 | Variables and Functions | 
 | 272 | ----------------------- | 
 | 273 | - functions should be used whenever possible for clarity | 
 | 274 | - functions should use ``local`` variables as much as possible to | 
 | 275 |   ensure they are isolated from the rest of the environment | 
 | 276 | - local variables should be lower case, global variables should be | 
 | 277 |   upper case | 
 | 278 | - function names should_have_underscores, NotCamelCase. | 
| Ian Wienand | aee18c7 | 2014-02-21 15:35:08 +1100 | [diff] [blame] | 279 | - functions should be declared as per the regex ^function foo {$ | 
 | 280 |   with code starting on the next line |