diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index ac65968d663c..de2c0b8b2fe0 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -1382,6 +1382,11 @@ def _build_job_explanation(self): return 'Previous Task Canceled: {"job_type": "%s", "job_name": "%s", "job_id": "%s"}' % (self.model_to_str(), self.name, self.id) return None + def fallback_cancel(self): + if not self.celery_task_id: + self.refresh_from_db(fields=['celery_task_id']) + self.cancel_dispatcher_process() + def cancel_dispatcher_process(self): """Returns True if dispatcher running this job acknowledged request and sent SIGTERM""" if not self.celery_task_id: @@ -1419,12 +1424,7 @@ def cancel(self, job_explanation=None, is_chain=False): else: # Avoid race condition where we have stale model from pending state but job has already started, # its checking signal but not cancel_flag, so re-send signal after this database commit - def try_to_cancel_one_last_time(): - if not self.celery_task_id: - self.refresh_from_db(fields=['celery_task_id']) - self.cancel_dispatcher_process() - - connection.on_commit(try_to_cancel_one_last_time) + connection.on_commit(self.fallback_cancel) # If a SIGTERM signal was sent to the control process, and acked by the dispatcher # then we want to let its own cleanup change status, otherwise change status now diff --git a/awx/main/tests/unit/models/test_unified_job_unit.py b/awx/main/tests/unit/models/test_unified_job_unit.py index 72fcc0ef9b89..b0567500fc4e 100644 --- a/awx/main/tests/unit/models/test_unified_job_unit.py +++ b/awx/main/tests/unit/models/test_unified_job_unit.py @@ -22,6 +22,10 @@ def test_unified_job_workflow_attributes(): assert job.workflow_job_id == 1 +def mock_on_commit(f): + f() + + @pytest.fixture def unified_job(mocker): mocker.patch.object(UnifiedJob, 'can_cancel', return_value=True) @@ -30,12 +34,13 @@ def unified_job(mocker): j.cancel_flag = None j.save = mocker.MagicMock() j.websocket_emit_status = mocker.MagicMock() + j.fallback_cancel = mocker.MagicMock() return j def test_cancel(unified_job): - with mock.patch('awx.main.models.unified_jobs.connection.on_commit'): + with mock.patch('awx.main.models.unified_jobs.connection.on_commit', wraps=mock_on_commit): unified_job.cancel() assert unified_job.cancel_flag is True @@ -55,7 +60,7 @@ def test_cancel_job_explanation(unified_job): unified_job.cancel(job_explanation=job_explanation) assert unified_job.job_explanation == job_explanation - unified_job.save.assert_called_with(update_fields=['cancel_flag', 'start_args', 'status', 'job_explanation']) + unified_job.save.assert_called_with(update_fields=['cancel_flag', 'start_args', 'job_explanation', 'status']) def test_organization_copy_to_jobs():