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

Fix race condition when creating/deleting namespace address set #4382

Merged
merged 1 commit into from
Jun 10, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 35 additions & 13 deletions go-controller/pkg/ovn/base_network_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -532,27 +532,49 @@ func (bnc *BaseNetworkController) deleteNamespaceLocked(ns string) (*namespaceIn
}

// Delete the address set after a short delay.
// This is so NetworkPolicy handlers can converge and stop referencing it.
// This is to avoid OVN warnings while the address set is still
// referenced from NBDB ACLs until the NetworkPolicy handlers remove
// them.
addressSet := nsInfo.addressSet
go func() {
select {
case <-bnc.stopChan:
return
case <-time.After(20 * time.Second):
// Check to see if the NS was re-added in the meanwhile. If so,
// only delete if the new NS's AddressSet shouldn't exist.
nsInfo, nsUnlock := bnc.getNamespaceLocked(ns, true)
if nsInfo != nil {
defer nsUnlock()
if nsInfo.addressSet != nil {
klog.V(5).Infof("Skipping deferred deletion of AddressSet for NS %s: re-created", ns)
return
maybeDeleteAddressSet := func() bool {
bnc.namespacesMutex.Lock()
nsInfo := bnc.namespaces[ns]
if nsInfo == nil {
defer bnc.namespacesMutex.Unlock()
} else {
bnc.namespacesMutex.Unlock()
nsInfo.Lock()
defer nsInfo.Unlock()
bnc.namespacesMutex.Lock()
defer bnc.namespacesMutex.Unlock()
if nsInfo != bnc.namespaces[ns] {
// somebody deleted the namespace while waiting for
// its lock, check again in case it was added back
return false
}
// if somebody recreated the namespace during the delay,
// check if it has an address set
if nsInfo.addressSet != nil {
klog.V(5).Infof("Skipping deferred deletion of AddressSet for NS %s: recreated", ns)
return true
}
}
klog.V(5).Infof("Finishing deferred deletion of AddressSet for NS %s", ns)
if err := addressSet.Destroy(); err != nil {
klog.Errorf("Failed to delete AddressSet for NS %s: %v", ns, err.Error())
}
return true
}

klog.V(5).Infof("Finishing deferred deletion of AddressSet for NS %s", ns)
if err := addressSet.Destroy(); err != nil {
klog.Errorf("Failed to delete AddressSet for NS %s: %v", ns, err.Error())
for {
done := maybeDeleteAddressSet()
if done {
break
}
}
}
}()
Expand Down