Code review comment for lp:~ajkavanagh/charm-helpers/add-service-checks-lp1524388

Revision history for this message
David Ames (thedac) wrote :

Needs fixing:

We are going to want to set the state to 'blocked' which means user action is required. The state 'unknown' is reserved for before a state has been declared or for charms which never set state.

Hacking around a bit with your keystone example (I had to add servcies to the set_os_workload_status). When I used a simple ['keystone'] it fails:

    # set the status according to the current state of the contexts
    set_os_workload_status(
        configs, REQUIRED_INTERFACES, charm_func=check_optional_relations,
        services=['keystone'])

Traceback (most recent call last):
  File "hooks/update-status", line 654, in <module>
    main()
  File "hooks/update-status", line 650, in main
    assess_status(CONFIGS)
  File "/var/lib/juju/agents/unit-keystone-0/charm/hooks/keystone_utils.py", line 1845, in assess_status
    services=['keystone'])
  File "/var/lib/juju/agents/unit-keystone-0/charm/hooks/charmhelpers/contrib/openstack/utils.py", line 936, in set_os_workload_status
    _s = [s['service'] for s in services]
TypeError: string indices must be integers, not str

Using the full dictionary works beautifully.
    set_os_workload_status(
        configs, REQUIRED_INTERFACES, charm_func=check_optional_relations,
        services=[{'service': 'keystone', 'ports': [5000, 4990, 35357, 35347]}])

For discussion:

We need to have a discussion about the default ports for a service vrs our haproxy with non-default ports for the service. For example keystone's default is 5000 which haproxy listens for but our charm has the keystone service itself listen on 4990.

Is there anyway we can automatically "guess" these?

« Back to merge proposal