config generator generates invalid configuration file

[Sergey Vilgelm]

The cinder's config generator generates an invalid configuration file. It put the options of DEFAULT section from external libraries to the last generated section of cinder.

An example:

#
# Options defined in cinder.....
#

....

#hash_algorithms=md5

[profiler]

#
# Options defined in cinder.service
#

# If False fully disable profiling feature. (boolean value)
#profiler_enabled=false

# If False doesn't trace SQL requests. (boolean value)
#trace_sqlalchemy=false

#
# From oslo.log
#

# Print debugging output (set logging level to DEBUG instead of
# default WARNING level). (boolean value)
#debug = false

# Print more verbose output (set logging level to INFO instead of
# default WARNING level). (boolean value)
#verbose = false

As we can see, the options from oslo.log[1] are placed in the [profiler] section, but they should be in the [DEFAULT].
I propose to switch to using the oslo-config-generator.

[1] https://github.com/openstack/oslo.log/blob/master/oslo_log/_options.py#L153

[Jay Bryant (jsbryant)]

Sergey,

Do you have a fix for this in progress, or something you are modeling it after. I had started with this patch in the last release and missed the Window for getting it in: https://review.openstack.org/#/c/165431/ That first attempt at a design was not well received.

I was going to some checking to see how others had solved this. Appreciate any input you can give on your plan.

Thanks!
Jay

[Sergey Vilgelm (sergey.vilgelm)]

Hi Jay,
Yes, I have created the fix, but i think there are some problems, we have to separate the common cinder options and the options specific to the drivers, because some drivers are declaring the same options and the configuration file will have the duplicates.
For example:
    zfssa_rest_timeout is declared in the two modules:
         https://github.com/openstack/cinder/blob/master/cinder/volume/drivers/zfssa/zfssaiscsi.py#L71
         https://github.com/openstack/cinder/blob/master/cinder/volume/drivers/zfssa/zfssanfs.py#L52

Maybe we have to include only common options with the default driver and create special entry points for the options of the other drivers. What do you think?

[Sergey Vilgelm (sergey.vilgelm)]

The last output of the compare_configs script:

NEW: ('DEFAULT', 'cb_add_qosgroup') -> ['graceallowed:false,iops:10,iopscontrol:true,latency:15,memlimit:0,networkspeed:0,throughput:0,tpcontrol:false']
OLD ('DEFAULT', 'cb_add_qosgroup') -> ['latency:15,iops:10,graceallowed:false,iopscontrol:true,memlimit:0,throughput:0,tpcontrol:false,networkspeed:0']
NEW: ('DEFAULT', 'cb_create_volume') -> ['blocklength:512B,compression:off,deduplication:off,protocoltype:ISCSI,recordsize:16k,sync:always']
OLD ('DEFAULT', 'cb_create_volume') -> ['compression:off,deduplication:off,blocklength:512B,sync:always,protocoltype:ISCSI,recordsize:16k']
NEW: ('DEFAULT', 'zfssa_rest_timeout') -> ['<None>', '<None>']
OLD ('DEFAULT', 'zfssa_rest_timeout') ->
NOTE: Duplicated options?
NEW: ('DEFAULT', 'my_ip') -> ['192.168.1.78']
OLD ('DEFAULT', 'my_ip') -> ['10.0.0.1']
NEW: ('DEFAULT', 'host') -> ['vilgelm.local']
OLD ('DEFAULT', 'host') -> ['cinder']
NEW: ('cors', 'allow_methods') -> ['GET,POST,PUT,DELETE,OPTIONS', 'GET,POST,PUT,DELETE,OPTIONS']
OLD ('cors', 'allow_methods') ->
NOTE: Duplicated options?
NEW: ('cors', 'allowed_origin') -> ['<None>', '<None>']
OLD ('cors', 'allowed_origin') ->
NOTE: Duplicated options?
NEW: ('cors', 'allow_headers') -> ['Content-Type,Cache-Control,Content-Language,Expires,Last-Modified,Pragma', 'Content-Type,Cache-Control,Content-Language,Expires,Last-Modified,Pragma']
OLD ('cors', 'allow_headers') ->
NOTE: Duplicated options?
NEW: ('cors', 'max_age') -> ['3600', '3600']
OLD ('cors', 'max_age') ->
NOTE: Duplicated options?
NEW: ('cors', 'expose_headers') -> ['Content-Type,Cache-Control,Content-Language,Expires,Last-Modified,Pragma', 'Content-Type,Cache-Control,Content-Language,Expires,Last-Modified,Pragma']
OLD ('cors', 'expose_headers') ->
NOTE: Duplicated options?
NEW: ('cors', 'allow_credentials') -> ['true', 'true']
OLD ('cors', 'allow_credentials') ->
NOTE: Duplicated options?

[Sergey Vilgelm (sergey.vilgelm)]

Please, forget my last message :) I had a bug in the my script
This is correct output:

NEW: ('DEFAULT', 'cb_add_qosgroup') -> ['graceallowed:false,iops:10,iopscontrol:true,latency:15,memlimit:0,networkspeed:0,throughput:0,tpcontrol:false']
OLD ('DEFAULT', 'cb_add_qosgroup') -> ['latency:15,iops:10,graceallowed:false,iopscontrol:true,memlimit:0,throughput:0,tpcontrol:false,networkspeed:0']
NEW: ('DEFAULT', 'cb_create_volume') -> ['blocklength:512B,compression:off,deduplication:off,protocoltype:ISCSI,recordsize:16k,sync:always']
OLD ('DEFAULT', 'cb_create_volume') -> ['compression:off,deduplication:off,blocklength:512B,sync:always,protocoltype:ISCSI,recordsize:16k']
NEW: ('DEFAULT', 'zfssa_rest_timeout') -> ['<None>', '<None>']
OLD ('DEFAULT', 'zfssa_rest_timeout') ->
NOTE: Duplicated options?
NEW: ('DEFAULT', 'my_ip') -> ['192.168.1.78']
OLD ('DEFAULT', 'my_ip') -> ['10.0.0.1']
NEW: ('DEFAULT', 'host') -> ['vilgelm.local']
OLD ('DEFAULT', 'host') -> ['cinder']

[Sergey Vilgelm (sergey.vilgelm)]

And yes, the main important thing. The link: https://review.openstack.org/#/c/200567/

[Jay Bryant (jsbryant)]

Thanks Sergey. I updated the review with some more info.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Sergey Vilgelm (<email address hidden>) on branch: master
Review: https://review.openstack.org/200567
Reason: The cinder team decided to find the way for generating the config files more dynamically

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/219700

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/219700
Committed: https://git.openstack.org/cgit/openstack/cinder/commit/?id=111a056c0f212ed38ff6127f6c9b9dc13fd536f0
Submitter: Jenkins
Branch: master

commit 111a056c0f212ed38ff6127f6c9b9dc13fd536f0
Author: Kendall Nelson <email address hidden>
Date: Thu Aug 13 10:17:36 2015 -0500

    Dynamically create cinder.conf.sample

    As it stands, the opts.py file that is passed into
    oslo-config-generator isn't being generated dynamically
    and the old way of generating the cinder.conf.sample is
    dependent on oslo-incubator which Cinder is trying to
    move away from. oslo-config-generator works differently
    than oslo-incubator so a number of changes had to be made
    in order to make this switch.

    This patch adds the config directory to Cinder and in it
    are two files:

        -generate_cinder_opts.py that will take the
         results of a grep command to create the opts.py
         file to be passed into oslo-config-generator.

        -cinder.conf which is the new configuration for
         oslo-config-generator. The file is inside the config
         directory to be consistent with other projects.

    Some changes were made to the generate_sample.sh file in
    order to give the base directories and target directories
    to the generate_cinder_opts.py program.

    tox.ini was edited to remove the checkonly option because
    all that needs to happen in check_uptodate.sh is a check to
    ensure that the cinder.conf.sample is actually being
    generated with no issues.

    All options were removed from the check_uptodate.sh
    because they were unnecessary given the new, more simple
    way of generating the cinder.conf.sample.

    setup.cfg was also edited in order to add information
    oslo-config-generator needs to run.

    Co-Authored By: Jay Bryant <email address hidden>
    Co-Authored By: Jacob Gregor <email address hidden>

    Change-Id: I643dbe5675ae9280e204f691781e617266f570d5
    Closes-Bug: 1473768
    Closes-Bug: 1437904
    Closes-Bug: 1381563