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

Cobbler Inventory: allow users to specify extra groups #9420

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
2 changes: 2 additions & 0 deletions changelogs/fragments/9419-inventory-cobbler-extra-vars.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
minor_changes:
- cobbler inventory plugin - fix typos, request full rendered variables for Cobbler system, allow end user to specify extra groups, should not change currently expected behavior (https://github.com/ansible-collections/community.general/issues/9419).
69 changes: 55 additions & 14 deletions plugins/inventory/cobbler.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,29 @@
description: Prefix to apply to cobbler groups.
default: cobbler_
want_facts:
description: Toggle, if V(true) the plugin will retrieve host facts from the server.
description: Toggle, if V(true) the plugin will retrieve all host facts from the server.
type: boolean
default: true
want_ip_addresses:
description:
- Toggle, if V(true) the plugin will add a C(cobbler_ipv4_addresses) and C(cobbleer_ipv6_addresses) dictionary to the defined O(group) mapping
- Toggle, if V(true) the plugin will add a C(cobbler_ipv4_addresses) and C(cobbler_ipv6_addresses) dictionary to the defined O(group) mapping
interface DNS names to IP addresses.
type: boolean
default: true
version_added: 7.1.0
facts_level:
Copy link
Contributor

Choose a reason for hiding this comment

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

We will want version_added for these as well.

Copy link
Author

Choose a reason for hiding this comment

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

Okay, I can do that - what version number should I put?

description:
- "normal: Gather only system-level variables"
- "as_rendered: Gather all variables as rolled up by Cobbler"
type: string
choices: [ 'normal', 'as_rendered' ]
default: normal
extra_groups:
description:
- Comma or space-separated string containing extra groups to be added to the Cobbler inventory
Copy link
Contributor

Choose a reason for hiding this comment

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

Why a comma or space-separated string? Why not a list of strings?

Copy link
Author

@Ty9000 Ty9000 Mar 13, 2025

Choose a reason for hiding this comment

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

I guess it can be a list of strings, I'm not tied to anything specific and that portion of the code would need to be modified to read over the list

- Requires O(want_facts) to be set to V(true)
type: string
default: ""
'''

EXAMPLES = '''
Expand All @@ -114,6 +127,7 @@
password: secure
'''

from re import split
import socket

from ansible.errors import AnsibleError
Expand Down Expand Up @@ -142,7 +156,7 @@ class InventoryModule(BaseInventoryPlugin, Cacheable):
def __init__(self):
super(InventoryModule, self).__init__()
self.cache_key = None
self.connection = None
self.cobbler_connection = None
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't get what's going on with cobbler_connection vs local_connection.

Copy link
Author

Choose a reason for hiding this comment

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

That was because @russoz wanted the self.c variable renamed to something more sensible, and that is just what I came up with. It can always be changed


def verify_file(self, path):
valid = False
Expand All @@ -157,13 +171,13 @@ def _get_connection(self):
if not HAS_XMLRPC_CLIENT:
raise AnsibleError('Could not import xmlrpc client library')

if self.connection is None:
if self.cobbler_connection is None:
self.display.vvvv(f'Connecting to {self.cobbler_url}\n')
self.connection = xmlrpc_client.Server(self.cobbler_url, allow_none=True)
self.cobbler_connection = xmlrpc_client.Server(self.cobbler_url, allow_none=True)
self.token = None
if self.get_option('user') is not None:
self.token = self.connection.login(text_type(self.get_option('user')), text_type(self.get_option('password')))
return self.connection
self.token = self.cobbler_connection.login(text_type(self.get_option('user')), text_type(self.get_option('password')))
return self.cobbler_connection

def _init_cache(self):
if self.cache_key not in self._cache:
Expand All @@ -178,12 +192,12 @@ def _reload_cache(self):

def _get_profiles(self):
if not self.use_cache or 'profiles' not in self._cache.get(self.cache_key, {}):
c = self._get_connection()
self.local_connection = self._get_connection()
try:
if self.token is not None:
data = c.get_profiles(self.token)
data = self.local_connection.get_profiles(self.token)
else:
data = c.get_profiles()
data = self.local_connection.get_profiles()
except (socket.gaierror, socket.error, xmlrpc_client.ProtocolError):
self._reload_cache()
else:
Expand All @@ -194,12 +208,11 @@ def _get_profiles(self):

def _get_systems(self):
if not self.use_cache or 'systems' not in self._cache.get(self.cache_key, {}):
c = self._get_connection()
try:
if self.token is not None:
data = c.get_systems(self.token)
data = self.local_connection.get_systems(self.token)
else:
data = c.get_systems()
data = self.local_connection.get_systems()
except (socket.gaierror, socket.error, xmlrpc_client.ProtocolError):
self._reload_cache()
else:
Expand Down Expand Up @@ -238,6 +251,8 @@ def parse(self, inventory, loader, path, cache=True):
self.include_profiles = self.get_option('include_profiles')
self.group_by = self.get_option('group_by')
self.inventory_hostname = self.get_option('inventory_hostname')
self.facts_level = self.get_option('facts_level')
self.extra_groups = self.get_option('extra_groups')

for profile in self._get_profiles():
if profile['parent']:
Expand Down Expand Up @@ -319,7 +334,7 @@ def parse(self, inventory, loader, path, cache=True):

# Add host to groups specified by group_by fields
for group_by in self.group_by:
if host[group_by] == '<<inherit>>':
if host[group_by] == '<<inherit>>' or host[group_by] == '':
groups = []
else:
groups = [host[group_by]] if isinstance(host[group_by], str) else host[group_by]
Expand Down Expand Up @@ -371,6 +386,32 @@ def parse(self, inventory, loader, path, cache=True):
if ipv6_address is not None:
self.inventory.set_variable(hostname, 'cobbler_ipv6_address', make_unsafe(ipv6_address))

# If more facts are requested, gather them all from Cobbler
if self.facts_level == "as_rendered":
self.display.vvvv(f"Gathering all facts for {hostname}\n")
key = "mgmt_parameters"
if self.token is not None:
all_data = self.local_connection.get_system_as_rendered(host['name'], self.token)
else:
all_data = self.local_connection.get_system_as_rendered(host['name'])

# If user requests extra groups to be added to the Cobbler inventory,
if self.extra_groups:
# Gather each extra group, split by ',' or ' '
for extra_group in split(',| ', self.extra_groups):
try:
# Gather each value of the extra group, split by ',' or ' '
# e.g. profile_role: 'test1,test2'
for entry in split(',| ', all_data[key][extra_group]):
self.display.vvvv(f"Added {hostname} to group {entry}")
self._add_safe_group_name(entry, child=hostname)
except KeyError as e:
self.display.vvvv(f"Could not find {e} value for {hostname}")

elif self.facts_level == "normal":
key = "autoinstall_meta"
all_data = host

if self.get_option('want_facts'):
try:
self.inventory.set_variable(hostname, 'cobbler', make_unsafe(host))
Expand Down