Add more tests

master/buildbot/libvirtbuildslave.py

@@ -17,7 +17,7 @@
 import os
 
 from twisted.internet import defer, utils, reactor, threads
-from twisted.python import log
+from twisted.python import log, failure
 from buildbot.buildslave import AbstractBuildSlave, AbstractLatentBuildSlave
 from buildbot import config
 
@@ -125,14 +125,14 @@ def __init__(self, uri):
     @defer.inlineCallbacks
     def lookupByName(self, name):
         """ I lookup an existing prefined domain """
-        d = yield queue.executeInThread(self.connection.lookupByName, name)
+        res = yield queue.executeInThread(self.connection.lookupByName, name)
         defer.returnValue(self.DomainClass(self, res))
 
     @defer.inlineCallbacks
     def create(self, xml):
         """ I take libvirt XML and start a new VM """
-        d = yield queue.executeInThread(self.connection.createXML, xml, 0)
-        defer.returnVlalue(self.DomainClass(self, res))
+        res = yield queue.executeInThread(self.connection.createXML, xml, 0)
+        defer.returnValue(self.DomainClass(self, res))
 
     @defer.inlineCallbacks
     def all(self):
@@ -168,7 +168,7 @@ def __init__(self, name, password, connection, hd_image, base_image = None, xml=
         self.domain = None
 
         self.ready = False
-        self._find_existing_instance()
+        self._find_existing_deferred = self._find_existing_instance()
 
     @defer.inlineCallbacks
     def _find_existing_instance(self):
@@ -241,7 +241,8 @@ def start_instance(self, build):
         in the list of defined virtual machines and start that.
         """
         if self.domain is not None:
-             raise ValueError('domain active')
+             log.msg("Cannot start_instance '%s' as already active" % self.name)
+             defer.returnValue(False)
 
         yield self._prepare_base_image()
 
@@ -253,7 +254,7 @@ def start_instance(self, build):
                 yield self.domain.create()
         except Exception, f:
             log.msg("Cannot start a VM (%s), failing gracefully and triggering a new build check" % self.name)
-            log.err(failure)
+            log.err(failure.Failure())
             self.domain = None
             defer.returnValue(False)
 

master/buildbot/test/unit/test_libvirtslave.py

@@ -15,34 +15,163 @@
 
 import mock
 from twisted.trial import unittest
-from twisted.internet import defer, reactor
+from twisted.internet import defer, reactor, utils
 from buildbot import libvirtbuildslave, config
 from buildbot.test.fake import fakemaster
 
+class FakeLibVirt(object):
+
+    def __init__(self, patch):
+        self.patch = patch
+        self.domains = {}
+
+        self.libvirt = mock.Mock()
+        self.patch(libvirtbuildslave, "libvirt", self.libvirt)
+
+        conn = self.libvirt_conn = self.libvirt.open.return_value = mock.Mock()
+        conn.listDomainsID.side_effect = self.domains.keys
+        conn.lookupByName.side_effect = lambda name: self.domains[name]
+        conn.lookupByID.side_effect = lambda name: self.domains[name]
+
+        self.conn = libvirtbuildslave.Connection("test:///")
+
+    def add_domain(self, name):
+        domain = mock.Mock()
+        domain.name.return_value = name
+        self.domains[name] = domain
+        return domain
+
+
 class TestLibVirtSlave(unittest.TestCase):
 
     class ConcreteBuildSlave(libvirtbuildslave.LibVirtSlave):
         pass
 
+    def setUp(self):
+        self.libvirt = FakeLibVirt(patch=self.patch)
+
     def test_constructor_nolibvirt(self):
         self.patch(libvirtbuildslave, "libvirt", None)
         self.assertRaises(config.ConfigErrors, self.ConcreteBuildSlave,
             'bot', 'pass', None, 'path', 'path')
 
     def test_constructor_minimal(self):
-        self.patch(libvirtbuildslave, "libvirt", mock.Mock())
-
-        connection = mock.Mock()
-        connection.all.return_value = []
-
-        bs = self.ConcreteBuildSlave('bot', 'pass', connection, 'path', 'otherpath')
+        conn = self.libvirt.conn
+        bs = self.ConcreteBuildSlave('bot', 'pass', conn, 'path', 'otherpath')
+        yield bs._find_existing_deferred
         self.assertEqual(bs.slavename, 'bot')
         self.assertEqual(bs.password, 'pass')
-        self.assertEqual(bs.connection, connection)
+        self.assertEqual(bs.connection, conn)
         self.assertEqual(bs.image, 'path')
         self.assertEqual(bs.base_image, 'otherpath')
         self.assertEqual(bs.keepalive_interval, 3600)
 
+    @defer.inlineCallbacks
+    def test_find_existing(self):
+        d = self.libvirt.add_domain("bot")
+
+        bs = self.ConcreteBuildSlave('bot', 'pass', self.libvirt.conn, 'p', 'o')
+        yield bs._find_existing_deferred        
+
+        self.assertEqual(bs.domain.domain, d)
+        self.assertEqual(bs.substantiated, True)
+
+    @defer.inlineCallbacks
+    def test_prepare_base_image_none(self):
+        self.patch(utils, "getProcessValue", mock.Mock())
+        utils.getProcessValue.side_effect = lambda x,y: defer.succeed(0)
+
+        bs = self.ConcreteBuildSlave('bot', 'pass', self.libvirt.conn, 'p', None)
+        yield bs._find_existing_deferred
+        yield bs._prepare_base_image()
+
+        self.assertEqual(utils.getProcessValue.call_count, 0)
+
+    @defer.inlineCallbacks
+    def test_prepare_base_image_cheap(self):
+        self.patch(utils, "getProcessValue", mock.Mock())
+        utils.getProcessValue.side_effect = lambda x,y: defer.succeed(0)
+
+        bs = self.ConcreteBuildSlave('bot', 'pass', self.libvirt.conn, 'p', 'o')
+        yield bs._find_existing_deferred
+        yield bs._prepare_base_image()
+
+        utils.getProcessValue.assert_called_with(
+            "qemu-img", ["create", "-b", "o", "-f", "qcow2", "p"])
+
+    @defer.inlineCallbacks
+    def test_prepare_base_image_full(self):
+        pass
+        self.patch(utils, "getProcessValue", mock.Mock())
+        utils.getProcessValue.side_effect = lambda x,y: defer.succeed(0)
+
+        bs = self.ConcreteBuildSlave('bot', 'pass', self.libvirt.conn, 'p', 'o')
+        yield bs._find_existing_deferred
+        bs.cheap_copy = False
+        yield bs._prepare_base_image()
+
+        utils.getProcessValue.assert_called_with(
+            "cp", ["o", "p"])
+
+    @defer.inlineCallbacks
+    def test_start_instance(self):
+        bs = self.ConcreteBuildSlave('b', 'p', self.libvirt.conn, 'p', 'o',
+            xml='<xml/>')
+
+        prep = mock.Mock()
+        prep.side_effect = lambda: defer.succeed(0)
+        self.patch(bs, "_prepare_base_image", prep)
+
+        yield bs._find_existing_deferred
+        started = yield bs.start_instance(mock.Mock())
+
+        self.assertEqual(started, True)
+
+    @defer.inlineCallbacks
+    def setup_canStartBuild(self):
+        bs = self.ConcreteBuildSlave('b', 'p', self.libvirt.conn, 'p', 'o')
+        yield bs._find_existing_deferred
+        bs.updateLocks()
+        defer.returnValue(bs)
+
+    @defer.inlineCallbacks
+    def test_canStartBuild(self):
+        bs = yield self.setup_canStartBuild()
+        self.assertEqual(bs.canStartBuild(), True)
+
+    @defer.inlineCallbacks
+    def test_canStartBuild_notready(self):
+        """
+        If a LibVirtSlave hasnt finished scanning for existing VMs then we shouldn't
+        start builds on it as it might create a 2nd VM when we want to reuse the existing
+        one.
+        """
+        bs = yield self.setup_canStartBuild()
+        bs.ready = False
+        self.assertEqual(bs.canStartBuild(), False)
+
+    @defer.inlineCallbacks
+    def test_canStartBuild_domain_and_not_connected(self):
+        """
+        If we've found that the VM this slave would instance already exists but hasnt
+        connected then we shouldn't start builds or we'll end up with a dupe.
+        """
+        bs = yield self.setup_canStartBuild()
+        bs.domain = mock.Mock()
+        self.assertEqual(bs.canStartBuild(), False)
+
+    @defer.inlineCallbacks
+    def test_canStartBuild_domain_and_connected(self):
+        """
+        If we've found an existing VM and it is connected then we should start builds
+        """
+        bs = yield self.setup_canStartBuild()
+        bs.domain = mock.Mock()
+        isconnected = mock.Mock()
+        isconnected.return_value = True
+        self.patch(bs, "isConnected", isconnected)
+        self.assertEqual(bs.canStartBuild(), True)
+
 
 class TestWorkQueue(unittest.TestCase):