diff --git a/changelogs/fragments/kubevirt_vm_fix_craete_template.yml b/changelogs/fragments/kubevirt_vm_fix_craete_template.yml new file mode 100644 index 00000000000..3c79318fb57 --- /dev/null +++ b/changelogs/fragments/kubevirt_vm_fix_craete_template.yml @@ -0,0 +1,3 @@ +--- +minor_changes: + - Improve creating VM from template. Merge VM disks/interfaces with the template defaults. diff --git a/lib/ansible/module_utils/kubevirt.py b/lib/ansible/module_utils/kubevirt.py index 247e498b931..4b6f4523bca 100644 --- a/lib/ansible/module_utils/kubevirt.py +++ b/lib/ansible/module_utils/kubevirt.py @@ -7,9 +7,11 @@ from collections import defaultdict from distutils.version import Version +from ansible.module_utils.common._collections_compat import Sequence from ansible.module_utils.k8s.common import list_dict_str from ansible.module_utils.k8s.raw import KubernetesRawModule +import copy import re MAX_SUPPORTED_API_VERSION = 'v1alpha3' @@ -126,21 +128,25 @@ class KubeVirtRawModule(KubernetesRawModule): super(KubeVirtRawModule, self).__init__(*args, **kwargs) @staticmethod - def merge_dicts(x, y): + def merge_dicts(base_dict, merging_dicts): + """This function merges a base dictionary with one or more other dictionaries. + The base dictionary takes precedence when there is a key collision. + merging_dicts can be a dict or a list or tuple of dicts. In the latter case, the + dictionaries at the front of the list have higher precedence over the ones at the end. """ - This function merge two dictionaries, where the first dict has - higher priority in merging two same keys. - """ - for k in set(x.keys()).union(y.keys()): - if k in x and k in y: - if isinstance(x[k], dict) and isinstance(y[k], dict): - yield (k, dict(KubeVirtRawModule.merge_dicts(x[k], y[k]))) - else: - yield (k, y[k]) - elif k in x: - yield (k, x[k]) - else: - yield (k, y[k]) + if not merging_dicts: + merging_dicts = ({},) + + if not isinstance(merging_dicts, Sequence): + merging_dicts = (merging_dicts,) + + new_dict = {} + for d in reversed(merging_dicts): + new_dict.update(d) + + new_dict.update(base_dict) + + return new_dict def get_resource(self, resource): try: @@ -215,16 +221,23 @@ class KubeVirtRawModule(KubernetesRawModule): 'disk': {'bus': 'virtio'}, }) - def _define_interfaces(self, interfaces, template_spec): + def _define_interfaces(self, interfaces, template_spec, defaults): """ Takes interfaces parameter of Ansible and create kubevirt API interfaces and networks strucutre out from it. """ + if not interfaces and defaults and 'interfaces' in defaults: + interfaces = copy.deepcopy(defaults['interfaces']) + for d in interfaces: + d['network'] = defaults['networks'][0] + if interfaces: # Extract interfaces k8s specification from interfaces list passed to Ansible: spec_interfaces = [] for i in interfaces: - spec_interfaces.append(dict((k, v) for k, v in i.items() if k != 'network')) + spec_interfaces.append( + self.merge_dicts(dict((k, v) for k, v in i.items() if k != 'network'), defaults['interfaces']) + ) if 'interfaces' not in template_spec['domain']['devices']: template_spec['domain']['devices']['interfaces'] = [] template_spec['domain']['devices']['interfaces'].extend(spec_interfaces) @@ -234,21 +247,28 @@ class KubeVirtRawModule(KubernetesRawModule): for i in interfaces: net = i['network'] net['name'] = i['name'] - spec_networks.append(net) + spec_networks.append(self.merge_dicts(net, defaults['networks'])) if 'networks' not in template_spec: template_spec['networks'] = [] template_spec['networks'].extend(spec_networks) - def _define_disks(self, disks, template_spec): + def _define_disks(self, disks, template_spec, defaults): """ Takes disks parameter of Ansible and create kubevirt API disks and volumes strucutre out from it. """ + if not disks and defaults and 'disks' in defaults: + disks = copy.deepcopy(defaults['disks']) + for d in disks: + d['volume'] = defaults['volumes'][0] + if disks: # Extract k8s specification from disks list passed to Ansible: spec_disks = [] for d in disks: - spec_disks.append(dict((k, v) for k, v in d.items() if k != 'volume')) + spec_disks.append( + self.merge_dicts(dict((k, v) for k, v in d.items() if k != 'volume'), defaults['disks']) + ) if 'disks' not in template_spec['domain']['devices']: template_spec['domain']['devices']['disks'] = [] template_spec['domain']['devices']['disks'].extend(spec_disks) @@ -258,7 +278,7 @@ class KubeVirtRawModule(KubernetesRawModule): for d in disks: volume = d['volume'] volume['name'] = d['name'] - spec_volumes.append(volume) + spec_volumes.append(self.merge_dicts(volume, defaults['volumes'])) if 'volumes' not in template_spec: template_spec['volumes'] = [] template_spec['volumes'].extend(spec_volumes) @@ -274,7 +294,7 @@ class KubeVirtRawModule(KubernetesRawModule): self.fail("API versions {0} are too recent. Max supported is {1}/{2}.".format( str([r.api_version for r in sr]), API_GROUP, MAX_SUPPORTED_API_VERSION)) - def _construct_vm_definition(self, kind, definition, template, params): + def _construct_vm_definition(self, kind, definition, template, params, defaults=None): self.client = self.get_api_client() disks = params.get('disks', []) @@ -328,7 +348,7 @@ class KubeVirtRawModule(KubernetesRawModule): template_spec['domain']['cpu']['model'] = cpu_model if labels: - template['metadata']['labels'] = dict(self.merge_dicts(labels, template['metadata']['labels'])) + template['metadata']['labels'] = self.merge_dicts(labels, template['metadata']['labels']) if machine_type: template_spec['domain']['machine']['type'] = machine_type @@ -343,7 +363,7 @@ class KubeVirtRawModule(KubernetesRawModule): template_spec['domain']['devices']['autoattachGraphicsDevice'] = not headless # Define disks - self._define_disks(disks, template_spec) + self._define_disks(disks, template_spec, defaults) # Define cloud init disk if defined: # Note, that this must be called after _define_disks, so the cloud_init @@ -351,18 +371,15 @@ class KubeVirtRawModule(KubernetesRawModule): self._define_cloud_init(cloud_init_nocloud, template_spec) # Define interfaces: - self._define_interfaces(interfaces, template_spec) + self._define_interfaces(interfaces, template_spec, defaults) # Define datavolumes: self._define_datavolumes(datavolumes, definition['spec']) - # Perform create/absent action: - definition = dict(self.merge_dicts(self.resource_definitions[0], definition)) - resource = self.find_supported_resource(kind) - return dict(self.merge_dicts(self.resource_definitions[0], definition)) + return self.merge_dicts(definition, self.resource_definitions[0]) - def construct_vm_definition(self, kind, definition, template): - definition = self._construct_vm_definition(kind, definition, template, self.params) + def construct_vm_definition(self, kind, definition, template, defaults=None): + definition = self._construct_vm_definition(kind, definition, template, self.params, defaults) resource = self.find_supported_resource(kind) definition = self.set_defaults(resource, definition) return resource, definition diff --git a/lib/ansible/modules/cloud/kubevirt/kubevirt_preset.py b/lib/ansible/modules/cloud/kubevirt/kubevirt_preset.py index 303ee05c1d8..54927938890 100644 --- a/lib/ansible/modules/cloud/kubevirt/kubevirt_preset.py +++ b/lib/ansible/modules/cloud/kubevirt/kubevirt_preset.py @@ -129,8 +129,11 @@ class KubeVirtVMPreset(KubeVirtRawModule): # attributes there, remove when we do: definition['spec']['domain']['devices'] = dict() + # defaults for template + defaults = {'disks': [], 'volumes': [], 'interfaces': [], 'networks': []} + # Execute the CURD of VM: - dummy, definition = self.construct_vm_definition(KIND, definition, definition) + dummy, definition = self.construct_vm_definition(KIND, definition, definition, defaults) result_crud = self.execute_crud(KIND, definition) changed = result_crud['changed'] result = result_crud.pop('result') diff --git a/lib/ansible/modules/cloud/kubevirt/kubevirt_pvc.py b/lib/ansible/modules/cloud/kubevirt/kubevirt_pvc.py index de707723374..6607150ab24 100644 --- a/lib/ansible/modules/cloud/kubevirt/kubevirt_pvc.py +++ b/lib/ansible/modules/cloud/kubevirt/kubevirt_pvc.py @@ -435,7 +435,7 @@ class KubevirtPVC(KubernetesRawModule): spec['volumeName'] = self.params.get('volume_name') # 'resource_definition:' has lower priority than module parameters - definition = dict(KubeVirtRawModule.merge_dicts(self.resource_definitions[0], definition)) + definition = KubeVirtRawModule.merge_dicts(self.resource_definitions[0], definition) self.client = self.get_api_client() resource = self.find_resource(KIND, API, fail=True) diff --git a/lib/ansible/modules/cloud/kubevirt/kubevirt_rs.py b/lib/ansible/modules/cloud/kubevirt/kubevirt_rs.py index 4611774ff67..d7b8d0ffefb 100644 --- a/lib/ansible/modules/cloud/kubevirt/kubevirt_rs.py +++ b/lib/ansible/modules/cloud/kubevirt/kubevirt_rs.py @@ -173,9 +173,12 @@ class KubeVirtVMIRS(KubeVirtRawModule): if replicas is not None: definition['spec']['replicas'] = replicas + # defaults for template + defaults = {'disks': [], 'volumes': [], 'interfaces': [], 'networks': []} + # Execute the CURD of VM: template = definition['spec']['template'] - dummy, definition = self.construct_vm_definition(KIND, definition, template) + dummy, definition = self.construct_vm_definition(KIND, definition, template, defaults) result_crud = self.execute_crud(KIND, definition) changed = result_crud['changed'] result = result_crud.pop('result') diff --git a/lib/ansible/modules/cloud/kubevirt/kubevirt_vm.py b/lib/ansible/modules/cloud/kubevirt/kubevirt_vm.py index 5fdcc92d2b7..c8fd0a7257e 100644 --- a/lib/ansible/modules/cloud/kubevirt/kubevirt_vm.py +++ b/lib/ansible/modules/cloud/kubevirt/kubevirt_vm.py @@ -217,8 +217,6 @@ kubevirt_vm: import copy import traceback -from ansible.module_utils.k8s.common import AUTH_ARG_SPEC - from ansible.module_utils.k8s.common import AUTH_ARG_SPEC from ansible.module_utils.kubevirt import ( virtdict, @@ -317,11 +315,31 @@ class KubeVirtVM(KubeVirtRawModule): return changed, k8s_obj + def _process_template_defaults(self, proccess_template, processedtemplate, defaults): + def set_template_default(default_name, default_name_index, definition_spec): + default_value = proccess_template['metadata']['annotations'][default_name] + if default_value: + values = definition_spec[default_name_index] + default_values = [d for d in values if d.get('name') == default_value] + defaults[default_name_index] = default_values + if definition_spec[default_name_index] is None: + definition_spec[default_name_index] = [] + definition_spec[default_name_index].extend([d for d in values if d.get('name') != default_value]) + + devices = processedtemplate['spec']['template']['spec']['domain']['devices'] + spec = processedtemplate['spec']['template']['spec'] + + set_template_default('defaults.template.cnv.io/disk', 'disks', devices) + set_template_default('defaults.template.cnv.io/volume', 'volumes', spec) + set_template_default('defaults.template.cnv.io/nic', 'interfaces', devices) + set_template_default('defaults.template.cnv.io/network', 'networks', spec) + def construct_definition(self, kind, our_state, ephemeral): definition = virtdict() processedtemplate = {} # Construct the API object definition: + defaults = {'disks': [], 'volumes': [], 'interfaces': [], 'networks': []} vm_template = self.params.get('template') if vm_template: # Find the template the VM should be created from: @@ -338,14 +356,16 @@ class KubeVirtVM(KubeVirtRawModule): processedtemplates_res = self.client.resources.get(api_version='template.openshift.io/v1', kind='Template', name='processedtemplates') processedtemplate = processedtemplates_res.create(proccess_template.to_dict()).to_dict()['objects'][0] + # Process defaults of the template: + self._process_template_defaults(proccess_template, processedtemplate, defaults) + if not ephemeral: definition['spec']['running'] = our_state == 'running' template = definition if ephemeral else definition['spec']['template'] template['metadata']['labels']['vm.cnv.io/name'] = self.params.get('name') - dummy, definition = self.construct_vm_definition(kind, definition, template) - definition = dict(self.merge_dicts(processedtemplate, definition)) + dummy, definition = self.construct_vm_definition(kind, definition, template, defaults) - return definition + return self.merge_dicts(definition, processedtemplate) def execute_module(self): # Parse parameters specific to this module: @@ -370,16 +390,18 @@ class KubeVirtVM(KubeVirtRawModule): if our_state != 'absent': self.params['state'] = k8s_state = 'present' + # Start with fetching the current object to make sure it exists + # If it does, but we end up not performing any operations on it, at least we'll be able to return + # its current contents as part of the final json self.client = self.get_api_client() self._kind_resource = self.find_supported_resource(kind) k8s_obj = self.get_resource(self._kind_resource) if not self.check_mode and not vm_spec_change and k8s_state != 'absent' and not k8s_obj: self.fail("It's impossible to create an empty VM or change state of a non-existent VM.") - # Changes in VM's spec or any changes to VMIs warrant a full CRUD, the latter because - # VMIs don't really have states to manage; they're either present or don't exist + # If there are (potential) changes to `spec:` or we want to delete the object, that warrants a full CRUD # Also check_mode always warrants a CRUD, as that'll produce a sane result - if vm_spec_change or ephemeral or k8s_state == 'absent' or self.check_mode: + if vm_spec_change or k8s_state == 'absent' or self.check_mode: definition = self.construct_definition(kind, our_state, ephemeral) result = self.execute_crud(kind, definition) changed = result['changed'] diff --git a/test/units/modules/cloud/kubevirt/test_kubevirt_vm.py b/test/units/modules/cloud/kubevirt/test_kubevirt_vm.py index 235dec4108f..9e8e4df9494 100644 --- a/test/units/modules/cloud/kubevirt/test_kubevirt_vm.py +++ b/test/units/modules/cloud/kubevirt/test_kubevirt_vm.py @@ -136,18 +136,18 @@ def test_simple_merge_dicts(self): dict1 = {'labels': {'label1': 'value'}} dict2 = {'labels': {'label2': 'value'}} dict3 = json.dumps({'labels': {'label1': 'value', 'label2': 'value'}}, sort_keys=True) - assert dict3 == json.dumps(dict(mymodule.KubeVirtVM.merge_dicts(dict1, dict2)), sort_keys=True) + assert dict3 == json.dumps(mymodule.KubeVirtVM.merge_dicts(dict1, dict2), sort_keys=True) def test_simple_multi_merge_dicts(self): dict1 = {'labels': {'label1': 'value', 'label3': 'value'}} dict2 = {'labels': {'label2': 'value'}} dict3 = json.dumps({'labels': {'label1': 'value', 'label2': 'value', 'label3': 'value'}}, sort_keys=True) - assert dict3 == json.dumps(dict(mymodule.KubeVirtVM.merge_dicts(dict1, dict2)), sort_keys=True) + assert dict3 == json.dumps(mymodule.KubeVirtVM.merge_dicts(dict1, dict2), sort_keys=True) def test_double_nested_merge_dicts(self): dict1 = {'metadata': {'labels': {'label1': 'value', 'label3': 'value'}}} dict2 = {'metadata': {'labels': {'label2': 'value'}}} dict3 = json.dumps({'metadata': {'labels': {'label1': 'value', 'label2': 'value', 'label3': 'value'}}}, sort_keys=True) - assert dict3 == json.dumps(dict(mymodule.KubeVirtVM.merge_dicts(dict1, dict2)), sort_keys=True) + assert dict3 == json.dumps(mymodule.KubeVirtVM.merge_dicts(dict1, dict2), sort_keys=True)