From d84bfd710d22871b0a345b26f8349bc60a9ce85c Mon Sep 17 00:00:00 2001 From: Paul Eggleton Date: Tue, 18 Sep 2018 17:31:57 +1200 Subject: [PATCH] Allow stopping update task For situations where the user launches a distro comparison update process and then shortly afterwards realises it is operating with the wrong configuration (or is otherwise broken) and is going to take a long time to finish, add a button to the task page to stop the task. This was tricky to get working, since the default behaviour of Celery's revoke() would either terminate both the Celery task process along with the update process (leaving us with no log saved to the database) or worse not even kill the update process, depending on the signal sent. To avoid this, send SIGUSR2, trap it in the task process and kill the child process, returning gracefully. To make that possible I had to rewrite runcmd() to use subprocess.Popen() instead of subprocess.check_call() as otherwise we can't get the child's PID. Signed-off-by: Paul Eggleton --- layerindex/urls.py | 5 ++++- layerindex/utils.py | 16 +++++++++++++--- layerindex/views.py | 11 +++++++++++ templates/layerindex/task.html | 11 +++++++++++ 4 files changed, 39 insertions(+), 4 deletions(-) diff --git a/layerindex/urls.py b/layerindex/urls.py index e8ba8c9..bab7f10 100644 --- a/layerindex/urls.py +++ b/layerindex/urls.py @@ -8,7 +8,7 @@ from django.conf.urls import * from django.views.generic import TemplateView, DetailView, ListView, RedirectView from django.views.defaults import page_not_found from django.core.urlresolvers import reverse_lazy -from layerindex.views import LayerListView, LayerReviewListView, LayerReviewDetailView, RecipeSearchView, MachineSearchView, LayerDetailView, edit_layer_view, delete_layer_view, edit_layernote_view, delete_layernote_view, HistoryListView, EditProfileFormView, AdvancedRecipeSearchView, BulkChangeView, BulkChangeSearchView, bulk_change_edit_view, bulk_change_patch_view, BulkChangeDeleteView, RecipeDetailView, RedirectParamsView, ClassicRecipeSearchView, ClassicRecipeDetailView, ClassicRecipeStatsView, LayerUpdateDetailView, UpdateListView, UpdateDetailView, StatsView, publish_view, LayerCheckListView, BBClassCheckListView, TaskStatusView, ComparisonRecipeSelectView, ComparisonRecipeSelectDetailView, task_log_view +from layerindex.views import LayerListView, LayerReviewListView, LayerReviewDetailView, RecipeSearchView, MachineSearchView, LayerDetailView, edit_layer_view, delete_layer_view, edit_layernote_view, delete_layernote_view, HistoryListView, EditProfileFormView, AdvancedRecipeSearchView, BulkChangeView, BulkChangeSearchView, bulk_change_edit_view, bulk_change_patch_view, BulkChangeDeleteView, RecipeDetailView, RedirectParamsView, ClassicRecipeSearchView, ClassicRecipeDetailView, ClassicRecipeStatsView, LayerUpdateDetailView, UpdateListView, UpdateDetailView, StatsView, publish_view, LayerCheckListView, BBClassCheckListView, TaskStatusView, ComparisonRecipeSelectView, ComparisonRecipeSelectDetailView, task_log_view, task_stop_view from layerindex.models import LayerItem, Recipe, RecipeChangeset from rest_framework import routers from . import restviews @@ -171,6 +171,9 @@ urlpatterns = [ url(r'^tasklog/(?P[-\w]+)/$', task_log_view, name='task_log'), + url(r'^stoptask/(?P[-\w]+)/$', + task_stop_view, + name='task_stop'), url(r'^ajax/layerchecklist/(?P[-\w]+)/$', LayerCheckListView.as_view( template_name='layerindex/layerchecklist.html'), diff --git a/layerindex/utils.py b/layerindex/utils.py index 04599a1..88b3bd6 100644 --- a/layerindex/utils.py +++ b/layerindex/utils.py @@ -285,6 +285,7 @@ def parse_layer_conf(layerdir, data, logger=None): data = parse_conf(conf_file, data) data.expandVarref('LAYERDIR') +child_pid = 0 def runcmd(cmd, destdir=None, printerr=True, outfile=None, logger=None): """ execute command, raise CalledProcessError if fail @@ -296,10 +297,17 @@ def runcmd(cmd, destdir=None, printerr=True, outfile=None, logger=None): out = open(outfile, 'wb+') else: out = tempfile.TemporaryFile() + + def onsigusr2(sig, frame): + # Kill the child process + os.kill(child_pid, signal.SIGTERM) + signal.signal(signal.SIGUSR2, onsigusr2) try: - try: - subprocess.check_call(cmd, stdout=out, stderr=out, cwd=destdir, shell=True) - except subprocess.CalledProcessError as e: + proc = subprocess.Popen(cmd, stdout=out, stderr=out, cwd=destdir, shell=True) + global child_pid + child_pid = proc.pid + proc.communicate() + if proc.returncode: out.seek(0) output = out.read() output = output.decode('utf-8', errors='replace').strip() @@ -308,6 +316,7 @@ def runcmd(cmd, destdir=None, printerr=True, outfile=None, logger=None): logger.error("%s" % output) else: sys.stderr.write("%s\n" % output) + e = subprocess.CalledProcessError(proc.returncode, cmd) e.output = output raise e @@ -317,6 +326,7 @@ def runcmd(cmd, destdir=None, printerr=True, outfile=None, logger=None): if logger: logger.debug("output: %s" % output.rstrip() ) finally: + signal.signal(signal.SIGUSR2, signal.SIG_DFL) if outfile: out.close() return output diff --git a/layerindex/views.py b/layerindex/views.py index f25b1bd..758ce84 100644 --- a/layerindex/views.py +++ b/layerindex/views.py @@ -1412,6 +1412,17 @@ def task_log_view(request, task_id): response['Task-Progress'] = preader.read() return response +def task_stop_view(request, task_id): + from celery.result import AsyncResult + import signal + if not request.user.is_authenticated(): + raise PermissionDenied + + result = AsyncResult(task_id) + result.revoke(terminate=True, signal=signal.SIGUSR2) + return HttpResponse('terminated') + + class ComparisonRecipeSelectView(ClassicRecipeSearchView): def _can_edit(self): if self.request.user.is_authenticated(): diff --git a/templates/layerindex/task.html b/templates/layerindex/task.html index 0937e03..597a07e 100644 --- a/templates/layerindex/task.html +++ b/templates/layerindex/task.html @@ -30,6 +30,10 @@ {% endif %} +{% if not update.finished %} +