diff --git a/coriolis/tests/integration/base.py b/coriolis/tests/integration/base.py index a6931f03..715234b8 100644 --- a/coriolis/tests/integration/base.py +++ b/coriolis/tests/integration/base.py @@ -162,7 +162,8 @@ def _create_transfer( @classmethod def _create_pool( - cls, endpoint_id, name="test-pool", skip_allocation=True): + cls, endpoint_id, name="test-pool", skip_allocation=True, + wait_for_allocation=False): pool = cls._client.minion_pools.create( name=name, endpoint=endpoint_id, @@ -178,6 +179,13 @@ def _create_pool( ) cls.addClassCleanup(cls._safe_delete_pool, pool.id) + if wait_for_allocation: + pool_obj = cls._wait_for_pool(pool.id, MINION_ALLOCATED_TERMINAL) + if pool_obj.status != constants.MINION_POOL_STATUS_ALLOCATED: + raise AssertionError( + "Pool did not reach ALLOCATED (got %s)" + % pool_obj.status) + return pool @classmethod @@ -269,15 +277,10 @@ def setUpClass(cls): cls._pool_id = None if cls._CREATE_MINION_POOLS: pool = cls._create_pool( - cls._dst_endpoint.id, "transfer-pool", skip_allocation=False) + cls._dst_endpoint.id, "transfer-pool", skip_allocation=False, + wait_for_allocation=True) cls._pool_id = pool.id - pool_obj = cls._wait_for_pool(pool.id, MINION_ALLOCATED_TERMINAL) - if pool_obj.status != constants.MINION_POOL_STATUS_ALLOCATED: - raise AssertionError( - "Pool did not reach ALLOCATED (got %s)" % pool_obj.status, - ) - # (re)init the scsi_debug module. test_utils.destroy_scsi_debug() test_utils.init_scsi_debug(size_mb=cls._SCSI_DEBUG_SIZE_MB) @@ -469,6 +472,7 @@ def assertExecutionErrored(self, execution_id, timeout=600): [ constants.EXECUTION_STATUS_ERROR, constants.EXECUTION_STATUS_DEADLOCKED, + constants.EXECUTION_STATUS_ERROR_ALLOCATING_MINIONS, ], "Expected an error status for execution %s, got %s" % (execution_id, execution.status), @@ -554,6 +558,20 @@ def assertDeploymentCompleted(self, deployment_id, timeout=600): % (deployment_id, deployment.last_execution_status), ) + def assertDeploymentErrored(self, deployment_id, timeout=600): + """Assert that *deployment_id* ends in an error state.""" + deployment = self.wait_for_deployment(deployment_id, timeout=timeout) + self.assertIn( + deployment.last_execution_status, + [ + constants.EXECUTION_STATUS_ERROR, + constants.EXECUTION_STATUS_DEADLOCKED, + constants.EXECUTION_STATUS_ERROR_ALLOCATING_MINIONS, + ], + "Expected an error status for deployment %s, got %s" + % (deployment_id, deployment.last_execution_status), + ) + def _patch_add_delay(self, obj, method_name): _orig = getattr(obj, method_name) diff --git a/coriolis/tests/integration/deployments/test_osmorphing.py b/coriolis/tests/integration/deployments/test_osmorphing.py index 48c21eaa..b648022f 100644 --- a/coriolis/tests/integration/deployments/test_osmorphing.py +++ b/coriolis/tests/integration/deployments/test_osmorphing.py @@ -10,6 +10,7 @@ import os import re import unittest +from unittest import mock import uuid from coriolis.db import api as db_api @@ -202,7 +203,14 @@ class OsMorphingMinionPoolDeploymentTest( integration_base.MinionPoolTestBase, OsMorphingDeploymentTestBase): """OS morphing deployment using a minion pool for the OS morphing phase.""" - _CREATE_MINION_POOLS = True + @classmethod + def setUpClass(cls): + super().setUpClass() + + pool = cls._create_pool( + cls._dst_endpoint.id, "osmorph-pool", skip_allocation=False, + wait_for_allocation=True) + cls._osmorph_pool_id = pool.id def test_deployment_with_os_morphing(self): self.assertFalse( @@ -213,7 +221,7 @@ def test_deployment_with_os_morphing(self): deployment_kwargs = { "instance_osmorphing_minion_pool_mappings": { - self._instance_name: self._pool_id, + self._instance_name: self._osmorph_pool_id, }, } self._execute_transfer_and_deployment(deployment_kwargs) @@ -226,7 +234,7 @@ def test_deployment_with_os_morphing(self): ctxt = self._get_db_context() pool = db_api.get_minion_pool( - ctxt, self._pool_id, include_machines=True) + ctxt, self._osmorph_pool_id, include_machines=True) self.assertTrue( pool.minion_machines, "OS morphing pool has no minion machines") @@ -236,3 +244,53 @@ def test_deployment_with_os_morphing(self): machine.last_used_at, "OS morphing minion machine %s was never used" % machine.id, ) + + def test_osmorphing_minion_allocation_failure_cleans_up(self): + """OS morphing minion pool allocation fail test. + + Steps: + 1. Run the transfer execution to completion. + 2. Deploy with the instance mapped to the osmorphing pool; + healthcheck_minion on the pool's pre-existing machine fails. + 3. The healthcheck decider falls back to deallocate + recreate; + create_minion fails too, so the recreation attempt is exhausted. + 4. Minion allocation for the deployment fails and the deployment + errors out via report_deployment_minions_allocation_error. + 5. The machine that failed both attempts is cleaned up, rather than + left dangling in a broken intermediate status. + """ + self._execute_and_wait(self._transfer.id) + + injected_error = Exception("injected minion failure") + deployment_kwargs = { + "instance_osmorphing_minion_pool_mappings": { + self._instance_name: self._osmorph_pool_id, + }, + } + + with mock.patch.object( + self._harness.imp_provider_class, "healthcheck_minion", + side_effect=injected_error) as mock_healthcheck, \ + mock.patch.object( + self._harness.imp_provider_class, "create_minion", + side_effect=injected_error) as mock_create: + deployment = self._client.deployments.create_from_transfer( + self._transfer.id, skip_os_morphing=False, + **deployment_kwargs) + self.addCleanup( + self._cleanup_deployment, deployment.id, deployment.instances) + + self.assertDeploymentErrored(deployment.id) + + mock_healthcheck.assert_called() + mock_create.assert_called() + + ctxt = self._get_db_context() + pool = db_api.get_minion_pool( + ctxt, self._osmorph_pool_id, include_machines=True) + self.assertEqual( + [], pool.minion_machines, + "Minion machine(s) left in an inconsistent state after " + "allocation failure: %s" + % [(m.id, m.allocation_status) for m in pool.minion_machines], + ) diff --git a/coriolis/tests/integration/management/test_region.py b/coriolis/tests/integration/management/test_region.py index a975839f..f835f124 100644 --- a/coriolis/tests/integration/management/test_region.py +++ b/coriolis/tests/integration/management/test_region.py @@ -3,10 +3,15 @@ """Integration tests for the regions APIs. -Exercises region CRUD operations via the Coriolis REST API. +Exercises region CRUD operations via the Coriolis REST API, as well as +scheduler behavior when workers and endpoints are mapped to regions. """ +from keystoneauth1.exceptions import http as http_exc + +from coriolis import constants from coriolis.tests.integration import base +from coriolis import utils as coriolis_utils class RegionTests(base.CoriolisIntegrationTestBase): @@ -44,3 +49,75 @@ def test_region_crud(self): regions = self._client.regions.list() ids = [r.id for r in regions] self.assertNotIn(region.id, ids) + + +class RegionSchedulingTests(base.ReplicaIntegrationTestBase): + + @classmethod + def setUpClass(cls): + super().setUpClass() + cls._worker_service = ( + cls._client.services.find_service_by_host_and_topic( + coriolis_utils.get_hostname(), + constants.WORKER_MAIN_MESSAGING_TOPIC)) + cls.addClassCleanup( + cls._client.services.update, + cls._worker_service.id, {"mapped_regions": []}) + + def _create_region(self, name, **kwargs): + region = self._client.regions.create(name, **kwargs) + self.addCleanup( + self._ignoreExc(self._client.regions.delete), region.id) + + return region + + def test_region_scheduling(self): + """Covers the scheduler's RegionsFilter. + + A matched-region transfer schedules and completes normally, while + a worker / endpoint region mismatch causes the transfer creation + itself to fail. + The scheduler needs a region-matching worker to service the endpoint + validation calls for the created transfer. A region mismatch surfaces + as an immediate BadRequest at creation time. + """ + # Matched region: reuse the src / dst endpoints and transfer created in + # ReplicaIntegrationTestBase.setUp. + matched_region = self._create_region("region-matched") + self._client.services.update( + self._worker_service.id, {"mapped_regions": [matched_region.id]}) + self._client.endpoints.update( + self._src_endpoint.id, {"mapped_regions": [matched_region.id]}) + self._client.endpoints.update( + self._dst_endpoint.id, {"mapped_regions": [matched_region.id]}) + + self._execute_and_wait(self._transfer.id) + + # Mismatched region: point the worker and a fresh pair of + # endpoints at different regions, and assert transfer creation + # itself is rejected. + worker_region = self._create_region("region-worker") + endpoint_region = self._create_region("region-endpoint") + + self._client.services.update( + self._worker_service.id, {"mapped_regions": [worker_region.id]}) + + src_endpoint = self._create_endpoint( + name="region-mismatch-src", + endpoint_type=self._exp_platform, + connection_info=self._exp_conn_info, + regions=[endpoint_region.id], + ) + dst_endpoint = self._create_endpoint( + name="region-mismatch-dst", + endpoint_type=self._imp_platform, + connection_info=self._imp_conn_info, + regions=[endpoint_region.id], + ) + + self.assertRaises( + http_exc.BadRequest, + self._create_transfer, + src_endpoint.id, dst_endpoint.id, + ["region-mismatch-instance"], + ) diff --git a/coriolis/tests/integration/management/test_service.py b/coriolis/tests/integration/management/test_service.py index 4ad2ae2b..a0b0d81b 100644 --- a/coriolis/tests/integration/management/test_service.py +++ b/coriolis/tests/integration/management/test_service.py @@ -8,6 +8,8 @@ import socket +from keystoneauth1.exceptions import http as http_exc + from coriolis.tests.integration import base @@ -55,3 +57,35 @@ def test_service_crud(self): services = self._client.services.list() ids = [s.id for s in services] self.assertNotIn(svc.id, ids) + self.assertRaises( + http_exc.NotFound, self._client.services.get, svc.id) + + def test_service_registration_conflict(self): + # ConductorServerEndpoint.register_service raises Conflict when a + # service with the same host / binary / topic is already registered. + hostname = socket.gethostname() + svc = self._create_service( + hostname, "conflict-binary", "coriolis_worker") + + self.assertRaises( + http_exc.Conflict, + self._client.services.create, + host=hostname, binary="conflict-binary", + topic="coriolis_worker", regions=[]) + + self._client.services.delete(svc.id) + + def test_service_registration_with_region(self): + # Creates a service with a mapped region. + region = self._client.regions.create("service-test-region") + self.addCleanup( + self._ignoreExc(self._client.regions.delete), region.id) + + hostname = socket.gethostname() + svc = self._client.services.create( + host=hostname, binary="region-binary", topic="coriolis_worker", + regions=[region.id]) + self.addCleanup(self._ignoreExc(self._client.services.delete), svc.id) + + fetched = self._client.services.get(svc.id) + self.assertIn(region.id, fetched.mapped_regions) diff --git a/coriolis/tests/integration/test_failure_recovery.py b/coriolis/tests/integration/test_failure_recovery.py index f995d25e..fafbb7f3 100644 --- a/coriolis/tests/integration/test_failure_recovery.py +++ b/coriolis/tests/integration/test_failure_recovery.py @@ -124,3 +124,51 @@ def _slow_then_fail(self_provider, *args, **kwargs): self.assertExecutionErrored(execution.id) self.assertTargetResourcesCleaned(execution.id) + + +class MinionPoolAllocationFailureTest(base.MinionPoolReplicaTestBase): + """Transfer minion pool allocation failure tests.""" + + def test_transfer_minion_allocation_failure_cleans_up(self): + """Transfer minion pool allocation fail test. + + Steps: + 1. healthcheck_minion on the pool's pre-existing machine fails. + 2. The healthcheck decider falls back to deallocate + recreate; + create_minion fails too, so the recreation attempt is exhausted. + 3. Allocation for the transfer execution fails and the execution + errors out. + 4. The machine that failed both attempts is cleaned up, rather than + left dangling in a broken intermediate status. The pool itself + stays ALLOCATED and usable. + """ + injected_error = Exception("injected minion failure") + + with mock.patch.object( + self._harness.imp_provider_class, "healthcheck_minion", + side_effect=injected_error) as mock_healthcheck, \ + mock.patch.object( + self._harness.imp_provider_class, "create_minion", + side_effect=injected_error) as mock_create: + execution = self._client.transfer_executions.create( + self._transfer.id, shutdown_instances=False) + self.assertExecutionErrored(execution.id) + + mock_healthcheck.assert_called() + mock_create.assert_called() + + # The pool itself stays usable. + self.assertPoolAllocated(self._pool_id) + + # Its only machine failed both the healthcheck and the recreation + # attempt. ending up as UNINITIALIZED. It then gets deleted, rather + # than left dangling in a broken intermediate status. + ctxt = self._get_db_context() + pool = db_api.get_minion_pool( + ctxt, self._pool_id, include_machines=True) + self.assertEqual( + [], pool.minion_machines, + "Minion machine(s) left in an inconsistent state after " + "allocation failure: %s" + % [(m.id, m.allocation_status) for m in pool.minion_machines], + ) diff --git a/coriolis/tests/integration/test_minion_pools.py b/coriolis/tests/integration/test_minion_pools.py index 3fde63b2..e8de9cae 100644 --- a/coriolis/tests/integration/test_minion_pools.py +++ b/coriolis/tests/integration/test_minion_pools.py @@ -12,10 +12,14 @@ import time +from oslo_config import cfg + from coriolis import constants from coriolis.db import api as db_api from coriolis.tests.integration import base +CONF = cfg.CONF + class MinionPoolLifecycleTest(base.MinionPoolTestBase): @@ -112,3 +116,48 @@ def test_allocate_deallocate(self): final.status, "Pool deallocation ended in unexpected status '%s'" % final.status, ) + + def test_cron_triggered_refresh(self): + """Cron-scheduled refresh. + + Minion pools refresh periodically based on on the config option + minion_manager.minion_pool_default_refresh_period_minutes. This test + verifies that the refresh actually fires. + """ + CONF.set_override( + "minion_pool_default_refresh_period_minutes", 1, + group="minion_manager") + self.addCleanup( + CONF.clear_override, + "minion_pool_default_refresh_period_minutes", + group="minion_manager") + + # Refresh jobs are registered at pool-creation time based on the + # CONF value above, so the pool must be created after the override. + pool = self._create_pool(self._endpoint.id) + + self._client.minion_pools.allocate_minion_pool(pool.id) + self._wait_for_pool(pool.id, base.MINION_ALLOCATED_TERMINAL) + + machine = self._wait_for_machine_status( + pool.id, constants.MINION_MACHINE_STATUS_AVAILABLE) + baseline_updated_at = machine.updated_at + + ctxt = self._get_db_context() + deadline = time.monotonic() + 120 + refreshed = False + while time.monotonic() < deadline: + pool = db_api.get_minion_pool(ctxt, pool.id, include_machines=True) + m = pool.minion_machines[0] + status = m.allocation_status + if (status == constants.MINION_MACHINE_STATUS_AVAILABLE and + m.updated_at != baseline_updated_at): + refreshed = True + break + time.sleep(2) + + self.assertTrue( + refreshed, + "Minion pool machine '%s' was not refreshed by the automatic " + "cron job in time" % pool.id, + )