Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ansible 2.4 group_vars not parsed lexicographically #31689

Closed
Saloshp opened this issue Oct 13, 2017 · 7 comments · Fixed by #32963
Closed

Ansible 2.4 group_vars not parsed lexicographically #31689

Saloshp opened this issue Oct 13, 2017 · 7 comments · Fixed by #32963
Labels
affects_2.4 This issue/PR affects Ansible v2.4 c:vars/variable_manager feature This issue/PR relates to a feature request. inventory Inventory category module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team.

Comments

@Saloshp
Copy link

Saloshp commented Oct 13, 2017

ISSUE TYPE
  • Bug Report
  • Feature Idea
COMPONENT NAME

group_vars

ANSIBLE VERSION
ansible 2.4.0.0
  config file = /opt/ansible/ansible.cfg
  configured module search path = [u'/usr/lib/pymodules/python2.7/ansible/module_utils']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.14 (default, Sep 20 2017, 01:25:59) [GCC 7.2.0]
CONFIGURATION
[root@local test]# ansible-config dump --only-changed
[root@local test]# 
OS / ENVIRONMENT
[root@local test]# uname -a
Linux local 4.13.4-1-ARCH #1 SMP PREEMPT Thu Sep 28 08:39:52 CEST 2017 x86_64 GNU/Linux
SUMMARY

group variable files of higher lexicographical order do not override lower ones

STEPS TO REPRODUCE
[root@local test]# for file in $(find -type f); do echo "#File: ${file}"; cat $file; echo; done
#File: ./playbooks/debug.yml
- hosts: localhost
  gather_facts: yes
  tasks:
  - debug: var=myvar

#File: ./playbooks/group_vars/all/02_vars
myvar: "02"

#File: ./playbooks/group_vars/all/00_vars
myvar: "00"

[root@local test]# ansible-playbook -i "127.0.0.1," -c local playbooks/debug.yml 
EXPECTED RESULTS

ok: [127.0.0.1] => {
"myvar": "02"
}

ACTUAL RESULTS
PLAY [localhost] *************************************************************************************************************************************

TASK [Gathering Facts] *******************************************************************************************************************************
ok: [127.0.0.1]

TASK [debug] *****************************************************************************************************************************************
ok: [127.0.0.1] => {
    "myvar": "00"
}

PLAY RECAP *******************************************************************************************************************************************
127.0.0.1                  : ok=2    changed=0    unreachable=0    failed=0   


@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 bug_report needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Oct 13, 2017
@s-hertel s-hertel added c:vars/variable_manager and removed needs_triage Needs a first human triage before being processed. labels Oct 13, 2017
@NomAnor
Copy link

NomAnor commented Nov 16, 2017

It seems the group_vars/host_vars files are found in the function _get_dir_files in plugins/vars/host_group_vars.py. This function uses os.listdir. According to the python documentation:

The list is in arbitrary order.

So one can't rely on any ordering. This behaviour should at least be documented.

I think it would be better to load the files in sorted order. This can be benificial for multi-stage environments to specify defaults for all environments as shown in the article How to Manage Multistage Environments with Ansible (The method shown is not reliable in the current version). Playbook group_vars can't be used beacuse they would override the inventory group_vars.

@bcoca
Copy link
Member

bcoca commented Nov 16, 2017

this was never sorted, previous versions always used os.listdir, this would be a change in behaviour.

@Saloshp
Copy link
Author

Saloshp commented Nov 16, 2017

@bcoca, so your'e saying a random behaviour is better than a predicted one, simply because you got used to it?
Also, I didn't dive into the code but can't reproduce under Ansible 2.3 (works fine)

@bcoca
Copy link
Member

bcoca commented Nov 16, 2017

@Saloshp its not random, it is just not determined by our code, there is a big difference there.

The code in 2.4 mostly 'moved' but was not changed when it came to finding files, you can check 2.3 as it also uses os.listdir and there is no sorting after that.

I'm not saying i'm rejecting the feature, I'm saying we need to add a toggle for those depending on the previous behaviour.

@ansibot
Copy link
Contributor

ansibot commented Nov 16, 2017

cc @jhoekx
click here for bot help

@ansibot ansibot added inventory Inventory category module This issue/PR relates to a module. labels Nov 16, 2017
@logan2211
Copy link
Contributor

logan2211 commented Nov 21, 2017

Actually, they were sorted, and 2.4 broke this behavior.

Here is the snippet from 2.3.1.0 which previously sorted the list of files:

try:
names = loader.list_directory(path)
except os.error as err:
raise AnsibleError("This folder cannot be listed: %s: %s." % (path, err.strerror))
# evaluate files in a stable order rather than whatever
# order the filesystem lists them.
names.sort()

This is a valid bug and the fix in the PR is valid imo. This regression is preventing a smooth upgrade to 2.4 in one of my projects currently.

@bcoca
Copy link
Member

bcoca commented Nov 21, 2017

hmm, seems we used to do 1/2 the vars one way, the other 1/2 the other ... sorted does seem more predictable so I'll add it w/o requiring a toggle.

@ansibot ansibot added feature This issue/PR relates to a feature request. and removed feature_idea labels Mar 3, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 c:vars/variable_manager feature This issue/PR relates to a feature request. inventory Inventory category module This issue/PR relates to a module. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants