From 3c631ea2e9a5c6f7b6e196d3acf6b13a34f413e8 Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <hansmi@google.com>
Date: Wed, 5 Dec 2012 13:42:45 +0100
Subject: [PATCH] jqueue: Don't modify input opcode when changing priority
Commit 4679547 implemented the ability to change job's priority after it
was submitted. The code contained a bug whereby it would modify the
input data for an opcode, something the job queue shouldn't do (logical
units do for historical reasons).
This patch removes the line modifying the opcode input and adjusts the
tests.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
---
lib/jqueue.py | 4 +---
test/ganeti.jqueue_unittest.py | 15 ++++++++++-----
2 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/lib/jqueue.py b/lib/jqueue.py
index 5e639950e..5bde17d28 100644
--- a/lib/jqueue.py
+++ b/lib/jqueue.py
@@ -517,9 +517,7 @@ class _QueuedJob(object):
changed = True
- # Note: this also changes the on-disk priority ("op.priority" is only in
- # memory)
- op.input.priority = priority
+ # Set new priority (doesn't modify opcode input)
op.priority = priority
if changed:
diff --git a/test/ganeti.jqueue_unittest.py b/test/ganeti.jqueue_unittest.py
index 34cb71b89..7b7567e54 100755
--- a/test/ganeti.jqueue_unittest.py
+++ b/test/ganeti.jqueue_unittest.py
@@ -398,7 +398,8 @@ class TestQueuedJob(unittest.TestCase):
result = job.ChangePriority(-10)
self.assertEqual(job.CalcPriority(), -10)
self.assertTrue(compat.all(op.priority == -10 for op in job.ops))
- self.assertTrue(compat.all(op.input.priority == -10 for op in job.ops))
+ self.assertFalse(compat.any(hasattr(op.input, "priority")
+ for op in job.ops))
self.assertEqual(result,
(True, ("Priorities of pending opcodes for job 24984 have"
" been changed to -10")))
@@ -468,8 +469,8 @@ class TestQueuedJob(unittest.TestCase):
self.assertEqual(job.CalcPriority(), constants.OP_PRIO_DEFAULT)
self.assertEqual(map(operator.attrgetter("priority"), job.ops),
[constants.OP_PRIO_DEFAULT, 7, 7, 7])
- self.assertEqual([getattr(op.input, "priority", None) for op in job.ops],
- [None, 7, 7, 7])
+ self.assertFalse(compat.any(hasattr(op.input, "priority")
+ for op in job.ops))
self.assertEqual(map(operator.attrgetter("status"), job.ops), [
constants.OP_STATUS_RUNNING,
constants.OP_STATUS_QUEUED,
@@ -520,8 +521,8 @@ class TestQueuedJob(unittest.TestCase):
self.assertEqual(map(operator.attrgetter("priority"), job.ops),
[constants.OP_PRIO_DEFAULT, constants.OP_PRIO_DEFAULT,
-19, -19])
- self.assertEqual([getattr(op.input, "priority", None) for op in job.ops],
- [None, None, -19, -19])
+ self.assertFalse(compat.any(hasattr(op.input, "priority")
+ for op in job.ops))
self.assertEqual(map(operator.attrgetter("status"), job.ops), [
constants.OP_STATUS_SUCCESS,
constants.OP_STATUS_RUNNING,
@@ -2221,6 +2222,10 @@ class TestJobProcessorTimeouts(unittest.TestCase, _JobProcessorTestUtils):
result = proc(_nextop_fn=self._NextOpcode)
assert self.curop is not None
+ # Input priority should never be set or modified
+ self.assertFalse(compat.any(hasattr(op.input, "priority")
+ for op in job.ops))
+
if result == jqueue._JobProcessor.FINISHED or self.gave_lock:
# Got lock and/or job is done, result must've been written
self.assertFalse(job.cur_opctx)
--
GitLab