VMware: fix the VNC port allocation

There is small chance for VNC port collisions with the current
implementation which choose the port number based on the MoRef id
of the VM.
This patch fixes this by running a query for all allocated ports
and then selects one which is not taken.

Closes-Bug: #1255609
(cherry picked from commit 2f49ed4b5dbb5c954fc7a9b42ee7b170c38c775c)

Conflicts:

	nova/exception.py
	nova/tests/virt/vmwareapi/test_vmwareapi.py
	nova/tests/virt/vmwareapi/test_vmwareapi_vm_util.py
	nova/virt/vmwareapi/vm_util.py
	nova/virt/vmwareapi/vmops.py

Change-Id: If7c3b14dd49ed05c5fde819c5a36d5608650cbbc

5 files changed, 99 insertions, 14 deletions

diff --git a/nova/exception.py b/nova/exception.py
index b0b6909124..7c17803541 100644
--- a/nova/exception.py
+++ b/nova/exception.py
@@ -891,6 +891,11 @@ class InstanceTypeNotFoundByName(InstanceTypeNotFound):
                 "could not be found.")
 
 
+class ConsolePortRangeExhausted(NovaException):
+    msg_fmt = _("The console port range %(min_port)d-%(max_port)d is "
+                "exhausted.")
+
+
 class FlavorNotFound(NotFound):
     msg_fmt = _("Flavor %(flavor_id)s could not be found.")
 
diff --git a/nova/tests/virt/vmwareapi/test_vmwareapi.py b/nova/tests/virt/vmwareapi/test_vmwareapi.py
index f01439f45c..6950a681ee 100755
--- a/nova/tests/virt/vmwareapi/test_vmwareapi.py
+++ b/nova/tests/virt/vmwareapi/test_vmwareapi.py
@@ -21,6 +21,7 @@
 Test suite for VMwareAPI.
 """
 
+import collections
 import contextlib
 import copy
 
@@ -977,11 +978,12 @@ class VMwareAPIVMTestCase(test.NoDBTestCase):
     def _test_get_vnc_console(self):
         self._create_vm()
         fake_vm = vmwareapi_fake._get_objects("VirtualMachine").objects[0]
-        fake_vm_id = int(fake_vm.obj.value.replace('vm-', ''))
+        OptionValue = collections.namedtuple('OptionValue', ['key', 'value'])
+        opt_val = OptionValue(key='', value=5906)
+        fake_vm.set(vm_util.VNC_CONFIG_KEY, opt_val)
         vnc_dict = self.conn.get_vnc_console(self.instance)
-        self.assertEquals(vnc_dict['host'], self.vnc_host)
-        self.assertEquals(vnc_dict['port'], cfg.CONF.vmware.vnc_port +
-                          fake_vm_id % cfg.CONF.vmware.vnc_port_total)
+        self.assertEqual(vnc_dict['host'], self.vnc_host)
+        self.assertEqual(vnc_dict['port'], 5906)
 
     def test_get_vnc_console(self):
         self._test_get_vnc_console()
@@ -990,6 +992,13 @@ class VMwareAPIVMTestCase(test.NoDBTestCase):
         self.flags(vnc_password='vmware', group='vmware')
         self._test_get_vnc_console()
 
+    def test_get_vnc_console_noport(self):
+        self._create_vm()
+        fake_vm = vmwareapi_fake._get_objects("VirtualMachine").objects[0]
+        self.assertRaises(exception.ConsoleTypeUnavailable,
+                          self.conn.get_vnc_console,
+                          self.instance)
+
     def test_host_ip_addr(self):
         self.assertEquals(self.conn.get_host_ip_addr(), "test_url")
 
diff --git a/nova/tests/virt/vmwareapi/test_vmwareapi_vm_util.py b/nova/tests/virt/vmwareapi/test_vmwareapi_vm_util.py
index a8cdce7bde..ebd495011f 100755
--- a/nova/tests/virt/vmwareapi/test_vmwareapi_vm_util.py
+++ b/nova/tests/virt/vmwareapi/test_vmwareapi_vm_util.py
@@ -373,6 +373,31 @@ class VMwareVMUtilTestCase(test.NoDBTestCase):
         result = re.sub(r'\s+', '', repr(result))
         self.assertEqual(expected, result)
 
+    def _create_fake_vms(self):
+        fake_vms = fake.FakeRetrieveResult()
+        OptionValue = collections.namedtuple('OptionValue', ['key', 'value'])
+        for i in range(10):
+            vm = fake.ManagedObject()
+            opt_val = OptionValue(key='', value=5900 + i)
+            vm.set(vm_util.VNC_CONFIG_KEY, opt_val)
+            fake_vms.add_object(vm)
+        return fake_vms
+
+    def test_get_vnc_port(self):
+        fake_vms = self._create_fake_vms()
+        self.flags(vnc_port=5900, group='vmware')
+        self.flags(vnc_port_total=10000, group='vmware')
+        actual = vm_util.get_vnc_port(fake_session(fake_vms))
+        self.assertEqual(actual, 5910)
+
+    def test_get_vnc_port_exhausted(self):
+        fake_vms = self._create_fake_vms()
+        self.flags(vnc_port=5900, group='vmware')
+        self.flags(vnc_port_total=10, group='vmware')
+        self.assertRaises(exception.ConsolePortRangeExhausted,
+                          vm_util.get_vnc_port,
+                          fake_session(fake_vms))
+
     def test_get_all_cluster_refs_by_name_none(self):
         fake_objects = fake.FakeRetrieveResult()
         refs = vm_util.get_all_cluster_refs_by_name(fake_session(fake_objects),
diff --git a/nova/virt/vmwareapi/vm_util.py b/nova/virt/vmwareapi/vm_util.py
index 981dcdfecd..e7031f5c17 100755
--- a/nova/virt/vmwareapi/vm_util.py
+++ b/nova/virt/vmwareapi/vm_util.py
@@ -23,13 +23,20 @@ The VMware API VM utility module to build SOAP object specs.
 import collections
 import copy
 
+from oslo.config import cfg
+
 from nova import exception
 from nova.openstack.common.gettextutils import _
 from nova.openstack.common import log as logging
+from nova import utils
 from nova.virt.vmwareapi import vim_util
 
+CONF = cfg.CONF
 LOG = logging.getLogger(__name__)
 
+# the config key which stores the VNC port
+VNC_CONFIG_KEY = 'config.extraConfig["RemoteDisplay.vnc.port"]'
+
 
 def build_datastore_path(datastore_name, path):
     """Build the datastore compliant path."""
@@ -600,6 +607,45 @@ def get_vnc_config_spec(client_factory, port, password):
     return virtual_machine_config_spec
 
 
+@utils.synchronized('vmware.get_vnc_port')
+def get_vnc_port(session):
+    """Return VNC port for an VM or None if there is no available port."""
+    min_port = CONF.vmware.vnc_port
+    port_total = CONF.vmware.vnc_port_total
+    allocated_ports = _get_allocated_vnc_ports(session)
+    max_port = min_port + port_total
+    for port in range(min_port, max_port):
+        if port not in allocated_ports:
+            return port
+    raise exception.ConsolePortRangeExhausted(min_port=min_port,
+                                              max_port=max_port)
+
+
+def _get_allocated_vnc_ports(session):
+    """Return an integer set of all allocated VNC ports."""
+    # TODO(rgerganov): bug #1256944
+    # The VNC port should be unique per host, not per vCenter
+    vnc_ports = set()
+    result = session._call_method(vim_util, "get_objects",
+                                  "VirtualMachine", [VNC_CONFIG_KEY])
+    while result:
+        for obj in result.objects:
+            if not hasattr(obj, 'propSet'):
+                continue
+            dynamic_prop = obj.propSet[0]
+            option_value = dynamic_prop.val
+            vnc_port = option_value.value
+            vnc_ports.add(int(vnc_port))
+        token = _get_token(result)
+        if token:
+            result = session._call_method(vim_util,
+                                          "continue_to_get_objects",
+                                          token)
+        else:
+            break
+    return vnc_ports
+
+
 def search_datastore_spec(client_factory, file_name):
     """Builds the datastore search spec."""
     search_spec = client_factory.create('ns0:HostDatastoreBrowserSearchSpec')
diff --git a/nova/virt/vmwareapi/vmops.py b/nova/virt/vmwareapi/vmops.py
index 9489cd5a81..c13a0b301c 100755
--- a/nova/virt/vmwareapi/vmops.py
+++ b/nova/virt/vmwareapi/vmops.py
@@ -309,8 +309,8 @@ class VMwareVMOps(object):
 
         # Set the vnc configuration of the instance, vnc port starts from 5900
         if CONF.vnc_enabled:
-            vnc_port = self._get_vnc_port(vm_ref)
             vnc_pass = CONF.vmware.vnc_password or ''
+            vnc_port = vm_util.get_vnc_port(self._session)
             self._set_vnc_config(client_factory, instance, vnc_port, vnc_pass)
 
         def _create_virtual_disk():
@@ -1361,9 +1361,17 @@ class VMwareVMOps(object):
     def get_vnc_console(self, instance):
         """Return connection info for a vnc console."""
         vm_ref = vm_util.get_vm_ref(self._session, instance)
+        opt_value = self._session._call_method(vim_util,
+                               'get_dynamic_property',
+                               vm_ref, 'VirtualMachine',
+                               vm_util.VNC_CONFIG_KEY)
+        if opt_value:
+            port = int(opt_value.value)
+        else:
+            raise exception.ConsoleTypeUnavailable(console_type='vnc')
 
         return {'host': CONF.vmware.host_ip,
-                'port': self._get_vnc_port(vm_ref),
+                'port': port,
                 'internal_access_path': None}
 
     def get_vnc_console_vcenter(self, instance):
@@ -1387,14 +1395,6 @@ class VMwareVMOps(object):
         return vnc_console
 
     @staticmethod
-    def _get_vnc_port(vm_ref):
-        """Return VNC port for an VM."""
-        vm_id = int(vm_ref.value.replace('vm-', ''))
-        port = CONF.vmware.vnc_port + vm_id % CONF.vmware.vnc_port_total
-
-        return port
-
-    @staticmethod
     def _get_machine_id_str(network_info):
         machine_id_str = ''
         for vif in network_info: