Merge pull request #7187 from ajithhub/nics_as_list

Reorganize NIC Profiles as list of interfaces

salt/modules/virt.py

@@ -219,6 +219,24 @@ def _prepare_serial_port_xml(serial_type='pty', telnet_port='', console=True, **
                            telnet_port=telnet_port,
                            console=console)
 
+def _prepare_nics_xml(interfaces):
+    '''
+    Prepares the network interface section of the VM xml
+
+    interfaces: list of dicts as returned from _nic_profile
+
+    Returns string representing interfaces devices suitable for
+    insertion into the VM XML definition
+    '''
+
+    template_name = 'interface.jinja'
+
+    try:
+        template = JINJA.get_template(template_name)
+    except jinja2.exceptions.TemplateNotFound:
+        log.error('Could not load template {0}'.format(template_name))
+        return ''
+    return template.render(interfaces=interfaces)
 
 def _gen_xml(name,
              cpu,
@@ -335,30 +353,9 @@ def _gen_xml(name,
             disk_str += disk_i
     data = data.replace('%%DISKS%%', disk_str)
 
-    nic_str = ''
-    for dev, args in nicp.items():
-        nic_t = '''
-                <interface type='%%TYPE%%'>
-                    <source %%SOURCE%%/>
-                    <mac address='%%MAC%%'/>
-                    <model type='%%MODEL%%'/>
-                </interface>
-'''
-        if 'bridge' in args:
-            nic_t = nic_t.replace('%%SOURCE%%', 'bridge=\'{0}\''.format(args['bridge']))
-            nic_t = nic_t.replace('%%TYPE%%', 'bridge')
-        elif 'network' in args:
-            nic_t = nic_t.replace('%%SOURCE%%', 'network=\'{0}\''.format(args['network']))
-            nic_t = nic_t.replace('%%TYPE%%', 'network')
-        if 'model' in args:
-            nic_t = nic_t.replace('%%MODEL%%', args['model'])
-        dmac = '{0}_mac'.format(dev)
-        if dmac in kwargs:
-            nic_t = nic_t.replace('%%MAC%%', kwargs[dmac])
-        else:
-            nic_t = nic_t.replace('%%MAC%%', salt.utils.gen_mac())
-        nic_str += nic_t
+    nic_str = _prepare_nics_xml(nicp)
     data = data.replace('%%NICS%%', nic_str)
+
     return data
 
 
@@ -430,7 +427,6 @@ def _image_type(vda):
     else:
         return 'raw'
 
-
 # TODO: this function is deprecated, should be merged and replaced
 # with _disk_profile()
 def _get_image_info(hypervisor, name, **kwargs):
@@ -450,6 +446,96 @@ def _get_image_info(hypervisor, name, **kwargs):
     return ret
 
 
+class _Interface(object):
+    def __init__(self,
+                 name=None,
+                 source=None,
+                 type=None,
+                 model='virtio',
+                 mac=None,
+                 **kwargs):
+        if not (source and type):
+            raise Exception('Need a source and type for _Interface, '
+                            'got: \'{0}\' and \'{1}\' respectively'
+                            .format(source, type))
+        self.name = name
+        self.model = model
+        self.source = source 
+        self.type = type 
+
+        if mac is not None:
+            self.mac = mac
+        else:
+            self.mac = salt.utils.gen_mac()
+
+    def __str__(self):
+        return ('Name: {0}, Type: {1}, Source: {2}, '
+                'Model: {3}, MAC: {4}'
+                .format(self.name, self.type, self.source, self.model, self.mac))
+
+    def __repr__(self):
+        return '{0}  ({1})'.format(self.__class__, self.__str__())
+
+
+class _NicProfile(object):
+    default = {'eth0': {'bridge': 'br0', 'model': 'virtio'}}
+
+    def __init__(self, name):
+        self.interfaces = []
+        self.name = name
+
+        config_data = __salt__['config.option']('virt.nic', {}) \
+                        .get(name, self.__class__.default)
+
+        # support old style dicts
+        if isinstance(config_data, dict):
+            for interface_name, attributes in config_data.items():
+                attributes['name'] = interface_name
+                self.interfaces.append(attributes)
+
+        # new style lists
+        elif isinstance(config_data, list):
+            self.interfaces = config_data
+
+        for interface in self.interfaces:
+            self._normalize_net_types(interface)
+
+    def _normalize_net_types(self, attributes):
+        '''
+        Guess which style of definition:
+
+            bridge: br0
+
+            network: net0
+
+            type: network
+            source: net0
+        '''
+        for type in ['bridge', 'network']:
+            if type in attributes:
+                attributes['type'] = type
+                # we want to discard the original key
+                attributes['source'] = attributes.pop(type)
+
+        attributes['type'] = attributes.get('type', None)
+        attributes['source'] = attributes.get('source', None)
+
+    def __str__(self):
+        lines = []
+        for interface in self.interfaces:
+            lines.append(interface.__str__())
+        return '\n'.join(lines) 
+
+    def create_interfaces(self, **kwargs):
+        results = []
+        for interface in self.interfaces:
+            dmac = '{0}_mac'.format(interface['name'])
+            if dmac in kwargs:
+                interface['mac'] = kwargs[dmac]
+            results.append(_Interface(**interface))
+        return results
+
+
 def _disk_profile(profile, hypervisor, **kwargs):
     '''
     Gather the disk profile from the config or apply the default based
@@ -498,40 +584,108 @@ def _disk_profile(profile, hypervisor, **kwargs):
     return disklist
 
 
-def _nic_profile(profile, hypervisor):
-    '''
-    Gather the nic profile from the config or apply the default based
-    on the active hypervisor
 
-    This is the ``default`` profile for KVM/QEMU, which can be
-    overridden in the configuration:
+def _nic_profile(profile_name, hypervisor, **kwargs):
 
-    .. code-block:: yaml
+    default = [{'eth0': {}}]
+    vmware_overlay = {'type': 'bridge', 'source': 'DEFAULT', 'model': 'e1000'}
+    kvm_overlay    = {'type': 'bridge', 'source': 'br0',     'model': 'virtio'}
+    overlays = {
+            'kvm': kvm_overlay,
+            'qemu': kvm_overlay,
+            'esxi': vmware_overlay,
+            'vmware': vmware_overlay,
+            }
 
-        virt:
-          nic:
-            default:
-              eth0:
-                bridge: br0
-                model: virtio
+    # support old location
+    config_data = __salt__['config.option']('virt.nic', {}) \
+                    .get(profile_name, None)
 
-    The ``model`` parameter is optional, and will default to whatever
-    is best suitable for the active hypervisor.
-    '''
-    default = {'eth0': {}}
-    if hypervisor in ['esxi', 'vmware']:
-        overlay = {'bridge': 'DEFAULT', 'model': 'e1000'}
-    elif hypervisor in ['qemu', 'kvm']:
-        overlay = {'bridge': 'br0', 'model': 'virtio'}
-    else:
-        overlay = {}
-    nics = __salt__['config.get']('virt:nic', {}).get(profile, default)
-    for key, val in overlay.items():
-        for nic in nics:
-            if key not in nics[nic]:
-                nics[nic][key] = val
-    return nics
+    if config_data is None:
+        config_data = __salt__['config.get']('virt:nic', {}).get(profile_name, default)
 
+    interfaces = []
+
+    def append_dict_profile_to_interface_list(profile_dict):
+        for interface_name, attributes in profile_dict.items():
+            attributes['name'] = interface_name
+            interfaces.append(attributes)
+
+    # support old style dicts (top-level dicts)
+    # virt:
+    #    nic:
+    #        eth0:
+    #            bridge: br0
+    #        eth1:
+    #            network: test_net
+    #
+    if isinstance(config_data, dict):
+        append_dict_profile_to_interface_list(config_data)
+
+    # virt:
+    #    nic:
+    #      -  eth0:
+    #           bridge: br0
+    #      -  eth1:
+    #           network: test_net
+    # or
+    #      -  name: eth0
+    #         bridge: br0
+    #      -  name: eth1
+    #         network: test_net
+    #
+    # new style lists (may contain dicts)
+    elif isinstance(config_data, list):
+        for interface in config_data:
+            if isinstance(interface, dict):
+                if len(interface.keys()) == 1:
+                    append_dict_profile_to_interface_list(interface)
+                else:
+                    interfaces.append(interface)
+
+    def _normalize_net_types(attributes):
+        '''
+        Guess which style of definition:
+
+            bridge: br0
+             
+             or
+
+            network: net0
+
+              or
+
+            type: network
+            source: net0
+        '''
+        for type in ['bridge', 'network']:
+            if type in attributes:
+                attributes['type'] = type
+                # we want to discard the original key
+                attributes['source'] = attributes.pop(type)
+
+        attributes['type'] = attributes.get('type', None)
+        attributes['source'] = attributes.get('source', None)
+
+    def _apply_default_overlay(attributes):
+        for key, value in overlays[hypervisor].items():
+            if key not in attributes or not attributes[key]:
+                attributes[key] = value
+
+    def _assign_mac(attributes):
+        dmac = '{0}_mac'.format(attributes['name'])
+        if dmac in kwargs:
+            attributes['mac'] = kwargs[dmac]
+        else:
+            attributes['mac'] = salt.utils.gen_mac()
+
+    for interface in interfaces:
+        _normalize_net_types(interface)
+        _assign_mac(interface)
+        if hypervisor in overlays:
+            _apply_default_overlay(interface)
+
+    return interfaces
 
 def init(name,
          cpu,
@@ -553,7 +707,9 @@ def init(name,
         salt 'hypervisor' virt.init vm_name 4 512 nic=profile disk=profile
     '''
     hypervisor = __salt__['config.get']('libvirt:hypervisor', hypervisor)
-    nicp = _nic_profile(nic, hypervisor)
+
+    nicp = _nic_profile(nic, hypervisor, **kwargs)
+
     diskp = None
     seedable = False
     if image:  # with disk template image

salt/templates/virt/interface.jinja

@@ -0,0 +1,7 @@
+{% for interface in interfaces %}
+                <interface type='{{ interface.type }}'>
+                    <source {{ interface.type }}='{{ interface.source }}'/>
+                    <mac address='{{ interface.mac }}'/>
+                    <model type='{{ interface.model }}'/>
+                </interface>
+{% endfor %}

salt/templates/virt/serial_port_pty.jinja

@@ -1,3 +1,4 @@
+
                 <serial type='pty'>
                     <target port='0'/>
                 </serial>

setup.py

@@ -313,7 +313,11 @@ SETUP_KWARGS = {'name': NAME,
                              'salt.log.handlers',
                              'salt.templates',
                              ],
-                'package_data': {'salt.templates': ['rh_ip/*.jinja']},
+                'package_data': {'salt.templates': [
+                                    'rh_ip/*.jinja',
+                                    'virt/*.jinja'
+                                    ]
+                                },
                 'data_files': [('share/man/man1',
                                 ['doc/man/salt-master.1',
                                  'doc/man/salt-key.1',

tests/unit/modules/virt_test.py

@@ -2,6 +2,8 @@
 
 # Import python libs
 import sys
+from xml.etree import ElementTree as ElementTree
+import re
 
 # Import Salt Testing libs
 from salttesting import skipIf, TestCase
@@ -12,7 +14,8 @@ ensure_in_syspath('../../')
 # Import salt libs
 from salt.modules import virt
 from salt.modules import config
-from salt._compat import StringIO as _StringIO, ElementTree as _ElementTree
+from salt._compat import ElementTree as _ElementTree
+import salt.utils
 
 # Import third party libs
 import yaml
@@ -29,6 +32,44 @@ virt.__salt__ = {
 @skipIf(NO_MOCK, NO_MOCK_REASON)
 class VirtTestCase(TestCase):
 
+    @skipIf(sys.version_info < (2, 7), 'ElementTree version 1.3 required'
+            ' which comes with Python 2.7')
+    def test_gen_xml_for_serial(self):
+        diskp = virt._disk_profile('default', 'kvm')
+        nicp = virt._nic_profile('default', 'kvm')
+        xml_data = virt._gen_xml(
+            'hello',
+            1,
+            512,
+            diskp,
+            nicp,
+            'kvm',
+            serial_type="pty",
+            console=True
+            )
+        root = ElementTree.fromstring(xml_data)
+        self.assertEquals(root.find('devices/serial').attrib['type'], 'pty')
+        self.assertEquals(root.find('devices/console').attrib['type'], 'pty')
+
+    @skipIf(sys.version_info < (2, 7), 'ElementTree version 1.3 required'
+            ' which comes with Python 2.7')
+    def test_gen_xml_for_serial_no_console(self):
+        diskp = virt._disk_profile('default', 'kvm')
+        nicp = virt._nic_profile('default', 'kvm')
+        xml_data = virt._gen_xml(
+            'hello',
+            1,
+            512,
+            diskp,
+            nicp,
+            'kvm',
+            serial_type="pty",
+            console=False
+            )
+        root = ElementTree.fromstring(xml_data)
+        self.assertEquals(root.find('devices/serial').attrib['type'], 'pty')
+        self.assertEquals(root.find('devices/console'), None)
+
     def test_default_disk_profile_hypervisor_esxi(self):
         mock = MagicMock(return_value={})
         with patch.dict(virt.__salt__, {'config.get': mock}):
@@ -56,9 +97,10 @@ class VirtTestCase(TestCase):
         with patch.dict(virt.__salt__, {'config.get': mock}):
             ret = virt._nic_profile('nonexistant', 'esxi')
             self.assertTrue(len(ret) == 1)
-            self.assertIn('eth0', ret)
-            eth0 = ret['eth0']
-            self.assertEqual(eth0['bridge'], 'DEFAULT')
+            eth0 = ret[0]
+            self.assertEqual(eth0['name'], 'eth0')
+            self.assertEqual(eth0['type'], 'bridge')
+            self.assertEqual(eth0['source'], 'DEFAULT')
             self.assertEqual(eth0['model'], 'e1000')
 
     def test_default_nic_profile_hypervisor_kvm(self):
@@ -66,9 +108,10 @@ class VirtTestCase(TestCase):
         with patch.dict(virt.__salt__, {'config.get': mock}):
             ret = virt._nic_profile('nonexistant', 'kvm')
             self.assertTrue(len(ret) == 1)
-            self.assertIn('eth0', ret)
-            eth0 = ret['eth0']
-            self.assertEqual(eth0['bridge'], 'br0')
+            eth0 = ret[0]
+            self.assertEqual(eth0['name'], 'eth0')
+            self.assertEqual(eth0['type'], 'bridge')
+            self.assertEqual(eth0['source'], 'br0')
             self.assertEqual(eth0['model'], 'virtio')
 
     @skipIf(sys.version_info < (2, 7), 'ElementTree version 1.3 required'
@@ -84,13 +127,25 @@ class VirtTestCase(TestCase):
             nicp,
             'kvm',
             )
-        tree = _ElementTree.parse(_StringIO(xml_data))
-        self.assertTrue(tree.getroot().attrib['type'] == 'kvm')
-        self.assertTrue(tree.find('vcpu').text == '1')
-        self.assertTrue(tree.find('memory').text == '524288')
-        self.assertTrue(tree.find('memory').attrib['unit'] == 'KiB')
-        self.assertTrue(len(tree.findall('.//disk')) == 1)
-        self.assertTrue(len(tree.findall('.//interface')) == 1)
+        root = ElementTree.fromstring(xml_data)
+        self.assertTrue(root.attrib['type'] == 'kvm')
+        self.assertTrue(root.find('vcpu').text == '1')
+        self.assertTrue(root.find('memory').text == '524288')
+        self.assertTrue(root.find('memory').attrib['unit'] == 'KiB')
+        self.assertTrue(len(root.findall('.//disk')) == 1)
+
+        interfaces = root.findall('.//interface')
+        self.assertEquals(len(interfaces), 1)
+
+        iface = interfaces[0]
+        self.assertEquals(iface.attrib['type'], 'bridge')
+        self.assertEquals(iface.find('source').attrib['bridge'], 'br0')
+        self.assertEquals(iface.find('model').attrib['type'], 'virtio')
+
+        mac = iface.find('mac').attrib['address']
+        self.assertTrue(
+              re.match('^([0-9A-F]{2}[:-]){5}([0-9A-F]{2})$', mac, re.I))
+
 
     @skipIf(sys.version_info < (2, 7), 'ElementTree version 1.3 required'
             ' which comes with Python 2.7')
@@ -105,13 +160,25 @@ class VirtTestCase(TestCase):
             nicp,
             'esxi',
             )
-        tree = _ElementTree.parse(_StringIO(xml_data))
-        self.assertTrue(tree.getroot().attrib['type'] == 'vmware')
-        self.assertTrue(tree.find('vcpu').text == '1')
-        self.assertTrue(tree.find('memory').text == '524288')
-        self.assertTrue(tree.find('memory').attrib['unit'] == 'KiB')
-        self.assertTrue(len(tree.findall('.//disk')) == 1)
-        self.assertTrue(len(tree.findall('.//interface')) == 1)
+        root = _ElementTree.fromstring(xml_data)
+        self.assertTrue(root.attrib['type'] == 'vmware')
+        self.assertTrue(root.find('vcpu').text == '1')
+        self.assertTrue(root.find('memory').text == '524288')
+        self.assertTrue(root.find('memory').attrib['unit'] == 'KiB')
+        self.assertTrue(len(root.findall('.//disk')) == 1)
+
+        interfaces = root.findall('.//interface')
+        self.assertEquals(len(interfaces), 1)
+
+        iface = interfaces[0]
+        self.assertEquals(iface.attrib['type'], 'bridge')
+        self.assertEquals(iface.find('source').attrib['bridge'], 'DEFAULT')
+        self.assertEquals(iface.find('model').attrib['type'], 'e1000')
+
+        mac = iface.find('mac').attrib['address']
+        self.assertTrue(
+              re.match('^([0-9A-F]{2}[:-]){5}([0-9A-F]{2})$', mac, re.I))
+
 
     @skipIf(sys.version_info < (2, 7), 'ElementTree version 1.3 required'
             ' which comes with Python 2.7')
@@ -123,18 +190,24 @@ class VirtTestCase(TestCase):
     size: 8192
     format: vmdk
     model: scsi
+    pool: datastore1
 - second:
     size: 4096
     format: vmdk  # FIX remove line, currently test fails
     model: scsi   # FIX remove line, currently test fails
+    pool: datastore2
 '''
         nicp_yaml = '''
-eth1:
-  bridge: ONENET
+- type: bridge
+  name: eth1
+  source: ONENET
   model: e1000
-eth2:
-  bridge: TWONET
-  model: e1000    # FIX remove line, currently test fails
+  mac: '00:00:00:00:00:00'
+- name: eth2
+  type: bridge
+  source: TWONET
+  model: e1000
+  mac: '00:00:00:00:00:00'
 '''
         disk_profile.return_value = yaml.load(diskp_yaml)
         nic_profile.return_value = yaml.load(nicp_yaml)
@@ -147,15 +220,14 @@ eth2:
             diskp,
             nicp,
             'esxi',
-            eth1_mac='00:00:00:00:00:00',  # FIX test for this
             )
-        tree = _ElementTree.parse(_StringIO(xml_data))
-        self.assertTrue(tree.getroot().attrib['type'] == 'vmware')
-        self.assertTrue(tree.find('vcpu').text == '1')
-        self.assertTrue(tree.find('memory').text == '524288')
-        self.assertTrue(tree.find('memory').attrib['unit'] == 'KiB')
-        self.assertTrue(len(tree.findall('.//disk')) == 2)
-        self.assertTrue(len(tree.findall('.//interface')) == 2)
+        root = _ElementTree.fromstring(xml_data)
+        self.assertTrue(root.attrib['type'] == 'vmware')
+        self.assertTrue(root.find('vcpu').text == '1')
+        self.assertTrue(root.find('memory').text == '524288')
+        self.assertTrue(root.find('memory').attrib['unit'] == 'KiB')
+        self.assertTrue(len(root.findall('.//disk')) == 2)
+        self.assertTrue(len(root.findall('.//interface')) == 2)
 
     @skipIf(sys.version_info < (2, 7), 'ElementTree version 1.3 required'
             ' which comes with Python 2.7')
@@ -167,18 +239,24 @@ eth2:
     size: 8192
     format: qcow2
     model: virtio
+    pool: /var/lib/images
 - second:
     size: 4096
     format: qcow2   # FIX remove line, currently test fails
     model: virtio   # FIX remove line, currently test fails
+    pool: /var/lib/images
 '''
         nicp_yaml = '''
-eth1:
-  bridge: br1
+- type: bridge
+  name: eth1
+  source: b2 
+  model: virtio 
+  mac: '00:00:00:00:00:00'
+- name: eth2
+  type: bridge
+  source: b2 
   model: virtio
-eth2:
-  bridge: b2
-  model: virtio     # FIX remove line, currently test fails
+  mac: '00:00:00:00:00:00'
 '''
         disk_profile.return_value = yaml.load(diskp_yaml)
         nic_profile.return_value = yaml.load(nicp_yaml)
@@ -191,15 +269,57 @@ eth2:
             diskp,
             nicp,
             'kvm',
-            eth1_mac='00:00:00:00:00:00',  # FIX test for this
             )
-        tree = _ElementTree.parse(_StringIO(xml_data))
-        self.assertTrue(tree.getroot().attrib['type'] == 'kvm')
-        self.assertTrue(tree.find('vcpu').text == '1')
-        self.assertTrue(tree.find('memory').text == '524288')
-        self.assertTrue(tree.find('memory').attrib['unit'] == 'KiB')
-        self.assertTrue(len(tree.findall('.//disk')) == 2)
-        self.assertTrue(len(tree.findall('.//interface')) == 2)
+        root = _ElementTree.fromstring(xml_data)
+        self.assertTrue(root.attrib['type'] == 'kvm')
+        self.assertTrue(root.find('vcpu').text == '1')
+        self.assertTrue(root.find('memory').text == '524288')
+        self.assertTrue(root.find('memory').attrib['unit'] == 'KiB')
+        self.assertTrue(len(root.findall('.//disk')) == 2)
+        self.assertTrue(len(root.findall('.//interface')) == 2)
+
+
+    def test_mixed_dict_and_list_as_profile_objects(self):
+
+        yaml_config = '''
+          virt.nic:
+             new-listonly-profile:
+                - bridge: br0
+                  name: eth0
+                - model: virtio
+                  name: eth1
+                  source: test_network
+                  type: network
+             new-list-with-legacy-names:
+                - eth0:
+                     bridge: br0
+                - eth1:
+                     bridge: br1
+                     model: virtio
+             non-default-legacy-profile:
+                eth0:
+                   bridge: br0
+                eth1:
+                   bridge: br1
+                   model: virtio
+        '''
+        mock_config = yaml.load(yaml_config)
+        salt.modules.config.__opts__ = mock_config
+
+        for name in mock_config['virt.nic'].keys():
+            profile = salt.modules.virt._nic_profile(name, 'kvm')
+            self.assertEquals(len(profile), 2)
+
+            interface_attrs = profile[0]
+            self.assertIn('source', interface_attrs)
+            self.assertIn('type', interface_attrs)
+            self.assertIn('name', interface_attrs)
+            self.assertIn('model', interface_attrs)
+            self.assertEquals(interface_attrs['model'], 'virtio')
+            self.assertIn('mac', interface_attrs)
+            self.assertTrue(
+                re.match('^([0-9A-F]{2}[:-]){5}([0-9A-F]{2})$',
+                interface_attrs['mac'] , re.I))
 
 
 if __name__ == '__main__':

tests/unit/simple.py

@@ -1,4 +1,7 @@
 from salttesting import TestCase, expectedFailure
+from salttesting.helpers import ensure_in_syspath
+
+ensure_in_syspath('../')
 
 
 class SimpleTest(TestCase):
@@ -8,3 +11,7 @@ class SimpleTest(TestCase):
     @expectedFailure
     def test_fail(self):
         assert False
+
+if __name__ == '__main__':
+    from integration import run_tests
+    run_tests(SimpleTest, needs_daemon=False)