RAPI client: Tidy and test WaitForJobCompletion

- Use constants
- Don't sleep if no delay is given
- Mark function as deprecated: it uses polling instead of waiting for changes
  (but the latter needs authentication); it can still be used
- Add unittests

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

lib/rapi/client.py

@@ -1184,32 +1184,38 @@ class GanetiRapiClient(object): # pylint: disable-msg=R0904
   def WaitForJobCompletion(self, job_id, period=5, retries=-1):
     """Polls cluster for job status until completion.
 
-    Completion is defined as any of the following states: "error",
-    "canceled", or "success".
+    Completion is defined as any of the following states listed in
+    L{JOB_STATUS_FINALIZED}.
 
     @type job_id: string
     @param job_id: job id to watch
-
     @type period: int
     @param period: how often to poll for status (optional, default 5s)
-
     @type retries: int
     @param retries: how many time to poll before giving up
                     (optional, default -1 means unlimited)
 
     @rtype: bool
-    @return: True if job succeeded or False if failed/status timeout
+    @return: C{True} if job succeeded or C{False} if failed/status timeout
+    @deprecated: It is recommended to use L{WaitForJobChange} wherever
+      possible; L{WaitForJobChange} returns immediately after a job changed and
+      does not use polling
 
     """
     while retries != 0:
       job_result = self.GetJobStatus(job_id)
-      if not job_result or job_result["status"] in ("error", "canceled"):
-        return False
-      if job_result["status"] == "success":
+
+      if job_result and job_result["status"] == JOB_STATUS_SUCCESS:
         return True
-      time.sleep(period)
+      elif not job_result or job_result["status"] in JOB_STATUS_FINALIZED:
+        return False
+
+      if period:
+        time.sleep(period)
+
       if retries > 0:
         retries -= 1
+
     return False
 
   def WaitForJobChange(self, job_id, fields, prev_job_info, prev_log_serial):

test/ganeti.rapi.client_unittest.py

@@ -103,6 +103,9 @@ class RapiMock(object):
     self._last_handler = None
     self._last_req_data = None
 
+  def ResetResponses(self):
+    del self._responses[:]
+
   def AddResponse(self, response, code=200):
     self._responses.insert(0, (code, response))
 
@@ -1213,6 +1216,79 @@ class GanetiRapiClientTests(testutils.GanetiTestCase):
 
         self.assertEqual(self.rapi.CountPending(), 0)
 
+  def testWaitForJobCompletionNoChange(self):
+    resp = serializer.DumpJson({
+      "status": constants.JOB_STATUS_WAITLOCK,
+      })
+
+    for retries in [1, 5, 25]:
+      for _ in range(retries):
+        self.rapi.AddResponse(resp)
+
+      self.assertFalse(self.client.WaitForJobCompletion(22789, period=None,
+                                                        retries=retries))
+      self.assertHandler(rlib2.R_2_jobs_id)
+      self.assertItems(["22789"])
+
+      self.assertEqual(self.rapi.CountPending(), 0)
+
+  def testWaitForJobCompletionAlreadyFinished(self):
+    self.rapi.AddResponse(serializer.DumpJson({
+      "status": constants.JOB_STATUS_SUCCESS,
+      }))
+
+    self.assertTrue(self.client.WaitForJobCompletion(22793, period=None,
+                                                     retries=1))
+    self.assertHandler(rlib2.R_2_jobs_id)
+    self.assertItems(["22793"])
+
+    self.assertEqual(self.rapi.CountPending(), 0)
+
+  def testWaitForJobCompletionEmptyResponse(self):
+    self.rapi.AddResponse("{}")
+    self.assertFalse(self.client.WaitForJobCompletion(22793, period=None,
+                                                     retries=10))
+    self.assertHandler(rlib2.R_2_jobs_id)
+    self.assertItems(["22793"])
+
+    self.assertEqual(self.rapi.CountPending(), 0)
+
+  def testWaitForJobCompletionOutOfRetries(self):
+    for retries in [3, 10, 21]:
+      for _ in range(retries):
+        self.rapi.AddResponse(serializer.DumpJson({
+          "status": constants.JOB_STATUS_RUNNING,
+          }))
+
+      self.assertFalse(self.client.WaitForJobCompletion(30948, period=None,
+                                                        retries=retries - 1))
+      self.assertHandler(rlib2.R_2_jobs_id)
+      self.assertItems(["30948"])
+
+      self.assertEqual(self.rapi.CountPending(), 1)
+      self.rapi.ResetResponses()
+
+  def testWaitForJobCompletionSuccessAndFailure(self):
+    for retries in [1, 4, 13]:
+      for (success, end_status) in [(False, constants.JOB_STATUS_ERROR),
+                                    (True, constants.JOB_STATUS_SUCCESS)]:
+        for _ in range(retries):
+          self.rapi.AddResponse(serializer.DumpJson({
+            "status": constants.JOB_STATUS_RUNNING,
+            }))
+
+        self.rapi.AddResponse(serializer.DumpJson({
+          "status": end_status,
+          }))
+
+        result = self.client.WaitForJobCompletion(3187, period=None,
+                                                  retries=retries + 1)
+        self.assertEqual(result, success)
+        self.assertHandler(rlib2.R_2_jobs_id)
+        self.assertItems(["3187"])
+
+        self.assertEqual(self.rapi.CountPending(), 0)
+
 
 class RapiTestRunner(unittest.TextTestRunner):
   def run(self, *args):