Nil config options not present during config-changed

[Michael Nelson]

I'm seeing a traceback [1] while deploying the apache charm in CI, which implies that during the config-changed hook context, the valid charm config option 'ssl_certlocation' is not known to juju.

The option has been explicitly set to '' empty string as a value (EDIT: turns out the yaml config is actually setting it explicitly to nil, not ''). Checking `juju config service` shows that the config option has been set correctly [2], but running `config-get` in a hook context shows the config option is missing [3] (rather than empty)- causing the error.

Comparing [3] with the actual options shows that the following options are all missing from config-get:

ssl_cert
ssl_certlocation
ssl_chain
ssl_chainlocation
ssl_key
ssl_keylocation

All of these options were given values of '' when the deployment happened.

With some debugging, we saw that setting the config item to an empty string failed, as juju already thought it had that value, but setting it to anything else caused it to then be available in config-get.

juju --version
2.0.3-trusty-amd64

[1] http://paste.ubuntu.com/24050566/
[2] http://paste.ubuntu.com/24050571/
[3] http://paste.ubuntu.com/24050668/

[Michael Nelson (michael.nelson)]

Unfortunately I can't reproduce this with the same charm locally using the lxd provider. I'll try in a scratch env on ps45.

[Michael Nelson (michael.nelson)]

...and unfortunately a small working example - deploying the same charm on the same cloud, but on its own, fails to reproduce the issue either:

http://paste.ubuntu.com/24051145/

I've hit it 2 from 2 though on CI. Sorry for the private link, but here's an example:

https://jenkins.canonical.com/is-mojo-ci/job/wip-ols-scasnap/49/console

[Michael Nelson (michael.nelson)]

OK, I needed to look into this again today (after my work-around had it's own issues), and was able to reproduce it both on the original cloud, as well as locally.

Details here [1] but it looks like the issue isn't the default values, but instead whether the deployment is configured to send empty values for some options - and if so - juju seems not to include those options at all in the unit's config-get, rather than including them with their set empty values.

[1] http://paste.ubuntu.com/24076192/

[Anastasia (anastasia-macmood)]

@Michael
I can see in the code that we are indeed omitting options that have keys but no value (nil) in config-get:
https://github.com/juju/juju/blob/develop/worker/uniter/runner/jujuc/config-get.go#L68
I'll try to find out if this was an oversight or if there was some logic guiding this decision.

This should not omit empty strings though "".

[Michael Nelson (michael.nelson)]

Thanks Anastasia - yeah, just confirmed it's fine with empty strings "" as the code indicates. So perhaps this is an intentional change with juju2 (that only surprised us because it worked fine on existing juju1 envs).

[Michael Nelson (michael.nelson)]

Though it still would seem incorrect - given that `juju config` reports the value as "" in this case (not nil), while the unit never sees the option at all:

http://paste.ubuntu.com/24076677/

[Anastasia (anastasia-macmood)]

@Michael Nelson (michael.nelson),
From the code above, it also looks like if you are requesting config with option all (i.e. either -a or --all), you will get all options including the ones with nil values. Does it not suit your needs?

by default, if not provided, "all" option is set to false and we omit keys with nil values :D

[Michael Nelson (michael.nelson)]

Possibly - I'm just using charmhelpers - so it could be that it should be using --all when getting the config. Still, as per http://paste.ubuntu.com/24076677/, if the value is nil shouldn't it be displayed as such in `juju config`?

Again, this only bit me on CI because it's different to juju1 behaviour - but I should be now unblocked.

[Michael Nelson (michael.nelson)]

@anastasia: Did you find out if there was a reason for this change in behaviour? If there is one, great, if not, I'm still not convinced this is a useful change from juju1 (where a config item is provided with its default value if a config.yaml didn't specify it, or specified it with a nil value). But if there's a good reason for the change, great.

[Anastasia (anastasia-macmood)]

@Michael Nelson (michael.nelson),
There was no change in behavior from Juju 1.x, specifically looked at 1.25x - the code was there already: https://github.com/juju/juju/blob/1.25/worker/uniter/runner/jujuc/config-get.go#L68

[Michael Nelson (michael.nelson)]

I think I mentioned a couple of times that I only hit this issue when testing an existing (passing) behaviour in a CI run with juju2? I think you'll find that although that piece of code didn't change, the behaviour did (perhaps in juju2 the units are passed a nil value, where as in juju1 they're passed the default). What I can be certain of is that in juju1, the 'nil' values from a config file are by default available with their default values in a hook context on the unit:

http://paste.ubuntu.com/24079777/

whereas with juju2, they are not (by default) - that's the change in behaviour that I'm asking about. Again, I'm unblocked so it's not a major concern of mine - I just want to be sure that the correct decision is made here. If the change in behaviour is because this is a feature which was always meant to be part of juju1 (as per the code) but it was a bug that it wasn't, and it's important to fix that now - great. If it's just happened by chance that the behaviour has been updated, I'd be warier of introducing a behavioural change (given that most charms will be expecting config-get to return all the available config options). But fine either way with me :)

[Anastasia (anastasia-macmood)]

@Michael Nelson (michael.nelson),
Since I am not seeing the change in Juju, is it possible that your testing is done on a different version of charm? What charm were using for 1.25 and 2.x testing?

[Michael Nelson (michael.nelson)]

Yep, sure - it's included that in the above pastes - both tested against the same version of the charm - cs:trusty/apache2-20. In juju1, the default value is provided to the charm in config-get, in juju2, it's not. I've not checked, but at a guess I'd be looking at the juju code which populates c.ctx.ConfigSettings() which is iterated by the config-get code.

[Anastasia (anastasia-macmood)]

PR against develop (2.2): https://github.com/juju/juju/pull/7055

@Michael Nelson (michael.nelson),
Thank you for highlighting this and patiently working with us through your example!

It has, indeed, highlighted an oversight that manifested as a regression in behavior. Unlike on 1.25, we have started storing application settings with nil values. And, although, it would have potentially gotten straighten out on the first settings update (since we delete settings with nil values then), it poorly manifested itself in "config-get": as you've observed, we've simply omitted settings with nil values instead of falling back to charm default.

For posterity, the best and proper way to specify that your application should not use charm default value for a setting is to set it to an empty string - either '' or "" - rather than nil.