From fd1749c12c2310aa438d50f37b2d31c757d345e7 Mon Sep 17 00:00:00 2001 From: Corbin Simpson Date: Wed, 9 May 2012 17:02:00 -0700 Subject: [PATCH] models: Remove "cleared" and "processed" fields from Jobs. Paves the way for a simpler world. If a Job has not finished, then it will not have a "finished" timestamp. If it has finished but not cleared, it will have a "finished" timestamp. If it is cleared, then it will be deleted from the DB. Refs #8439. --- ganeti_web/cache/job.py | 9 +-- ganeti_web/models.py | 8 +-- ganeti_web/tests/job.py | 81 +++++++++++++------------- ganeti_web/tests/views/virtual_machine/edit.py | 7 +-- ganeti_web/views/cluster.py | 6 +- ganeti_web/views/general.py | 4 +- ganeti_web/views/jobs.py | 39 +++++-------- ganeti_web/views/node.py | 5 +- ganeti_web/views/virtual_machine.py | 8 +-- 9 files changed, 81 insertions(+), 86 deletions(-) diff --git a/ganeti_web/cache/job.py b/ganeti_web/cache/job.py index 7c282da9..ed5ccb79 100644 --- a/ganeti_web/cache/job.py +++ b/ganeti_web/cache/job.py @@ -274,10 +274,11 @@ class JobCacheUpdater(object): ct = ContentType.objects.get_for_model(model) # create job - job = Job(cluster=cluster, job_id=id, content_type=ct, object_id=obj_id) - job.cleared = info['status'] in COMPLETE_STATUS - job.info = info - job.save() + if info["status"] not in COMPLETE_STATUS: + job = Job(cluster=cluster, job_id=id, content_type=ct, + object_id=obj_id) + job.info = info + job.save() updated += 1 callback(id) diff --git a/ganeti_web/models.py b/ganeti_web/models.py index 0c8bad3c..f06b992d 100644 --- a/ganeti_web/models.py +++ b/ganeti_web/models.py @@ -332,7 +332,7 @@ class CachedClusterObject(models.Model): if self.last_job_id: ct = ContentType.objects.get_for_model(self) job_ids = Job.objects\ - .filter(content_type=ct, object_id=self.pk, processed=False) \ + .filter(content_type=ct, object_id=self.pk) \ .order_by('job_id') \ .values_list('job_id', flat=True) @@ -347,14 +347,14 @@ class CachedClusterObject(models.Model): if status in ('success', 'error'): job_updates = Job.parse_persistent_info(data) Job.objects.filter(pk=job_id) \ - .update(processed=True, **job_updates) + .update(**job_updates) except GanetiApiError: status = 'unknown' op = None if status == 'unknown': Job.objects.filter(pk=job_id) \ - .update(status=status, ignore_cache=False, processed=True) + .update(status=status, ignore_cache=False) if status in ('success','error','unknown'): _updates = self._complete_job(self.cluster_id, @@ -452,8 +452,6 @@ class Job(CachedClusterObject): cluster = models.ForeignKey('Cluster', editable=False, related_name='jobs') cluster_hash = models.CharField(max_length=40, editable=False) - processed = models.BooleanField(default=False) - cleared = models.BooleanField(default=False) finished = models.DateTimeField(null=True) status = models.CharField(max_length=10) op = models.CharField(max_length=50) diff --git a/ganeti_web/tests/job.py b/ganeti_web/tests/job.py index f9e8cdfe..56701ddf 100644 --- a/ganeti_web/tests/job.py +++ b/ganeti_web/tests/job.py @@ -15,7 +15,6 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, # USA. -from django.contrib.auth.models import User from django.test import TestCase from django.test.client import Client @@ -216,7 +215,6 @@ class TestJobViews(TestJobMixin, TestCase, UserTestMixin, ViewTestMixin): c_error.info = JOB_ERROR c_error.save() c_error = Job.objects.get(pk=c_error.pk) - self.assertFalse(c_error.cleared) self.cluster.last_job = c_error self.cluster.ignore_cache = True self.cluster.save() @@ -225,7 +223,6 @@ class TestJobViews(TestJobMixin, TestCase, UserTestMixin, ViewTestMixin): vm_error.info = JOB_ERROR vm_error.save() vm_error = Job.objects.get(pk=vm_error.pk) - self.assertFalse(vm_error.cleared) self.vm.last_job = vm_error self.vm.ignore_cache = True self.vm.save() @@ -249,7 +246,6 @@ class TestJobViews(TestJobMixin, TestCase, UserTestMixin, ViewTestMixin): vm_error2.info = JOB_ERROR vm_error2.save() vm_error2 = Job.objects.get(pk=vm_error.pk) - self.assertFalse(vm_error.cleared) self.vm.last_job = vm_error self.vm.ignore_cache = True self.vm.save() @@ -259,7 +255,7 @@ class TestJobViews(TestJobMixin, TestCase, UserTestMixin, ViewTestMixin): self.assertEqual(vm_error.pk, updated['last_job_id']) self.assertTrue(updated['ignore_cache']) - def test_clear_job_authorized_cluster(self): + def test_clear_job_superuser(self): url = '/cluster/%s/job/%s/clear/' @@ -268,35 +264,24 @@ class TestJobViews(TestJobMixin, TestCase, UserTestMixin, ViewTestMixin): c_error.info = JOB_ERROR c_error.save() c_error = Job.objects.get(pk=c_error.pk) - self.assertFalse(c_error.cleared) self.cluster.last_job = c_error self.cluster.ignore_cache = True self.cluster.save() - vm_error = Job.objects.create(cluster=self.cluster, obj=self.vm, - job_id=2) - vm_error.info = JOB_ERROR - vm_error.save() - vm_error = Job.objects.get(pk=vm_error.pk) - self.assertFalse(vm_error.cleared) - self.vm.last_job = vm_error - self.vm.ignore_cache = True - self.vm.save() args = (self.cluster.slug, c_error.job_id) # authorized for cluster def tests(user, response): - error = Job.objects.get(pk=c_error.pk) - self.assertTrue(error.cleared) - Job.objects.all().update(cleared=False) + qs = Job.objects.filter(pk=c_error.pk) + self.assertFalse(qs) updated = Cluster.objects.filter(pk=self.cluster.pk).values('last_job_id','ignore_cache')[0] self.assertEqual(None, updated['last_job_id']) self.assertFalse(updated['ignore_cache']) - self.assert_200(url, args, users=[self.superuser, self.cluster_admin], + self.assert_200(url, args, users=[self.superuser], data={'id':c_error.pk}, tests=tests, method='post', mime='application/json') - def test_clear_job_authorized_vm(self): + def test_clear_job_authorized_cluster(self): url = '/cluster/%s/job/%s/clear/' @@ -305,34 +290,52 @@ class TestJobViews(TestJobMixin, TestCase, UserTestMixin, ViewTestMixin): c_error.info = JOB_ERROR c_error.save() c_error = Job.objects.get(pk=c_error.pk) - self.assertFalse(c_error.cleared) self.cluster.last_job = c_error self.cluster.ignore_cache = True self.cluster.save() - vm_error = Job.objects.create(cluster=self.cluster, obj=self.vm, - job_id=2) - vm_error.info = JOB_ERROR - vm_error.save() - vm_error = Job.objects.get(pk=vm_error.pk) - self.assertFalse(vm_error.cleared) - self.vm.last_job = vm_error - self.vm.ignore_cache = True - self.vm.save() - # authorized for vm - args = (self.cluster.slug, vm_error.job_id) + args = (self.cluster.slug, c_error.job_id) + + # authorized for cluster def tests(user, response): - error = Job.objects.get(pk=vm_error.pk) - self.assertTrue(error.cleared, 'error was not marked cleared') - Job.objects.all().update(cleared=False) - updated = VirtualMachine.objects.filter(pk=self.vm.pk).values('last_job_id','ignore_cache')[0] + qs = Job.objects.filter(pk=c_error.pk) + self.assertFalse(qs) + updated = Cluster.objects.filter(pk=self.cluster.pk).values('last_job_id','ignore_cache')[0] self.assertEqual(None, updated['last_job_id']) self.assertFalse(updated['ignore_cache']) - self.assert_200(url, args, users=[self.superuser, self.cluster_admin, - self.vm_admin, self.vm_owner], - data={'id':vm_error.id}, tests=tests, method='post', + self.assert_200(url, args, users=[self.cluster_admin], + data={'id':c_error.pk}, tests=tests, method='post', mime='application/json') + def test_clear_job_authorized_vm(self): + + url = '/cluster/%s/job/%s/clear/' + + # XXX ugh, sorry for this! + for user in [self.superuser, self.cluster_admin, self.vm_admin, + self.vm_owner]: + + vm_error = Job.objects.create(cluster=self.cluster, obj=self.vm, + job_id=2) + vm_error.info = JOB_ERROR + vm_error.save() + vm_error = Job.objects.get(pk=vm_error.pk) + self.vm.last_job = vm_error + self.vm.ignore_cache = True + self.vm.save() + + args = (self.cluster.slug, vm_error.job_id) + + def tests(user, response): + qs = Job.objects.filter(pk=vm_error.pk) + self.assertFalse(qs.exists(), "job error was not deleted") + updated = VirtualMachine.objects.filter(pk=self.vm.pk).values('last_job_id','ignore_cache')[0] + self.assertEqual(None, updated['last_job_id']) + self.assertFalse(updated['ignore_cache']) + self.assert_200(url, args, users=[user], data={'id':vm_error.id}, + tests=tests, method='post', + mime='application/json') + def test_job_detail(self): """ tests viewing job detail diff --git a/ganeti_web/tests/views/virtual_machine/edit.py b/ganeti_web/tests/views/virtual_machine/edit.py index 609b0497..af364acc 100644 --- a/ganeti_web/tests/views/virtual_machine/edit.py +++ b/ganeti_web/tests/views/virtual_machine/edit.py @@ -605,8 +605,7 @@ class TestVirtualMachineDeleteViews(TestVirtualMachineViewsBase): """ job = models.Job.objects.create(job_id=42, obj=self.vm, - cluster_id=self.vm.cluster_id, - cleared=False) + cluster_id=self.vm.cluster_id) self.assertTrue(self.c.login(username=self.superuser.username, password='secret')) @@ -623,8 +622,8 @@ class TestVirtualMachineDeleteViews(TestVirtualMachineViewsBase): self.assertTrue(job_id) # Refresh and make sure it's cleared. - job = models.Job.objects.get(job_id=job.job_id) - self.assertTrue(job.cleared) + qs = models.Job.objects.filter(job_id=job.job_id) + self.assertFalse(qs.exists()) def test_view_delete_post_cluster_admin(self): self.user.grant('admin', self.cluster) diff --git a/ganeti_web/views/cluster.py b/ganeti_web/views/cluster.py index d356a586..68c2830b 100644 --- a/ganeti_web/views/cluster.py +++ b/ganeti_web/views/cluster.py @@ -329,9 +329,11 @@ def job_status(request, id, rest=False): """ Return a list of basic info for running jobs. """ - q = Q(status__in=('running','waiting')) | Q(status='error', cleared=False) + ct = ContentType.objects.get_for_model(Cluster) - jobs = Job.objects.filter(q, content_type=ct, object_id=id).order_by('job_id') + jobs = Job.objects.filter(status__in=("error", "running", "waiting"), + content_type=ct, + object_id=id).order_by('job_id') jobs = [j.info for j in jobs] if rest: diff --git a/ganeti_web/views/general.py b/ganeti_web/views/general.py index 215e3ae5..5c2856a4 100644 --- a/ganeti_web/views/general.py +++ b/ganeti_web/views/general.py @@ -207,9 +207,9 @@ def overview(request, rest=False): # # XXX all jobs have the cluster listed, filtering by cluster includes jobs # for both the cluster itself and any of its VMs or Nodes - error_clause = Q(status='error', cleared=False) + error_clause = Q(status='error') vm_type = ContentType.objects.get_for_model(VirtualMachine) - select_clause = Q(content_type=vm_type, object_id__in=vms,) + select_clause = Q(content_type=vm_type, object_id__in=vms) if admin: select_clause |= Q(cluster__in=clusters) job_errors = Job.objects.filter(error_clause & select_clause) \ diff --git a/ganeti_web/views/jobs.py b/ganeti_web/views/jobs.py index 0ca56e20..58180006 100644 --- a/ganeti_web/views/jobs.py +++ b/ganeti_web/views/jobs.py @@ -56,9 +56,9 @@ def status(request, cluster_slug, job_id, rest=False): @login_required -def clear(request, cluster_slug, job_id, rest=False): +def clear(request, cluster_slug, job_id): """ - Clear a single failed job error message + Remove a job. """ user = request.user @@ -71,32 +71,23 @@ def clear(request, cluster_slug, job_id, rest=False): if not cluster_admin: if isinstance(obj, (Cluster, Node)): - if rest: - return HttpResponseForbidden - else: - raise Http403(NO_PRIVS) + raise Http403(NO_PRIVS) elif isinstance(obj, (VirtualMachine,)): # object is a virtual machine, check perms on VM and on Cluster - if not (obj.owner_id == user.get_profile().pk \ - or user.has_perm('admin', obj) \ + if not (obj.owner_id == user.get_profile().pk + or user.has_perm('admin', obj) or user.has_perm('admin', obj.cluster)): raise Http403(NO_PRIVS) - - # clear the error. - Job.objects.filter(pk=job.pk).update(cleared=True) - - # clear the job from the object, but only if it is the last job. It's - # possible another job was started after this job, and the error message - # just wasn't cleared. - # - # XXX object could be none, in which case we dont need to clear its last_job + # If the job points to an object, and the job is the most recent job on + # the object, then clear it from the object. if obj is not None: - ObjectModel = obj.__class__ - ObjectModel.objects.filter(pk=job.object_id, last_job=job) \ - .update(last_job=None, ignore_cache=False) + if obj.last_job == job: + obj.last_job = None + obj.ignore_cache = False + obj.save() - if rest: - return 1 - else: - return HttpResponse('1', mimetype='application/json') + # "Clear" the job. With extreme prejudice. + job.delete() + + return HttpResponse('1', mimetype='application/json') diff --git a/ganeti_web/views/node.py b/ganeti_web/views/node.py index 0315c9f4..bba8d30b 100644 --- a/ganeti_web/views/node.py +++ b/ganeti_web/views/node.py @@ -272,9 +272,10 @@ def job_status(request, id, rest=False): """ Return a list of basic info for running jobs. """ - q = Q(status__in=('running','waiting')) | Q(status='error', cleared=False) ct = ContentType.objects.get_for_model(Node) - jobs = Job.objects.filter(q, content_type=ct, object_id=id).order_by('job_id') + jobs = Job.objects.filter(status__in=("error", "running", "waiting"), + content_type=ct, + object_id=id).order_by('job_id') jobs = [j.info for j in jobs] if rest: diff --git a/ganeti_web/views/virtual_machine.py b/ganeti_web/views/virtual_machine.py index 04e81859..3004af84 100644 --- a/ganeti_web/views/virtual_machine.py +++ b/ganeti_web/views/virtual_machine.py @@ -200,8 +200,7 @@ class VMDeleteView(LoginRequiredMixin, DeleteView): # Clear any old jobs for this VM. ct = ContentType.objects.get_for_model(VirtualMachine) - Job.objects.filter(content_type=ct, - object_id=instance.id).update(cleared=True) + Job.objects.filter(content_type=ct, object_id=instance.id).delete() # Create the deletion job. job_id = instance.rapi.DeleteInstance(instance.hostname) @@ -1178,9 +1177,10 @@ def job_status(request, id, rest=False): """ Return a list of basic info for running jobs. """ - q = Q(status__in=('running','waiting')) | Q(status='error', cleared=False) ct = ContentType.objects.get_for_model(VirtualMachine) - jobs = Job.objects.filter(q, content_type=ct, object_id=id).order_by('job_id') + jobs = Job.objects.filter(status__in=("error", "running", "waiting"), + content_type=ct, + object_id=id).order_by('job_id') jobs = [j.info for j in jobs] if rest: -- 2.11.4.GIT