Adam Litke has uploaded a new change for review.
Change subject: jobs: Allow run and abort only from valid states
......................................................................
jobs: Allow run and abort only from valid states
Before this patch we allow abort and run from any state even when it
does not make sense. Abort is valid only for pending or running jobs
and run is valid only for pending jobs. When an invalid call is
attempted raise a new error.
Change-Id: Ia8bc1ab264c5786b43fb409c3fab8913337778bf
Signed-off-by: Adam Litke <ali...@redhat.com>
---
M lib/vdsm/define.py
M lib/vdsm/exception.py
M lib/vdsm/jobs.py
M tests/jobsTests.py
4 files changed, 55 insertions(+), 4 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/11/63711/1
diff --git a/lib/vdsm/define.py b/lib/vdsm/define.py
index d75e919..3edb881 100644
--- a/lib/vdsm/define.py
+++ b/lib/vdsm/define.py
@@ -88,6 +88,8 @@
'migrateLimit': exception.MigrationLimitExceeded().response(),
'recovery': exception.RecoveryInProgress().response(),
'hostdevDetachErr': exception.HostdevDetachFailed().response(),
+ 'CannotAbortJob': exception.CannotAbortJob().response(),
+ 'CannotRunJob': exception.CannotRunJob().response(),
}
diff --git a/lib/vdsm/exception.py b/lib/vdsm/exception.py
index 40d13f5..671067a 100644
--- a/lib/vdsm/exception.py
+++ b/lib/vdsm/exception.py
@@ -347,6 +347,16 @@
message = 'Could not detach host device'
+class CannotAbortJob(VdsmException):
+ code = 84
+ message = 'Job cannot be aborted from this state'
+
+
+class CannotRunJob(VdsmException):
+ code = 85
+ message = 'Job cannot be run from this state'
+
+
class RecoveryInProgress(VdsmException):
code = 99
message = 'Recovering from crash or Initializing'
diff --git a/lib/vdsm/jobs.py b/lib/vdsm/jobs.py
index fb81bdd..ba71008 100644
--- a/lib/vdsm/jobs.py
+++ b/lib/vdsm/jobs.py
@@ -60,6 +60,21 @@
name = 'JobNotDone'
+class CannotAbortJob(ClientError):
+ ''' Job is not running or pending '''
+ name = 'CannotAbortJob'
+
+
+class CannotRunJob(ClientError):
+ ''' Only pending jobs may be run '''
+ name = 'CannotRunJob'
+
+
+class JobNotRunnable(ClientError):
+ ''' Job cannot be run from its current state '''
+ name = 'JobNotRunnable'
+
+
class AbortNotSupported(ClientError):
''' This type of job does not support aborting '''
name = 'AbortNotSupported'
@@ -121,7 +136,8 @@
return self.status in (STATUS.PENDING, STATUS.RUNNING)
def abort(self):
- # TODO: Don't abort if not pending and not running
+ if not self.active:
+ raise CannotAbortJob()
self._status = STATUS.ABORTED
logging.info('Job %r aborting...', self._id)
self._abort()
@@ -129,7 +145,8 @@
self._autodelete()
def run(self):
- # TODO: Don't run if aborted or not pending
+ if self.status != STATUS.PENDING:
+ raise CannotRunJob()
self._status = STATUS.RUNNING
try:
self._run()
diff --git a/tests/jobsTests.py b/tests/jobsTests.py
index 2cb578c..1f0aeed 100644
--- a/tests/jobsTests.py
+++ b/tests/jobsTests.py
@@ -173,12 +173,24 @@
self.assertEqual({}, jobs.info(job_type=bar.job_type,
job_ids=[foo.id]))
- def test_abort_job(self):
- job = TestingJob()
+ @permutations([[jobs.STATUS.PENDING], [jobs.STATUS.RUNNING]])
+ def test_abort_job(self, status):
+ job = TestingJob(status)
jobs.add(job)
jobs.abort(job.id)
self.assertEqual(jobs.STATUS.ABORTED, job.status)
self.assertTrue(job._aborted)
+
+ @permutations([
+ [jobs.STATUS.ABORTED, jobs.CannotAbortJob.name],
+ [jobs.STATUS.DONE, jobs.CannotAbortJob.name],
+ [jobs.STATUS.FAILED, jobs.CannotAbortJob.name]
+ ])
+ def test_abort_from_invalid_state(self, status, err):
+ job = TestingJob(status)
+ jobs.add(job)
+ res = jobs.abort(job.id)
+ self.assertEqual(response.error(err), res)
def test_abort_unknown_job(self):
self.assertEqual(response.error(jobs.NoSuchJob.name),
@@ -265,6 +277,16 @@
self.run_job(job)
self.assertEqual(jobs.STATUS.DONE, job.status)
+ @permutations([
+ [jobs.STATUS.RUNNING],
+ [jobs.STATUS.ABORTED],
+ [jobs.STATUS.DONE],
+ [jobs.STATUS.FAILED]
+ ])
+ def test_run_from_invalid_state(self, state):
+ job = TestingJob(state)
+ self.assertRaises(jobs.CannotRunJob, job.run)
+
def test_default_exception(self):
message = "testing failure"
job = TestingJob(exception=Exception(message))
--
To view, visit https://gerrit.ovirt.org/63711
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia8bc1ab264c5786b43fb409c3fab8913337778bf
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <ali...@redhat.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches@lists.fedorahosted.org
https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
