Add time.sleep call before set VLAN call for port in vCenter to avoid…

… race in vmware code

Add new handler for VmRegisteredEvent
Refactored VLAN ID assignment logic

Partial-Bug: #1792538
Change-Id: I28bc48c0cf3622cfb3130671c33adfa2d442317b
Depends-on: Ic332e37c3de9462c32dac81bc370b313460935be
(cherry picked from commit 44ca1b6)

cvm/__main__.py

@@ -21,7 +21,7 @@
                          VRouterAPIClient)
 from cvm.controllers import (GuestNetHandler, PowerStateHandler, UpdateHandler,
                              VmReconfiguredHandler, VmRemovedHandler,
-                             VmRenamedHandler, VmUpdatedHandler,
+                             VmRenamedHandler, VmUpdatedHandler, VmRegisteredHandler,
                              VmwareController, VmwareToolsStatusHandler)
 from cvm.database import Database
 from cvm.models import VlanIdPool
@@ -79,6 +79,7 @@ def build_monitor(config, lock, database):
     vm_reconfigured_handler = VmReconfiguredHandler(vm_service, vn_service,
                                                     vmi_service, vrouter_port_service)
     vm_removed_handler = VmRemovedHandler(vm_service, vmi_service, vrouter_port_service)
+    vm_registered_handler = VmRegisteredHandler(vm_service, vn_service, vmi_service, vrouter_port_service)
     guest_net_handler = GuestNetHandler(vmi_service, vrouter_port_service)
     vmware_tools_status_handler = VmwareToolsStatusHandler(vm_service)
     power_state_handler = PowerStateHandler(vm_service, vrouter_port_service)
@@ -87,6 +88,7 @@ def build_monitor(config, lock, database):
         vm_renamed_handler,
         vm_reconfigured_handler,
         vm_removed_handler,
+        vm_registered_handler,
         guest_net_handler,
         vmware_tools_status_handler,
         power_state_handler,

cvm/controllers.py

@@ -78,13 +78,6 @@ class VmUpdatedHandler(AbstractEventHandler):
         vim.event.VmDeployedEvent,
         vim.event.VmMacChangedEvent,
         vim.event.VmMacAssignedEvent,
-        vim.event.DrsVmMigratedEvent,
-        vim.event.DrsVmPoweredOnEvent,
-        vim.event.VmMigratedEvent,
-        vim.event.VmRegisteredEvent,
-        vim.event.VmPoweredOnEvent,
-        vim.event.VmPoweredOffEvent,
-        vim.event.VmSuspendedEvent,
     )
 
     def __init__(self, vm_service, vn_service, vmi_service, vrouter_port_service):
@@ -104,6 +97,26 @@ def _handle_event(self, event):
             logger.info('Skipping event for a non-existent VM.')
 
 
+class VmRegisteredHandler(AbstractEventHandler):
+    EVENTS = (vim.event.VmRegisteredEvent,)
+
+    def __init__(self, vm_service, vn_service, vmi_service, vrouter_port_service):
+        self._vm_service = vm_service
+        self._vn_service = vn_service
+        self._vmi_service = vmi_service
+        self._vrouter_port_service = vrouter_port_service
+
+    def _handle_event(self, event):
+        vmware_vm = event.vm.vm
+        try:
+            self._vm_service.update(vmware_vm)
+            self._vn_service.update_vns()
+            self._vmi_service.update_vmis(vm_registered=True)
+            self._vrouter_port_service.sync_ports()
+        except vmodl.fault.ManagedObjectNotFound:
+            logger.info('Skipping event for a non-existent VM.')
+
+
 class VmRenamedHandler(AbstractEventHandler):
     EVENTS = (vim.event.VmRenamedEvent,)
 

cvm/services.py

@@ -1,5 +1,6 @@
 import ipaddress
 import logging
+import time
 
 from vnc_api.vnc_api import PermType2
 
@@ -212,11 +213,11 @@ def sync_vlan_ids(self):
             for vlan_id in reserved_vlan_ids:
                 self._vlan_id_pool.reserve(vlan_id)
 
-    def update_vmis(self):
+    def update_vmis(self, vm_registered=False):
         vmis_to_update = [vmi_model for vmi_model in self._database.vmis_to_update]
         for vmi_model in vmis_to_update:
             logger.info('Updating %s', vmi_model)
-            self._update_vmis_vn(vmi_model)
+            self._update_vmis_vn(vmi_model, vm_registered=vm_registered)
             self._database.vmis_to_update.remove(vmi_model)
             logger.info('Updated %s', vmi_model)
 
@@ -225,36 +226,50 @@ def update_vmis(self):
             self._delete(vmi_model)
             self._database.vmis_to_delete.remove(vmi_model)
 
-    def _update_vmis_vn(self, new_vmi_model):
+    def _update_vmis_vn(self, new_vmi_model, vm_registered=False):
         old_vmi_model = self._database.get_vmi_model_by_uuid(new_vmi_model.uuid)
         new_vmi_model.vn_model = self._database.get_vn_model_by_key(new_vmi_model.vcenter_port.portgroup_key)
         if old_vmi_model and old_vmi_model.vn_model != new_vmi_model.vn_model:
             self._delete(old_vmi_model)
         if new_vmi_model.vn_model is not None:
-            self._create_or_update(new_vmi_model)
+            self._create_or_update(new_vmi_model, vm_registered=vm_registered)
         else:
             with self._vcenter_api_client:
                 dpg = self._vcenter_api_client.get_dpg_by_key(new_vmi_model.vcenter_port.portgroup_key)
                 logger.error('Interface of VM: %s is connected to portgroup: %s, which is not handled by Contrail',
                              new_vmi_model.vm_model.name, dpg.name)
 
-    def _create_or_update(self, vmi_model):
-        self._assign_vlan_id(vmi_model)
+    def _create_or_update(self, vmi_model, vm_registered=False):
+        self._assign_vlan_id(vmi_model, vm_registered=vm_registered)
         self._add_vnc_info_to(vmi_model)
         self._update_in_vnc(vmi_model)
         self._add_instance_ip_to(vmi_model)
         self._update_vrouter_port(vmi_model)
         self._database.save(vmi_model)
 
-    def _assign_vlan_id(self, vmi_model):
+    def _assign_vlan_id(self, vmi_model, vm_registered=False):
         with self._vcenter_api_client:
             current_vlan_id = self._vcenter_api_client.get_vlan_id(vmi_model.vcenter_port)
-            if current_vlan_id and self._vlan_id_pool.is_available(current_vlan_id):
-                vmi_model.vcenter_port.vlan_id = current_vlan_id
-                self._vlan_id_pool.reserve(current_vlan_id)
+            if vm_registered:
+                if current_vlan_id and self._vlan_id_pool.is_available(current_vlan_id):
+                    self._preserve_old_vlan_id(current_vlan_id, vmi_model)
+                else:
+                    self._assign_new_vlan_id(vmi_model)
+                return
+            if current_vlan_id:
+                self._preserve_old_vlan_id(current_vlan_id, vmi_model)
             else:
-                vmi_model.vcenter_port.vlan_id = self._vlan_id_pool.get_available()
-                self._vcenter_api_client.set_vlan_id(vmi_model.vcenter_port)
+                self._assign_new_vlan_id(vmi_model)
+
+    def _preserve_old_vlan_id(self, current_vlan_id, vmi_model):
+        vmi_model.vcenter_port.vlan_id = current_vlan_id
+        self._vlan_id_pool.reserve(current_vlan_id)
+
+    def _assign_new_vlan_id(self, vmi_model):
+        vmi_model.vcenter_port.vlan_id = self._vlan_id_pool.get_available()
+        # Purpose of this sleep is avoid to race in vmware code
+        time.sleep(3)
+        self._vcenter_api_client.set_vlan_id(vmi_model.vcenter_port)
 
     def _add_vnc_info_to(self, vmi_model):
         vmi_model.parent = self._project

tests/conftest.py

@@ -6,7 +6,7 @@
 
 from cvm.controllers import (GuestNetHandler, PowerStateHandler, UpdateHandler,
                              VmReconfiguredHandler, VmRemovedHandler,
-                             VmRenamedHandler, VmUpdatedHandler,
+                             VmRenamedHandler, VmUpdatedHandler, VmRegisteredHandler,
                              VmwareController, VmwareToolsStatusHandler)
 from cvm.database import Database
 from cvm.models import (VirtualMachineInterfaceModel, VirtualMachineModel,
@@ -159,6 +159,13 @@ def vm_created_update(vmware_vm_1):
     return wrap_into_update_set(event=event)
 
 
+@pytest.fixture()
+def vm_registered_update(vmware_vm_1):
+    event = Mock(spec=vim.event.VmRegisteredEvent())
+    event.vm.vm = vmware_vm_1
+    return wrap_into_update_set(event=event)
+
+
 @pytest.fixture()
 def vm_removed_update():
     event = Mock(spec=vim.event.VmRemovedEvent())
@@ -341,6 +348,7 @@ def controller(vm_service, vn_service, vmi_service, vrouter_port_service, lock):
         VmRenamedHandler(vm_service, vmi_service, vrouter_port_service),
         VmReconfiguredHandler(vm_service, vn_service, vmi_service, vrouter_port_service),
         VmRemovedHandler(vm_service, vmi_service, vrouter_port_service),
+        VmRegisteredHandler(vm_service, vn_service, vmi_service, vrouter_port_service),
         GuestNetHandler(vmi_service, vrouter_port_service),
         PowerStateHandler(vm_service, vrouter_port_service),
         VmwareToolsStatusHandler(vm_service)

tests/functional/test_vm_created_vlan_id.py

@@ -36,36 +36,3 @@ def test_vm_created_vlan_id(controller, database, vcenter_api_client, vm_created
         vn_model=vn_model_1,
         vm_model=vm_model
     )
-
-
-def test_vmotion_vlan_unavailable(controller, database, vcenter_api_client, vm_created_update, vn_model_1,
-                                  vlan_id_pool):
-    """ When the VLAN ID is unavailable on a host, we should change it to a new value"""
-    # Virtual Networks are already created for us and after synchronization,
-    # their models are stored in our database
-    database.save(vn_model_1)
-
-    # Some vlan ids should be already reserved
-    reserve_vlan_ids(vlan_id_pool, [0, 1, 5])
-
-    # When we ask vCenter for the VLAN ID it turns out that the VLAN ID has already been overridden
-    vcenter_api_client.get_vlan_id.return_value = 5
-
-    # A new update containing VmCreatedEvent arrives and is being handled by the controller
-    controller.handle_update(vm_created_update)
-
-    # Check if VLAN ID has been changed
-    vmi_model = database.get_all_vmi_models()[0]
-    vcenter_api_client.set_vlan_id.assert_called_once_with(vmi_model.vcenter_port)
-
-    # Check inner VMI model state
-    vm_model = database.get_vm_model_by_uuid('vmware-vm-uuid-1')
-    assert_vmi_model_state(
-        vmi_model,
-        mac_address='mac-address',
-        ip_address='192.168.100.5',
-        vlan_id=2,
-        display_name='vmi-DPG1-VM1',
-        vn_model=vn_model_1,
-        vm_model=vm_model
-    )

tests/functional/test_vm_registered_vlan_id.py

@@ -0,0 +1,67 @@
+from tests.utils import assert_vmi_model_state, reserve_vlan_ids
+
+
+def test_vmotion_vlan_unavailable(controller, database, vcenter_api_client, vm_registered_update, vn_model_1,
+                                  vlan_id_pool):
+    """ When the VLAN ID is unavailable on a host, we should change it to a new value"""
+    # Virtual Networks are already created for us and after synchronization,
+    # their models are stored in our database
+    database.save(vn_model_1)
+
+    # Some vlan ids should be already reserved
+    reserve_vlan_ids(vlan_id_pool, [0, 1, 5])
+
+    # When we ask vCenter for the VLAN ID it turns out that the VLAN ID has already been overridden
+    vcenter_api_client.get_vlan_id.return_value = 5
+
+    # A new update containing VmRegisteredEvent arrives and is being handled by the controller
+    controller.handle_update(vm_registered_update)
+
+    # Check if VLAN ID has been changed
+    vmi_model = database.get_all_vmi_models()[0]
+    vcenter_api_client.set_vlan_id.assert_called_once_with(vmi_model.vcenter_port)
+
+    # Check inner VMI model state
+    vm_model = database.get_vm_model_by_uuid('vmware-vm-uuid-1')
+    assert_vmi_model_state(
+        vmi_model,
+        mac_address='mac-address',
+        ip_address='192.168.100.5',
+        vlan_id=2,
+        display_name='vmi-DPG1-VM1',
+        vn_model=vn_model_1,
+        vm_model=vm_model
+    )
+
+
+def test_vmotion_vlan_available(controller, database, vcenter_api_client, vm_registered_update, vn_model_1,
+                                  vlan_id_pool):
+    """ When the VLAN ID is available on a host, we should not change it"""
+    # Virtual Networks are already created for us and after synchronization,
+    # their models are stored in our database
+    database.save(vn_model_1)
+
+    # Some vlan ids should be already reserved
+    reserve_vlan_ids(vlan_id_pool, [0, 1])
+
+    # When we ask vCenter for the VLAN ID it turns out that the VLAN ID has already been overridden
+    vcenter_api_client.get_vlan_id.return_value = 5
+
+    # A new update containing VmRegisteredEvent arrives and is being handled by the controller
+    controller.handle_update(vm_registered_update)
+
+    # Check if VLAN ID has been changed
+    vmi_model = database.get_all_vmi_models()[0]
+    vcenter_api_client.set_vlan_id.assert_not_called()
+
+    # Check inner VMI model state
+    vm_model = database.get_vm_model_by_uuid('vmware-vm-uuid-1')
+    assert_vmi_model_state(
+        vmi_model,
+        mac_address='mac-address',
+        ip_address='192.168.100.5',
+        vlan_id=5,
+        display_name='vmi-DPG1-VM1',
+        vn_model=vn_model_1,
+        vm_model=vm_model
+    )

tests/unit/services/test_vlan_ids.py

@@ -39,17 +39,28 @@ def test_retain_old_vlan_id(vmi_service, database, vcenter_api_client, vmi_model
     assert vmi_model.vcenter_port.vlan_id == 20
 
 
-def test_current_not_available(vmi_service, database, vcenter_api_client,
+def test_current_not_available_with_register_event(vmi_service, database, vcenter_api_client,
                                vlan_id_pool, vmi_model):
     database.vmis_to_update.append(vmi_model)
     vcenter_api_client.get_vlan_id.return_value = 20
     reserve_vlan_ids(vlan_id_pool, [20])
 
-    vmi_service.update_vmis()
+    vmi_service.update_vmis(vm_registered=True)
 
     assert vmi_model.vcenter_port.vlan_id == 0
 
 
+def test_current_not_available_without_register_event(vmi_service, database, vcenter_api_client,
+                               vlan_id_pool, vmi_model):
+    database.vmis_to_update.append(vmi_model)
+    vcenter_api_client.get_vlan_id.return_value = 20
+    reserve_vlan_ids(vlan_id_pool, [20])
+
+    vmi_service.update_vmis(vm_registered=False)
+
+    assert vmi_model.vcenter_port.vlan_id == 20
+
+
 def test_restore_vlan_id(vmi_service, database, vcenter_api_client,
                          vlan_id_pool, vmi_model):
     reserve_vlan_ids(vlan_id_pool, [20])