Jump to: navigation, search

ConfigOptionsConsistency

Revision as of 12:49, 16 June 2016 by John Garbutt (talk | contribs) (fix_opt_description)

Maintaining consistency for Config options

This page explains the set of tasks we need to do to ensure the consistency for centralizing config options. Every file in nova/conf starts with the following comments at the top:

   # needs:fix_opt_description
   # needs:check_deprecation_status
   # needs:check_opt_group_and_type
   # needs:fix_opt_description_indentation
   # needs:fix_opt_registration_consistency

For review purposes, you should ideally submit 1 review per step per file.

fix_opt_description

"Services which consume this" section should be dropped from the opt description since this is going to be hard to maintain, and hard to review check.

Below template should be followed for opt description.

 help="""#A short description what it does. If it is a unit (e.g. timeout) 
# describe the unit which is used (seconds, megabyte, mebibyte, ...)

# A long description what the impact and scope is. The operators should
# know the expected change in the behavior of Nova if they tweak this.
 
Possible values:
 
# description of possible values. Especially if this is an option
# with numeric values (int, float), describe the edge cases (like the
# min value, max value, 0, -1).
  1. Omit this section if it is only duplicating the machine readable
  2. info in the option definition, such as for all boolean settings,
  3. unless the true/false values need describing for some reason.
Related options:

# Which other config options have to be considered when I change this
# one? If it stand solely on its own, use "None"
 
""")

check_deprecation_status

For options that are already deprecated, check if we are ready to remove them (at least one cycle, and at least three months of deprecation).

We need to look for configuration options that should be deprecated. For example, things that allow the loading of arbitry driver code should be moved to stevedoor extensions. Some configuration variables may be no longer used, and should be removed.

check_opt_group_and_type

If an option should really be part of an option group, such as a libvirt specific option that is in the default group, we should move that option into the group, using the deprecated group and name as required.

In a similar way we should look at the mutable=true flag, which says its safe to reload the configuration value when the service is sent a SIG_HUP. To be safe, an option must be rechecked each time. It cannot be cached, EG in a local var outside a long-running loop, or used to set something up. The former case can easily be fixed by checking the option direct from CONF each time around the loop. The latter case requires the use of a mutation hook, registered by CONF.register_mutate_hook, which repeats the setup. Examples:

fix_opt_description_indentation

Positive Example: Check enabled_opt in https://github.com/openstack/nova/blob/master/nova/conf/serial_console.py for opt description indentation.

Note: * should be to iterate items after a headline.

The best way to check the indentation is to look at the generated sample configuration file, and check it renders correctly, i.e. without any excessive indentation.

fix_opt_registration_consistency

  • Multiple options should be grouped into lists, not every option named.
Example: https://review.openstack.org/#/c/321470/1/nova/conf/availability_zone.py.         
  • One register_opts call per group, to match list_opts.
Example: https://review.openstack.org/#/c/321470/1/nova/conf/api.py
  • ALL_OPTS created by composing lots of lists.

Lists should be concatenated using using + for consistency. Some config files use list(itertools.chain to concatenate lists. This should be changed.

Example: Check ALL_OPTS in https://github.com/openstack/nova/blob/master/nova/conf/api.py     
  • Using string keys in list_opts until the oslo bug is fixed.

String keys for groups should be used in list_opts().

Example: https://github.com/openstack/nova/blob/master/nova/conf/neutron.py
                  NEUTRON_GROUP = 'neutron'
                 def list_opts():
                       return {
                           neutron_group: ALL_OPTS + _gen_opts_from_plugins()
                       }

This should be changed to:

                 def list_opts():
                       return {
                           "neutron": ALL_OPTS + _gen_opts_from_plugins()
                       }