Allow adding libvirt name for node

This patch adds new field in node_list config that allows
specifying name of libvirt domain associated with a node.

* Now only 'ip' is required field for node.
* Host moved from namedtuple to custom class.

Change-Id: I47410d4e77c18a3874975812e09ea3bb7f3a574d

os_faults/api/node_collection.py

@@ -11,20 +11,31 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-import collections
 import logging
 import random
 import warnings
 
 from os_faults.api import error
 from os_faults.api.util import public
+from os_faults import utils
 
 LOG = logging.getLogger(__name__)
 
-Host = collections.namedtuple('Host', ['ip', 'mac', 'fqdn'])
+
+class Host(utils.ComparableMixin, utils.ReprMixin):
+
+    ATTRS = ('ip', 'mac', 'fqdn', 'libvirt_name')
+
+    def __init__(self, ip, mac=None, fqdn=None, libvirt_name=None):
+        self.ip = ip
+        self.mac = mac
+        self.fqdn = fqdn
+        self.libvirt_name = libvirt_name
 
 
-class NodeCollection(object):
+class NodeCollection(utils.ReprMixin):
+
+    ATTRS = ('hosts', )
 
     def __init__(self, cloud_management=None, hosts=None):
         self.cloud_management = cloud_management
@@ -34,9 +45,6 @@ class NodeCollection(object):
     def hosts(self):
         return sorted(self._hosts)
 
-    def __repr__(self):
-        return '{}({})'.format(self.__class__.__name__, repr(self.hosts))
-
     def __len__(self):
         return len(self._hosts)
 

os_faults/drivers/libvirt_driver.py

@@ -68,49 +68,51 @@ class LibvirtDriver(power_management.PowerDriver):
 
         return self._cached_conn
 
-    def _find_domain_by_mac_address(self, mac_address):
+    def _find_domain_by_host(self, host):
         for domain in self.conn.listAllDomains():
-            if mac_address in domain.XMLDesc():
+            if host.libvirt_name and host.libvirt_name == domain.name():
+                return domain
+            if host.mac and host.mac in domain.XMLDesc():
                 return domain
 
         raise error.PowerManagementError(
-            'Domain with MAC address %s not found!' % mac_address)
+            'Domain not found for host %s.' % host)
 
     def supports(self, host):
         try:
-            self._find_domain_by_mac_address(host.mac)
+            self._find_domain_by_host(host)
         except error.PowerManagementError:
             return False
         return True
 
     def poweroff(self, host):
-        LOG.debug('Power off domain with MAC address: %s', host.mac)
-        domain = self._find_domain_by_mac_address(host.mac)
+        domain = self._find_domain_by_host(host)
+        LOG.debug('Power off domain with name: %s', host.mac)
         domain.destroy()
         LOG.info('Domain powered off: %s', host.mac)
 
     def poweron(self, host):
-        LOG.debug('Power on domain with MAC address: %s', host.mac)
-        domain = self._find_domain_by_mac_address(host.mac)
+        domain = self._find_domain_by_host(host)
+        LOG.debug('Power on domain with name: %s', domain.name())
         domain.create()
-        LOG.info('Domain powered on: %s', host.mac)
+        LOG.info('Domain powered on: %s', domain.name())
 
     def reset(self, host):
-        LOG.debug('Reset domain with MAC address: %s', host.mac)
-        domain = self._find_domain_by_mac_address(host.mac)
+        domain = self._find_domain_by_host(host)
+        LOG.debug('Reset domain with name: %s', domain.name())
         domain.reset()
-        LOG.info('Domain reset: %s', host.mac)
+        LOG.info('Domain reset: %s', domain.name())
 
     def shutdown(self, host):
-        LOG.debug('Shutdown domain with MAC address: %s', host.mac)
-        domain = self._find_domain_by_mac_address(host.mac)
+        domain = self._find_domain_by_host(host)
+        LOG.debug('Shutdown domain with name: %s', domain.name())
         domain.shutdown()
-        LOG.info('Domain is off: %s', host.mac)
+        LOG.info('Domain is off: %s', domain.name())
 
     def snapshot(self, host, snapshot_name, suspend):
-        LOG.debug('Create snapshot "%s" for domain with MAC address: %s',
-                  snapshot_name, host.mac)
-        domain = self._find_domain_by_mac_address(host.mac)
+        domain = self._find_domain_by_host(host)
+        LOG.debug('Create snapshot "%s" for domain with name: %s',
+                  snapshot_name, domain.name())
         if suspend:
             domain.suspend()
         domain.snapshotCreateXML(
@@ -118,18 +120,18 @@ class LibvirtDriver(power_management.PowerDriver):
                 snapshot_name))
         if suspend:
             domain.resume()
-        LOG.debug('Created snapshot "%s" for domain with MAC address: %s',
-                  snapshot_name, host.mac)
+        LOG.debug('Created snapshot "%s" for domain with name: %s',
+                  snapshot_name, domain.name())
 
     def revert(self, host, snapshot_name, resume):
-        LOG.debug('Revert snapshot "%s" for domain with MAC address: %s',
-                  snapshot_name, host.mac)
-        domain = self._find_domain_by_mac_address(host.mac)
+        domain = self._find_domain_by_host(host)
+        LOG.debug('Revert snapshot "%s" for domain with name: %s',
+                  snapshot_name, domain.name())
         snapshot = domain.snapshotLookupByName(snapshot_name)
         if domain.isActive():
             domain.destroy()
         domain.revertToSnapshot(snapshot)
         if resume:
             domain.resume()
-        LOG.debug('Reverted snapshot "%s" for domain with MAC address: %s',
-                  snapshot_name, host.mac)
+        LOG.debug('Reverted snapshot "%s" for domain with name: %s',
+                  snapshot_name, domain.name())

os_faults/drivers/node_list.py

@@ -32,6 +32,7 @@ class NodeListDiscover(node_discover.NodeDiscover):
           - ip: 10.0.0.51
             mac: aa:bb:cc:dd:ee:01
             fqdn: node1.local
+            libvirt_name: node1
           - ip: 10.0.0.52
             mac: aa:bb:cc:dd:ee:02
             fqdn: node2.local
@@ -54,16 +55,16 @@ class NodeListDiscover(node_discover.NodeDiscover):
                     'pattern': utils.MACADDR_REGEXP,
                 },
                 'fqdn': {'type': 'string'},
+                'libvirt_name': {'type': 'string'},
             },
-            'required': ['ip', 'mac', 'fqdn'],
+            'required': ['ip'],
             'additionalProperties': False,
         },
         'minItems': 1,
     }
 
     def __init__(self, conf):
-        self.hosts = [node_collection.Host(ip=host['ip'], mac=host['mac'],
-                                           fqdn=host['fqdn']) for host in conf]
+        self.hosts = [node_collection.Host(**host) for host in conf]
 
     def discover_hosts(self):
         """Discover hosts

os_faults/tests/unit/api/test_node_collection.py

@@ -52,14 +52,14 @@ class NodeCollectionTestCase(test.TestCase):
             hosts=copy.deepcopy(self.hosts))
 
         self.hosts2 = [
-            node_collection.Host(ip='10.0.0.2', mac='09:7b:74:90:63:c1',
-                                 fqdn='node1.com'),
+            node_collection.Host(ip='10.0.0.7', mac='09:7b:74:90:63:c7',
+                                 fqdn='node6.com'),
             node_collection.Host(ip='10.0.0.3', mac='09:7b:74:90:63:c2',
                                  fqdn='node2.com'),
             node_collection.Host(ip='10.0.0.6', mac='09:7b:74:90:63:c6',
                                  fqdn='node5.com'),
-            node_collection.Host(ip='10.0.0.7', mac='09:7b:74:90:63:c7',
-                                 fqdn='node6.com'),
+            node_collection.Host(ip='10.0.0.2', mac='09:7b:74:90:63:c1',
+                                 fqdn='node1.com'),
         ]
 
         self.node_collection2 = node_collection.NodeCollection(

os_faults/tests/unit/api/test_power_management.py

@@ -101,7 +101,7 @@ class PowerManagerTestCase(test.TestCase):
                                 self.pm.reset, self.hosts)
         self.assertEqual("No supported driver found for host "
                          "Host(ip='10.0.0.3', mac='09:7b:74:90:63:c2', "
-                         "fqdn='node2.com')", str(exc))
+                         "fqdn='node2.com', libvirt_name=None)", str(exc))
 
     def test_run_no_drivers(self):
         self.pm = power_management.PowerManager()
@@ -110,4 +110,4 @@ class PowerManagerTestCase(test.TestCase):
                                 self.pm.reset, self.hosts)
         self.assertEqual("No supported driver found for host "
                          "Host(ip='10.0.0.2', mac='09:7b:74:90:63:c1', "
-                         "fqdn='node1.com')", str(exc))
+                         "fqdn='node1.com', libvirt_name=None)", str(exc))

os_faults/tests/unit/drivers/test_libvirt_driver.py

@@ -49,19 +49,35 @@ class LibvirtDriverTestCase(test.TestCase):
         self.assertEqual(conn, 'some cached connection')
 
     @mock.patch(DRIVER_PATH + '.LibvirtDriver._get_connection')
-    def test__find_domain_by_mac_address(self, mock__get_connection):
+    def test__find_domain_by_host_mac(self, mock__get_connection):
+        host = node_collection.Host(ip='10.0.0.2', mac=':54:00:f9:b8:f9')
         domain1 = mock.MagicMock()
         domain1.XMLDesc.return_value = '52:54:00:ab:64:42'
         domain2 = mock.MagicMock()
         domain2.XMLDesc.return_value = '52:54:00:f9:b8:f9'
         self.driver.conn.listAllDomains.return_value = [domain1, domain2]
 
-        domain = self.driver._find_domain_by_mac_address('52:54:00:f9:b8:f9')
+        domain = self.driver._find_domain_by_host(host)
         self.assertEqual(domain, domain2)
 
     @mock.patch(DRIVER_PATH + '.LibvirtDriver._get_connection')
-    def test__find_domain_by_mac_address_domain_not_found(
+    def test__find_domain_by_host_name(self, mock__get_connection):
+        host = node_collection.Host(ip='10.0.0.2', libvirt_name='foo')
+        domain1 = mock.MagicMock()
+        domain1.XMLDesc.return_value = '52:54:00:ab:64:42'
+        domain1.name.return_value = 'bar'
+        domain2 = mock.MagicMock()
+        domain2.XMLDesc.return_value = '52:54:00:f9:b8:f9'
+        domain2.name.return_value = 'foo'
+        self.driver.conn.listAllDomains.return_value = [domain1, domain2]
+
+        domain = self.driver._find_domain_by_host(host)
+        self.assertEqual(domain, domain2)
+
+    @mock.patch(DRIVER_PATH + '.LibvirtDriver._get_connection')
+    def test__find_domain_by_host_domain_not_found(
             self, mock__get_connection):
+        host = node_collection.Host(ip='10.0.0.2')
         domain1 = mock.MagicMock()
         domain1.XMLDesc.return_value = '52:54:00:ab:64:42'
         domain2 = mock.MagicMock()
@@ -69,8 +85,7 @@ class LibvirtDriverTestCase(test.TestCase):
         self.driver.conn.listAllDomains.return_value = [domain1, domain2]
 
         self.assertRaises(error.PowerManagementError,
-                          self.driver._find_domain_by_mac_address,
-                          '00:00:00:00:00:01')
+                          self.driver._find_domain_by_host, host)
 
     @mock.patch(DRIVER_PATH + '.LibvirtDriver._get_connection')
     def test_supports(self, mock__get_connection):
@@ -88,25 +103,25 @@ class LibvirtDriverTestCase(test.TestCase):
 
         self.assertFalse(self.driver.supports(self.host))
 
-    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_mac_address')
+    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_host')
     @ddt.data(('poweroff', 'destroy'), ('poweron', 'create'),
               ('reset', 'reset'), ('shutdown', 'shutdown'))
-    def test_driver_actions(self, actions, mock__find_domain_by_mac_address):
+    def test_driver_actions(self, actions, mock__find_domain_by_host):
         getattr(self.driver, actions[0])(self.host)
-        domain = mock__find_domain_by_mac_address.return_value
+        domain = mock__find_domain_by_host.return_value
         getattr(domain, actions[1]).assert_called_once_with()
 
-    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_mac_address')
-    def test_snapshot(self, mock__find_domain_by_mac_address):
+    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_host')
+    def test_snapshot(self, mock__find_domain_by_host):
         self.driver.snapshot(self.host, 'foo', suspend=False)
-        domain = mock__find_domain_by_mac_address.return_value
+        domain = mock__find_domain_by_host.return_value
         domain.snapshotCreateXML.assert_called_once_with(
             '<domainsnapshot><name>foo</name></domainsnapshot>')
 
-    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_mac_address')
-    def test_snapshot_suspend(self, mock__find_domain_by_mac_address):
+    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_host')
+    def test_snapshot_suspend(self, mock__find_domain_by_host):
         self.driver.snapshot(self.host, 'foo', suspend=True)
-        domain = mock__find_domain_by_mac_address.return_value
+        domain = mock__find_domain_by_host.return_value
         domain.assert_has_calls((
             mock.call.suspend(),
             mock.call.snapshotCreateXML(
@@ -114,34 +129,34 @@ class LibvirtDriverTestCase(test.TestCase):
             mock.call.resume(),
         ))
 
-    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_mac_address')
-    def test_revert(self, mock__find_domain_by_mac_address):
+    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_host')
+    def test_revert(self, mock__find_domain_by_host):
         self.driver.revert(self.host, 'foo', resume=False)
-        domain = mock__find_domain_by_mac_address.return_value
+        domain = mock__find_domain_by_host.return_value
         snapshot = domain.snapshotLookupByName.return_value
         domain.snapshotLookupByName.assert_called_once_with('foo')
         domain.revertToSnapshot.assert_called_once_with(snapshot)
         self.assertFalse(domain.resume.called)
 
-    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_mac_address')
-    def test_revert_resume(self, mock__find_domain_by_mac_address):
+    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_host')
+    def test_revert_resume(self, mock__find_domain_by_host):
         self.driver.revert(self.host, 'foo', resume=True)
-        domain = mock__find_domain_by_mac_address.return_value
+        domain = mock__find_domain_by_host.return_value
         snapshot = domain.snapshotLookupByName.return_value
         domain.snapshotLookupByName.assert_called_once_with('foo')
         domain.revertToSnapshot.assert_called_once_with(snapshot)
         domain.resume.assert_called_once_with()
 
-    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_mac_address')
-    def test_revert_destroy(self, mock__find_domain_by_mac_address):
-        domain = mock__find_domain_by_mac_address.return_value
+    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_host')
+    def test_revert_destroy(self, mock__find_domain_by_host):
+        domain = mock__find_domain_by_host.return_value
         domain.isActive.return_value = True
         self.driver.revert(self.host, 'foo', resume=True)
         domain.destroy.assert_called_once_with()
 
-    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_mac_address')
-    def test_revert_destroy_nonactive(self, mock__find_domain_by_mac_address):
-        domain = mock__find_domain_by_mac_address.return_value
+    @mock.patch(DRIVER_PATH + '.LibvirtDriver._find_domain_by_host')
+    def test_revert_destroy_nonactive(self, mock__find_domain_by_host):
+        domain = mock__find_domain_by_host.return_value
         domain.isActive.return_value = False
         self.driver.revert(self.host, 'foo', resume=True)
         self.assertFalse(domain.destroy.called)

os_faults/tests/unit/test_utils.py

@@ -85,3 +85,60 @@ class RequiredVariablesTestCase(test.TestCase):
                                 inst.method_that_miss_variables)
         msg = 'BAR, BAZ required for MyClass.method_that_miss_variables'
         self.assertEqual(str(err), msg)
+
+
+class MyPoint(utils.ComparableMixin):
+    ATTRS = ('a', 'b')
+
+    def __init__(self, a, b):
+        self.a = a
+        self.b = b
+
+
+class ComparableMixinTestCase(test.TestCase):
+
+    def test_operations(self):
+        p1 = MyPoint(1, 'a')
+        p2 = MyPoint(1, 'b')
+        p3 = MyPoint(2, 'c')
+        p4 = MyPoint(2, 'c')
+
+        self.assertTrue(p1 < p2)
+        self.assertTrue(p1 <= p2)
+        self.assertFalse(p1 == p2)
+        self.assertFalse(p1 >= p2)
+        self.assertFalse(p1 > p2)
+        self.assertTrue(p1 != p2)
+        self.assertTrue(hash(p1) != hash(p2))
+
+        self.assertTrue(p2 < p3)
+        self.assertTrue(p2 <= p3)
+        self.assertFalse(p2 == p3)
+        self.assertFalse(p2 >= p3)
+        self.assertFalse(p2 > p3)
+        self.assertTrue(p2 != p3)
+        self.assertTrue(hash(p2) != hash(p3))
+
+        self.assertFalse(p3 < p4)
+        self.assertTrue(p3 <= p4)
+        self.assertTrue(p3 == p4)
+        self.assertTrue(p3 >= p4)
+        self.assertFalse(p3 > p4)
+        self.assertFalse(p3 != p4)
+        self.assertEqual(hash(p3), hash(p4))
+
+
+class MyRepr(utils.ReprMixin):
+    REPR_ATTRS = ('a', 'b', 'c')
+
+    def __init__(self):
+        self.a = 'foo'
+        self.b = {'foo': 'bar'}
+        self.c = 42
+
+
+class ReprMixinTestCase(test.TestCase):
+
+    def test_repr(self):
+        r = MyRepr()
+        self.assertEqual("MyRepr(a='foo', b={'foo': 'bar'}, c=42)", repr(r))

os_faults/utils.py

@@ -61,3 +61,44 @@ def require_variables(*variables):
             return fn(self, *args, **kawrgs)
         return wrapper
     return decorator
+
+
+class ComparableMixin(object):
+
+    ATTRS = ()
+
+    def _cmp_attrs(self):
+        return tuple(getattr(self, attr) for attr in self.ATTRS)
+
+    def __lt__(self, other):
+        return self._cmp_attrs() < other._cmp_attrs()
+
+    def __le__(self, other):
+        return self._cmp_attrs() <= other._cmp_attrs()
+
+    def __eq__(self, other):
+        return self._cmp_attrs() == other._cmp_attrs()
+
+    def __ge__(self, other):
+        return self._cmp_attrs() >= other._cmp_attrs()
+
+    def __gt__(self, other):
+        return self._cmp_attrs() > other._cmp_attrs()
+
+    def __ne__(self, other):
+        return not self.__eq__(other)
+
+    def __hash__(self):
+        return hash(self._cmp_attrs())
+
+
+class ReprMixin(object):
+
+    ATTRS = ()
+    REPR_ATTRS = ()
+
+    def __repr__(self):
+        return '{}({})'.format(
+            self.__class__.__name__,
+            ', '.join('{}={}'.format(attr, repr(getattr(self, attr)))
+                      for attr in self.REPR_ATTRS or self.ATTRS))