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.

Sign up for GitHub

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

Jump to bottom

Foreman: add options to support backwards compatibility with script #67070

Closed
wants to merge 2 commits into from
Closed

Foreman: add options to support backwards compatibility with script #67070

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2047c19
foreman plugin: add options to support compatibility with script
Feb 4, 2020
4861aad
drop legacy_groups option
Feb 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 41 additions & 16 deletions lib/ansible/plugins/inventory/foreman.py
View file
Open in desktop
Expand Up @@ -69,6 +69,13 @@
type: boolean
default: False
version_added: '2.10'
legacy_hostvars:
description:
- Toggle, if true the plugin will build legacy hostvars present in the foreman script
- Places hostvars in a dictionary with keys `foreman`, `foreman_facts`, and `foreman_params`
type: boolean
default: False
version_added: '2.10'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe you could add the deprecated flag to this data?

Copy link
Contributor Author

@jladdjr jladdjr Feb 19, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean, we should basically say in advance something like, "in 2.13, we'll drop support for this (and officially break with the old script)" - is that what you mean?

@wenottingham - do we have a general policy about when awx can officially break ties with the old inv. scripts? Guess the answer to that may vary for each inventory source.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why would it be deprecated? is it that complex to maintain it as an option?

It's a breaking change regardless of when it happens.

Copy link
Contributor Author

@jladdjr jladdjr Feb 21, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlanCoding per @wenottingham's note, it seems like this option is something that we'll need to maintain for a while unless (1) there's some other way we plan on providing compatibility in the future (I can't think of any) or (2) we plan on actually transitioning the awx foreman plugin towards its vanilla behavior (but that would be a breaking change for awx users and I'm not sure we plan on ever doing that). In short, doesn't seem like we are planning on deprecating support for the new legacy_hostvars option.


'''

Expand Down Expand Up @@ -208,6 +215,13 @@ def _get_facts(self, host):
raise ValueError("More than one set of facts returned for '%s'" % host)
return facts

def _get_hostvars(self, host, vars_prefix='', omitted_vars=()):
hostvars = {}
for k, v in host.items():
if k not in omitted_vars:
hostvars[vars_prefix + k] = v
return hostvars

def _populate(self):

for host in self._get_hosts():
Expand All @@ -222,25 +236,36 @@ def _populate(self):
group_name = self.inventory.add_group(group_name)
self.inventory.add_child(group_name, host_name)

# set host vars from host info
try:
for k, v in host.items():
if k not in ('name', 'hostgroup_title', 'hostgroup_name'):
try:
self.inventory.set_variable(host_name, self.get_option('vars_prefix') + k, v)
except ValueError as e:
self.display.warning("Could not set host info hostvar for %s, skipping %s: %s" % (host, k, to_text(e)))
except ValueError as e:
self.display.warning("Could not get host info for %s, skipping: %s" % (host_name, to_text(e)))
if self.get_option('legacy_hostvars'):
hostvars = self._get_hostvars(host)
self.inventory.set_variable(host_name, 'foreman', hostvars)
else:
omitted_vars = ('name', 'hostgroup_title', 'hostgroup_name')
hostvars = self._get_hostvars(host, self.get_option('vars_prefix'), omitted_vars)

# set host vars from params
if self.get_option('want_params'):
for p in self._get_all_params_by_id(host['id']):
for k, v in hostvars.items():
try:
self.inventory.set_variable(host_name, p['name'], p['value'])
self.inventory.set_variable(host_name, k, v)
except ValueError as e:
self.display.warning("Could not set hostvar %s to '%s' for the '%s' host, skipping: %s" %
(p['name'], to_native(p['value']), host, to_native(e)))
self.display.warning("Could not set host info hostvar for %s, skipping %s: %s" % (host, k, to_text(e)))

# set host vars from params
if self.get_option('want_params'):
params = self._get_all_params_by_id(host['id'])
filtered_params = {}
for p in params:
if 'name' in p and 'value' in p:
filtered_params[p['name']] = p['value']

if self.get_option('legacy_hostvars'):
self.inventory.set_variable(host_name, 'foreman_params', filtered_params)
else:
for k, v in filtered_params:
try:
self.inventory.set_variable(host_name, p['name'], p['value'])
except ValueError as e:
self.display.warning("Could not set hostvar %s to '%s' for the '%s' host, skipping: %s" %
(p['name'], to_native(p['value']), host, to_native(e)))

# set host vars from facts
if self.get_option('want_facts'):
Expand Down