search

theforeman / foreman_ansible_inventory

Foreman dynamic inventory script for ansible - Now merged into Ansible itself
GNU General Public License v3.0
70 stars 31 forks source link

Use Foreman to calculate resulting set of params #15

Closed arielsalvo closed 8 years ago

arielsalvo commented 8 years ago

I noticed the host returned by Foreman's API contains an all_parameters entry which contains Organization / Location / and global as well has host and group parameters. This would probably be enough to satisfy #9 . I added an option server_param_resolution in the section foreman of the ini file to make this change optional and backwards compatible; if missing or set to False, the original behavior is maintained.

It would be nice to have a way to query all_parameters from the API itself instead of having to use the host.

agx commented 8 years ago

@dLobatog can you think of the case where all_parameters wouldn't be correct? I remember a case where all_parameters was empty in early 1.10 since there were only params on the hostgroups but not but can't seem to reproduce it now.

If not we should just drop the manual resolution and totally rely on all_parameters.

dLobatog commented 8 years ago

@agx As far as I know all_parameters is 100% reliable. The id field though isn't easy to understand as you may have duplicate ids (e.g: param 'a' from a host has id 1, param 'b' from hostgroup has id 1 too as they don't come from the same source). That shouldn't affect this plugin though, so please merge at will :smile:

Jenpatrick commented 8 years ago

I'd totally be in favor of this too. Would save so much time and effort.

arielsalvo commented 8 years ago

If appropriate, I can remove the option in the config file and move to all_parameters. Just say the word ;)

agx commented 8 years ago

@arielsalvo thanks for the patch. I've pulled this in and removed the config variable.