Fix UnregisterAllVirtualCenters to clear vCenterInstances map#3890
Fix UnregisterAllVirtualCenters to clear vCenterInstances map#3890chethanv28 wants to merge 1 commit intokubernetes-sigs:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: chethanv28 The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
cncf-cla: yes
|
Triggering CSI-WCP Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete |
Added clear(vCenterInstances) to ensure the global vCenterInstances map is cleared when unregistering all virtual centers. Previously, only the VirtualCenterManager's internal map was cleared, leaving the global map with stale entries. Also added unit tests for virtualcenter package.
chethanv28
force-pushed
the
topic/chethanv28/fix-unregister-virtualcenters
branch
from
116d3c5 to
b42f651
Compare
|
Triggering CSI-WCP Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete |
|
SUCCESS --- Jenkins Build #964 |
|
Triggering CSI-WCP Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete |
|
Triggering CSI-WCP Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete |
|
SUCCESS --- Jenkins Build #966 |
|
Triggering CSI-WCP Pre-checkin Pipeline for this PR... Job takes approximately an hour to complete |
|
SUCCESS --- Jenkins Build #968 |
3 participants
What this PR does / why we need it:
This PR fixes a bug in UnregisterAllVirtualCenters() where the global vCenterInstances map was not being cleared after unregistering all virtual centers.
Data Consistency: The vCenterInstances map is used by GetVirtualCenterInstanceForVCenterHost() to retrieve vCenter instances. If entries remain after unregistration, subsequent lookups could return stale/disconnected instances.
Clean Shutdown: The function's purpose (as documented) is to "unregister and logout all registered vCenter instances" before exiting. Leaving entries in the map contradicts this intent.
Memory Cleanup: Clearing the map ensures proper cleanup of references to VirtualCenter objects, allowing garbage collection.
Test Coverage: Added unit tests (virtualcenter_test.go) that validate this behavior, ensuring vCenterInstances is empty after calling UnregisterAllVirtualCenters().
Which issue this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close that issue when PR gets merged):The UnregisterAllVirtualCenters() function is called during container shutdown to logout all registered vCenter sessions. However, it only cleared the VirtualCenterManager's internal sync.Map via virtualcentermanager.UnregisterAllVirtualCenters(), but did not clear the global vCenterInstances map defined at the package level.
This results in:
Stale entries remaining in vCenterInstances after unregistration
Inconsistent state between the VirtualCenterManager and the global map
Potential issues if the driver were to re-register vCenters without a full restart
Testing done:
TBA
Special notes for your reviewer:
Release note: