xapi: fix create hypervisor pool

Fixes bug 1049099.

Fixing problems with the rpc api when creating hypervisor pools with
xenapi. Rpc calls were not using the compute_rpcapi approach, thus were
not properly versioned. Apart from that, the slave parameters were not
forwarded to the master. A new (2.2) version is introduced where the rpc
calls have the slave_info payload. Added tests to cover the pool cases.
Some trivial extract methods performed on pool, to decouple the pool
functionality from its dependencies.

Change-Id: Ie44a1c09ef204affc4a657c71557691e83b22c22

3 files changed, 163 insertions, 13 deletions

diff --git a/nova/tests/compute/test_compute.py b/nova/tests/compute/test_compute.py
index de07d4d68..14edaa7a0 100644
--- a/nova/tests/compute/test_compute.py
+++ b/nova/tests/compute/test_compute.py
@@ -4656,7 +4656,7 @@ class ComputeAggrTestCase(BaseTestCase):
         self.aggr = db.aggregate_create(self.context, values)
 
     def test_add_aggregate_host(self):
-        def fake_driver_add_to_aggregate(context, aggregate, host):
+        def fake_driver_add_to_aggregate(context, aggregate, host, **_ignore):
             fake_driver_add_to_aggregate.called = True
             return {"foo": "bar"}
         self.stubs.Set(self.compute.driver, "add_to_aggregate",
@@ -4666,7 +4666,8 @@ class ComputeAggrTestCase(BaseTestCase):
         self.assertTrue(fake_driver_add_to_aggregate.called)
 
     def test_remove_aggregate_host(self):
-        def fake_driver_remove_from_aggregate(context, aggregate, host):
+        def fake_driver_remove_from_aggregate(context, aggregate, host,
+                                              **_ignore):
             fake_driver_remove_from_aggregate.called = True
             self.assertEqual("host", host, "host")
             return {"foo": "bar"}
@@ -4676,6 +4677,32 @@ class ComputeAggrTestCase(BaseTestCase):
         self.compute.remove_aggregate_host(self.context, self.aggr.id, "host")
         self.assertTrue(fake_driver_remove_from_aggregate.called)
 
+    def test_add_aggregate_host_passes_slave_info_to_driver(self):
+        def driver_add_to_aggregate(context, aggregate, host, **kwargs):
+            self.assertEquals(self.context, context)
+            self.assertEquals(aggregate.id, self.aggr.id)
+            self.assertEquals(host, "the_host")
+            self.assertEquals("SLAVE_INFO", kwargs.get("slave_info"))
+
+        self.stubs.Set(self.compute.driver, "add_to_aggregate",
+                       driver_add_to_aggregate)
+
+        self.compute.add_aggregate_host(self.context, self.aggr.id,
+            "the_host", slave_info="SLAVE_INFO")
+
+    def test_remove_from_aggregate_passes_slave_info_to_driver(self):
+        def driver_remove_from_aggregate(context, aggregate, host, **kwargs):
+            self.assertEquals(self.context, context)
+            self.assertEquals(aggregate.id, self.aggr.id)
+            self.assertEquals(host, "the_host")
+            self.assertEquals("SLAVE_INFO", kwargs.get("slave_info"))
+
+        self.stubs.Set(self.compute.driver, "remove_from_aggregate",
+                       driver_remove_from_aggregate)
+
+        self.compute.remove_aggregate_host(self.context,
+            self.aggr.id, "the_host", slave_info="SLAVE_INFO")
+
 
 class ComputePolicyTestCase(BaseTestCase):
 
diff --git a/nova/tests/compute/test_rpcapi.py b/nova/tests/compute/test_rpcapi.py
index 5728bdf91..559a56a0a 100644
--- a/nova/tests/compute/test_rpcapi.py
+++ b/nova/tests/compute/test_rpcapi.py
@@ -99,7 +99,7 @@ class ComputeRpcAPITestCase(test.TestCase):
 
     def test_add_aggregate_host(self):
         self._test_compute_api('add_aggregate_host', 'cast', aggregate_id='id',
-                host_param='host', host='host')
+                host_param='host', host='host', slave_info={}, version='2.2')
 
     def test_add_fixed_ip_to_instance(self):
         self._test_compute_api('add_fixed_ip_to_instance', 'cast',
@@ -249,7 +249,8 @@ class ComputeRpcAPITestCase(test.TestCase):
 
     def test_remove_aggregate_host(self):
         self._test_compute_api('remove_aggregate_host', 'cast',
-                aggregate_id='id', host_param='host', host='host')
+                aggregate_id='id', host_param='host', host='host',
+                slave_info={}, version='2.2')
 
     def test_remove_fixed_ip_from_instance(self):
         self._test_compute_api('remove_fixed_ip_from_instance', 'cast',
diff --git a/nova/tests/test_xenapi.py b/nova/tests/test_xenapi.py
index dced3ddd9..b543f3e0f 100644
--- a/nova/tests/test_xenapi.py
+++ b/nova/tests/test_xenapi.py
@@ -44,6 +44,7 @@ from nova.tests.xenapi import stubs
 from nova.virt.xenapi import agent
 from nova.virt.xenapi import driver as xenapi_conn
 from nova.virt.xenapi import fake as xenapi_fake
+from nova.virt.xenapi import pool
 from nova.virt.xenapi import pool_states
 from nova.virt.xenapi import vm_utils
 from nova.virt.xenapi import vmops
@@ -1870,15 +1871,44 @@ class XenAPIAggregateTestCase(stubs.XenAPITestBase):
                               'host': xenapi_fake.get_record('host',
                                                              host_ref)['uuid']}
 
-    def test_add_to_aggregate_called(self):
-        def fake_add_to_aggregate(context, aggregate, host):
-            fake_add_to_aggregate.called = True
+    def test_pool_add_to_aggregate_called_by_driver(self):
+
+        calls = []
+
+        def pool_add_to_aggregate(context, aggregate, host, slave_info=None):
+            self.assertEquals("CONTEXT", context)
+            self.assertEquals("AGGREGATE", aggregate)
+            self.assertEquals("HOST", host)
+            self.assertEquals("SLAVEINFO", slave_info)
+            calls.append(pool_add_to_aggregate)
         self.stubs.Set(self.conn._pool,
                        "add_to_aggregate",
-                       fake_add_to_aggregate)
+                       pool_add_to_aggregate)
+
+        self.conn.add_to_aggregate("CONTEXT", "AGGREGATE", "HOST",
+                                   slave_info="SLAVEINFO")
 
-        self.conn.add_to_aggregate(None, None, None)
-        self.assertTrue(fake_add_to_aggregate.called)
+        self.assertTrue(pool_add_to_aggregate in calls)
+
+    def test_pool_remove_from_aggregate_called_by_driver(self):
+
+        calls = []
+
+        def pool_remove_from_aggregate(context, aggregate, host,
+                                       slave_info=None):
+            self.assertEquals("CONTEXT", context)
+            self.assertEquals("AGGREGATE", aggregate)
+            self.assertEquals("HOST", host)
+            self.assertEquals("SLAVEINFO", slave_info)
+            calls.append(pool_remove_from_aggregate)
+        self.stubs.Set(self.conn._pool,
+                       "remove_from_aggregate",
+                       pool_remove_from_aggregate)
+
+        self.conn.remove_from_aggregate("CONTEXT", "AGGREGATE", "HOST",
+                                        slave_info="SLAVEINFO")
+
+        self.assertTrue(pool_remove_from_aggregate in calls)
 
     def test_add_to_aggregate_for_first_host_sets_metadata(self):
         def fake_init_pool(id, name):
@@ -1900,11 +1930,11 @@ class XenAPIAggregateTestCase(stubs.XenAPITestBase):
         aggregate = self._aggregate_setup(hosts=['host', 'host2'],
                                           metadata=self.fake_metadata)
         self.conn._pool.add_to_aggregate(self.context, aggregate, "host2",
-                                         compute_uuid='fake_uuid',
+                                         dict(compute_uuid='fake_uuid',
                                          url='fake_url',
                                          user='fake_user',
                                          passwd='fake_pass',
-                                         xenhost_uuid='fake_uuid')
+                                         xenhost_uuid='fake_uuid'))
         self.assertTrue(fake_join_slave.called)
 
     def test_add_to_aggregate_first_host(self):
@@ -2060,7 +2090,7 @@ class XenAPIAggregateTestCase(stubs.XenAPITestBase):
 
     def test_add_aggregate_host_raise_err(self):
         """Ensure the undo operation works correctly on add."""
-        def fake_driver_add_to_aggregate(context, aggregate, host):
+        def fake_driver_add_to_aggregate(context, aggregate, host, **_ignore):
             raise exception.AggregateError
         self.stubs.Set(self.compute.driver, "add_to_aggregate",
                        fake_driver_add_to_aggregate)
@@ -2078,6 +2108,98 @@ class XenAPIAggregateTestCase(stubs.XenAPITestBase):
         self.assertEqual(excepted.hosts, [])
 
 
+class Aggregate(object):
+    def __init__(self, id=None, hosts=None):
+        self.id = id
+        self.hosts = hosts or []
+
+
+class MockComputeAPI(object):
+    def __init__(self):
+        self._mock_calls = []
+
+    def add_aggregate_host(self, ctxt, aggregate_id,
+                                     host_param, host, slave_info):
+        self._mock_calls.append((
+            self.add_aggregate_host, ctxt, aggregate_id,
+            host_param, host, slave_info))
+
+    def remove_aggregate_host(self, ctxt, aggregate_id, host_param,
+                              host, slave_info):
+        self._mock_calls.append((
+            self.remove_aggregate_host, ctxt, aggregate_id,
+            host_param, host, slave_info))
+
+
+class StubDependencies(object):
+    """Stub dependencies for ResourcePool"""
+
+    def __init__(self):
+        self.compute_rpcapi = MockComputeAPI()
+
+    def _is_hv_pool(self, *_ignore):
+        return True
+
+    def _get_metadata(self, *_ignore):
+        return {
+            pool_states.KEY: {},
+            'master_compute': 'master'
+        }
+
+    def _create_slave_info(self, *ignore):
+        return "SLAVE_INFO"
+
+
+class ResourcePoolWithStubs(StubDependencies, pool.ResourcePool):
+    """ A ResourcePool, use stub dependencies """
+
+
+class HypervisorPoolTestCase(test.TestCase):
+
+    def test_slave_asks_master_to_add_slave_to_pool(self):
+        slave = ResourcePoolWithStubs()
+        aggregate = Aggregate(id=98, hosts=[])
+
+        slave.add_to_aggregate("CONTEXT", aggregate, "slave")
+
+        self.assertIn(
+            (slave.compute_rpcapi.add_aggregate_host,
+            "CONTEXT", 98, "slave", "master", "SLAVE_INFO"),
+            slave.compute_rpcapi._mock_calls)
+
+    def test_slave_asks_master_to_remove_slave_from_pool(self):
+        slave = ResourcePoolWithStubs()
+        aggregate = Aggregate(id=98, hosts=[])
+
+        slave.remove_from_aggregate("CONTEXT", aggregate, "slave")
+
+        self.assertIn(
+            (slave.compute_rpcapi.remove_aggregate_host,
+            "CONTEXT", 98, "slave", "master", "SLAVE_INFO"),
+            slave.compute_rpcapi._mock_calls)
+
+
+class SwapXapiHostTestCase(test.TestCase):
+
+    def test_swapping(self):
+        self.assertEquals(
+            "http://otherserver:8765/somepath",
+            pool.swap_xapi_host(
+                "http://someserver:8765/somepath", 'otherserver'))
+
+    def test_no_port(self):
+        self.assertEquals(
+            "http://otherserver/somepath",
+            pool.swap_xapi_host(
+                "http://someserver/somepath", 'otherserver'))
+
+    def test_no_path(self):
+        self.assertEquals(
+            "http://otherserver",
+            pool.swap_xapi_host(
+                "http://someserver", 'otherserver'))
+
+
 class VmUtilsTestCase(test.TestCase):
     """Unit tests for xenapi utils."""