Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cherry-pick]Refactor Subnet lock for SubnetPort creation (#974) #979

Merged
merged 1 commit into from
Dec 27, 2024

Conversation

yanjunz97
Copy link
Contributor

Instead of using lock for Subnet, this PR adds a cache in SubnetPort service to record the number of SubnetPorts on each Subnet. Read/Write operation for this cache is protected by lock to avoid race condition and ensure the number of SubnetPort on the Subnet will be controlled below the Subnet capacity.

Testing done:

  • Create 16 Pods on default SubnetSet with size 16 (capacity 12), got all Pods ready; delete all Pods and observe the Subnets under the SubnetSet will be deleted by GC.
  • Create 16 VMs on DHCPServer SubnetSet with size 16 (capacity 12), all Vms can get IP; delete all VMs and observe the Subnet under the SubnetSet will be deleted by GC.
  • Create 16 VMs on DHCPServer Subnet with size 16 (capacity 12), only 12 VMs can PowerOn and get IP. The remainings will have VirtualMachineNetworkReady as NotReady and Message like network interface "eth0" error: subnetPort is not ready: SubnetPortNotReady - error occurred while processing the SubnetPort CR. Error: no valid IP in Subnet /orgs/default/projects/project-quality/vpcs/ns-1_7fcad11c-da38-4137-96ec-f0f80ae8d96b/subnets/subnet-test-1_60ebfa36-bbe2-4e6e-85df-8228be4e190a

Instead of using lock for Subnet, this PR adds a cache in SubnetPort service
to record the number of SubnetPorts on each Subnet.
Read/Write operation for this cache is protected by lock to avoid race condition
and ensure the number of SubnetPort on the Subnet will be controlled below
the Subnet capacity.

Testing done:
- Create 16 Pods on default SubnetSet with size 16 (capacity 12), got all Pods ready; 
   delete all Pods and observe the Subnets under the SubnetSet will be deleted by GC.
- Create 16 VMs on DHCPServer SubnetSet with size 16 (capacity 12), all Vms can get IP; 
   delete all VMs and observe the Subnet under the SubnetSet will be deleted by GC.
- Create 16 VMs on DHCPServer Subnet with size 16 (capacity 12), only 12 VMs can PowerOn
   and get IP. The remainings will have `VirtualMachineNetworkReady` as `NotReady` and
   Message like `network interface "eth0" error: subnetPort is not ready: SubnetPortNotReady - error occurred while processing the SubnetPort CR. Error: no valid IP in Subnet /orgs/default/projects/project-quality/vpcs/ns-1_7fcad11c-da38-4137-96ec-f0f80ae8d96b/subnets/subnet-test-1_60ebfa36-bbe2-4e6e-85df-8228be4e190a`

Signed-off-by: Yanjun Zhou <[email protected]>
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 77.77778% with 28 lines in your changes missing coverage. Please review.

Project coverage is 73.97%. Comparing base (5a693bc) to head (efbfebf).

Files with missing lines Patch % Lines
pkg/nsx/services/subnet/subnet.go 37.50% 9 Missing and 1 partial ⚠️
...kg/controllers/subnetport/subnetport_controller.go 41.66% 5 Missing and 2 partials ⚠️
pkg/nsx/services/subnetport/subnetport.go 91.66% 4 Missing and 1 partial ⚠️
pkg/controllers/common/utils.go 72.72% 2 Missing and 1 partial ⚠️
pkg/controllers/subnetset/subnetset_controller.go 82.35% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           v4.2.0     #979      +/-   ##
==========================================
- Coverage   74.03%   73.97%   -0.06%     
==========================================
  Files         118      118              
  Lines       16397    16396       -1     
==========================================
- Hits        12139    12129      -10     
- Misses       3477     3483       +6     
- Partials      781      784       +3     
Flag Coverage Δ
unit-tests 73.97% <77.77%> (-0.06%) ⬇️
Files with missing lines Coverage Δ
pkg/controllers/pod/pod_controller.go 61.57% <100.00%> (-0.41%) ⬇️
pkg/controllers/subnet/subnet_controller.go 63.74% <100.00%> (+0.10%) ⬆️
pkg/nsx/services/subnet/store.go 71.64% <ø> (-3.90%) ⬇️
pkg/nsx/services/subnetport/store.go 77.10% <ø> (ø)
pkg/controllers/common/utils.go 88.62% <72.72%> (-1.86%) ⬇️
pkg/controllers/subnetset/subnetset_controller.go 66.08% <82.35%> (+0.09%) ⬆️
pkg/nsx/services/subnetport/subnetport.go 82.08% <91.66%> (+1.25%) ⬆️
...kg/controllers/subnetport/subnetport_controller.go 74.74% <41.66%> (-0.95%) ⬇️
pkg/nsx/services/subnet/subnet.go 64.78% <37.50%> (-1.97%) ⬇️

@yanjunz97 yanjunz97 merged commit db604dc into vmware-tanzu:v4.2.0 Dec 27, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants