Status set multiple times in OpenStack charms with optional relations

I guess we're trying to minimise the changes to the full set of charms, but I think that this might not be avoidable.

The main issue, as David has investigated, is that the optional charm_func= function passed to assess_status call 'set_os_workload_status()' which sets the status, followed by 'assess_status()' doing the same thing after calling the charm_func.

The reason the optional charm_func sets the status is to discover what it will be prior to returning it to assess_status() which then sets it.

Obviously, this is a design failure. In charm_helpers, the consequence of the optional relations charm_func function, is that _determine_os_workload_status() is called TWICE, once before the optional relations, and then afterwards as part of the optional relations.

The fix is to change optional_relations such that it returns a dictionary of the possible optional interfaces that can be dynamically added to the required_interfaces, which is then passed to assess_status() so that _determine_os_workload_status() [in charmhelpers] is only called once. I'll do this on glance and propose it for review.

Related fix proposed to branch: master
Review: https://review.openstack.org/325650

Reviewed: https://review.openstack.org/325650
Committed: https://git.openstack.org/cgit/openstack/charm-glance/commit/?id=1ab2c809ad7251f6d8a30c053d8c7818ac051547
Submitter: Jenkins
Branch: master

commit 1ab2c809ad7251f6d8a30c053d8c7818ac051547
Author: Alex Kavanagh <email address hidden>
Date: Sun Jun 5 12:57:53 2016 +0000

    Fix for status-set race - related to bug 1588462

    This change fixes the obvious race for a status_set() between
    check_optional_interfaces() and assess_status() as the later calls the former
    which calls status_set(), returns the status, which is then potentially set
    again by the assess_status() function. This cleans up the code so that only a
    single status_set() is performed when calling assess_status().

    Change-Id: I06efbcaade8f0c99b5931104e6887d24cb10e35a
    Related-Bug:#1588462

Related fix proposed to branch: master
Review: https://review.openstack.org/329219

Reviewed: https://review.openstack.org/329219
Committed: https://git.openstack.org/cgit/openstack/charm-nova-cloud-controller/commit/?id=b570e36cba93affe67594ef820afae731b746ad7
Submitter: Jenkins
Branch: master

commit b570e36cba93affe67594ef820afae731b746ad7
Author: David Ames <email address hidden>
Date: Mon Jun 13 15:55:40 2016 -0700

    Fix for status-set race - related to bug 1588462

    This change fixes the obvious race for a status_set() between
    check_optional_interfaces() and assess_status() as the later calls the former
    which calls status_set(), returns the status, which is then potentially set
    again by the assess_status() function. This cleans up the code so that only a
    single status_set() is performed when calling assess_status().

    Change-Id: Ie37a4d98de9c5e7bd26304e096796ce6287ea52b
    Related-Bug:#1588462

Related fix proposed to branch: master
Review: https://review.openstack.org/329879

Related fix proposed to branch: master
Review: https://review.openstack.org/329916

Related fix proposed to branch: master
Review: https://review.openstack.org/329959

Reviewed: https://review.openstack.org/329916
Committed: https://git.openstack.org/cgit/openstack/charm-cinder/commit/?id=734fb80b08e3fba6249ce911baa068a8eda65a7a
Submitter: Jenkins
Branch: master

commit 734fb80b08e3fba6249ce911baa068a8eda65a7a
Author: Alex Kavanagh <email address hidden>
Date: Wed Jun 15 13:10:31 2016 +0000

    Fix for multiple status-set - related to bug 1588462

    This change fixes the obvious race for a status_set() between
    check_optional_interfaces() and assess_status() as the later calls the former
    which calls status_set(), returns the status, which is then potentially set
    again by the assess_status() function. This cleans up the code so that only a
    single status_set() is performed when calling assess_status().

    Change-Id: I3604ed3b36af91afb95d43e0e5e24483c9919fd1
    Related-Bug:#1588462

Reviewed: https://review.openstack.org/329959
Committed: https://git.openstack.org/cgit/openstack/charm-keystone/commit/?id=61047ac055fa79d1d8142f51612088bac042155f
Submitter: Jenkins
Branch: master

commit 61047ac055fa79d1d8142f51612088bac042155f
Author: Alex Kavanagh <email address hidden>
Date: Wed Jun 15 14:05:01 2016 +0000

    Fix for multiple status-set - related to bug 1588462

    This change fixes the obvious race for a status_set() between
    check_optional_interfaces() and assess_status() as the later calls the former
    which calls status_set(), returns the status, which is then potentially set
    again by the assess_status() function. This cleans up the code so that only a
    single status_set() is performed when calling assess_status().

    Change-Id: I928f60967e4a7588df2b25136525391c283cda14
    Related-Bug:#1588462

Related fix proposed to branch: master
Review: https://review.openstack.org/330446

Related fix proposed to branch: master
Review: https://review.openstack.org/330461

Reviewed: https://review.openstack.org/330446
Committed: https://git.openstack.org/cgit/openstack/charm-neutron-api/commit/?id=638e06de085ea156cbed060b1c965d617fe9fdc6
Submitter: Jenkins
Branch: master

commit 638e06de085ea156cbed060b1c965d617fe9fdc6
Author: Alex Kavanagh <email address hidden>
Date: Thu Jun 16 10:35:21 2016 +0000

    Fix for multiple status-set - related to bug 1588462

    This change fixes the obvious race for a status_set() between
    check_optional_interfaces() and assess_status() as the later calls the former
    which calls status_set(), returns the status, which is then potentially set
    again by the assess_status() function. This cleans up the code so that only a
    single status_set() is performed when calling assess_status().

    Change-Id: Ic5d0be6e1f7a2283e4dd0594c6465a99497dbbec
    Related-Bug:#1588462

Reviewed: https://review.openstack.org/329879
Committed: https://git.openstack.org/cgit/openstack/charm-ceph-radosgw/commit/?id=36ee1afc8dde730b719382ee1331bffac6f58c37
Submitter: Jenkins
Branch: master

commit 36ee1afc8dde730b719382ee1331bffac6f58c37
Author: Alex Kavanagh <email address hidden>
Date: Wed Jun 15 11:27:31 2016 +0000

    Fix for multiple status-set - related to bug 1588462

    This change fixes the obvious race for a status_set() between
    check_optional_interfaces() and assess_status() as the later calls the former
    which calls status_set(), returns the status, which is then potentially set
    again by the assess_status() function. This cleans up the code so that only a
    single status_set() is performed when calling assess_status().

    Change-Id: Idb11019cec20061622b5f36911e49adfb9bac14e
    Related-Bug:#1588462

Reviewed: https://review.openstack.org/330461
Committed: https://git.openstack.org/cgit/openstack/charm-neutron-gateway/commit/?id=0322be6cc64f9d2c108095c77f5fd18df9c88fc0
Submitter: Jenkins
Branch: master

commit 0322be6cc64f9d2c108095c77f5fd18df9c88fc0
Author: Alex Kavanagh <email address hidden>
Date: Thu Jun 16 10:55:05 2016 +0000

    Fix for multiple status-set - related to bug 1588462

    This change fixes the obvious race for a status_set() between
    check_optional_interfaces() and assess_status() as the later calls the former
    which calls status_set(), returns the status, which is then potentially set
    again by the assess_status() function. This cleans up the code so that only a
    single status_set() is performed when calling assess_status().

    Change-Id: I05e071d635819c516ab76684842a25a9c2f81d83
    Related-Bug:#1588462

The initially listed charms have been checked, and the rest of the charms reviewed for the multiple status_set issue:

charm-ceilometer - OK
charm-ceilometer-agent - OK
charm-ceph - different implementation - OK
charm-ceph-mon - different implementation - OK
charm-ceph-osd - different implementation - OK
charm-ceph-radosgw - DONE
charm-cinder - DONE
charm-cinder-backup - no implementation of assess_status - OK
charm-cinder-ceph - no implementation of assess_status - OK
charm-glance - DONE
charm-hacluster - no implementation of assess_status - OK
charm-heat - no implementation of assess_status - OK
charm-keystone - DONE
charm-lxd - different implementation of assess_status - OK
charm-neutron-api - DONE
charm-neutron-api-odl - no implementation of assess_status - OK
charm-neutron-gateway - DONE
charm-neutron-openvswitch - has a slightly different implementation - OK
charm-nova-cloud-controller - DONE
charm-nova-compute - has a slightly different implementation - OK
charm-odl-controller - no implementation of assess_status - OK
charm-openstack-dashboard - no optional interfaces - OK
charm-openvswitch-odl - reactive charm with no assess_status implementation - OK
charm-percona-cluster - has different implementation - not affected - OK
charm-rabbitmq-server - Not OK - has different multiple-status-set problem !!!
charm-swift-proxy - OK - but needs an edit to fix bad function names
charm-swift-storage - has simple implementation of assess_status - OK
charm-tempest - new, so has no assess_status() implementation yet. - OK

This pulls out that the rabbitmq-server charm needs to be resolved as well.

Related fix proposed to branch: master
Review: https://review.openstack.org/331150

Reviewed: https://review.openstack.org/331150
Committed: https://git.openstack.org/cgit/openstack/charm-rabbitmq-server/commit/?id=406c8b69a3d71f2088bb9918831b5fe88c78c376
Submitter: Jenkins
Branch: master

commit 406c8b69a3d71f2088bb9918831b5fe88c78c376
Author: Alex Kavanagh <email address hidden>
Date: Fri Jun 17 13:53:34 2016 +0000

    Fix for multiple status_set() in assess_status()

    This fixes a multiple status_set() bug in the implementation of assess_status()
    on the charm, when it is determining whether it is active AND clustered.
    The change hooks into the _determine_os_workload_status() directly in
    charmhelpers to ensure that status_set() is only called once when
    assess_status() is called.

    Change-Id: Idfd93126c3edb41f83897c82ee7f20fe5f366559
    Related-Bug:#1588462