Fix false positive server group functional tests

Most of the evacuate tests in nova.tests.functional.test_server_group
could produce false positive results becuase of multiple reasons:
* test did not wait for the migration to finish in done or error state
  before checked the instance host
* test did not asserted that the instance host is changed during the
  evacuation

Also the evacuation tests that starts up a third compute failed to add
the magic fake.set_nodes() call making the third node unusable and
causing unexpected NoValidHost during the evacuation. However it was
not visible until the above mistakes are fixed in the test

Closes-Bug: #1739013

 Conflicts:
	nova/tests/functional/test_servers.py

NOTE(melwitt): The conflict was from methods in Pike that don't exist
in Ocata. Also, in Ocata, conductor doesn't set the migration status
to 'error' upon NoValidHost during rebuild so we can't wait for that
in the tests.

Change-Id: Idc4e3d8ac7c99c09c6f4860dc0c7a05f28a35627
(cherry picked from commit ec04052736)
(cherry picked from commit 1558893de1)

nova/tests/functional/integrated_helpers.py

@@ -286,3 +286,23 @@ class InstanceHelperMixin(object):
         if completion_event is None:
             self.fail('Timed out waiting for %s failure event. Current '
                       'instance actions: %s' % (event_name, actions))
+
+    def _wait_for_migration_status(self, server, expected_status):
+        """Waits for a migration record with the given status to be found
+        for the given server, else the test fails. The migration record, if
+        found, is returned.
+        """
+        api = getattr(self, 'admin_api', None)
+        if api is None:
+            api = self.api
+
+        for attempt in range(10):
+            migrations = api.api_get('/os-migrations').body['migrations']
+            for migration in migrations:
+                if (migration['instance_uuid'] == server['id'] and
+                        migration['status'].lower() ==
+                        expected_status.lower()):
+                    return migration
+            time.sleep(0.5)
+        self.fail('Timed out waiting for migration with status "%s" for '
+                  'instance: %s' % (expected_status, server['id']))

nova/tests/functional/test_server_group.py

@@ -139,7 +139,8 @@ class ServerGroupTestV21(ServerGroupTestBase):
         # tree.
         self.stub_out('nova.virt.driver.load_compute_driver',
                       _fake_load_compute_driver)
-        self.compute = self.start_service('compute')
+        fake.set_nodes(['compute'])
+        self.compute = self.start_service('compute', host='compute')
 
         # NOTE(gibi): start a second compute host to be able to test affinity
         # NOTE(sbauza): Make sure the FakeDriver returns a different nodename
@@ -421,13 +422,19 @@ class ServerGroupTestV21(ServerGroupTestBase):
         time.sleep(self._service_down_time)
 
         # Start additional host to test evacuation
+        fake.set_nodes(['host3'])
         self.start_service('compute', host='host3')
 
         post = {'evacuate': {'onSharedStorage': False}}
         self.admin_api.post_server_action(servers[1]['id'], post)
+        self._wait_for_migration_status(servers[1], 'done')
         evacuated_server = self._wait_for_state_change(
             self.admin_api, servers[1], 'ACTIVE')
 
+        # check that the server is evacuated to another host
+        self.assertNotEqual(evacuated_server['OS-EXT-SRV-ATTR:host'],
+                            servers[1]['OS-EXT-SRV-ATTR:host'])
+        # check that anti-affinity policy is kept during evacuation
         self.assertNotEqual(evacuated_server['OS-EXT-SRV-ATTR:host'],
                             servers[0]['OS-EXT-SRV-ATTR:host'])
 
@@ -612,13 +619,19 @@ class ServerGroupTestV215(ServerGroupTestV21):
         time.sleep(self._service_down_time)
 
         # Start additional host to test evacuation
+        fake.set_nodes(['host3'])
         compute3 = self.start_service('compute', host='host3')
 
         post = {'evacuate': {}}
         self.admin_api.post_server_action(servers[1]['id'], post)
+        self._wait_for_migration_status(servers[1], 'done')
         evacuated_server = self._wait_for_state_change(
             self.admin_api, servers[1], 'ACTIVE')
 
+        # check that the server is evacuated
+        self.assertNotEqual(evacuated_server['OS-EXT-SRV-ATTR:host'],
+                            servers[1]['OS-EXT-SRV-ATTR:host'])
+        # check that policy is kept
         self.assertNotEqual(evacuated_server['OS-EXT-SRV-ATTR:host'],
                             servers[0]['OS-EXT-SRV-ATTR:host'])
 
@@ -804,6 +817,7 @@ class ServerGroupTestV215(ServerGroupTestV21):
 
         post = {'evacuate': {}}
         self.admin_api.post_server_action(servers[1]['id'], post)
+        self._wait_for_migration_status(servers[1], 'done')
         evacuated_server = self._wait_for_state_change(
             self.admin_api, servers[1], 'ACTIVE')