Bug: ini values need to be python booleans

[jmatsushita]

We have a few places in ini files where we use lower case booleans, which won't work as expected:

• https://github.com/wireapp/wire-server-deploy/blob/develop/ansible/inventory/prod/hosts.example.ini#L148
• https://github.com/wireapp/wire-server-deploy/blob/develop/ansible/inventory/prod/hosts.example.ini#L155

Not sure what happens with yml files but we're not consistent:

• https://github.com/wireapp/wire-server-deploy/blob/develop/ansible/inventory/offline/group_vars/all/offline.yml#L50
• https://github.com/wireapp/wire-server-deploy/blob/develop/ansible/inventory/offline/group_vars/all/offline.yml#L46

[jmatsushita]

I suppose

 when: not (offline|default(false))

in https://github.com/wireapp/wire-server-deploy/pull/468/files#diff-bde0839bd8eb7f9d9cd0c74de702d90fec560b0ab57f74658639de4d4d1e3231R18

Also needs to be False.

[jmatsushita]

cc @arianvp

[arthurwolf]

So this is as simple as "find all lowercase booleans and uppercase them", or am I missing any nuance here?

[tjanson]

If I may: unfortunately boolean parsing in Ansible is a wildly inconsistent mess, so it's best to assume the worst at the place of consumption, i.e., handle conversion from string.

That way you also don't get thrown off when a var is set as string via passing --extra-args via CLI.

From docs:

• If a variable value set in an INI inventory must be a certain type (for example, a string or a boolean value), always specify the type with a filter in your task. Do not rely on types set in INI inventories when consuming variables.

• Consider using YAML format for inventory sources to avoid confusion on the actual type of a variable. The YAML inventory plugin processes variable values consistently and correctly.

(Is there a reason for the hosts file being ini-style?)

[arianvp]

(Is there a reason for the hosts file being ini-style?)

No; we could very well convert it to .yaml style and then just worry about 1 layer of footguns instead of 2. Food for thought

[arthurwolf]

If lowercase false are an issue (including as strings), then there are a lot of issues around

arthur@debian-wire:~/dev/wire-server-deploy$ ack True | wc -l
8
arthur@debian-wire:~/dev/wire-server-deploy$ ack true | wc -l
125
arthur@debian-wire:~/dev/wire-server-deploy$ ack False | wc -l
6
arthur@debian-wire:~/dev/wire-server-deploy$ ack false | wc -l
87

A few of those are in text/sentences, but most are actual values, see pastebin:

https://pastebin.com/u50b7aG3

I can make a PR changing things around, but I need some sort of clear rule on exactly what to do.

Is it really as simple as finding in all ini and yaml files, all lowercase boolean values (including strings), and replacing them with non-string uppercase booleans of equal value ? If it is, tell me and I'll just do that.

[arthurwolf]

At this point, i don't have clear instructions.

Do I just change the few values in the original comment above 15 days ago, or do I do a more general edit of all values that match all around all files, following a clear rule (and if so, what is that rule?).

I can do a PR easily as soon as I know what to do, which I don't yet.

[jmatsushita]

@arthurwolf as I see it, there are several possible follow up tasks from here, sorted below from smaller scope to larger scope.
0. Familiarise yourself with ansible boolean parsing (from ini files, from yaml files, in playbooks/tasks). Could be that convincing yourself that passing false instead of False does break things (its not obvious since failing to parse will sometimes be parsed as false).

1. Focus on the initial issue, identifying in ini files (including comments that are meant to be uncommented) lower case false, and change them to capitalised False.
2. In the initial issue I mention yaml files. Find what should be the capitalisation there and make sure we're consistent across our inventory group_vars.
3. In my follow up comment I also identified that some ansible tasks we wrote (not config files like 1. and 2.) might have improperly capitalised booleans too. Find them and fix them too.
4. Now as @tjanson suggested, the best practice is to check for possible type errors at the use site in ansible tasks (using filters, which you'll need to find more about), since we can't expect the world (when copy pasting our inventory or writing their own) to not make mistakes. So find out about how to filter for badly typed input, in the places we use inventory configuration values. Since we also use external playbooks, which may or may not implement this best practice, we might have control over only some of the parsing, but if we are thorough and defensive, we could filter in our wrapper playbooks.
5. Finally, when we feel we have caught all the possible weird booleans the world could throw at us, and you've become a master of ansible type safety, then we could as @arianvp suggested remove ini files from our ansible code, and convert them to yaml, so that we have to think only about one type of inputs format for our configuration which is slightly less error prone than ini files (while congratulating ourselves that we've made our playbooks more type safe in the case our users choose to keep their ini files or pass --extra-args via the command line).

I'd advise you start from the top to familiarise yourself with the issue, submit a PR addressing it and once that's reviewed and merged then move on to the the next.