Prechádzať zdrojové kódy

Fix deployment input validation

Adds logic to validate manually launched deployments before creating
the deployment, while also validating auto deployments' inputs
before creating the deployment execution.
Daniel Vincze 1 rok pred
rodič
commit
6d5c21e9bb

+ 13 - 8
coriolis/conductor/rpc/server.py

@@ -1399,13 +1399,7 @@ class ConductorServerEndpoint(object):
                 "No provider found for: %s" % endpoint.type)
                 "No provider found for: %s" % endpoint.type)
         return provider_types["types"]
         return provider_types["types"]
 
 
-    def _execute_deployment(self, ctxt, deployment, force):
-        transfer = self._get_transfer(
-            ctxt, deployment.transfer_id, include_task_info=True)
-        skip_os_morphing = deployment.skip_os_morphing
-        clone_disks = deployment.clone_disks
-        user_scripts = deployment.user_scripts
-
+    def _validate_deployment_inputs(self, ctxt, deployment, transfer, force):
         self._check_transfer_running_executions(ctxt, transfer)
         self._check_transfer_running_executions(ctxt, transfer)
         self._check_valid_transfer_tasks_execution(transfer, force)
         self._check_valid_transfer_tasks_execution(transfer, force)
         for instance, info in transfer.info.items():
         for instance, info in transfer.info.items():
@@ -1415,10 +1409,19 @@ class ConductorServerEndpoint(object):
                     f"for instance: {instance}. If transferred disks are "
                     f"for instance: {instance}. If transferred disks are "
                     "deleted, the transfer needs to be executed anew "
                     "deleted, the transfer needs to be executed anew "
                     "before a deployment can occur")
                     "before a deployment can occur")
-        deployment.info = transfer.info
         self._check_minion_pools_for_action(ctxt, deployment)
         self._check_minion_pools_for_action(ctxt, deployment)
         self._check_reservation_for_transfer(transfer)
         self._check_reservation_for_transfer(transfer)
 
 
+    def _execute_deployment(self, ctxt, deployment, force):
+        transfer = self._get_transfer(
+            ctxt, deployment.transfer_id, include_task_info=True)
+        skip_os_morphing = deployment.skip_os_morphing
+        clone_disks = deployment.clone_disks
+        user_scripts = deployment.user_scripts
+
+        if deployment.deployer_id:
+            self._validate_deployment_inputs(ctxt, deployment, transfer, force)
+        deployment.info = transfer.info
         destination_endpoint = self.get_endpoint(
         destination_endpoint = self.get_endpoint(
             ctxt, transfer.destination_endpoint_id)
             ctxt, transfer.destination_endpoint_id)
         destination_provider_types = self._get_provider_types(
         destination_provider_types = self._get_provider_types(
@@ -1709,6 +1712,8 @@ class ConductorServerEndpoint(object):
         if instance_osmorphing_minion_pool_mappings:
         if instance_osmorphing_minion_pool_mappings:
             deployment.instance_osmorphing_minion_pool_mappings.update(
             deployment.instance_osmorphing_minion_pool_mappings.update(
                 instance_osmorphing_minion_pool_mappings)
                 instance_osmorphing_minion_pool_mappings)
+        if not wait_for_execution:
+            self._validate_deployment_inputs(ctxt, deployment, transfer, force)
 
 
         db_api.add_deployment(ctxt, deployment)
         db_api.add_deployment(ctxt, deployment)
         LOG.info("Deployment created: %s", deployment.id)
         LOG.info("Deployment created: %s", deployment.id)

+ 8 - 2
coriolis/tests/conductor/rpc/test_server.py

@@ -2216,6 +2216,8 @@ class ConductorServerEndpointTestCase(test_base.CoriolisBaseTestCase):
         mock_get_available_providers.assert_called_once_with(
         mock_get_available_providers.assert_called_once_with(
             mock.sentinel.context)
             mock.sentinel.context)
 
 
+    @mock.patch.object(server.ConductorServerEndpoint,
+                       '_validate_deployment_inputs')
     @mock.patch.object(models, 'Deployment')
     @mock.patch.object(models, 'Deployment')
     @mock.patch.object(db_api, 'add_deployment')
     @mock.patch.object(db_api, 'add_deployment')
     @mock.patch.object(server.ConductorServerEndpoint, '_get_transfer')
     @mock.patch.object(server.ConductorServerEndpoint, '_get_transfer')
@@ -2223,7 +2225,8 @@ class ConductorServerEndpointTestCase(test_base.CoriolisBaseTestCase):
     @mock.patch.object(server.ConductorServerEndpoint, 'get_deployment')
     @mock.patch.object(server.ConductorServerEndpoint, 'get_deployment')
     def test_deploy_transfer_instances(
     def test_deploy_transfer_instances(
             self, mock_get_deployment, mock_execute_deployment,
             self, mock_get_deployment, mock_execute_deployment,
-            mock_get_transfer, mock_add_deployment, mock_deployment_model):
+            mock_get_transfer, mock_add_deployment, mock_deployment_model,
+            mock_validate_deployment_inputs):
         transfer_mock = mock.MagicMock()
         transfer_mock = mock.MagicMock()
         transfer_mock.instance_osmorphing_minion_pool_mappings = {
         transfer_mock.instance_osmorphing_minion_pool_mappings = {
             mock.sentinel.instance1: mock.sentinel.pool1}
             mock.sentinel.instance1: mock.sentinel.pool1}
@@ -2234,13 +2237,14 @@ class ConductorServerEndpointTestCase(test_base.CoriolisBaseTestCase):
         mock_get_transfer.return_value = transfer_mock
         mock_get_transfer.return_value = transfer_mock
         osm_pool_mappings = {mock.sentinel.instance1: mock.sentinel.pool2}
         osm_pool_mappings = {mock.sentinel.instance1: mock.sentinel.pool2}
         deployment = mock_deployment_model.return_value
         deployment = mock_deployment_model.return_value
+        force = False
 
 
         result = testutils.get_wrapped_function(
         result = testutils.get_wrapped_function(
             self.server.deploy_transfer_instances)(
             self.server.deploy_transfer_instances)(
                 self.server,
                 self.server,
                 mock.sentinel.ctxt,
                 mock.sentinel.ctxt,
                 mock.sentinel.transfer_id,
                 mock.sentinel.transfer_id,
-                force=False,
+                force=force,
                 clone_disks=True,
                 clone_disks=True,
                 instance_osmorphing_minion_pool_mappings=osm_pool_mappings,
                 instance_osmorphing_minion_pool_mappings=osm_pool_mappings,
                 skip_os_morphing=False,
                 skip_os_morphing=False,
@@ -2268,6 +2272,8 @@ class ConductorServerEndpointTestCase(test_base.CoriolisBaseTestCase):
             mock.sentinel.ctxt, deployment, False)
             mock.sentinel.ctxt, deployment, False)
         mock_get_deployment.assert_called_once_with(
         mock_get_deployment.assert_called_once_with(
             mock.sentinel.ctxt, deployment.id)
             mock.sentinel.ctxt, deployment.id)
+        mock_validate_deployment_inputs.assert_called_once_with(
+            mock.sentinel.ctxt, deployment, transfer_mock, force)
 
 
     @mock.patch.object(models, 'Deployment')
     @mock.patch.object(models, 'Deployment')
     @mock.patch.object(db_api, 'add_deployment')
     @mock.patch.object(db_api, 'add_deployment')