Repository: zhmcclient/python-zhmcclient

Issue:
Starting a paused partition fails
### Actual behavior
This is the expected behavior by DPM (confirmed that) - but it's really strange. For nova-dpm we need ot fix it - the question is, if we fix it in zhmcclient or in nova-dpm.

- do an inband shutdown of a partition (e.g. via linux shell:  shutdown -P now)
- partition state switches "Paused"
- try to start that partition using "Daily - Start"

This results in the following error on the UI
"STS00005E
One or more targeted partitions are already started. Only partitions in the Stopped state can be started.
To restart partitions, first run the Stop task, and then try the operation again."


The cli does not care about this issue at all, it just leaves the partition state to paused:
$ zhmc partition list P000S67B
/+-------------------------------------------------------------+------------+
| name                                                        | status     |
|-------------------------------------------------------------+------------|
| andreas_cloud_init_tests                                    | paused     |

$ zhmc partition start P000S67B andreas_cloud_init_tests
|Partition andreas_cloud_init_tests has been started.


$ zhmc partition list P000S67B
/+-------------------------------------------------------------+------------+
| name                                                        | status     |
|-------------------------------------------------------------+------------|
| andreas_cloud_init_tests                                    | paused     |


The python binding seems to behave like the cli. It returns immediately even if wait_for_completion ==True. There's no indication that the operation failed.

### Expected behavior


### Execution environment

* zhmcclient version: 0.9.0
* Operating system (type+version):


Discussion:
coveralls: 
[![Coverage Status](https://coveralls.io/builds/10867587/badge)](https://coveralls.io/builds/10867587)

Coverage increased (+0.01%) to 88.898% when pulling **1bac9182e06c1152388d9c04aee56487528676ed on andy/fix-job-error** into **c6e1335c4e1b93540dae6494b98e82f1514f93b4 on master**.


Patch:
diff --git a/docs/changes.rst b/docs/changes.rst
index 3c7bbd6..3075dd5 100755
--- a/docs/changes.rst
+++ b/docs/changes.rst
@@ -48,6 +48,8 @@ Released: not yet
 * Improved robustness of timestats tests by measuring the actual sleep time
   instead of going by the requested sleep time.
   
+* Added support for 'error' field in 'job-results' (fixes issue #228).
+
 **Enhancements:**
 
 * Improved the mock support by adding the typical attributes of its superclass
diff --git a/zhmcclient/_session.py b/zhmcclient/_session.py
index 14b80fa..81ceb9b 100644
--- a/zhmcclient/_session.py
+++ b/zhmcclient/_session.py
@@ -824,7 +824,7 @@ class Session(object):
             elif result.status_code == 202:
                 result_object = _result_object(result)
                 job_uri = result_object['job-uri']
-                job = Job(self, job_uri)
+                job = Job(self, job_uri, 'POST', uri)
                 if wait_for_completion:
                     return job.wait_for_completion(operation_timeout)
                 else:
@@ -954,7 +954,7 @@ class Job(object):
     completion.
     """
 
-    def __init__(self, session, uri):
+    def __init__(self, session, uri, op_method, op_uri):
         """
         Parameters:
 
@@ -967,9 +967,25 @@ class Job(object):
             Must not be `None`.
 
             Example: ``"/api/jobs/{job-id}"``
+
+          op_method (:term:`string`):
+            Name of the HTTP method of the operation that is executing
+            asynchronously on the HMC.
+            Must not be `None`.
+
+            Example: ``"POST"``
+
+          op_uri (:term:`string`):
+            Canonical URI of the operation that is executing asynchronously on
+            the HMC.
+            Must not be `None`.
+
+            Example: ``"/api/partitions/{partition-id}/stop"``
         """
         self._session = session
         self._uri = uri
+        self._op_method = op_method
+        self._op_uri = op_uri
 
     @property
     def session(self):
@@ -987,6 +1003,26 @@ class Job(object):
         """
         return self._uri
 
+    @property
+    def op_method(self):
+        """
+        :term:`string`: Name of the HTTP method of the operation that is
+        executing asynchronously on the HMC.
+
+        Example: ``"POST"``
+        """
+        return self._op_method
+
+    @property
+    def op_uri(self):
+        """
+        :term:`string`: Canonical URI of the operation that is executing
+        asynchronously on the HMC.
+
+        Example: ``"/api/partitions/{partition-id}/stop"``
+        """
+        return self._op_uri
+
     @logged_api_call
     def check_for_completion(self):
         """
@@ -1027,25 +1063,33 @@ class Job(object):
         job_status = job_result_obj['status']
         if job_status == 'complete':
             self.session.delete(self.uri)
-            oper_status_code = job_result_obj['job-status-code']
-            if oper_status_code in (200, 201):
-                oper_result_obj = job_result_obj.get('job-results', None)
-            elif oper_status_code == 204:
+            op_status_code = job_result_obj['job-status-code']
+            if op_status_code in (200, 201):
+                op_result_obj = job_result_obj.get('job-results', None)
+            elif op_status_code == 204:
                 # No content
-                oper_result_obj = None
+                op_result_obj = None
             else:
                 error_result_obj = job_result_obj.get('job-results', None)
-                message = error_result_obj.get('message', None) \
-                    if error_result_obj else None
+                if not error_result_obj:
+                    message = None
+                elif 'message' in error_result_obj:
+                    message = error_result_obj['message']
+                elif 'error' in error_result_obj:
+                    message = error_result_obj['error']
+                else:
+                    message = None
                 error_obj = {
-                    'http-status': oper_status_code,
+                    'http-status': op_status_code,
                     'reason': job_result_obj['job-reason-code'],
                     'message': message,
+                    'request-method': self.op_method,
+                    'request-uri': self.op_uri,
                 }
                 raise HTTPError(error_obj)
         else:
-            oper_result_obj = None
-        return job_status, oper_result_obj
+            op_result_obj = None
+        return job_status, op_result_obj
 
     @logged_api_call
     def wait_for_completion(self, operation_timeout=None):
@@ -1098,13 +1142,13 @@ class Job(object):
             start_time = time.time()
 
         while True:
-            job_status, oper_result_obj = self.check_for_completion()
+            job_status, op_result_obj = self.check_for_completion()
 
             # We give completion of status priority over strictly achieving
             # the timeout, so we check status first. This may cause a longer
             # duration of the method than prescribed by the timeout.
             if job_status == 'complete':
-                return oper_result_obj
+                return op_result_obj
 
             if operation_timeout > 0:
                 current_time = time.time()


Test patch:
diff --git a/tests/unit/test_session.py b/tests/unit/test_session.py
index 2f5dba2..64102b4 100755
--- a/tests/unit/test_session.py
+++ b/tests/unit/test_session.py
@@ -291,27 +291,37 @@ class JobTests(unittest.TestCase):
         """Test initialization of Job object."""
         session = Session('fake-host', 'fake-user', 'fake-pw')
 
-        job = Job(session, self.job_uri)
+        # Jobs exist only for POST, but we want to test that the specified HTTP
+        # method comes back regardless:
+        op_method = 'GET'
+
+        op_uri = '/api/bla'
+
+        job = Job(session, self.job_uri, op_method, op_uri)
 
         self.assertEqual(job.uri, self.job_uri)
         self.assertEqual(job.session, session)
+        self.assertEqual(job.op_method, op_method)
+        self.assertEqual(job.op_uri, op_uri)
 
     def test_check_incomplete(self):
         """Test check_for_completion() with incomplete job."""
         with requests_mock.mock() as m:
             self.mock_server_1(m)
             session = Session('fake-host', 'fake-user', 'fake-pw')
-            job = Job(session, self.job_uri)
+            op_method = 'POST'
+            op_uri = '/api/foo'
+            job = Job(session, self.job_uri, op_method, op_uri)
             query_job_status_result = {
                 'status': 'running',
             }
             m.get(self.job_uri, json=query_job_status_result)
             m.delete(self.job_uri)
 
-            job_status, oper_result = job.check_for_completion()
+            job_status, op_result = job.check_for_completion()
 
             self.assertEqual(job_status, 'running')
-            self.assertIsNone(oper_result)
+            self.assertIsNone(op_result)
 
     def test_check_complete_success_noresult(self):
         """Test check_for_completion() with successful complete job without
@@ -319,7 +329,9 @@ class JobTests(unittest.TestCase):
         with requests_mock.mock() as m:
             self.mock_server_1(m)
             session = Session('fake-host', 'fake-user', 'fake-pw')
-            job = Job(session, self.job_uri)
+            op_method = 'POST'
+            op_uri = '/api/foo'
+            job = Job(session, self.job_uri, op_method, op_uri)
             query_job_status_result = {
                 'status': 'complete',
                 'job-status-code': 200,
@@ -329,10 +341,10 @@ class JobTests(unittest.TestCase):
             m.get(self.job_uri, json=query_job_status_result)
             m.delete(self.job_uri)
 
-            job_status, oper_result = job.check_for_completion()
+            job_status, op_result = job.check_for_completion()
 
             self.assertEqual(job_status, 'complete')
-            self.assertIsNone(oper_result)
+            self.assertIsNone(op_result)
 
     def test_check_complete_success_result(self):
         """Test check_for_completion() with successful complete job with a
@@ -340,48 +352,133 @@ class JobTests(unittest.TestCase):
         with requests_mock.mock() as m:
             self.mock_server_1(m)
             session = Session('fake-host', 'fake-user', 'fake-pw')
-            job = Job(session, self.job_uri)
-            exp_oper_result = {
+            op_method = 'POST'
+            op_uri = '/api/foo'
+            job = Job(session, self.job_uri, op_method, op_uri)
+            exp_op_result = {
                 'foo': 'bar',
             }
             query_job_status_result = {
                 'status': 'complete',
                 'job-status-code': 200,
                 # 'job-reason-code' omitted because HTTP status good
-                'job-results': exp_oper_result,
+                'job-results': exp_op_result,
             }
             m.get(self.job_uri, json=query_job_status_result)
             m.delete(self.job_uri)
 
-            job_status, oper_result = job.check_for_completion()
+            job_status, op_result = job.check_for_completion()
 
             self.assertEqual(job_status, 'complete')
-            self.assertEqual(oper_result, exp_oper_result)
+            self.assertEqual(op_result, exp_op_result)
 
-    def test_check_complete_error(self):
-        """Test check_for_completion() with complete job in error."""
+    def test_check_complete_error1(self):
+        """Test check_for_completion() with complete job in error (1)."""
         with requests_mock.mock() as m:
             self.mock_server_1(m)
             session = Session('fake-host', 'fake-user', 'fake-pw')
-            job = Job(session, self.job_uri)
-            error_result = {
-                'message': 'bla',
+            op_method = 'POST'
+            op_uri = '/api/foo'
+            job = Job(session, self.job_uri, op_method, op_uri)
+            query_job_status_result = {
+                'status': 'complete',
+                'job-status-code': 500,
+                'job-reason-code': 42,
+                # no 'job-results' field (it is not guaranteed to be there)
             }
+
+            m.get(self.job_uri, json=query_job_status_result)
+            m.delete(self.job_uri)
+
+            with self.assertRaises(HTTPError) as cm:
+                job_status, op_result = job.check_for_completion()
+
+            self.assertEqual(cm.exception.http_status, 500)
+            self.assertEqual(cm.exception.reason, 42)
+            self.assertEqual(cm.exception.message, None)
+
+    def test_check_complete_error2(self):
+        """Test check_for_completion() with complete job in error (2)."""
+        with requests_mock.mock() as m:
+            self.mock_server_1(m)
+            session = Session('fake-host', 'fake-user', 'fake-pw')
+            op_method = 'POST'
+            op_uri = '/api/foo'
+            job = Job(session, self.job_uri, op_method, op_uri)
             query_job_status_result = {
                 'status': 'complete',
                 'job-status-code': 500,
                 'job-reason-code': 42,
-                'job-results': error_result,
+                'job-results': {},  # it is not guaranteed to have any content
             }
+
+            m.get(self.job_uri, json=query_job_status_result)
+            m.delete(self.job_uri)
+
+            with self.assertRaises(HTTPError) as cm:
+                job_status, op_result = job.check_for_completion()
+
+            self.assertEqual(cm.exception.http_status, 500)
+            self.assertEqual(cm.exception.reason, 42)
+            self.assertEqual(cm.exception.message, None)
+
+    def test_check_complete_error3(self):
+        """Test check_for_completion() with complete job in error (3)."""
+        with requests_mock.mock() as m:
+            self.mock_server_1(m)
+            session = Session('fake-host', 'fake-user', 'fake-pw')
+            op_method = 'POST'
+            op_uri = '/api/foo'
+            job = Job(session, self.job_uri, op_method, op_uri)
+            query_job_status_result = {
+                'status': 'complete',
+                'job-status-code': 500,
+                'job-reason-code': 42,
+                'job-results': {
+                    # Content is not documented for the error case.
+                    # Some failures result in an 'error' field.
+                    'error': 'bla message',
+                },
+            }
+
+            m.get(self.job_uri, json=query_job_status_result)
+            m.delete(self.job_uri)
+
+            with self.assertRaises(HTTPError) as cm:
+                job_status, op_result = job.check_for_completion()
+
+            self.assertEqual(cm.exception.http_status, 500)
+            self.assertEqual(cm.exception.reason, 42)
+            self.assertEqual(cm.exception.message, 'bla message')
+
+    def test_check_complete_error4(self):
+        """Test check_for_completion() with complete job in error (4)."""
+        with requests_mock.mock() as m:
+            self.mock_server_1(m)
+            session = Session('fake-host', 'fake-user', 'fake-pw')
+            op_method = 'POST'
+            op_uri = '/api/foo'
+            job = Job(session, self.job_uri, op_method, op_uri)
+            query_job_status_result = {
+                'status': 'complete',
+                'job-status-code': 500,
+                'job-reason-code': 42,
+                'job-results': {
+                    # Content is not documented for the error case.
+                    # Some failures result in an 'message' field.
+                    'message': 'bla message',
+                },
+            }
+
             m.get(self.job_uri, json=query_job_status_result)
             m.delete(self.job_uri)
 
             with self.assertRaises(HTTPError) as cm:
-                job_status, oper_result = job.check_for_completion()
+                job_status, op_result = job.check_for_completion()
 
             self.assertEqual(cm.exception.http_status, 500)
             self.assertEqual(cm.exception.reason, 42)
-            self.assertEqual(cm.exception.message, 'bla')
+            self.assertEqual(cm.exception.message, 'bla message')
 
     def test_wait_complete1_success_result(self):
         """Test wait_for_completion() with successful complete job with a
@@ -389,22 +486,24 @@ class JobTests(unittest.TestCase):
         with requests_mock.mock() as m:
             self.mock_server_1(m)
             session = Session('fake-host', 'fake-user', 'fake-pw')
-            job = Job(session, self.job_uri)
-            exp_oper_result = {
+            op_method = 'POST'
+            op_uri = '/api/foo'
+            job = Job(session, self.job_uri, op_method, op_uri)
+            exp_op_result = {
                 'foo': 'bar',
             }
             query_job_status_result = {
                 'status': 'complete',
                 'job-status-code': 200,
                 # 'job-reason-code' omitted because HTTP status good
-                'job-results': exp_oper_result,
+                'job-results': exp_op_result,
             }
             m.get(self.job_uri, json=query_job_status_result)
             m.delete(self.job_uri)
 
-            oper_result = job.wait_for_completion()
+            op_result = job.wait_for_completion()
 
-            self.assertEqual(oper_result, exp_oper_result)
+            self.assertEqual(op_result, exp_op_result)
 
     def test_wait_complete3_success_result(self):
         """Test wait_for_completion() with successful complete job with a
@@ -412,8 +511,10 @@ class JobTests(unittest.TestCase):
         with requests_mock.mock() as m:
             self.mock_server_1(m)
             session = Session('fake-host', 'fake-user', 'fake-pw')
-            job = Job(session, self.job_uri)
-            exp_oper_result = {
+            op_method = 'POST'
+            op_uri = '/api/foo'
+            job = Job(session, self.job_uri, op_method, op_uri)
+            exp_op_result = {
                 'foo': 'bar',
             }
             m.get(self.job_uri,
@@ -423,16 +524,18 @@ class JobTests(unittest.TestCase):
                   ])
             m.delete(self.job_uri)
 
-            oper_result = job.wait_for_completion()
+            op_result = job.wait_for_completion()
 
-            self.assertEqual(oper_result, exp_oper_result)
+            self.assertEqual(op_result, exp_op_result)
 
     def test_wait_complete3_timeout(self):
         """Test wait_for_completion() with timeout."""
         with requests_mock.mock() as m:
             self.mock_server_1(m)
             session = Session('fake-host', 'fake-user', 'fake-pw')
-            job = Job(session, self.job_uri)
+            op_method = 'POST'
+            op_uri = '/api/foo'
+            job = Job(session, self.job_uri, op_method, op_uri)
             m.get(self.job_uri,
                   [
                       {'text': result_running_callback},
@@ -475,14 +578,14 @@ def result_running_callback(request, context):
 
 
 def result_complete_callback(request, context):
-    exp_oper_result = {
+    exp_op_result = {
         'foo': 'bar',
     }
     job_result_complete = {
         'status': 'complete',
         'job-status-code': 200,
         # 'job-reason-code' omitted because HTTP status good
-        'job-results': exp_oper_result,
+        'job-results': exp_op_result,
     }
     time.sleep(1)
     return json.dumps(job_result_complete)