From fe5a454441d68d8f599ee20be819577037f03c2c Mon Sep 17 00:00:00 2001 From: Chris Church Date: Mon, 13 May 2013 10:43:43 -0400 Subject: [PATCH] Basic support for job start/cancel via REST API with supporting tests. --- lib/main/access.py | 4 + lib/main/admin.py | 4 +- lib/main/forms.py | 8 +- lib/main/models/__init__.py | 13 ++- lib/main/rbac.py | 4 +- lib/main/serializers.py | 4 + lib/main/tests/base.py | 3 + lib/main/tests/jobs.py | 193 +++++++++++++++++++++++++++++------- lib/main/views.py | 50 +++++++++- 9 files changed, 235 insertions(+), 48 deletions(-) diff --git a/lib/main/access.py b/lib/main/access.py index 97c334b7f9..382b7db56e 100644 --- a/lib/main/access.py +++ b/lib/main/access.py @@ -517,6 +517,10 @@ class JobAccess(BaseAccess): model = Job + def can_change(self, obj, data): + print 'CAN_CHANGE', obj, data + return self.user.is_superuser and obj.status == 'new' + def can_start(self, obj): return False # FIXME diff --git a/lib/main/admin.py b/lib/main/admin.py index 357f56f5d4..664239d234 100644 --- a/lib/main/admin.py +++ b/lib/main/admin.py @@ -359,9 +359,9 @@ class JobAdmin(BaseModelAdmin): fsets = [fs for fs in fsets if 'created' not in fs[1]['fields'] and 'celery_task_id' not in fs[1]['fields']] - if not obj or (obj and obj.pk and obj.status != 'new'): + if not obj or (obj and obj.pk and not obj.can_start): fsets = [fs for fs in fsets if 'start_job' not in fs[1]['fields']] - if not obj or (obj and obj.pk and obj.status not in ('pending', 'running')): + if not obj or (obj and obj.pk and not obj.can_cancel): for fs in fsets: if 'celery_task_id' in fs[1]['fields']: fs[1]['fields'] = ('status', 'get_result_stdout_display', diff --git a/lib/main/forms.py b/lib/main/forms.py index a94ef1b0ff..311a15434a 100644 --- a/lib/main/forms.py +++ b/lib/main/forms.py @@ -115,7 +115,7 @@ class JobAdminForm(JobTemplateAdminForm): if self.instance.pk and self.instance.status != 'new': self.fields.pop('playbook', None) if (not self.data or self.data.get('start_job', '')) and \ - self.instance.credential and self.instance.status == 'new': + self.instance.credential and self.instance.can_start: for field in self.instance.get_passwords_needed_to_start(): if field not in self.fields: continue @@ -128,7 +128,7 @@ class JobAdminForm(JobTemplateAdminForm): return self.cleaned_data.get('cancel_job', False) def clean(self): - if self.instance.credential and self.instance.status == 'new': + if self.instance.credential and self.instance.can_start: for field in self.instance.get_passwords_needed_to_start(): if field in self.fields: self.fields[field].required = True @@ -138,7 +138,7 @@ class JobAdminForm(JobTemplateAdminForm): instance = super(JobAdminForm, self).save(commit) save_m2m = getattr(self, 'save_m2m', lambda: None) should_start = bool(self.cleaned_data.get('start_job', '') and - instance.status == 'new') + instance.can_start) start_opts = {} for field in ('ssh_password', 'sudo_password', 'ssh_key_unlock'): value = self.cleaned_data.get(field, '') @@ -146,7 +146,7 @@ class JobAdminForm(JobTemplateAdminForm): start_opts[field] = value #print 'should_start', should_start should_cancel = bool(self.cleaned_data.get('cancel_job', '') and - instance.status in ('new', 'pending')) + instance.can_cancel) def new_save_m2m(): save_m2m() if should_start: diff --git a/lib/main/models/__init__.py b/lib/main/models/__init__.py index 986b9ce50c..124d112899 100644 --- a/lib/main/models/__init__.py +++ b/lib/main/models/__init__.py @@ -644,6 +644,7 @@ class Job(CommonModel): ) failed = models.BooleanField( default=False, + editable=False, ) result_stdout = models.TextField( blank=True, @@ -703,9 +704,13 @@ class Job(CommonModel): needed.append(field) return needed + @property + def can_start(self): + return bool(self.status == 'new') + def start(self, **kwargs): from lib.main.tasks import RunJob - if self.status != 'new': + if not self.can_start: return False needed = self.get_passwords_needed_to_start() opts = dict([(field, kwargs.get(field, '')) for field in needed]) @@ -720,8 +725,12 @@ class Job(CommonModel): self.save(update_fields=['celery_task_id']) return True + @property + def can_cancel(self): + return bool(self.status in ('pending', 'running')) + def cancel(self): - if self.status in ('pending', 'running'): + if self.can_cancel: if not self.cancel_flag: self.cancel_flag = True self.save(update_fields=['cancel_flag']) diff --git a/lib/main/rbac.py b/lib/main/rbac.py index ef8a2cba93..7e724e4c76 100644 --- a/lib/main/rbac.py +++ b/lib/main/rbac.py @@ -70,6 +70,8 @@ class CustomRbac(permissions.BasePermission): return check_user_access(request.user, view.model, 'delete', obj) def _check_permissions(self, request, view, obj=None): + #if not obj and hasattr(view, 'get_object'): + # obj = view.get_object() # Check that obj (if given) is active, otherwise raise a 404. active = getattr(obj, 'active', getattr(obj, 'is_active', True)) if callable(active): @@ -81,7 +83,7 @@ class CustomRbac(permissions.BasePermission): return False # Don't allow inactive users (and respond with a 403). if not request.user.is_active: - raise PermissionDenied() + raise PermissionDenied('your account is inactive') # Always allow superusers (as long as they are active). if request.user.is_superuser: return True diff --git a/lib/main/serializers.py b/lib/main/serializers.py index 2667e5fb3f..3213cfaf9c 100644 --- a/lib/main/serializers.py +++ b/lib/main/serializers.py @@ -407,6 +407,10 @@ class JobSerializer(BaseSerializer): )) if obj.job_template: res['job_template'] = reverse('main:job_template_detail', args=(obj.job_template.pk,)) + if obj.can_start or True: + res['start'] = reverse('main:job_start', args=(obj.pk,)) + if obj.can_cancel or True: + res['cancel'] = reverse('main:job_cancel', args=(obj.pk,)) return res def from_native(self, data, files): diff --git a/lib/main/tests/base.py b/lib/main/tests/base.py index 4bcf2f9e5c..7531a6c71b 100644 --- a/lib/main/tests/base.py +++ b/lib/main/tests/base.py @@ -203,6 +203,9 @@ class BaseTestMixin(object): def put(self, url, data, expect=200, auth=None): return self._generic_rest(url, data=data, expect=expect, auth=auth, method='put') + def patch(self, url, data, expect=200, auth=None): + return self._generic_rest(url, data=data, expect=expect, auth=auth, method='patch') + def delete(self, url, expect=201, auth=None): return self._generic_rest(url, data=None, expect=expect, auth=auth, method='delete') diff --git a/lib/main/tests/jobs.py b/lib/main/tests/jobs.py index 4b98013710..409b7c89c2 100644 --- a/lib/main/tests/jobs.py +++ b/lib/main/tests/jobs.py @@ -18,11 +18,13 @@ import datetime import json from django.contrib.auth.models import User as DjangoUser from django.core.urlresolvers import reverse +import django.test from django.test.client import Client +from django.test.utils import override_settings from lib.main.models import * -from lib.main.tests.base import BaseTest +from lib.main.tests.base import BaseTestMixin -__all__ = ['JobTemplateTest', 'JobTest'] +__all__ = ['JobTemplateTest', 'JobTest', 'JobStartCancelTest'] TEST_PLAYBOOK = '''- hosts: all gather_facts: false @@ -31,7 +33,7 @@ TEST_PLAYBOOK = '''- hosts: all command: test 1 = 1 ''' -class BaseJobTest(BaseTest): +class BaseJobTestMixin(BaseTestMixin): '''''' def _create_inventory(self, name, organization, created_by, @@ -273,7 +275,7 @@ class BaseJobTest(BaseTest): ) self.cred_iris = self.user_iris.credentials.create( ssh_username='iris', - ssh_password='', + ssh_password='ASK', created_by=self.user_sue, ) @@ -414,15 +416,9 @@ class BaseJobTest(BaseTest): ) def setUp(self): - super(BaseJobTest, self).setUp() + super(BaseJobTestMixin, self).setUp() self.populate() - -class JobTemplateTest(BaseJobTest): - - def setUp(self): - super(JobTemplateTest, self).setUp() - def _test_invalid_creds(self, url, data=None, methods=None): data = data or {} methods = methods or ('options', 'head', 'get') @@ -430,11 +426,13 @@ class JobTemplateTest(BaseJobTest): with self.current_user(*auth): for method in methods: f = getattr(self, method) - if method in ('post', 'put'): + if method in ('post', 'put', 'patch'): f(url, data, expect=401) else: f(url, expect=401) +class JobTemplateTest(BaseJobTestMixin, django.test.TestCase): + def test_get_job_template_list(self): url = reverse('main:job_template_list') @@ -556,12 +554,15 @@ class JobTemplateTest(BaseJobTest): url = reverse('main:job_template_detail', args=(jt.pk,)) # Test with no auth and with invalid login. - self._test_invalid_creds(url, methods=('put',)) + self._test_invalid_creds(url, methods=('put',))# 'patch')) # sue can update the job template detail. with self.current_user(self.user_sue): data = self.get(url) + data['name'] = '%s-updated' % data['name'] response = self.put(url, data) + #patch_data = dict(name='%s-changed' % data['name']) + #response = self.patch(url, patch_data) # FIXME: Check other credentials and optional fields. @@ -600,40 +601,100 @@ class JobTemplateTest(BaseJobTest): # FIXME: Check other credentials and optional fields. -class JobTest(BaseJobTest): - - def setUp(self): - super(JobTest, self).setUp() +class JobTest(BaseJobTestMixin, django.test.TestCase): def test_get_job_list(self): - pass + url = reverse('main:job_list') + + # Test with no auth and with invalid login. + self._test_invalid_creds(url) + + # sue's credentials (superuser) == 200, full list + with self.current_user(self.user_sue): + self.options(url) + self.head(url) + response = self.get(url) + qs = Job.objects.all() + self.check_pagination_and_size(response, qs.count()) + self.check_list_ids(response, qs) + + # FIXME: Check individual job result fields. + # FIXME: Check with other credentials. def test_post_job_list(self): - pass + url = reverse('main:job_list') + data = dict( + name='new job without template', + job_type=PERM_INVENTORY_DEPLOY, + inventory=self.inv_ops_east.pk, + project=self.proj_prod.pk, + playbook=self.proj_prod.playbooks[0], + credential=self.cred_ops_east.pk, + ) + + # Test with no auth and with invalid login. + self._test_invalid_creds(url, data, methods=('post',)) + + # sue can create a new job without a template. + with self.current_user(self.user_sue): + response = self.post(url, data, expect=201) + + # sue can also create a job here from a template + jt = self.jt_ops_east_run + data = dict( + name='new job from template', + job_template=jt.pk, + ) + with self.current_user(self.user_sue): + response = self.post(url, data, expect=201) + + # FIXME: Check with other credentials and optional fields. def test_get_job_detail(self): - pass + job = self.job_ops_east_run + url = reverse('main:job_detail', args=(job.pk,)) + + # Test with no auth and with invalid login. + self._test_invalid_creds(url) + + # sue can read the job detail. + with self.current_user(self.user_sue): + self.options(url) + self.head(url) + response = self.get(url) + self.assertEqual(response['url'], url) + + # FIXME: Check with other credentials and optional fields. def test_put_job_detail(self): - pass + job = self.job_ops_west_run + url = reverse('main:job_detail', args=(job.pk,)) - def test_get_job_start(self): - pass + # Test with no auth and with invalid login. + self._test_invalid_creds(url, methods=('put',))# 'patch')) - def test_post_job_start(self): - pass + # sue can update the job detail only if the job is new. + self.assertEqual(job.status, 'new') + with self.current_user(self.user_sue): + data = self.get(url) + data['name'] = '%s-updated' % data['name'] + response = self.put(url, data) + #patch_data = dict(name='%s-changed' % data['name']) + #response = self.patch(url, patch_data) - def test_get_job_cancel(self): - pass + # sue cannot update the job detail if it is in any other state. + for status in ('pending', 'running', 'successful', 'failed', 'error', + 'canceled'): + job.status = status + job.save() + with self.current_user(self.user_sue): + data = self.get(url) + data['name'] = '%s-updated' % data['name'] + self.put(url, data, expect=405) + #patch_data = dict(name='%s-changed' % data['name']) + #self.patch(url, patch_data, expect=405) - def test_post_job_cancel(self): - pass - - def test_get_job_host_list(self): - pass - - def test_get_job_job_event_list(self): - pass + # FIXME: Check with other credentials and readonly fields. def _test_mainline(self): url = reverse('main:job_list') @@ -683,4 +744,66 @@ class JobTest(BaseJobTest): # and that jobs come back nicely serialized with related resources and so on ... # that we can drill all the way down and can get at host failure lists, etc ... +@override_settings(CELERY_ALWAYS_EAGER=True, + CELERY_EAGER_PROPAGATES_EXCEPTIONS=True, + ANSIBLE_TRANSPORT='local') +class JobStartCancelTest(BaseJobTestMixin, django.test.TransactionTestCase): + '''Job API tests that need to use the celery task backend.''' + + def setUp(self): + super(JobStartCancelTest, self).setUp() + # Pass test database name in environment for use by the inventory script. + os.environ['ACOM_TEST_DATABASE_NAME'] = settings.DATABASES['default']['NAME'] + + def tearDown(self): + super(JobStartCancelTest, self).tearDown() + os.environ.pop('ACOM_TEST_DATABASE_NAME', None) + + def test_job_start(self): + job = self.job_ops_east_run + url = reverse('main:job_start', args=(job.pk,)) + + # Test with no auth and with invalid login. + self._test_invalid_creds(url) + self._test_invalid_creds(url, methods=('post',)) + + self.assertEqual(job.status, 'new') + with self.current_user(self.user_sue): + response = self.get(url) + self.assertTrue(response['can_start']) + self.assertFalse(response['passwords_needed_to_start']) + response = self.post(url, {}, expect=202) + + # FIXME: Test with other users, test when passwords are required. + + def test_job_cancel(self): + job = self.job_ops_east_run + url = reverse('main:job_cancel', args=(job.pk,)) + + # Test with no auth and with invalid login. + self._test_invalid_creds(url) + self._test_invalid_creds(url, methods=('post',)) + + # sue can cancel the job, but only when it is pending or running. + for status in [x[0] for x in Job.STATUS_CHOICES]: + job.status = status + job.save() + with self.current_user(self.user_sue): + response = self.get(url) + if status in ('pending', 'running'): + self.assertTrue(response['can_cancel']) + response = self.post(url, {}, expect=202) + else: + self.assertFalse(response['can_cancel']) + response = self.post(url, {}, expect=405) + + # FIXME: Test with other users. + + def test_get_job_host_list(self): + pass + + def test_get_job_job_event_list(self): + pass + + diff --git a/lib/main/views.py b/lib/main/views.py index 5a1561e14f..f24520ffcd 100644 --- a/lib/main/views.py +++ b/lib/main/views.py @@ -954,15 +954,57 @@ class JobDetail(BaseDetail): serializer_class = JobSerializer permission_classes = (CustomRbac,) -class JobStart(BaseDetail): + def update(self, request, *args, **kwargs): + obj = self.get_object() + # Only allow changes (PUT/PATCH) when job status is "new". + if obj.status != 'new': + return self.http_method_not_allowed(request, *args, **kwargs) + return super(JobDetail, self).update(request, *args, **kwargs) + +class JobStart(generics.RetrieveAPIView): + + model = Job + permission_classes = (CustomRbac,) + + def get(self, request, *args, **kwargs): + obj = self.get_object() + data = dict( + can_start=obj.can_start, + ) + if obj.can_start: + data['passwords_needed_to_start'] = obj.get_passwords_needed_to_start() + return Response(data) def post(self, request, *args, **kwargs): - pass # FIXME + obj = self.get_object() + if obj.can_start: + result = obj.start(**request.DATA) + if not result: + return Response(status=400) + else: + return Response(status=202) + else: + return Response(status=405) -class JobCancel(BaseDetail): +class JobCancel(generics.RetrieveAPIView): + + model = Job + permission_classes = (CustomRbac,) + + def get(self, request, *args, **kwargs): + obj = self.get_object() + data = dict( + can_cancel=obj.can_cancel, + ) + return Response(data) def post(self, request, *args, **kwargs): - pass # FIXME + obj = self.get_object() + if obj.can_cancel: + result = obj.cancel() + return Response(status=202) + else: + return Response(status=405) class JobHostsList(BaseSubList): pass