From f1128206ff6bc0accf1844a8f25624a7c692d3f3 Mon Sep 17 00:00:00 2001 From: Nikolay Mahotkin Date: Fri, 7 Jul 2017 15:56:52 +0300 Subject: [PATCH] Fixing deleting cron-trigger trusts * Removed extra client creation from trust * Delete trust only together with deleting trigger (so only one node deletes the trust) * Fix trust context with session Change-Id: Ic207b0364b6bc45a6e24e561ef1f540208795530 --- mistral/services/periodic.py | 15 ++++++++------- mistral/services/security.py | 10 +++++++++- mistral/services/triggers.py | 8 ++++++-- mistral/tests/unit/api/v2/test_cron_triggers.py | 2 +- .../tests/unit/services/test_trigger_service.py | 5 +++-- 5 files changed, 27 insertions(+), 13 deletions(-) diff --git a/mistral/services/periodic.py b/mistral/services/periodic.py index 438424bff..9fc72516a 100644 --- a/mistral/services/periodic.py +++ b/mistral/services/periodic.py @@ -38,14 +38,15 @@ class MistralPeriodicTasks(periodic_task.PeriodicTasks): for trigger in triggers.get_next_cron_triggers(): LOG.debug("Processing cron trigger: %s" % trigger) - # Setup admin context before schedule triggers. - ctx = security.create_context(trigger.trust_id, trigger.project_id) - - auth_ctx.set_ctx(ctx) - - LOG.debug("Cron trigger security context: %s" % ctx) - try: + # Setup admin context before schedule triggers. + ctx = security.create_context( + trigger.trust_id, trigger.project_id + ) + + auth_ctx.set_ctx(ctx) + LOG.debug("Cron trigger security context: %s" % ctx) + # Try to advance the cron trigger next_execution_time and # remaining_executions if relevant. modified = advance_cron_trigger(trigger) diff --git a/mistral/services/security.py b/mistral/services/security.py index 497e638bb..048747b65 100644 --- a/mistral/services/security.py +++ b/mistral/services/security.py @@ -85,7 +85,15 @@ def delete_trust(trust_id): if not trust_id: return - keystone_client = keystone.client_for_trusts(trust_id) + ctx = auth_ctx.ctx() + + # If this trust is already in the context then it means that + # context already has trust scoped token from exactly this trust_id. + # So we don't need request the token from the trust one more time. + if ctx.is_trust_scoped and ctx.trust_id == trust_id: + keystone_client = keystone.client() + else: + keystone_client = keystone.client_for_trusts(trust_id) try: keystone_client.trusts.delete(trust_id) diff --git a/mistral/services/triggers.py b/mistral/services/triggers.py index 2777065dd..b6497b4a7 100644 --- a/mistral/services/triggers.py +++ b/mistral/services/triggers.py @@ -138,8 +138,12 @@ def delete_cron_trigger(name, trust_id=None): trigger = db_api.get_cron_trigger(name) trust_id = trigger.trust_id - security.delete_trust(trust_id) - return db_api.delete_cron_trigger(name) + modified_count = db_api.delete_cron_trigger(name) + if modified_count: + # Delete trust only together with deleting trigger. + security.delete_trust(trust_id) + + return modified_count def create_event_trigger(name, exchange, topic, event, workflow_id, diff --git a/mistral/tests/unit/api/v2/test_cron_triggers.py b/mistral/tests/unit/api/v2/test_cron_triggers.py index c0e882a12..beb93bd40 100644 --- a/mistral/tests/unit/api/v2/test_cron_triggers.py +++ b/mistral/tests/unit/api/v2/test_cron_triggers.py @@ -60,7 +60,7 @@ TRIGGER_DB.update(trigger_values) MOCK_WF = mock.MagicMock(return_value=WF) MOCK_TRIGGER = mock.MagicMock(return_value=TRIGGER_DB) MOCK_TRIGGERS = mock.MagicMock(return_value=[TRIGGER_DB]) -MOCK_DELETE = mock.MagicMock(return_value=None) +MOCK_DELETE = mock.MagicMock(return_value=1) MOCK_EMPTY = mock.MagicMock(return_value=[]) MOCK_NOT_FOUND = mock.MagicMock(side_effect=exc.DBEntityNotFoundError()) MOCK_DUPLICATE = mock.MagicMock(side_effect=exc.DBDuplicateEntryError()) diff --git a/mistral/tests/unit/services/test_trigger_service.py b/mistral/tests/unit/services/test_trigger_service.py index 4681642e3..e78079f25 100644 --- a/mistral/tests/unit/services/test_trigger_service.py +++ b/mistral/tests/unit/services/test_trigger_service.py @@ -227,14 +227,15 @@ class TriggerServiceV2Test(base.DbTestCase): @mock.patch.object(security, 'create_trust', type('trust', (object,), {'id': 'my_trust_id'})) - @mock.patch.object(security, 'create_context', mock.Mock()) + @mock.patch.object(security, 'create_context') @mock.patch.object(rpc.EngineClient, 'start_workflow', mock.Mock()) @mock.patch( 'mistral.services.periodic.advance_cron_trigger', mock.MagicMock(side_effect=new_advance_cron_trigger) ) @mock.patch.object(security, 'delete_trust') - def test_create_delete_trust_in_trigger(self, delete_trust): + def test_create_delete_trust_in_trigger(self, create_ctx, delete_trust): + create_ctx.return_value = self.ctx cfg.CONF.set_default('auth_enable', True, group='pecan') trigger_thread = periodic.setup() self.addCleanup(trigger_thread.stop)