virt: let libvirt generate MAC addresses

There is no need to generate MAC addresses in the virt module if the
user hasn't provided any. This only makes it harder to make the
difference between a real mac address change from the user and a new
generated one.

Now the mac address is not written in the domain XML definition if not
provided by the user. This avoids unnecessary changes when applying
virt.running.

salt/modules/virt.py

@@ -1272,19 +1272,8 @@ def _complete_nics(interfaces, hypervisor):
             if key not in attributes or not attributes[key]:
                 attributes[key] = value
 
-    def _assign_mac(attributes, hypervisor):
-        """
-        Compute mac address for NIC depending on hypervisor
-        """
-        if hypervisor in ["qemu", "kvm"]:
-            attributes["mac"] = salt.utils.network.gen_mac(prefix="52:54:00")
-        else:
-            attributes["mac"] = salt.utils.network.gen_mac()
-
     for interface in interfaces:
         _normalize_net_types(interface)
-        if interface.get("mac", None) is None:
-            _assign_mac(interface, hypervisor)
         if hypervisor in overlays:
             _apply_default_overlay(interface)
 
@@ -1850,15 +1839,22 @@ def _nics_equal(nic1, nic2):
             "source": nic.find("source").attrib[nic.attrib["type"]]
             if nic.find("source") is not None
             else None,
-            "mac": nic.find("mac").attrib["address"].lower()
-            if nic.find("mac") is not None
-            else None,
             "model": nic.find("model").attrib["type"]
             if nic.find("model") is not None
             else None,
         }
 
-    return _filter_nic(nic1) == _filter_nic(nic2)
+    def _get_mac(nic):
+        return (
+            nic.find("mac").attrib["address"].lower()
+            if nic.find("mac") is not None
+            else None
+        )
+
+    mac1 = _get_mac(nic1)
+    mac2 = _get_mac(nic2)
+    macs_equal = not mac1 or not mac2 or mac1 == mac2
+    return _filter_nic(nic1) == _filter_nic(nic2) and macs_equal
 
 
 def _graphics_equal(gfx1, gfx2):
@@ -1969,16 +1965,7 @@ def _diff_interface_lists(old, new):
     :param old: list of ElementTree nodes representing the old interfaces
     :param new: list of ElementTree nodes representing the new interfaces
     """
-    diff = _diff_lists(old, new, _nics_equal)
-
-    # Remove duplicated addresses mac addresses and let libvirt generate them for us
-    macs = [nic.find("mac").get("address") for nic in diff["unchanged"]]
-    for nic in diff["new"]:
-        mac = nic.find("mac")
-        if mac.get("address") in macs:
-            nic.remove(mac)
-
-    return diff
+    return _diff_lists(old, new, _nics_equal)
 
 
 def _diff_graphics_lists(old, new):

salt/templates/virt/libvirt_domain.jinja

@@ -52,7 +52,9 @@
                 {% for nic in nics %}
                 <interface type='{{ nic.type }}'>
                     <source {{ nic.type }}='{{ nic.source }}'/>
+                    {% if nic.get('mac') -%}
                     <mac address='{{ nic.mac }}'/>
+                    {%- endif %}
                     {% if nic.model %}<model type='{{ nic.model }}'/>{% endif %}
                 </interface>
                 {% endfor %}

tests/unit/modules/test_virt.py

@@ -10,7 +10,6 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 import datetime
 import os
-import re
 import shutil
 import tempfile
 
@@ -634,9 +633,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual(iface.find("source").attrib["bridge"], "br0")
         self.assertEqual(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))
-
     def test_gen_xml_for_esxi_default_profile(self):
         """
         Test virt._gen_xml(), ESXi/vmware default profile case
@@ -668,9 +664,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual(iface.find("source").attrib["bridge"], "DEFAULT")
         self.assertEqual(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))
-
     def test_gen_xml_for_xen_default_profile(self):
         """
         Test virt._gen_xml(), XEN PV default profile case
@@ -915,8 +908,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
         controllers = root.findall(".//devices/controller")
         # There should be no controller
         self.assertTrue(len(controllers) == 0)
-        # kvm mac address shoud start with 52:54:00
-        self.assertTrue("mac address='52:54:00" in xml_data)
 
     def test_diff_disks(self):
         """
@@ -1751,12 +1742,7 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
                         "source": "default",
                         "mac": "52:54:00:39:02:b1",
                     },
-                    {
-                        "name": "eth1",
-                        "type": "network",
-                        "source": "oldnet",
-                        "mac": "52:54:00:39:02:b2",
-                    },
+                    {"name": "eth1", "type": "network", "source": "oldnet"},
                 ],
                 graphics={
                     "type": "spice",
@@ -1995,14 +1981,6 @@ class VirtTestCase(TestCase, LoaderModuleMockMixin):
                 self.assertIn("name", interface_attrs)
                 self.assertIn("model", interface_attrs)
                 self.assertEqual(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,
-                    )
-                )
 
     def test_get_xml(self):
         """