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

Already on GitHub? Sign in to your account

Merging hash vars is inconsistent #2329

Closed
chrishoffman opened this Issue Mar 7, 2013 · 10 comments

Comments

Projects
None yet
6 participants
Member

chrishoffman commented Mar 7, 2013

When setting the hash_behavior=merge in the ansible.cfg, hash variables are only merged in group/host vars. When hash variables are defined at other levels, such as at the playbook, the variable precedence takes over and hashes are overwritten.

Contributor

leucos commented Mar 8, 2013

Yes, it defitively should merge hashes in a logical order, which is
described here
http://ansible.cc/docs/playbooks2.html?highlight=precedence#understanding-variable-precedence
(the later overrides the former). Here is how I understand things. Please correct me when I'm wrong :

1/ Any variables specified with –extra-vars (-e) on the ansible-playbook
command line.

I don't think complex vars can be defined here (?)

2/ Variables loaded from YAML files mentioned in ‘vars_files’ in a playbook.

I suppose complex vars can be defined here.
If this is the first source of complex vars, the merge only happens
after this step.

3/ facts, whether built in or custom, or variables assigned from the
‘register’ keyword.

Should merge over previous vars.

4/ variables passed to parameterized task include statements

I don't think parameters to tasks can be complex (?).

5/ ‘vars’ as defined in the playbook.

Should merge over previous vars.

6/ Host variables from inventory.

As far as the discussion went, @mpdehaan doesn't want complex vars to be
defined at that level. However, I have a patch here, so if this sounds
reasonable in the end, I can push that. Since this would be new, it
probably can't harm existing pbooks. It seems reasonable to me to make
things behave consistently here too.

7/ Group variables from inventory in inheritance order

This is the only place where merging works for now.

Am I getting it right here ?

BTW I don't understand why 2/ and 5/ are so far away, since they are two
different ways of defining the same thing (one "modular", one "inline").

I'd like to hear people opinion on this, especially @mpdehaan 's.

Contributor

mpdehaan commented Mar 8, 2013

Will reply more on this later, but you can definitely define complex
variables in inventory (see group_vars/host_vars).

On Fri, Mar 8, 2013 at 2:02 AM, Michel Blanc notifications@github.comwrote:

Yes, it defitively should merge hashes in a logical order, which is
described here

http://ansible.cc/docs/playbooks2.html?highlight=precedence#understanding-variable-precedence
(the later overrides the former). Here is how I understand things. Please
correct me when I'm wrong :

1/ Any variables specified with –extra-vars (-e) on the ansible-playbook
command line.

I don't think complex vars can be defined here (?)

2/ Variables loaded from YAML files mentioned in ‘vars_files’ in a
playbook.

I suppose complex vars can be defined here.
If this is the first source of complex vars, the merge only happens
after this step.

3/ facts, whether built in or custom, or variables assigned from the
‘register’ keyword.

Should merge over previous vars.

4/ variables passed to parameterized task include statements

I don't think parameters to tasks can be complex (?).

5/ ‘vars’ as defined in the playbook.

Should merge over previous vars.

6/ Host variables from inventory.

As far as the discussion went, @mpdehaan https://github.com/mpdehaandoesn't want complex vars to be
defined at that level. However, I have a patch here, so if this sounds
reasonable in the end, I can push that. Since this would be new, it
probably can't harm existing pbooks. It seems reasonable to me to make
things behave consistently here too.

7/ Group variables from inventory in inheritance order

This is the only place where merging works for now.

Am I getting it right here ?

BTW I don't understand why 2/ and 5/ are so far away, since they are two
different ways of defining the same thing (one "modular", one "inline").

I'd like to hear people opinion on this, especially @mpdehaanhttps://github.com/mpdehaan's.


Reply to this email directly or view it on GitHubhttps://github.com/ansible/ansible/issues/2329#issuecomment-14606478
.

Contributor

leucos commented Mar 8, 2013

Yes @mpdehaan, but I was thinking of ini style inventory, something along the lines like :

[group]
server some.complex.var=42
Contributor

mpdehaan commented Mar 26, 2013

Just wanted to share I reviewed things, and I like the looks of the solution proposed in #2476 and we can include it once it gets some test issues cleared up.

Contributor

mpdehaan commented Apr 5, 2013

waiting on getting a pull request finalized for the above, expecting an update to #2476 per comments.

@mpdehaan mpdehaan closed this Apr 5, 2013

Just a note for anyone landing here from google... the config key is hash_behaviour, not hash_behavior

http://www.ansibleworks.com/docs/intro_configuration.html#hash-behaviour

I wonder if they would accept this :) Seems like American English should be used to lessen confusion.

diff --git a/lib/ansible/constants.py b/lib/ansible/constants.py
index 861dd53..d0e09ca 100644
--- a/lib/ansible/constants.py
+++ b/lib/ansible/constants.py
@@ -126,6 +126,7 @@ DEFAULT_SUDO              = get_config(p, DEFAULTS, 'sudo', 'ANSIBLE_SUDO', Fals
 DEFAULT_SUDO_EXE          = get_config(p, DEFAULTS, 'sudo_exe', 'ANSIBLE_SUDO_EXE', 'sudo')
 DEFAULT_SUDO_FLAGS        = get_config(p, DEFAULTS, 'sudo_flags', 'ANSIBLE_SUDO_FLAGS', '-H')
 DEFAULT_HASH_BEHAVIOUR    = get_config(p, DEFAULTS, 'hash_behaviour', 'ANSIBLE_HASH_BEHAVIOUR', 'replace')
+DEFAULT_HASH_BEHAVIOUR    = get_config(p, DEFAULTS, 'hash_behavior', 'ANSIBLE_HASH_BEHAVIOR', DEFAULT_HASH_BEHAVIOUR)
 DEFAULT_JINJA2_EXTENSIONS = get_config(p, DEFAULTS, 'jinja2_extensions', 'ANSIBLE_JINJA2_EXTENSIONS', None)
 DEFAULT_EXECUTABLE        = get_config(p, DEFAULTS, 'executable', 'ANSIBLE_EXECUTABLE', '/bin/sh')
 DEFAULT_SU_EXE            = get_config(p, DEFAULTS, 'su_exe', 'ANSIBLE_SU_EXE', 'su')
Contributor

mpdehaan commented Dec 3, 2014

Ah, this is just the docs anchor so not earth crushing, but I agree, docs
we should generalize standardize on American English, despite my love of
certain BBC programming.

(Still waiting for Doctor Who episode involving Top Gear Stig being alien)

On Wed, Dec 3, 2014 at 2:27 PM, John Villalovos notifications@github.com
wrote:

I wonder if they would accept this :) Seems like American English should
be used to lessen confusion.

diff --git a/lib/ansible/constants.py b/lib/ansible/constants.py
index 861dd53..d0e09ca 100644--- a/lib/ansible/constants.py+++ b/lib/ansible/constants.py@@ -126,6 +126,7 @@ DEFAULT_SUDO = get_config(p, DEFAULTS, 'sudo', 'ANSIBLE_SUDO', Fals
DEFAULT_SUDO_EXE = get_config(p, DEFAULTS, 'sudo_exe', 'ANSIBLE_SUDO_EXE', 'sudo')
DEFAULT_SUDO_FLAGS = get_config(p, DEFAULTS, 'sudo_flags', 'ANSIBLE_SUDO_FLAGS', '-H')
DEFAULT_HASH_BEHAVIOUR = get_config(p, DEFAULTS, 'hash_behaviour', 'ANSIBLE_HASH_BEHAVIOUR', 'replace')+DEFAULT_HASH_BEHAVIOUR = get_config(p, DEFAULTS, 'hash_behavior', 'ANSIBLE_HASH_BEHAVIOR', DEFAULT_HASH_BEHAVIOUR)
DEFAULT_JINJA2_EXTENSIONS = get_config(p, DEFAULTS, 'jinja2_extensions', 'ANSIBLE_JINJA2_EXTENSIONS', None)
DEFAULT_EXECUTABLE = get_config(p, DEFAULTS, 'executable', 'ANSIBLE_EXECUTABLE', '/bin/sh')
DEFAULT_SU_EXE = get_config(p, DEFAULTS, 'su_exe', 'ANSIBLE_SU_EXE', 'su')


Reply to this email directly or view it on GitHub
#2329 (comment).

Member

bcoca commented Dec 3, 2014

I think that is the next Xmas special when they test drive the tardis.

On Wed, Dec 3, 2014 at 4:47 PM, Michael DeHaan notifications@github.com
wrote:

(Still waiting for Doctor Who episode involving Top Gear Stig being alien)

Brian Coca

The actual configuration option does use the British English word 'behaviour'. So I think it is more than the doc anchor. If by doc anchor you mean the url...#hash-behaviour part.

My simplistic patch idea was just to allow using 'hash_behavior' in addition to 'hash_behaviour'

robinro pushed a commit to robinro/ansible that referenced this issue Dec 9, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment