Jump to: navigation, search

Difference between revisions of "ConfigOptionsConsistency"

(fix_opt_description)
(check_opt_group_and_type)
Line 49: Line 49:
 
=== check_opt_group_and_type ===
 
=== 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.
+
If an option should really be in a group, lets move that into the group. We do that by using the deprecated group and name as required. Don't forget the reno note!
 +
 
 +
Every option group should have a good title and help text set. For cases where there are lots of required options in the group, dependent on a particular setting like the virt driver setting, be sure to describe that in the group configuration help.
 +
 
 +
Check the type of the option, link Int or IP, and make sure you use things like string option choices, and int min values. For more info see: http://docs.openstack.org/developer/oslo.config/opts.html
 +
 
 +
=== Future stuff ===
 +
 
 +
==== mutable ====
  
 
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:
 
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:

Revision as of 14:37, 29 June 2016

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).
# Omit this section if it is only duplicating the machine readable
# info in the option definition, such as for all boolean settings,
# 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 there is nothing interesting, just omit this section.
 
""")

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 in a group, lets move that into the group. We do that by using the deprecated group and name as required. Don't forget the reno note!

Every option group should have a good title and help text set. For cases where there are lots of required options in the group, dependent on a particular setting like the virt driver setting, be sure to describe that in the group configuration help.

Check the type of the option, link Int or IP, and make sure you use things like string option choices, and int min values. For more info see: http://docs.openstack.org/developer/oslo.config/opts.html

Future stuff

mutable

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()
                       }