storage_interface problem

[Bartłomiej Daca]

This bug is connected with bug #1586524 and bug #1603885. It is only valid for versions liberty and mitaka. It does not affect master, because master uses ansible 2.0 and this bug is connected with ansible < 2.0.0.

So, first let's forget about kolla and focus on ansible( version 1.9.X ):

First, create three files:

cat <<'EOF' > globals.yml
testing_var: "from global.yml"
EOF

cat <<'EOF' > inventory
[a]
gs0104 testing_var="from inventory, only for this host"
gs0105
gs0106

[a:vars]
testing_var="from inventory, for group a"
EOF

cat <<'EOF' > test.yml
- name: testing
  hosts: all
  tasks:
    - debug:
        msg: "global context: '{{ testing_var }}'; hostvars context: '{{ hostvars[inventory_hostname]['testing_var']}}'"
EOF

command: ansible-playbook -i inventory -e @./globals.yml test.yml

Expected output:
...
ok: [gs0106] => {
    "msg": "global context: 'from global.yml'; hostvars context: 'from global.yml'"
}
ok: [gs0104] => {
    "msg": "global context: 'from global.yml'; hostvars context: 'from global.yml'"
}
ok: [gs0105] => {
    "msg": "global context: 'from global.yml'; hostvars context: 'from global.yml'"
}
...

Actual output:
...
ok: [gs0104] => {
    "msg": "global context: 'from global.yml'; hostvars context: 'from inventory, only for this host'"
}
ok: [gs0106] => {
    "msg": "global context: 'from global.yml'; hostvars context: 'from inventory, for group a'"
}
ok: [gs0105] => {
    "msg": "global context: 'from global.yml'; hostvars context: 'from inventory, for group a'"
}
...

As one can see, extra vars provided in file 'globals.yml' do not propagate to hostvars.

Going back to kolla (only liberty and mitaka versions), see the file ansible/roles/ceph/templates/ceph.conf.j2. In lines 7, 9 and 11 there are loops that look something like:
{% for host in groups['ceph-mon'] %}{{ hostvars[host]['ansible_' + hostvars[host]['storage_interface']]['ipv4']['address'] }}{% if not loop.last %}, {% endif %}{% endfor %}

These loops will generate wrong output when one will specify custom storage_interface. In the output one will see addresses from api_interface, not from the storage_interface, because storage_interface is read from hostvars (just like in this pure-ansible example above).

By changing the way of getting to storage_interface value in these three loops from:
{{ hostvars[host]['ansible_' + hostvars[host]['storage_interface']]['ipv4']['address'] }}
to
{{ hostvars[host]['ansible_' + storage_interface]['ipv4']['address'] }}
we can fix this problem. But this solution will create another bug:

If ceph-mon group will looks like this in inventory:

[ceph-mon]
gs0104
gs0105
gs0106 storage_interface=eth7

and the storage_interface will be specified in globals.yml as 'eth1', then the content of the ceph.conf will be invalid again. And this looks like won't fix to me.

To summerize:
# I suggest changing the ansible/roles/ceph/templates/ceph.conf.j2 in stable/liberty and stable/mitaka branches;
# Maybe this missbehaviour should be documented somehow?

[Bartłomiej Daca (bartek-daca)]

I think the statement that it will not affect master is wrong unfortunately...

In master:

Just like in the pure-ansible example, everything specified in extra-vars is on the top, so when one will specify storage_interface both in globals.yml and inventory, only globals.yml will be taken into consideration. But I am not sure if it is a bug, because it works exactly as it should, according to ansible documentation. Maybe it should be described in kolla documentation, because on the one hand we can see all this *_interface vars in global.yml, and on the other hand there are two lines in multinode inventory:

# you can specify "api_interface" and another interfaces like below:
#compute01 neutron_external_interface=eth0 api_interface=em1 storage_interface=em1 tunnel_interface=em1

Hence, it should be specified either in global.yml if the value is proper for all hosts, or in inventory if the value may vary between hosts.

[Dave Walker (davewalker)]

I think that is backwards to expected behaviour. I would have thought that the default is in globals.yml, and we override it in the inventory file for nodes that required specific settings.

[Bartłomiej Daca (bartek-daca)]

Exactly, this is what you expect, but this is not what ansible gives. Ansible documentation states clearly, that extra vars always win precedence.

One way of solving this can be:

* specifying default_storage_interface in globals.yml
* specifying storage_interface in inventory for specific hosts when needed
* reference to the var by {{ storage_interface | default( default_storage_interface ) }} in roles.

The same thing must be done with all *_interface vars.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/345972

[Jeffrey Zhang (jeffrey4l)]

First, I think you do the description in a now. the Expected output and the Actual output is reversed.

cat <<'EOF' > inventory
[a]
gs0104 testing_var="from inventory, only for this host"
gs0105
gs0106

[a:vars]
testing_var="from inventory, for group a"
EOF

cat <<'EOF' > test.yml
- name: testing
  hosts: all
  tasks:
    - debug:
        msg: "global context: '{{ testing_var }}'; hostvars context: '{{ hostvars[inventory_hostname]['testing_var']}}'"
EOF

command: ansible-playbook -i inventory -e @./globals.yml test.yml

Expected output: <--------- Here should be the Actual output
...
ok: [gs0106] => {
    "msg": "global context: 'from global.yml'; hostvars context: 'from global.yml'"
}
ok: [gs0104] => {
    "msg": "global context: 'from global.yml'; hostvars context: 'from global.yml'"
}
ok: [gs0105] => {
    "msg": "global context: 'from global.yml'; hostvars context: 'from global.yml'"
}
...

Actual output: <------------ Here should be the expected output
...
ok: [gs0104] => {
    "msg": "global context: 'from global.yml'; hostvars context: 'from inventory, only for this host'"
}
ok: [gs0106] => {
    "msg": "global context: 'from global.yml'; hostvars context: 'from inventory, for group a'"
}
ok: [gs0105] => {
    "msg": "global context: 'from global.yml'; hostvars context: 'from inventory, for group a'"
}
...

[Jeffrey Zhang (jeffrey4l)]

Please refer to the official doc[0].

> In 2.x, we have made the order of precedence more specific (with the last listed variables winning prioritization):

>role defaults [1]
>inventory vars [2]
>inventory group_vars
>inventory host_vars
>playbook group_vars
>playbook host_vars
>host facts
>play vars
>play vars_prompt
>play vars_files
>registered vars
>set_facts
>role and include vars
>block vars (only for tasks in block)
>task vars (only for the task)
>extra vars (always win precedence)

the extra vars always win. So it is hard to make it overwrote by inventory host_vars.

You solution to use a `default_xxx` variable work. But

1. not back compatible
2. make thing hard. In the bad case, we need convert more and more variable to `default_xxx`.

But I think the right way is remove the variables in the extra vars(globals.yml), and define the variables for all the host in the host_vars.

Here is a test.

    # globals.yml (comment out the cluster_interface )
    # cluster_interface = xxx

    # multihost ( the inventory file )
    [all:vars]
    cluster_interface = eth1

    [control]
    control[1:2]
    control3 clusteer_interface = eth4

see, it is still simple and clearly. we may need to document this.

[0] http://docs.ansible.com/ansible/playbooks_variables.html#variable-precedence-where-should-i-put-a-variable

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/354173
Committed: https://git.openstack.org/cgit/openstack/kolla/commit/?id=158a852998495c9289ff9899adc7dfbf9a06910a
Submitter: Jenkins
Branch: master

commit 158a852998495c9289ff9899adc7dfbf9a06910a
Author: Paul Bourke <email address hidden>
Date: Thu Aug 11 15:41:29 2016 +0000

    Add defaults for interfaces to all.yml

    The values for 'network_interface' and 'neutron_external_interface' are
    missing from all.yml, meaning it is impossible to override them on a per
    node / per group basis. (globals.yml get's top precedence).

    Make these consistent with the rest of the variables and move the
    defaults into all.yml. Operators can still override / update these in
    globals.yml as before, but those wanting more flexibility now have it
    via host / group variables.

    Change-Id: I2575921f76a8e245106da765757c70353bd6762c
    Closes-Bug: #1604129

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/kolla 3.0.0.0b3 development milestone.

[Bartłomiej Daca (bartek-daca)]

It is much better solution than this with the 'default_' prefix and it works, so perfect :)

What about the liberty/mitaka versions? They use ansible < 2.0, and the solution that works for ansible 2.0 won't work for the older versions.

Here is a test:

# globals.yml ( cluster_interface is commented out )
# cluster_interface = eth0

# multihost ( the inventory file )
[all:vars]
cluster_interface = eth1

[control]
gs0104
gs0105 cluster_interface = eth4

#group_vars/all.yml
cluster_interface = eth77

#test.yml, testing playbook
- name: debug
  hosts: all
  tasks:
    - debug:
        msg: "{{ hostvars[inventory_hostname][ 'cluster_interface' ] }}"

Actual output:
PLAY [debug] ******************************************************************

GATHERING FACTS ***************************************************************
ok: [gs0104]
ok: [gs0105]

TASK: [debug ] ****************************************************************
ok: [gs0104] => {
    "msg": "eth4"
}
ok: [gs0105] => {
    "msg": "eth77"
}

PLAY RECAP ********************************************************************
gs0104 : ok=2 changed=0 unreachable=0 failed=0
gs0105 : ok=2 changed=0 unreachable=0 failed=0

Expected output:
PLAY [debug] ******************************************************************

GATHERING FACTS ***************************************************************
ok: [gs0104]
ok: [gs0105]

TASK: [debug ] ****************************************************************
ok: [gs0104] => {
    "msg": "eth4"
}
ok: [gs0105] => {
    "msg": "eth1"
}

PLAY RECAP ********************************************************************
gs0104 : ok=2 changed=0 unreachable=0 failed=0
gs0105 : ok=2 changed=0 unreachable=0 failed=0

As we can see, group_vars/all.yml wins precedence. group_vars/all.yml is managed by kolla and user shouldn't modify it.

So what are the options?

1. We can comment out all *_interface vars on group_vars/all.yml and enforce users to specify this vars in inventory file, what is ugly.
2. We can talk again about the default_ prefix which is equally ugly.

Personally I use the second option in my env, this way I can cry every time i see it. It reminds me that this needs to be resolved. But being serious, anyone has a better idea?

[OpenStack Infra (hudson-openstack)]

Change abandoned by Mark Goddard (<email address hidden>) on branch: master
Review: https://review.opendev.org/345972
Reason: Very old