quickstart delployments broken in 1.24

[Curtis Hovey]

All quickstart jobs deploying HA Proxy broke.
    http://reports.vapour.ws/releases/3052/job/aws-quickstart-bundle/attempt/1002
    http://reports.vapour.ws/releases/3052/job/hp-quickstart-bundle/attempt/1141
    http://reports.vapour.ws/releases/3052/job/joyent-quickstart-bundle/attempt/1367
    http://reports.vapour.ws/releases/3052/job/maas17-quickstart-bundle/attempt/1085

All due to:

haproxy:
    charm: cs:trusty/haproxy-1
    exposed: false
    service-status:
      current: error
      message: 'hook failed: "config-changed"'

From the unit log:

<http://data.vapour.ws/juju-ci/products/version-3052/aws-quickstart-bundle/build-1002/machine-2/unit-haproxy-0.log>

... INFO config-changed Traceback (most recent call last):
... INFO config-changed File "/var/lib/juju/agents/unit-haproxy-0/charm/hooks/config-changed", line 1019, in <module>
... INFO config-changed main(hook_name)
... INFO config-changed File "/var/lib/juju/agents/unit-haproxy-0/charm/hooks/config-changed", line 985, in main
... INFO config-changed config_changed()
... INFO config-changed File "/var/lib/juju/agents/unit-haproxy-0/charm/hooks/config-changed", line 777, in config_changed
... INFO config-changed if not create_services():
... INFO config-changed File "/var/lib/juju/agents/unit-haproxy-0/charm/hooks/config-changed", line 470, in create_services
... INFO config-changed services_dict = get_config_services()
... INFO config-changed File "/var/lib/juju/agents/unit-haproxy-0/charm/hooks/config-changed", line 370, in get_config_services
... INFO config-changed return parse_services_yaml(services, config_data['services'])
... INFO config-changed KeyError: 'services'
... ERROR juju.worker.uniter.operation runhook.go:103 hook "config-changed" failed: exit status 1

[Curtis Hovey (sinzui)]

The three suspect commits are
    https://github.com/juju/juju/commit/bb38bf13c00283f572f05139428961b259275079
    https://github.com/juju/juju/commit/01bab327cfc3d53df5626da07c47298d350718a5
    https://github.com/juju/juju/commit/629fc0507c4b61be93fd79453244cb8fc853f413

[Eric Snow (ericsnowcurrently)]

The logs lead to the config-changed hook. In the hook code [1] it looks like one of the expected config options is not set. [2] This implies that one of those 3 changes is causing settings to not be set.

Thus, I expect that https://github.com/juju/juju/commit/629fc0507c4b61be93fd79453244cb8fc853f413 is the culprit. It changed how settings are set for charms.

Offhand, I wonder if it's related to how default setting values are handled...

[1] "return parse_services_yaml(services, config_data['services'])" in https://api.jujucharms.com/charmstore/v4/trusty/haproxy-13/archive/hooks/hooks.py
[2] "services" in https://api.jujucharms.com/charmstore/v4/trusty/haproxy-13/archive/config.yaml

[Eric Snow (ericsnowcurrently)]

FTR, the bundle looks like this one:

  https://api.jujucharms.com/charmstore/v4/~landscape/bundle/landscape-scalable-9/archive/bundle.yaml

Related resources:

* http://bazaar.launchpad.net/~juju-qa/juju-ci-tools/trunk/view/head:/quickstart_deploy.py
* http://bazaar.launchpad.net/~juju-qa/juju-ci-tools/trunk/view/head:/jujupy.py#L382
* https://jujucharms.com/haproxy/trusty/13
* https://api.jujucharms.com/charmstore/v4/trusty/haproxy-13/archive/config.yaml
* http://bazaar.launchpad.net/~charmers/charms/trusty/haproxy/trunk/view/head:/hooks/hooks.py#L468
* http://bazaar.launchpad.net/~juju-gui/juju-quickstart/trunk/view/head:/quickstart/manage.py#L648
* http://bazaar.launchpad.net/~juju-deployers/python-jujuclient/trunk/view/head:/jujuclient.py#L306
* https://jujucharms.com/u/landscape/landscape-scalable/9

[Eric Snow (ericsnowcurrently)]

Also:

* https://github.com/juju/charm/blob/v5/config.go
* https://github.com/juju/juju/blob/1.24/state/settings.go
* https://github.com/juju/juju/blob/1.24/state/service.go
* https://github.com/juju/juju/blob/1.24/juju/deploy.go

[Eric Snow (ericsnowcurrently)]

Also:

* https://github.com/juju/juju/blob/master/apiserver/service/service.go#L159

[Eric Snow (ericsnowcurrently)]

natefinch was able to reproduce the failure by manually running the quickstart_deploy.py script. He is going to follow up by running it again after reverting the suspect merge (629fc0507c4b).

Also, I've been unable to identify any semantic difference before and after 629fc0507c4b. I've added a test to check this in my fix branch. [1] The results of Nate's follow-up will help settle whether or not the suspect merge is at fault.

[1] https://github.com/ericsnowcurrently/juju/tree/fix-service-settings-1495681

[Eric Snow (ericsnowcurrently)]

mgz verified that the tests pass if 629fc0507c4b is reverted. So something is subtly different. We still need to determine what that is. It certainly isn't obvious to me at this point, so I'll keep at it.

[Eric Snow (ericsnowcurrently)]

Hmm. The API converts empty string config setting values into nil values. [1]

[1] https://github.com/juju/juju/blob/master/apiserver/service/service.go#L209

[Eric Snow (ericsnowcurrently)]

Yep, that's it! I should have a fix up shortly.

[Eric Snow (ericsnowcurrently)]

http://reviews.vapour.ws/r/2679/

[Canonical Juju QA Bot (juju-qa-bot)]

Juju-CI verified that this issue is Fix Released in juju-core 1.24:
    http://reports.vapour.ws/releases/3064