Skip to content

Commit 87fa976

Browse files
committed
Fix CNIEnv concurrency issue
Signed-off-by: apostasie <spam_blackhole@farcloser.world>
1 parent 28d77b6 commit 87fa976

File tree

1 file changed

+25
-48
lines changed

1 file changed

+25
-48
lines changed

pkg/netutil/netutil.go

+25-48
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,9 @@ func (e *CNIEnv) NetworkMap() (map[string]*NetworkConfig, error) { //nolint:revi
242242
return m, nil
243243
}
244244

245+
// FIXME: this is currently walking networkConfigList in an unsafe way.
246+
// However, enforcing locking here will deadlock.
247+
// To be investigated.
245248
func (e *CNIEnv) NetworkByNameOrID(key string) (*NetworkConfig, error) {
246249
networks, err := e.networkConfigList()
247250
if err != nil {
@@ -260,20 +263,6 @@ func (e *CNIEnv) NetworkByNameOrID(key string) (*NetworkConfig, error) {
260263
return nil, fmt.Errorf("no such network: %q", key)
261264
}
262265

263-
func (e *CNIEnv) filterNetworks(filterf func(*NetworkConfig) bool) ([]*NetworkConfig, error) {
264-
networkConfigs, err := e.networkConfigList()
265-
if err != nil {
266-
return nil, err
267-
}
268-
result := []*NetworkConfig{}
269-
for _, networkConfig := range networkConfigs {
270-
if filterf(networkConfig) {
271-
result = append(result, networkConfig)
272-
}
273-
}
274-
return result, nil
275-
}
276-
277266
func (e *CNIEnv) getConfigPathForNetworkName(netName string) string {
278267
if netName == DefaultNetworkName || e.Namespace == "" {
279268
return filepath.Join(e.NetconfPath, "nerdctl-"+netName+".conflist")
@@ -359,47 +348,35 @@ func (e *CNIEnv) RemoveNetwork(net *NetworkConfig) error {
359348
// label, or falls back to checking whether any network bears the
360349
// `DefaultNetworkName` name.
361350
func (e *CNIEnv) GetDefaultNetworkConfig() (*NetworkConfig, error) {
362-
// Search for networks bearing the `labels.NerdctlDefaultNetwork` label.
363-
defaultLabelFilterF := func(nc *NetworkConfig) bool {
364-
if nc.NerdctlLabels == nil {
365-
return false
366-
} else if _, ok := (*nc.NerdctlLabels)[labels.NerdctlDefaultNetwork]; ok {
367-
return true
368-
}
369-
return false
370-
}
371-
labelMatches, err := e.filterNetworks(defaultLabelFilterF)
372-
if err != nil {
373-
return nil, err
374-
}
375-
if len(labelMatches) >= 1 {
376-
if len(labelMatches) > 1 {
377-
log.L.Warnf("returning the first network bearing the %q label out of the multiple found: %#v", labels.NerdctlDefaultNetwork, labelMatches)
378-
}
379-
return labelMatches[0], nil
380-
}
351+
var networkConfigs []*NetworkConfig
352+
var err error
353+
// NOTE: we cannot lock NetconfPath directly, as Cilium (maybe others) are also locking it.
354+
err = lockutil.WithDirLock(filepath.Join(e.NetconfPath, ".nerdctl.lock"), func() error {
355+
networkConfigs, err = e.networkConfigList()
356+
return err
357+
})
381358

382-
// Search for networks bearing the DefaultNetworkName.
383-
defaultNameFilterF := func(nc *NetworkConfig) bool {
384-
return nc.Name == DefaultNetworkName
385-
}
386-
nameMatches, err := e.filterNetworks(defaultNameFilterF)
387359
if err != nil {
388360
return nil, err
389361
}
390-
if len(nameMatches) >= 1 {
391-
if len(nameMatches) > 1 {
392-
log.L.Warnf("returning the first network bearing the %q default network name out of the multiple found: %#v", DefaultNetworkName, nameMatches)
393-
}
394362

395-
// Warn the user if the default network was not created by nerdctl.
396-
match := nameMatches[0]
397-
_, statErr := os.Stat(e.getConfigPathForNetworkName(DefaultNetworkName))
398-
if match.NerdctlID == nil || statErr != nil {
399-
log.L.Warnf("default network named %q does not have an internal nerdctl ID or nerdctl-managed config file, it was most likely NOT created by nerdctl", DefaultNetworkName)
363+
for _, networkConfig := range networkConfigs {
364+
if networkConfig.NerdctlLabels == nil {
365+
continue
366+
}
367+
if _, ok := (*networkConfig.NerdctlLabels)[labels.NerdctlDefaultNetwork]; ok {
368+
return networkConfig, nil
400369
}
370+
}
401371

402-
return nameMatches[0], nil
372+
for _, networkConfig := range networkConfigs {
373+
if networkConfig.Name == DefaultNetworkName {
374+
_, statErr := os.Stat(e.getConfigPathForNetworkName(DefaultNetworkName))
375+
if networkConfig.NerdctlID == nil || statErr != nil {
376+
log.L.Warnf("default network named %q does not have an internal nerdctl ID or nerdctl-managed config file, it was most likely NOT created by nerdctl", DefaultNetworkName)
377+
}
378+
return networkConfig, nil
379+
}
403380
}
404381

405382
return nil, nil

0 commit comments

Comments
 (0)