Proxies don't handle reusing the SmartConnect instances very well. D…

salt/utils/vmware.py

@@ -87,7 +87,7 @@
 
 # Import Third Party Libs
 try:
-    from pyVim.connect import GetSi, SmartConnect, Disconnect
+    from pyVim.connect import GetSi, SmartConnect, Disconnect, GetStub
     from pyVmomi import vim, vmodl
     HAS_PYVMOMI = True
 except ImportError:
@@ -207,9 +207,17 @@ def _get_service_instance(host, username, password, protocol,
             port=port,
             b64token=token,
             mechanism=mechanism)
+    except TypeError as exc:
+        if 'unexpected keyword argument' in exc.message:
+            log.error('Initial connect to the VMware endpoint failed with {0}'.format(exc.message))
+            log.error('This may mean that a version of PyVmomi EARLIER than 6.0.0.2016.6 is installed.')
+            log.error('We recommend updating to that version or later.')
+            raise
     except Exception as exc:
+
         default_msg = 'Could not connect to host \'{0}\'. ' \
                       'Please check the debug log for more information.'.format(host)
+
         try:
             if (isinstance(exc, vim.fault.HostConnectFault) and
                 '[SSL: CERTIFICATE_VERIFY_FAILED]' in exc.msg) or \
@@ -302,14 +310,16 @@ def get_service_instance(host, username=None, password=None, protocol=None,
 
     service_instance = GetSi()
     if service_instance:
-        if service_instance._GetStub().host == ':'.join([host, str(port)]):
-            log.trace('Using cached service instance')
-            if salt.utils.is_proxy():
-                service_instance._GetStub().GetConnection()
-        else:
-            # Invalidate service instance
+        stub = GetStub()
+        if salt.utils.is_proxy() or (hasattr(stub, 'host') and stub.host != ':'.join([host, str(port)])):
+            # Proxies will fork and mess up the cached service instance.
+            # If this is a proxy or we are connecting to a different host
+            # invalidate the service instance to avoid a potential memory leak
+            # and reconnect
             Disconnect(service_instance)
             service_instance = None
+        else:
+            return service_instance
 
     if not service_instance:
         service_instance = _get_service_instance(host,

tests/unit/utils/vmware_test/connection_test.py

@@ -574,16 +574,10 @@ def test_default_params(self):
                                                 None)
 
     @patch('salt.utils.is_proxy', MagicMock(return_value=True))
-    def test_cached_service_instace_same_host_on_proxy(self):
-        mock_si = MagicMock()
-        mock_si_stub_value = MagicMock()
-        mock_si_get_connection = MagicMock()
-        mock_si_stub = MagicMock(return_value=mock_si_stub_value)
-        mock_get_si = MagicMock(return_value=mock_si)
-        mock_si._GetStub = mock_si_stub
-        mock_si_stub_value.host = 'fake_host:1'
-        mock_si_stub_value.GetConnection = mock_si_get_connection
-        with patch('salt.utils.vmware.GetSi', mock_get_si):
+    def test_no_cached_service_instance_same_host_on_proxy(self):
+        # Service instance is uncached when using class default mock objs
+        mock_get_si = MagicMock()
+        with patch('salt.utils.vmware._get_service_instance', mock_get_si):
             salt.utils.vmware.get_service_instance(
                 host='fake_host',
                 username='fake_username',
@@ -594,30 +588,36 @@ def test_cached_service_instace_same_host_on_proxy(self):
                 principal='fake_principal',
                 domain='fake_domain'
             )
-            self.assertEqual(mock_get_si.call_count, 1)
-            self.assertEqual(mock_si_stub.call_count, 2)
-            self.assertEqual(mock_si_get_connection.call_count, 1)
+            mock_get_si.assert_called_once_with('fake_host',
+                                                'fake_username',
+                                                'fake_password',
+                                                'fake_protocol',
+                                                1,
+                                                'fake_mechanism',
+                                                'fake_principal',
+                                                'fake_domain')
 
-    def test_cached_service_instace_dffierent_host(self):
+    def test_cached_service_instance_different_host(self):
         mock_si = MagicMock()
         mock_si_stub = MagicMock()
         mock_disconnect = MagicMock()
         mock_get_si = MagicMock(return_value=mock_si)
-        mock_si._GetStub = mock_si_stub
+        mock_getstub = MagicMock()
         with patch('salt.utils.vmware.GetSi', mock_get_si):
-            with patch('salt.utils.vmware.Disconnect', mock_disconnect):
-                salt.utils.vmware.get_service_instance(
-                    host='fake_host',
-                    username='fake_username',
-                    password='fake_password',
-                    protocol='fake_protocol',
-                    port=1,
-                    mechanism='fake_mechanism',
-                    principal='fake_principal',
-                    domain='fake_domain'
-                )
+            with patch('salt.utils.vmware.GetStub', mock_getstub):
+                with patch('salt.utils.vmware.Disconnect', mock_disconnect):
+                    salt.utils.vmware.get_service_instance(
+                        host='fake_host',
+                        username='fake_username',
+                        password='fake_password',
+                        protocol='fake_protocol',
+                        port=1,
+                        mechanism='fake_mechanism',
+                        principal='fake_principal',
+                        domain='fake_domain'
+                    )
             self.assertEqual(mock_get_si.call_count, 1)
-            self.assertEqual(mock_si_stub.call_count, 1)
+            self.assertEqual(mock_getstub.call_count, 1)
             self.assertEqual(mock_disconnect.call_count, 1)
 
     def test_uncached_service_instance(self):