yaml inventory: Support host-list (not a dictionary)

[dagwieers]

SUMMARY

Currently the YAML parser expects that every hosts-entry comprises of a dictionary of hosts with variables. Unfortunately that's not the case, inventories can simply consist of a list of hosts (because variables are defined elsewhere) and this would result in:

ERROR! Attempted to read "hosts.yaml" as YAML: list indices must be integers, not AnsibleUnicode

This PR allows for listing hosts as one would expect in YAML, just as one can do in a JSON inventory.
And it also support empty values.

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

yaml inventory

ANSIBLE VERSION

v2.3

[bcoca]

I'm not sure what you are trying to do here, can you show examples?

[dagwieers]

In a YAML inventory it now expects for groups:

all:
  hosts:
    host1:
      var1: value1a
      var2: value2
    host2:
      var1: value1b
      var2: value2
windows:
  hosts:
    host1: null
    host2: null

Whereas in various cases we have, a simple list would do. (JSON supports this fine)

all:
  hosts:
    host1:
      var1: value1a
      var2: value2
    host2:
      var1: value1b
      var2: value2
windows:
  hosts:
  - host1
  - host2

In an inventory you only need to specify the host-vars for a host only once. So what we do is define the hostvars for group "all" only, and use lists (and not dictionaries) everywhere else.

[bcoca]

the reason it does not use lists is to avoid duplicates

[dagwieers]

The JSON inventory doesn't have this restriction.
Why don't we simply make it a set internally if this is a concern ?

Dictionaries are used for key-value pairs, we don't have key-value pairs here.
And dictionaries can also have duplicate entries, causing the last entry to overwrite prior entries silently.
So I guess also in that case proper handling/escalating will be required.

[bcoca]

I'm torn on this one, i see your point, but I also think that keeping them at keys allows users to go back n forth with vars w/o having to restructure the entries.

[dagwieers]

True, I am not preventing people from doing that though. I am just supporting lists as well.

But I don't mind doing the legwork for ensuring lists are like sets. A simply existence check in add_host would be sufficient, or turning them into a set(), which is what I prefer.

(To be honest, I think there's only one place where host-vars should be defined and there's no need to move them around. That is why we defined them as part of the "all" group.)

[bcoca]

people define them in diff levels, we give flexibility towards organizing them as best as it suits their environment, I would loathe forcing the 1 way.

[dagwieers]

Nope, I wouldn't force anyone. it's just that picking one place to define host-vars prevents duplicate definitions of host-vars and headaches finding what's going on. Because I doubt it's obvious which host-vars win in this case (as they are processed sequentially, I'd guess the last one wins, but that's likely depending on the YAML parser).

[dagwieers]

@bcoca I updated the PR, it now reports when a list has duplicate entries:

[WARNING]: While parsing hosts list, found a duplicate entry for host 'localhost' in group 'linux'

much like how it does for duplicate keys in a dictionary.

[bcoca]

dupes would have not made their way to inventory, my point was letting the YAML parser dedupe early and cheaply.

[dagwieers]

I know they would not ended up in the inventory, but with dictionaries you get a warning if there are duplicate entries. Now the same will happen for duplicate entries in a list. I assumed that this was your concern. If it is not, and you prefer to get rid of this warning, I can remove it again.

I can't do this in the YAML parser, because for some lists duplicates are intended, so it should be very specific to 'hosts' dictionaries in groups.

[bcoca]

you can remove the warning

[dagwieers]

@bcoca I have updated my patch to the new inventory model. So this patch is to support a list of hosts rather than a dictionary of hosts as key. Especially for groups where you don't need to add host variables, you don't want to use dictionaries, especially because generated YAML lists with dictionary using empty values results in:

vmware:
  hosts:
    vmware-guest-01: null
    vmware-guest-02: null

Rather than:

vmware:
  hosts:
  - vmware-guest-01
  - vmware-guest-02

[abadger]

Code looks good to me.

[abadger]

@bcoca does your last message mean that you're okay with this (it does mirror what can be done with the ini format) and just wanted the message updated?

[bcoca]

No, i would still like to keep current format w/o complicating it with variations, I was also working on a schema validator for ansible-inventory, something like this for DTD:

name: Ansible YAML Inventory
type: map
mapping:
    desc: name of a group
    type: map
    unique: yes
    mapping: *group
      "hosts":
        desc: hosts that are part of this group
        type: map
        mapping:
          unique: yes
          desc: name of each host (inventory_hostname)
          type: map
          mapping:
            desc: vars associated with this host
      "children":
        type: map
        desc: child groups
        mapping: &group
      "vars":
        desc: vars associated with this group
        type: map
        mapping:
          unique: yes

[dagwieers]

So I disagree with defining host-lists as dictionary with empty values and allowing to have host-vars defined anywhere a host is added to a group in the YAML inventory:

all:
    hosts:
        host2:
            dns_server: a.b.c.d
    vars:
        dns_server: c.d.e.f
datacenter1:
    hosts:
        host2:
            dns_server: 2.3.4.5
        host3:
            dns_server: 3.4.5.6
    vars:
        dns_server: 6.7.8.9
windows:
    hosts:
        host1: null
            dns_server: 1.2.3.4
        host2: null
    vars:
        dns_server: 7.8.9.1

for the following reasons:

• It does not make YAML convenient or human-editable (a list of hosts should just be a YAML list)
• Having your hostvars (potentially) distributed over your complex YAML inventory is a potential support nightmare (especially if that file is managed by more than one person)

I much rather preferred a single location in the specification to add your host-vars (just like group-vars now are defined in a single location) and have lists of hosts as YAML lists.

_hostvars:
    host1:
        dns_server: 1.2.3.4
    host2:
        dns_server: 2.3.4.5
datacenter1:
    hosts:
    - host2
    - host3
windows:
    hosts:
    - host1
    - host2

IMO this is a design mistake, one we apparently can't or won't change.