From aa96eba4ce23ac26822f45c8af04f181d8d66f51 Mon Sep 17 00:00:00 2001 From: InsanePrawn Date: Thu, 16 Jul 2020 16:41:09 +0200 Subject: [PATCH 1/8] lxd_container module: Automate CONFIG_PARAM handling. Signed-off-by: InsanePrawn --- plugins/modules/lxd_container.py | 44 ++++++++++++++------------------ 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/plugins/modules/lxd_container.py b/plugins/modules/lxd_container.py index 30dc855617d..7d26fd7a94a 100644 --- a/plugins/modules/lxd_container.py +++ b/plugins/modules/lxd_container.py @@ -430,6 +430,10 @@ 'architecture', 'config', 'devices', 'ephemeral', 'profiles', 'source' ] +# CONFIG_CREATION_PARAMS is a list of attribute names that are only applied +# on instance creation. +CONFIG_CREATION_PARAMS = ['source'] + class LXDContainerManagement(object): def __init__(self, module): @@ -677,34 +681,24 @@ def _needs_to_change_instance_config(self, key): return self.config[key] != old_configs def _needs_to_apply_instance_configs(self): - return ( - self._needs_to_change_instance_config('architecture') or - self._needs_to_change_instance_config('config') or - self._needs_to_change_instance_config('ephemeral') or - self._needs_to_change_instance_config('devices') or - self._needs_to_change_instance_config('profiles') - ) + for param in set(CONFIG_PARAMS) - set(CONFIG_CREATION_PARAMS): + if self._needs_to_change_instance_config(param): + return True + return False def _apply_instance_configs(self): old_metadata = self.old_instance_json['metadata'] - body_json = { - 'architecture': old_metadata['architecture'], - 'config': old_metadata['config'], - 'devices': old_metadata['devices'], - 'profiles': old_metadata['profiles'] - } - - if self._needs_to_change_instance_config('architecture'): - body_json['architecture'] = self.config['architecture'] - if self._needs_to_change_instance_config('config'): - for k, v in self.config['config'].items(): - body_json['config'][k] = v - if self._needs_to_change_instance_config('ephemeral'): - body_json['ephemeral'] = self.config['ephemeral'] - if self._needs_to_change_instance_config('devices'): - body_json['devices'] = self.config['devices'] - if self._needs_to_change_instance_config('profiles'): - body_json['profiles'] = self.config['profiles'] + body_json = {} + for param in set(CONFIG_PARAMS) - set(CONFIG_CREATION_PARAMS): + if param in old_metadata: + body_json[param] = old_metadata[param] + + if self._needs_to_change_instance_config(param): + if param == 'config': + for k, v in self.config['config'].items(): + body_json['config'][k] = v + else: + body_json[param] = self.config[param] url = '{0}/{1}'.format(self.api_endpoint, self.name) if self.project: From 6900f635911739e701dc2570dc0b5f39a7dab03b Mon Sep 17 00:00:00 2001 From: InsanePrawn Date: Thu, 1 Oct 2020 05:17:31 +0200 Subject: [PATCH 2/8] lxd_container: check- and diff mode Signed-off-by: InsanePrawn --- plugins/modules/lxd_container.py | 39 +++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 11 deletions(-) diff --git a/plugins/modules/lxd_container.py b/plugins/modules/lxd_container.py index 7d26fd7a94a..0dd75a82a5f 100644 --- a/plugins/modules/lxd_container.py +++ b/plugins/modules/lxd_container.py @@ -396,6 +396,7 @@ type: list sample: ["create", "start"] ''' +import copy import datetime import os import time @@ -492,6 +493,7 @@ def __init__(self, module): self.module.fail_json(msg=e.msg) self.trust_password = self.module.params.get('trust_password', None) self.actions = [] + self.diff = {'before': {}, 'after': {}} def _build_config(self): self.config = {} @@ -525,7 +527,8 @@ def _change_state(self, action, force_stop=False): body_json = {'action': action, 'timeout': self.timeout} if force_stop: body_json['force'] = True - return self.client.do('PUT', url, body_json=body_json) + if not self.module.check_mode: + return self.client.do('PUT', url, body_json=body_json) def _create_instance(self): url = self.api_endpoint @@ -538,7 +541,8 @@ def _create_instance(self): url = '{0}?{1}'.format(url, urlencode(url_params)) config = self.config.copy() config['name'] = self.name - self.client.do('POST', url, config, wait_for_container=self.wait_for_container) + if not self.module.check_mode: + self.client.do('POST', url, config, wait_for_container=self.wait_for_container) self.actions.append('create') def _start_instance(self): @@ -557,7 +561,8 @@ def _delete_instance(self): url = '{0}/{1}'.format(self.api_endpoint, self.name) if self.project: url = '{0}?{1}'.format(url, urlencode(dict(project=self.project))) - self.client.do('DELETE', url) + if not self.module.check_mode: + self.client.do('DELETE', url) self.actions.append('delete') def _freeze_instance(self): @@ -687,7 +692,7 @@ def _needs_to_apply_instance_configs(self): return False def _apply_instance_configs(self): - old_metadata = self.old_instance_json['metadata'] + old_metadata = copy.deepcopy(self.old_instance_json['metadata']) body_json = {} for param in set(CONFIG_PARAMS) - set(CONFIG_CREATION_PARAMS): if param in old_metadata: @@ -699,11 +704,12 @@ def _apply_instance_configs(self): body_json['config'][k] = v else: body_json[param] = self.config[param] - + self.diff['after']['instance'] = body_json url = '{0}/{1}'.format(self.api_endpoint, self.name) if self.project: url = '{0}?{1}'.format(url, urlencode(dict(project=self.project))) - self.client.do('PUT', url, body_json=body_json) + if not self.module.check_mode: + self.client.do('PUT', url, body_json=body_json) self.actions.append('apply_instance_configs') def run(self): @@ -714,8 +720,17 @@ def run(self): self.client.authenticate(self.trust_password) self.ignore_volatile_options = self.module.params.get('ignore_volatile_options') - self.old_instance_json = self._get_instance_json() - self.old_state = self._instance_json_to_module_state(self.old_instance_json) + self.diff['before']['state'] = self.old_state + self.diff['after']['state'] = self.state + + old_sections = dict((k, v) for k, v in self.old_container_json['metadata'].items() if k in set(CONFIG_PARAMS) - set(CONFIG_CREATION_PARAMS)) + self.diff['before']['instance'] = dict( + (section, content) if not isinstance(content, dict) + else (section, dict((k, v) for k, v in content.items() + if not (self.ignore_volatile_options and k.startswith('volatile.')))) + for section, content in old_sections.items() + ) + action = getattr(self, LXD_ANSIBLE_STATES[self.state]) action() @@ -724,7 +739,8 @@ def run(self): 'log_verbosity': self.module._verbosity, 'changed': state_changed, 'old_state': self.old_state, - 'actions': self.actions + 'actions': self.actions, + 'diff': self.diff } if self.client.debug: result_json['logs'] = self.client.logs @@ -736,7 +752,8 @@ def run(self): fail_params = { 'msg': e.msg, 'changed': state_changed, - 'actions': self.actions + 'actions': self.actions, + 'diff': self.diff } if self.client.debug: fail_params['logs'] = e.kwargs['logs'] @@ -824,7 +841,7 @@ def main(): ), trust_password=dict(type='str', no_log=True) ), - supports_check_mode=False, + supports_check_mode=True, ) lxd_manage = LXDContainerManagement(module=module) From 052cfc3a9f1f023bcfdc581bfee081988adfb515 Mon Sep 17 00:00:00 2001 From: InsanePrawn Date: Fri, 9 Oct 2020 09:04:46 +0200 Subject: [PATCH 3/8] Make JSON lookups safer and fix crashes in check mode when instance is absent --- plugins/modules/lxd_container.py | 40 +++++++++++++++++++------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/plugins/modules/lxd_container.py b/plugins/modules/lxd_container.py index 0dd75a82a5f..e3bba547618 100644 --- a/plugins/modules/lxd_container.py +++ b/plugins/modules/lxd_container.py @@ -494,6 +494,8 @@ def __init__(self, module): self.trust_password = self.module.params.get('trust_password', None) self.actions = [] self.diff = {'before': {}, 'after': {}} + self.old_instance_json = {} + self.old_sections = {} def _build_config(self): self.config = {} @@ -575,11 +577,9 @@ def _unfreeze_instance(self): def _instance_ipv4_addresses(self, ignore_devices=None): ignore_devices = ['lo'] if ignore_devices is None else ignore_devices - - resp_json = self._get_instance_state_json() - network = resp_json['metadata']['network'] or {} - network = dict((k, v) for k, v in network.items() if k not in ignore_devices) or {} - addresses = dict((k, [a['address'] for a in v['addresses'] if a['family'] == 'inet']) for k, v in network.items()) or {} + data = (self._get_instance_state_json() or {}).get('metadata', None) or {} + network = dict((k, v) for k, v in (data.get('network', None) or {}).items() if k not in ignore_devices) + addresses = dict((k, [a['address'] for a in v['addresses'] if a['family'] == 'inet']) for k, v in network.items()) return addresses @staticmethod @@ -592,7 +592,7 @@ def _get_addresses(self): while datetime.datetime.now() < due: time.sleep(1) addresses = self._instance_ipv4_addresses() - if self._has_all_ipv4_addresses(addresses): + if self._has_all_ipv4_addresses(addresses) or self.module.check_mode: self.addresses = addresses return except LXDClientException as e: @@ -665,8 +665,9 @@ def _frozen(self): def _needs_to_change_instance_config(self, key): if key not in self.config: return False + if key == 'config' and self.ignore_volatile_options: # the old behavior is to ignore configurations by keyword "volatile" - old_configs = dict((k, v) for k, v in self.old_instance_json['metadata'][key].items() if not k.startswith('volatile.')) + old_configs = dict((k, v) for k, v in self.old_sections.get(key, {}).items() if not k.startswith('volatile.')) for k, v in self.config['config'].items(): if k not in old_configs: return True @@ -674,7 +675,7 @@ def _needs_to_change_instance_config(self, key): return True return False elif key == 'config': # next default behavior - old_configs = dict((k, v) for k, v in self.old_instance_json['metadata'][key].items()) + old_configs = dict((k, v) for k, v in self.old_sections.get(key, {}).items()) for k, v in self.config['config'].items(): if k not in old_configs: return True @@ -682,7 +683,7 @@ def _needs_to_change_instance_config(self, key): return True return False else: - old_configs = self.old_instance_json['metadata'][key] + old_configs = self.old_sections.get(key, {}) return self.config[key] != old_configs def _needs_to_apply_instance_configs(self): @@ -692,7 +693,7 @@ def _needs_to_apply_instance_configs(self): return False def _apply_instance_configs(self): - old_metadata = copy.deepcopy(self.old_instance_json['metadata']) + old_metadata = copy.deepcopy(self.old_instance_json).get('metadata', None) or {} body_json = {} for param in set(CONFIG_PARAMS) - set(CONFIG_CREATION_PARAMS): if param in old_metadata: @@ -700,6 +701,7 @@ def _apply_instance_configs(self): if self._needs_to_change_instance_config(param): if param == 'config': + body_json['config'] = body_json.get('config', None) or {} for k, v in self.config['config'].items(): body_json['config'][k] = v else: @@ -720,17 +722,23 @@ def run(self): self.client.authenticate(self.trust_password) self.ignore_volatile_options = self.module.params.get('ignore_volatile_options') - self.diff['before']['state'] = self.old_state - self.diff['after']['state'] = self.state - - old_sections = dict((k, v) for k, v in self.old_container_json['metadata'].items() if k in set(CONFIG_PARAMS) - set(CONFIG_CREATION_PARAMS)) - self.diff['before']['instance'] = dict( + self.old_instance_json = self._get_instance_json() + self.old_sections = dict( (section, content) if not isinstance(content, dict) else (section, dict((k, v) for k, v in content.items() if not (self.ignore_volatile_options and k.startswith('volatile.')))) - for section, content in old_sections.items() + for section, content in (self.old_instance_json.get('metadata', None) or {}).items() + if section in set(CONFIG_PARAMS) - set(CONFIG_CREATION_PARAMS) ) + self.diff['before']['instance'] = self.old_sections + # preliminary, will be overwritten in _apply_instance_configs() if called + self.diff['after']['instance'] = self.config + + self.old_state = self._instance_json_to_module_state(self.old_instance_json) + self.diff['before']['state'] = self.old_state + self.diff['after']['state'] = self.state + action = getattr(self, LXD_ANSIBLE_STATES[self.state]) action() From c3d33b7e899464b48bb069c8f59c6e8595b2d5c3 Mon Sep 17 00:00:00 2001 From: InsanePrawn Date: Sat, 21 Jan 2023 05:15:49 +0100 Subject: [PATCH 4/8] lxd_profile: fix docstring typos --- plugins/modules/lxd_profile.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/modules/lxd_profile.py b/plugins/modules/lxd_profile.py index 3d1b28b8fc8..1410d16e02f 100644 --- a/plugins/modules/lxd_profile.py +++ b/plugins/modules/lxd_profile.py @@ -35,7 +35,7 @@ type: str config: description: - - 'The config for the container (e.g. {"limits.memory": "4GB"}). + - 'The config for the instance (e.g. {"limits.memory": "4GB"}). See U(https://github.com/lxc/lxd/blob/master/doc/rest-api.md#patch-3)' - If the profile already exists and its "config" value in metadata obtained from @@ -247,7 +247,7 @@ class LXDProfileManagement(object): def __init__(self, module): - """Management of LXC containers via Ansible. + """Management of LXC profiles via Ansible. :param module: Processed Ansible Module. :type module: ``object`` From b0a8ee02bd3942a724f1e8142c98463a63a68d46 Mon Sep 17 00:00:00 2001 From: InsanePrawn Date: Sun, 22 Jan 2023 02:48:56 +0100 Subject: [PATCH 5/8] lxd_container: simplify _needs_to_change_instance_config() --- plugins/modules/lxd_container.py | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/plugins/modules/lxd_container.py b/plugins/modules/lxd_container.py index e3bba547618..329a916f60c 100644 --- a/plugins/modules/lxd_container.py +++ b/plugins/modules/lxd_container.py @@ -666,16 +666,9 @@ def _needs_to_change_instance_config(self, key): if key not in self.config: return False - if key == 'config' and self.ignore_volatile_options: # the old behavior is to ignore configurations by keyword "volatile" - old_configs = dict((k, v) for k, v in self.old_sections.get(key, {}).items() if not k.startswith('volatile.')) - for k, v in self.config['config'].items(): - if k not in old_configs: - return True - if old_configs[k] != v: - return True - return False - elif key == 'config': # next default behavior - old_configs = dict((k, v) for k, v in self.old_sections.get(key, {}).items()) + if key == 'config': + # self.old_sections is already filtered for volatile keys if necessary + old_configs = dict(self.old_sections.get(key, None) or {}) for k, v in self.config['config'].items(): if k not in old_configs: return True From d16fe451e8de9f4ffe30bc278d447f4f5528376a Mon Sep 17 00:00:00 2001 From: InsanePrawn Date: Sun, 22 Jan 2023 18:01:01 +0100 Subject: [PATCH 6/8] lxd_container: add docstring for check- and diff-mode and changelog fragment --- .../fragments/5866-lxd_container-diff-and-check-mode.yml | 2 ++ plugins/modules/lxd_container.py | 9 +++++++++ 2 files changed, 11 insertions(+) create mode 100644 changelogs/fragments/5866-lxd_container-diff-and-check-mode.yml diff --git a/changelogs/fragments/5866-lxd_container-diff-and-check-mode.yml b/changelogs/fragments/5866-lxd_container-diff-and-check-mode.yml new file mode 100644 index 00000000000..eb337cd42a3 --- /dev/null +++ b/changelogs/fragments/5866-lxd_container-diff-and-check-mode.yml @@ -0,0 +1,2 @@ +minor_changes: + - lxd_container - add diff and check mode (https://github.com/ansible-collections/community.general/pull/5866). diff --git a/plugins/modules/lxd_container.py b/plugins/modules/lxd_container.py index 329a916f60c..fea0dd48a02 100644 --- a/plugins/modules/lxd_container.py +++ b/plugins/modules/lxd_container.py @@ -16,6 +16,15 @@ description: - Management of LXD containers and virtual machines. author: "Hiroaki Nakamura (@hnakamur)" +extends_documentation_fragment: + - community.general.attributes +attributes: + check_mode: + support: full + version_added: 6.4.0 + diff_mode: + support: full + version_added: 6.4.0 options: name: description: From 1ce5cdba72c51f460e39920418b04bce7f631aae Mon Sep 17 00:00:00 2001 From: InsanePrawn Date: Mon, 13 Feb 2023 18:12:03 +0100 Subject: [PATCH 7/8] style fixes --- plugins/modules/lxd_container.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/plugins/modules/lxd_container.py b/plugins/modules/lxd_container.py index fea0dd48a02..da978d01754 100644 --- a/plugins/modules/lxd_container.py +++ b/plugins/modules/lxd_container.py @@ -421,7 +421,7 @@ 'stopped': '_stopped', 'restarted': '_restarted', 'absent': '_destroyed', - 'frozen': '_frozen' + 'frozen': '_frozen', } # ANSIBLE_LXD_STATES is a map of states of lxd containers to the Ansible @@ -750,7 +750,7 @@ def run(self): 'changed': state_changed, 'old_state': self.old_state, 'actions': self.actions, - 'diff': self.diff + 'diff': self.diff, } if self.client.debug: result_json['logs'] = self.client.logs @@ -763,7 +763,7 @@ def run(self): 'msg': e.msg, 'changed': state_changed, 'actions': self.actions, - 'diff': self.diff + 'diff': self.diff, } if self.client.debug: fail_params['logs'] = e.kwargs['logs'] @@ -777,7 +777,7 @@ def main(): argument_spec=dict( name=dict( type='str', - required=True + required=True, ), project=dict( type='str', @@ -807,7 +807,7 @@ def main(): ), state=dict( choices=list(LXD_ANSIBLE_STATES.keys()), - default='started' + default='started', ), target=dict( type='str', @@ -823,33 +823,33 @@ def main(): ), wait_for_container=dict( type='bool', - default=False + default=False, ), wait_for_ipv4_addresses=dict( type='bool', - default=False + default=False, ), force_stop=dict( type='bool', - default=False + default=False, ), url=dict( type='str', - default=ANSIBLE_LXD_DEFAULT_URL + default=ANSIBLE_LXD_DEFAULT_URL, ), snap_url=dict( type='str', - default='unix:/var/snap/lxd/common/lxd/unix.socket' + default='unix:/var/snap/lxd/common/lxd/unix.socket', ), client_key=dict( type='path', - aliases=['key_file'] + aliases=['key_file'], ), client_cert=dict( type='path', - aliases=['cert_file'] + aliases=['cert_file'], ), - trust_password=dict(type='str', no_log=True) + trust_password=dict(type='str', no_log=True), ), supports_check_mode=True, ) From 05d6c1f2cb42122830ebd094a8d011ad97c389ae Mon Sep 17 00:00:00 2001 From: InsanePrawn Date: Mon, 13 Feb 2023 18:12:13 +0100 Subject: [PATCH 8/8] lxd_container: fix typo in actions: "unfreez" lacks an "e" --- plugins/modules/lxd_container.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/lxd_container.py b/plugins/modules/lxd_container.py index da978d01754..00649a076fc 100644 --- a/plugins/modules/lxd_container.py +++ b/plugins/modules/lxd_container.py @@ -582,7 +582,7 @@ def _freeze_instance(self): def _unfreeze_instance(self): self._change_state('unfreeze') - self.actions.append('unfreez') + self.actions.append('unfreeze') def _instance_ipv4_addresses(self, ignore_devices=None): ignore_devices = ['lo'] if ignore_devices is None else ignore_devices