Parameter definitions can be inconsistent

Bug #1700664 reported by Ben Nemec on 2017-06-26
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
tripleo
High
Ben Nemec

Bug Description

Many of our parameters are defined in multiple templates, but
currently there is no easy way of checking that all of those
definitions match. It can be confusing when a parameter is defined
one way in one file and another way in a different file. For example,
the NovaWorkers description is:

Number of workers for Nova API service.

and

Number of workers for Nova Placement API service.

and

Number of workers for Nova Conductor service.

Which is it actually? All of them. That one parameter controls
the workers for all of the nova services, and its description should
reflect that, no matter which template you happen to look at.

I'm opening this bug to track the work to clean this up. Expect a lot of patches to be linked to it. :-)

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

Changed in tripleo:
assignee: nobody → Ben Nemec (bnemec)
status: Triaged → In Progress

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

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

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

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

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

Reviewed: https://review.openstack.org/477695
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=f572b764f18cb3c6b54e24c1015a7e4dfcefd65f
Submitter: Jenkins
Branch: master

commit f572b764f18cb3c6b54e24c1015a7e4dfcefd65f
Author: Ben Nemec <email address hidden>
Date: Mon Jun 26 16:58:13 2017 -0500

    Add validation check for divergent parameter definitions

    Many of our parameters are defined in multiple templates, but
    currently there is no easy way of checking that all of those
    definitions match. It can be confusing when a parameter is defined
    one way in one file and another way in a different file. For example,
    the NovaWorkers description is:

    Number of workers for Nova API service.

    and

    Number of workers for Nova Placement API service.

    and

    Number of workers for Nova Conductor service.

    Which is it actually? All of them. That one parameter controls
    the workers for all of the nova services, and its description should
    reflect that, no matter which template you happen to look at.

    This change adds a check to yaml-validate.py to catch these sorts of
    inconsistencies and allow us to eventually prevent new ones from
    getting into the templates.

    An exclusion mechanism is included because there are some parameter
    definitions we probably can't/shouldn't change. In particular, this
    includes the network cidrs which are defaulted to ipv4 addresses in
    the ipv4 net-iso templates and ipv6 in the ipv6 templates. It's
    possible a user would be relying on one of those defaults in their
    configuration, so if we change it they might break.

    To get around that, the tool explicitly ignores the default field of
    those parameters, while still checking the description and type fields
    so we maintain some sanity. There may be other parameters where this
    is an issue, but those can be added later as they are found.

    For the moment any inconsistencies are soft-fails. A failure message
    will be printed, but the return value will not be affected so we can
    add the tool without first having to fix every divergent parameter
    definition in tripleo-heat-templates (and there appear to be plenty).
    This will allow us to gradually fix the parameters over time, and
    once that is done we can make this a hard-fail.

    Change-Id: Ib8b2cb5e610022d2bbcec9f2e2d30d9a7c2be511
    Partial-Bug: 1700664

Reviewed: https://review.openstack.org/477696
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=0db2831da60b0936b31109bf7bbf3ab5ceb7432d
Submitter: Jenkins
Branch: master

commit 0db2831da60b0936b31109bf7bbf3ab5ceb7432d
Author: Ben Nemec <email address hidden>
Date: Mon Jun 26 17:09:24 2017 -0500

    Make NovaWorkers descriptions consistent

    Partial-Bug: 1700664
    Change-Id: I12ee7ab825069c1741438499f8df835014afc37f

Reviewed: https://review.openstack.org/477697
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=bb291c5a9b5a74628c889b76c68f622f7ba31dd6
Submitter: Jenkins
Branch: master

commit bb291c5a9b5a74628c889b76c68f622f7ba31dd6
Author: Ben Nemec <email address hidden>
Date: Mon Jun 26 17:20:09 2017 -0500

    Make *AdminStateUp parameters consistent

    Change-Id: I1849663744dc1ce9aba8067201c03090796df8bb
    Partial-Bug: 1700664

Reviewed: https://review.openstack.org/477702
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=ccc37ba8613772542ba0de8b9c78d48f38c68ab2
Submitter: Jenkins
Branch: master

commit ccc37ba8613772542ba0de8b9c78d48f38c68ab2
Author: Ben Nemec <email address hidden>
Date: Mon Jun 26 17:35:24 2017 -0500

    Make CephValidationDelay/Retries default consistent

    Also fix one instance of ManagementIpSubnet that was missing a
    description.

    Change-Id: I7c5b31d9ef464cefee1dd6ae7ebb9c017cbbd894
    Partial-Bug: 1700664

Reviewed: https://review.openstack.org/477703
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=697d816cd840795579d32e31e3289390d381233f
Submitter: Jenkins
Branch: master

commit 697d816cd840795579d32e31e3289390d381233f
Author: Ben Nemec <email address hidden>
Date: Mon Jun 26 17:29:34 2017 -0500

    Make Rabbit parameters consistent

    The Qdr service appears to have hijacked these parameters for its
    own use. I don't think it should have done that in the first place,
    but at least the parameter descriptions need to be kept consistent
    with the other services.

    Partial-Bug: 1700664
    Change-Id: I6d9a075a99f33e9deacaf5b10a6ea7b0a234b942

Reviewed: https://review.openstack.org/477704
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=5867dd5f3227a6dbc263174da14909d6197e218e
Submitter: Jenkins
Branch: master

commit 5867dd5f3227a6dbc263174da14909d6197e218e
Author: Ben Nemec <email address hidden>
Date: Mon Jun 26 17:43:45 2017 -0500

    Make NeutronEnableDVR parameter consistent

    Change-Id: I4bc74ccfa9bd143b203dd9ad97dacddf56949727
    Partial-Bug: 1700664

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

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

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

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

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

Changed in tripleo:
assignee: Ben Nemec (bnemec) → Emilien Macchi (emilienm)

Reviewed: https://review.openstack.org/483536
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=8fb3da3c6016341d1f7158b9841db63de9b86fdf
Submitter: Jenkins
Branch: master

commit 8fb3da3c6016341d1f7158b9841db63de9b86fdf
Author: Ben Nemec <email address hidden>
Date: Tue Jun 27 10:01:31 2017 -0500

    Make EnablePackageInstall and Debug descriptions consistent

    Change-Id: I3ea7c0c7ea049043668e68c6e637fd2aaf992622
    Partial-Bug: 1700664

Reviewed: https://review.openstack.org/483537
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=c02a3436239df79ddecddde171b286f92aea2c10
Submitter: Jenkins
Branch: master

commit c02a3436239df79ddecddde171b286f92aea2c10
Author: Ben Nemec <email address hidden>
Date: Tue Jun 27 10:26:08 2017 -0500

    Make Deploy/UpdateIdentifier definition semi-consistent

    It seems UpdateIdentifier is an overloaded parameter - it is used
    both to trigger package updates in the minor update case as well as
    to trigger the upgrade steps during a major upgrade. I'm not sure
    it's appropriate to change either of the descriptions as a result,
    so for the moment that is added to the exclusion list.

    Change-Id: Ied36cf259f6a6e5c8cfa7a01722fb7fda6900976
    Partial-Bug: 1700664

Reviewed: https://review.openstack.org/483538
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=c54e9b681b44ab962c4503cf1d88c44b683a972e
Submitter: Jenkins
Branch: master

commit c54e9b681b44ab962c4503cf1d88c44b683a972e
Author: Ben Nemec <email address hidden>
Date: Tue Jun 27 11:01:50 2017 -0500

    Make various password descriptions consistent

    Since these are obviously global parameters they shouldn't specify
    what will be using them because they are used in multiple places.

    Change-Id: I5054c2d67dffe802e37f8391dd7bad4721e29831
    Partial-Bug: 1700664

Changed in tripleo:
assignee: Emilien Macchi (emilienm) → Ben Nemec (bnemec)

Reviewed: https://review.openstack.org/483539
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=4502b7cba6b86cfb041459dbcebe77219236ecaf
Submitter: Jenkins
Branch: master

commit 4502b7cba6b86cfb041459dbcebe77219236ecaf
Author: Ben Nemec <email address hidden>
Date: Tue Jun 27 11:07:52 2017 -0500

    Make RoleParameters and key_name descriptions consistent

    The key_name default is ignored because the parameter is used in
    some mutually exclusive environments where the default doesn't
    need to be the same.

    Change-Id: I77c1a1159fae38d03b0e59b80ae6bee491d734d7
    Partial-Bug: 1700664

Reviewed: https://review.openstack.org/483540
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=c05e72cd720b16383f5e4be8b774270f8fce6ca5
Submitter: Jenkins
Branch: master

commit c05e72cd720b16383f5e4be8b774270f8fce6ca5
Author: Ben Nemec <email address hidden>
Date: Thu Jul 13 13:14:51 2017 -0500

    Make many networking parameters consistent

    These are mostly the low hanging fruit that only required a few
    minor changes to fix. There are more that require a lot of changes
    or might be more controversial that will be done later.

    Change-Id: I55cebc92ef37a3bb167f5fae0debe77339395e62
    Partial-Bug: 1700664

Reviewed: https://review.openstack.org/484041
Committed: https://git.openstack.org/cgit/openstack/tripleo-heat-templates/commit/?id=7f84409a6a21ce09042d4923236d2b05251af3af
Submitter: Jenkins
Branch: master

commit 7f84409a6a21ce09042d4923236d2b05251af3af
Author: Ben Nemec <email address hidden>
Date: Fri Jul 14 15:36:56 2017 -0500

    Make UpgradeLevelNovaCompute parameters consistent

    There is logic in nova-base.yaml that depends on the default for
    this parameter being '', and the nova-compute service only needs it
    set to auto during upgrade. That will be done by [1] anyway, so it
    doesn't matter what the default is. It's also not clear to me that
    the nova-compute task is even needed now that we're post-Ocata, but
    that's not a change I feel comfortable making.

    1: https://github.com/openstack/tripleo-heat-templates/blob/master/environments/major-upgrade-composable-steps.yaml

    Change-Id: Iccfcb5b68e406db1b942375803cfedbb929b4307
    Partial-Bug: 1700664

Changed in tripleo:
milestone: queens-1 → queens-2
Changed in tripleo:
milestone: queens-2 → queens-3
Changed in tripleo:
milestone: queens-3 → queens-rc1
Changed in tripleo:
status: In Progress → Fix Released
Ben Nemec (bnemec) wrote :

This is not entirely fixed. There is still a significant list of inconsistent parameters in the check tool: https://github.com/openstack/tripleo-heat-templates/blob/6894552a0df1903dba3f0cf1510509bfed3de5cc/tools/yaml-validate.py#L87

Changed in tripleo:
status: Fix Released → In Progress
milestone: queens-rc1 → rocky-1
Changed in tripleo:
milestone: rocky-1 → rocky-2
Changed in tripleo:
milestone: rocky-2 → rocky-3
Changed in tripleo:
milestone: rocky-3 → rocky-rc1
Changed in tripleo:
milestone: rocky-rc1 → stein-1
Changed in tripleo:
milestone: stein-1 → stein-2
Changed in tripleo:
milestone: stein-2 → stein-3
Changed in tripleo:
status: In Progress → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers