Fix setting the SOAP headers for remote calls.2.17.0

We use SOAP headers to set 'opID' for all service calls and
'vcSessionCookie' for PBM service calls. Commit 6a6336 introduced
regression in setting the vcSessionCookie. This patch fixes this by
setting all headers in one place.

Change-Id: If9630a14d6275f30275825f1822f2cf9af2959a7

3 files changed, 25 insertions, 18 deletions

diff --git a/oslo_vmware/pbm.py b/oslo_vmware/pbm.py
index df5b33f..007d81f 100644
--- a/oslo_vmware/pbm.py
+++ b/oslo_vmware/pbm.py
@@ -25,7 +25,6 @@ import os
 
 import six.moves.urllib.parse as urlparse
 import six.moves.urllib.request as urllib
-import suds.sax.element as element
 
 from oslo_vmware._i18n import _LW
 from oslo_vmware import service
@@ -70,8 +69,7 @@ class Pbm(service.Service):
 
         :param cookie: cookie to set
         """
-        elem = element.Element('vcSessionCookie').setText(cookie)
-        self.client.set_options(soapheaders=elem)
+        self._vc_session_cookie = cookie
 
     def retrieve_service_content(self):
         ref = vim_util.get_moref(service.SERVICE_INSTANCE, SERVICE_TYPE)
diff --git a/oslo_vmware/service.py b/oslo_vmware/service.py
index 0a67cad..b242ccb 100644
--- a/oslo_vmware/service.py
+++ b/oslo_vmware/service.py
@@ -213,6 +213,7 @@ class Service(object):
                                     plugins=[ServiceMessagePlugin()],
                                     cache=_CACHE)
         self._service_content = None
+        self._vc_session_cookie = None
 
     @staticmethod
     def build_base_url(protocol, host, port):
@@ -264,16 +265,24 @@ class Service(object):
                                                fault_string,
                                                details=details)
 
-    def _add_operation_id(self, op_id):
-        """Add operation ID for the next remote call to vCenter.
+    def _set_soap_headers(self, op_id):
+        """Set SOAP headers for the next remote call to vCenter.
 
+        SOAP headers may include operation ID and vcSessionCookie.
         The operation ID is a random string which allows to correlate log
         messages across different systems (OpenStack, vCenter, ESX).
+        vcSessionCookie is needed when making PBM calls.
         """
-        headers = [element.Element('operationID').setText(op_id)]
-        if self.client.options.soapheaders is not None:
-            headers.append(self.client.options.soapheaders)
-        self.client.set_options(soapheaders=headers)
+        headers = []
+        if self._vc_session_cookie:
+            elem = element.Element('vcSessionCookie').setText(
+                self._vc_session_cookie)
+            headers.append(elem)
+        if op_id:
+            elem = element.Element('operationID').setText(op_id)
+            headers.append(elem)
+        if headers:
+            self.client.set_options(soapheaders=headers)
 
     @property
     def service_content(self):
@@ -314,6 +323,7 @@ class Service(object):
                     return
 
                 skip_op_id = kwargs.pop('skip_op_id', False)
+                op_id = None
                 if not skip_op_id:
                     # Generate opID. It will appear in vCenter and ESX logs for
                     # this particular remote call.
@@ -323,7 +333,7 @@ class Service(object):
                               managed_object._type,
                               attr_name,
                               op_id)
-                    self._add_operation_id(op_id)
+                self._set_soap_headers(op_id)
                 request = getattr(self.client.service, attr_name)
                 response = request(managed_object, **kwargs)
                 if (attr_name.lower() == 'retrievepropertiesex'):
diff --git a/oslo_vmware/tests/test_service.py b/oslo_vmware/tests/test_service.py
index 1accb92..27168bf 100644
--- a/oslo_vmware/tests/test_service.py
+++ b/oslo_vmware/tests/test_service.py
@@ -374,7 +374,7 @@ class ServiceTest(base.TestCase):
         svc_obj.client.options.transport.cookiejar = [cookie]
         self.assertIsNone(svc_obj.get_http_cookie())
 
-    def test_add_operation_id(self):
+    def test_set_soap_headers(self):
         def fake_set_options(*args, **kwargs):
             headers = kwargs['soapheaders']
             self.assertEqual(1, len(headers))
@@ -384,20 +384,19 @@ class ServiceTest(base.TestCase):
         svc_obj = service.Service()
         svc_obj.client.options.soapheaders = None
         setattr(svc_obj.client, 'set_options', fake_set_options)
-        svc_obj._add_operation_id('fira-12345')
+        svc_obj._set_soap_headers('fira-12345')
 
-    def test_add_operation_id_with_existing_header(self):
+    def test_soap_headers_pbm(self):
         def fake_set_options(*args, **kwargs):
             headers = kwargs['soapheaders']
             self.assertEqual(2, len(headers))
-            txt = headers[0].getText()
-            self.assertEqual('fira-12345', txt)
-            self.assertEqual('vc-session-cookie', headers[1])
+            self.assertEqual('vc-session-cookie', headers[0].getText())
+            self.assertEqual('fira-12345', headers[1].getText())
 
         svc_obj = service.Service()
-        svc_obj.client.options.soapheaders = 'vc-session-cookie'
+        svc_obj._vc_session_cookie = 'vc-session-cookie'
         setattr(svc_obj.client, 'set_options', fake_set_options)
-        svc_obj._add_operation_id('fira-12345')
+        svc_obj._set_soap_headers('fira-12345')
 
 
 class MemoryCacheTest(base.TestCase):