Modify the proxmox VM when using clone in the salt-cloud profile if settings are present

salt/cloud/clouds/proxmox.py

@@ -492,6 +492,71 @@ def list_nodes_select(call=None):
     )
 
 
+def _stringlist_to_dictionary(input_string):
+    '''
+    Convert a stringlist (comma separated settings) to a dictionary
+
+    The result of the string setting1=value1,setting2=value2 will be a python dictionary:
+
+    {'setting1':'value1','setting2':'value2'}
+    '''
+    return dict(item.strip().split("=") for item in input_string.split(",") if item)
+
+
+def _dictionary_to_stringlist(input_dict):
+    '''
+    Convert a dictionary to a stringlist (comma separated settings)
+
+    The result of the dictionary {'setting1':'value1','setting2':'value2'} will be:
+
+    setting1=value1,setting2=value2
+    '''
+    return ','.join('{}={}'.format(k, input_dict[k]) for k in sorted(input_dict.keys()))
+
+
+def _reconfigure_clone(vm_, vmid):
+    '''
+    If we cloned a machine, see if we need to reconfigure any of the options such as net0,
+    ide2, etc. This enables us to have a different cloud-init ISO mounted for each VM that's brought up
+    :param vm_:
+    :return:
+    '''
+    if not vm_.get('technology') == 'qemu':
+        log.warning('Reconfiguring clones is only available under `qemu`')
+        return
+
+    # TODO: Support other settings here too as these are not the only ones that can be modified after a clone operation
+    log.info('Configuring cloned VM')
+
+    # Modify the settings for the VM one at a time so we can see any problems with the values
+    # as quickly as possible
+    for setting in vm_:
+        if re.match(r'^(ide|sata|scsi)(\d+)$', setting):
+            postParams = {setting: vm_[setting]}
+            query('post', 'nodes/{0}/qemu/{1}/config'.format(vm_['host'], vmid), postParams)
+
+        elif re.match(r'^net(\d+)$', setting):
+            # net strings are a list of comma seperated settings. We need to merge the settings so that
+            # the setting in the profile only changes the settings it touches and the other settings
+            # are left alone. An example of why this is necessary is because the MAC address is set
+            # in here and generally you don't want to alter or have to know the MAC address of the new
+            # instance, but you may want to set the VLAN bridge
+            data = query('get', 'nodes/{0}/qemu/{1}/config'.format(vm_['host'], vmid))
+
+            # Generate a dictionary of settings from the existing string
+            new_setting = {}
+            if setting in data:
+                new_setting.update(_stringlist_to_dictionary(data[setting]))
+
+            # Merge the new settings (as a dictionary) into the existing dictionary to get the
+            # new merged settings
+            new_setting.update(_stringlist_to_dictionary(vm_[setting]))
+
+            # Convert the dictionary back into a string list
+            postParams = {setting: _dictionary_to_stringlist(new_setting)}
+            query('post', 'nodes/{0}/qemu/{1}/config'.format(vm_['host'], vmid), postParams)
+
+
 def create(vm_):
     '''
     Create a single VM from a data dict
@@ -575,6 +640,9 @@ def create(vm_):
     if not wait_for_created(data['upid'], timeout=300):
         return {'Error': 'Unable to create {0}, command timed out'.format(name)}
 
+    if vm_.get('clone') is True:
+        _reconfigure_clone(vm_, vmid)
+
     # VM has been created. Starting..
     if not start(name, vmid, call='action'):
         log.error('Node %s (%s) failed to start!', name, vmid)

tests/unit/cloud/clouds/test_proxmox.py

@@ -0,0 +1,94 @@
+# -*- coding: utf-8 -*-
+'''
+    :codeauthor: Tyler Johnson <[email protected]>
+'''
+
+# Import Salt Libs
+from __future__ import absolute_import, print_function, unicode_literals
+
+# Import Salt Testing Libs
+from tests.support.mixins import LoaderModuleMockMixin
+from tests.support.unit import TestCase
+from tests.support.mock import (
+    MagicMock,
+    patch,
+)
+
+# Import Salt Libs
+from salt.cloud.clouds import proxmox
+
+
+class ProxmoxTest(TestCase, LoaderModuleMockMixin):
+    def setup_loader_modules(self):
+        return {
+            proxmox: {
+                '__utils__': {
+                    'cloud.fire_event': MagicMock(),
+                    'cloud.bootstrap': MagicMock()
+                },
+                '__opts__': {
+                    'sock_dir': True,
+                    'transport': True,
+                    'providers': {'my_proxmox': {}},
+                    'profiles': {'my_proxmox': {}}
+                },
+                '__active_provider_name__': 'my_proxmox:proxmox'
+            }
+        }
+
+    def setUp(self):
+        self.vm_ = {
+            'profile': 'my_proxmox',
+            'name': 'vm4',
+            'driver': 'proxmox',
+            'technology': 'qemu',
+            'host': '127.0.0.1',
+            'clone': True,
+            'ide0': 'data',
+            'sata0': 'data',
+            'scsi0': 'data',
+            'net0': 'a=b,c=d',
+        }
+
+    def tearDown(self):
+        del self.vm_
+
+    def test__stringlist_to_dictionary(self):
+        result = proxmox._stringlist_to_dictionary('')
+        self.assertEqual(result, {})
+
+        result = proxmox._stringlist_to_dictionary('foo=bar, ignored_space=bar,internal space=bar')
+        self.assertEqual(result, {'foo': 'bar', 'ignored_space': 'bar', 'internal space': 'bar'})
+
+        # Negative cases
+        self.assertRaises(ValueError, proxmox._stringlist_to_dictionary, 'foo=bar,foo')
+        self.assertRaises(ValueError, proxmox._stringlist_to_dictionary, 'foo=bar,totally=invalid=assignment')
+
+    def test__dictionary_to_stringlist(self):
+        result = proxmox._dictionary_to_stringlist({})
+        self.assertEqual(result, '')
+
+        result = proxmox._dictionary_to_stringlist({'a': 'a'})
+        self.assertEqual(result, 'a=a')
+
+        result = proxmox._dictionary_to_stringlist({'a': 'a', 'b': 'b'})
+        self.assertEqual(result, 'a=a,b=b')
+
+    def test__reconfigure_clone(self):
+        # The return_value is for the net reconfigure assertions, it is irrelevant for the rest
+        with patch.object(proxmox, 'query', return_value={'net0': 'c=overwritten,g=h'}) as query:
+            # Test a vm that lacks the required attributes
+            proxmox._reconfigure_clone({}, 0)
+            query.assert_not_called()
+
+            # Test a fully mocked vm
+            proxmox._reconfigure_clone(self.vm_, 0)
+
+            # net reconfigure
+            query.assert_any_call('get', 'nodes/127.0.0.1/qemu/0/config')
+            query.assert_any_call('post', 'nodes/127.0.0.1/qemu/0/config', {'net0': 'a=b,c=d,g=h'})
+
+            # hdd reconfigure
+            query.assert_any_call('post', 'nodes/127.0.0.1/qemu/0/config', {'ide0': 'data'})
+            query.assert_any_call('post', 'nodes/127.0.0.1/qemu/0/config', {'sata0': 'data'})
+            query.assert_any_call('post', 'nodes/127.0.0.1/qemu/0/config', {'scsi0': 'data'})