Skip to content

Commit 6d5c21e

Browse files
committed
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.
1 parent c430714 commit 6d5c21e

File tree

2 files changed

+21
-10
lines changed

2 files changed

+21
-10
lines changed

coriolis/conductor/rpc/server.py

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1399,13 +1399,7 @@ def _get_provider_types(self, ctxt, endpoint):
13991399
"No provider found for: %s" % endpoint.type)
14001400
return provider_types["types"]
14011401

1402-
def _execute_deployment(self, ctxt, deployment, force):
1403-
transfer = self._get_transfer(
1404-
ctxt, deployment.transfer_id, include_task_info=True)
1405-
skip_os_morphing = deployment.skip_os_morphing
1406-
clone_disks = deployment.clone_disks
1407-
user_scripts = deployment.user_scripts
1408-
1402+
def _validate_deployment_inputs(self, ctxt, deployment, transfer, force):
14091403
self._check_transfer_running_executions(ctxt, transfer)
14101404
self._check_valid_transfer_tasks_execution(transfer, force)
14111405
for instance, info in transfer.info.items():
@@ -1415,10 +1409,19 @@ def _execute_deployment(self, ctxt, deployment, force):
14151409
f"for instance: {instance}. If transferred disks are "
14161410
"deleted, the transfer needs to be executed anew "
14171411
"before a deployment can occur")
1418-
deployment.info = transfer.info
14191412
self._check_minion_pools_for_action(ctxt, deployment)
14201413
self._check_reservation_for_transfer(transfer)
14211414

1415+
def _execute_deployment(self, ctxt, deployment, force):
1416+
transfer = self._get_transfer(
1417+
ctxt, deployment.transfer_id, include_task_info=True)
1418+
skip_os_morphing = deployment.skip_os_morphing
1419+
clone_disks = deployment.clone_disks
1420+
user_scripts = deployment.user_scripts
1421+
1422+
if deployment.deployer_id:
1423+
self._validate_deployment_inputs(ctxt, deployment, transfer, force)
1424+
deployment.info = transfer.info
14221425
destination_endpoint = self.get_endpoint(
14231426
ctxt, transfer.destination_endpoint_id)
14241427
destination_provider_types = self._get_provider_types(
@@ -1709,6 +1712,8 @@ def deploy_transfer_instances(
17091712
if instance_osmorphing_minion_pool_mappings:
17101713
deployment.instance_osmorphing_minion_pool_mappings.update(
17111714
instance_osmorphing_minion_pool_mappings)
1715+
if not wait_for_execution:
1716+
self._validate_deployment_inputs(ctxt, deployment, transfer, force)
17121717

17131718
db_api.add_deployment(ctxt, deployment)
17141719
LOG.info("Deployment created: %s", deployment.id)

coriolis/tests/conductor/rpc/test_server.py

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2216,14 +2216,17 @@ def test_get_provider_types(self, mock_get_available_providers):
22162216
mock_get_available_providers.assert_called_once_with(
22172217
mock.sentinel.context)
22182218

2219+
@mock.patch.object(server.ConductorServerEndpoint,
2220+
'_validate_deployment_inputs')
22192221
@mock.patch.object(models, 'Deployment')
22202222
@mock.patch.object(db_api, 'add_deployment')
22212223
@mock.patch.object(server.ConductorServerEndpoint, '_get_transfer')
22222224
@mock.patch.object(server.ConductorServerEndpoint, '_execute_deployment')
22232225
@mock.patch.object(server.ConductorServerEndpoint, 'get_deployment')
22242226
def test_deploy_transfer_instances(
22252227
self, mock_get_deployment, mock_execute_deployment,
2226-
mock_get_transfer, mock_add_deployment, mock_deployment_model):
2228+
mock_get_transfer, mock_add_deployment, mock_deployment_model,
2229+
mock_validate_deployment_inputs):
22272230
transfer_mock = mock.MagicMock()
22282231
transfer_mock.instance_osmorphing_minion_pool_mappings = {
22292232
mock.sentinel.instance1: mock.sentinel.pool1}
@@ -2234,13 +2237,14 @@ def test_deploy_transfer_instances(
22342237
mock_get_transfer.return_value = transfer_mock
22352238
osm_pool_mappings = {mock.sentinel.instance1: mock.sentinel.pool2}
22362239
deployment = mock_deployment_model.return_value
2240+
force = False
22372241

22382242
result = testutils.get_wrapped_function(
22392243
self.server.deploy_transfer_instances)(
22402244
self.server,
22412245
mock.sentinel.ctxt,
22422246
mock.sentinel.transfer_id,
2243-
force=False,
2247+
force=force,
22442248
clone_disks=True,
22452249
instance_osmorphing_minion_pool_mappings=osm_pool_mappings,
22462250
skip_os_morphing=False,
@@ -2268,6 +2272,8 @@ def test_deploy_transfer_instances(
22682272
mock.sentinel.ctxt, deployment, False)
22692273
mock_get_deployment.assert_called_once_with(
22702274
mock.sentinel.ctxt, deployment.id)
2275+
mock_validate_deployment_inputs.assert_called_once_with(
2276+
mock.sentinel.ctxt, deployment, transfer_mock, force)
22712277

22722278
@mock.patch.object(models, 'Deployment')
22732279
@mock.patch.object(db_api, 'add_deployment')

0 commit comments

Comments
 (0)