diff --git a/pkg/controllers/common/utils.go b/pkg/controllers/common/utils.go index c1a27d8fa..329b273ba 100644 --- a/pkg/controllers/common/utils.go +++ b/pkg/controllers/common/utils.go @@ -23,14 +23,14 @@ import ( ) var ( - log = &logger.Log - lock = &sync.Mutex{} + log = &logger.Log + SubnetSetLocks sync.Map ) func AllocateSubnetFromSubnetSet(subnetSet *v1alpha1.SubnetSet, vpcService servicecommon.VPCServiceProvider, subnetService servicecommon.SubnetServiceProvider, subnetPortService servicecommon.SubnetPortServiceProvider) (string, error) { - // TODO: For now, this is a global lock. In the future, we need to narrow its scope down to improve the performance. - lock.Lock() - defer lock.Unlock() + // Use SubnetSet uuid lock to make sure when multiple ports are created on the same SubnetSet, only one Subnet will be created + lockSubnetSet(subnetSet.GetUID()) + defer unlockSubnetSet(subnetSet.GetUID()) subnetList := subnetService.GetSubnetsByIndex(servicecommon.TagScopeSubnetSetCRUID, string(subnetSet.GetUID())) for _, nsxSubnet := range subnetList { portNums := len(subnetPortService.GetPortsOfSubnet(*nsxSubnet.Id)) @@ -240,3 +240,17 @@ func NewStatusUpdater(client k8sclient.Client, nsxConfig *config.NSXOperatorConf ResourceType: resourceType, } } + +func lockSubnetSet(uuid types.UID) { + lock := sync.Mutex{} + subnetSetLock, _ := SubnetSetLocks.LoadOrStore(uuid, &lock) + log.V(1).Info("Lock SubnetSet", "uuid", uuid) + subnetSetLock.(*sync.Mutex).Lock() +} + +func unlockSubnetSet(uuid types.UID) { + if subnetSetLock, existed := SubnetSetLocks.Load(uuid); existed { + log.V(1).Info("Unlock SubnetSet", "uuid", uuid) + subnetSetLock.(*sync.Mutex).Unlock() + } +} diff --git a/pkg/controllers/subnetset/subnetset_controller.go b/pkg/controllers/subnetset/subnetset_controller.go index a0ca8e5c4..ba1f72bff 100644 --- a/pkg/controllers/subnetset/subnetset_controller.go +++ b/pkg/controllers/subnetset/subnetset_controller.go @@ -12,6 +12,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" apimachineryruntime "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/client-go/tools/record" ctrl "sigs.k8s.io/controller-runtime" @@ -260,6 +261,15 @@ func (r *SubnetSetReconciler) CollectGarbage(ctx context.Context) { r.StatusUpdater.IncreaseDeleteSuccessTotal() } } + + // clean the SubnetSet lock used to create Subnet + common.SubnetSetLocks.Range(func(key, value interface{}) bool { + uuid := key.(types.UID) + if !crdSubnetSetIDsSet.Has(string(uuid)) { + common.SubnetSetLocks.Delete(key) + } + return true + }) } func (r *SubnetSetReconciler) deleteSubnetBySubnetSetName(ctx context.Context, subnetSetName, ns string) error { diff --git a/pkg/controllers/subnetset/subnetset_controller_test.go b/pkg/controllers/subnetset/subnetset_controller_test.go index 4dd1266ee..cb1a6145f 100644 --- a/pkg/controllers/subnetset/subnetset_controller_test.go +++ b/pkg/controllers/subnetset/subnetset_controller_test.go @@ -6,10 +6,12 @@ import ( "fmt" "net/http" "reflect" + "sync" "testing" "time" "github.com/agiledragon/gomonkey/v2" + "github.com/google/uuid" "github.com/stretchr/testify/assert" "github.com/vmware/vsphere-automation-sdk-go/services/nsxt/model" v12 "k8s.io/api/core/v1" @@ -603,7 +605,15 @@ func TestSubnetSetReconciler_CollectGarbage(t *testing.T) { } }) + // fake SubnetSetLocks + lock := sync.Mutex{} + subnetSetId := types.UID(uuid.NewString()) + ctlcommon.SubnetSetLocks.LoadOrStore(subnetSetId, &lock) + r.CollectGarbage(ctx) + // the lock for should be deleted + _, ok := ctlcommon.SubnetSetLocks.Load(subnetSetId) + assert.False(t, ok) } type MockManager struct {