fix go-cni race condition

Signed-off-by: Michael Zappa <michael.zappa@gmail.com>

vendor/github.com/containerd/go-cni/cni.go

@@ -80,7 +80,10 @@ type libcni struct {
 	cniConfig    cnilibrary.CNI
 	networkCount int // minimum network plugin configurations needed to initialize cni
 	networks     []*Network
-	sync.RWMutex
+	// Mutex contract:
+	// - lock in public methods: write lock when mutating the state, read lock when reading the state.
+	// - never lock in private methods.
+	RWMutex
 }
 
 func defaultCNIConfig() *libcni {
@@ -135,11 +138,11 @@ func (c *libcni) Load(opts ...Opt) error {
 
 // Status returns the status of CNI initialization.
 func (c *libcni) Status() error {
+	c.RLock()
+	defer c.RUnlock()
 	if err := c.ready(); err != nil {
 		return err
 	}
-	c.RLock()
-	defer c.RUnlock()
 	// STATUS is only called for CNI Version 1.1.0 or greater. It is ignored for previous versions.
 	for _, v := range c.networks {
 		err := c.cniConfig.GetStatusNetworkList(context.Background(), v.config)
@@ -162,11 +165,11 @@ func (c *libcni) Networks() []*Network {
 
 // Setup setups the network in the namespace and returns a Result
 func (c *libcni) Setup(ctx context.Context, id string, path string, opts ...NamespaceOpts) (*Result, error) {
+	c.RLock()
+	defer c.RUnlock()
 	if err := c.ready(); err != nil {
 		return nil, err
 	}
-	c.RLock()
-	defer c.RUnlock()
 	ns, err := newNamespace(id, path, opts...)
 	if err != nil {
 		return nil, err
@@ -180,11 +183,11 @@ func (c *libcni) Setup(ctx context.Context, id string, path string, opts ...Name
 
 // SetupSerially setups the network in the namespace and returns a Result
 func (c *libcni) SetupSerially(ctx context.Context, id string, path string, opts ...NamespaceOpts) (*Result, error) {
+	c.RLock()
+	defer c.RUnlock()
 	if err := c.ready(); err != nil {
 		return nil, err
 	}
-	c.RLock()
-	defer c.RUnlock()
 	ns, err := newNamespace(id, path, opts...)
 	if err != nil {
 		return nil, err
@@ -198,7 +201,7 @@ func (c *libcni) SetupSerially(ctx context.Context, id string, path string, opts
 
 func (c *libcni) attachNetworksSerially(ctx context.Context, ns *Namespace) ([]*types100.Result, error) {
 	var results []*types100.Result
-	for _, network := range c.Networks() {
+	for _, network := range c.networks {
 		r, err := network.Attach(ctx, ns)
 		if err != nil {
 			return nil, err
@@ -223,15 +226,15 @@ func asynchAttach(ctx context.Context, index int, n *Network, ns *Namespace, wg
 func (c *libcni) attachNetworks(ctx context.Context, ns *Namespace) ([]*types100.Result, error) {
 	var wg sync.WaitGroup
 	var firstError error
-	results := make([]*types100.Result, len(c.Networks()))
+	results := make([]*types100.Result, len(c.networks))
 	rc := make(chan asynchAttachResult)
 
-	for i, network := range c.Networks() {
+	for i, network := range c.networks {
 		wg.Add(1)
 		go asynchAttach(ctx, i, network, ns, &wg, rc)
 	}
 
-	for range c.Networks() {
+	for range c.networks {
 		rs := <-rc
 		if rs.err != nil && firstError == nil {
 			firstError = rs.err
@@ -245,16 +248,16 @@ func (c *libcni) attachNetworks(ctx context.Context, ns *Namespace) ([]*types100
 
 // Remove removes the network config from the namespace
 func (c *libcni) Remove(ctx context.Context, id string, path string, opts ...NamespaceOpts) error {
+	c.RLock()
+	defer c.RUnlock()
 	if err := c.ready(); err != nil {
 		return err
 	}
-	c.RLock()
-	defer c.RUnlock()
 	ns, err := newNamespace(id, path, opts...)
 	if err != nil {
 		return err
 	}
-	for _, network := range c.Networks() {
+	for _, network := range c.networks {
 		if err := network.Remove(ctx, ns); err != nil {
 			// Based on CNI spec v0.7.0, empty network namespace is allowed to
 			// do best effort cleanup. However, it is not handled consistently
@@ -275,16 +278,16 @@ func (c *libcni) Remove(ctx context.Context, id string, path string, opts ...Nam
 
 // Check checks if the network is still in desired state
 func (c *libcni) Check(ctx context.Context, id string, path string, opts ...NamespaceOpts) error {
+	c.RLock()
+	defer c.RUnlock()
 	if err := c.ready(); err != nil {
 		return err
 	}
-	c.RLock()
-	defer c.RUnlock()
 	ns, err := newNamespace(id, path, opts...)
 	if err != nil {
 		return err
 	}
-	for _, network := range c.Networks() {
+	for _, network := range c.networks {
 		err := network.Check(ctx, ns)
 		if err != nil {
 			return err
@@ -329,8 +332,6 @@ func (c *libcni) reset() {
 }
 
 func (c *libcni) ready() error {
-	c.RLock()
-	defer c.RUnlock()
 	if len(c.networks) < c.networkCount {
 		return ErrCNINotInitialized
 	}

vendor/github.com/containerd/go-cni/deprecated.go

@@ -30,5 +30,7 @@ type CNIResult = Result //revive:disable // type name will be used as cni.CNIRes
 // results fails, or if a network could not be found.
 // Deprecated: do not use
 func (c *libcni) GetCNIResultFromResults(results []*types100.Result) (*Result, error) {
+	c.RLock()
+	defer c.RUnlock()
 	return c.createResult(results)
 }

vendor/github.com/containerd/go-cni/mutex.go

@@ -0,0 +1,23 @@
+//go:build !deadlocks && !race
+
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package cni
+
+import "sync"
+
+type RWMutex = sync.RWMutex

vendor/github.com/containerd/go-cni/mutex_deadlocks.go

@@ -0,0 +1,25 @@
+//go:build deadlocks || race
+
+/*
+   Copyright The containerd Authors.
+
+   Licensed under the Apache License, Version 2.0 (the "License");
+   you may not use this file except in compliance with the License.
+   You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+*/
+
+package cni
+
+import (
+	"github.com/sasha-s/go-deadlock"
+)
+
+type RWMutex = deadlock.RWMutex

vendor/github.com/containerd/go-cni/result.go

@@ -69,8 +69,6 @@ type Config struct {
 // interfaces created in the namespace. It returns an error if validation of
 // results fails, or if a network could not be found.
 func (c *libcni) createResult(results []*types100.Result) (*Result, error) {
-	c.RLock()
-	defer c.RUnlock()
 	r := &Result{
 		Interfaces: make(map[string]*Config),
 		raw:        results,

vendor/github.com/containerd/go-cni/testutils.go

@@ -23,22 +23,11 @@ import (
 	"testing"
 )
 
-func makeTmpDir(prefix string) (string, error) {
-	tmpDir, err := os.MkdirTemp("", prefix)
-	if err != nil {
-		return "", err
-	}
-	return tmpDir, nil
-}
-
 func makeFakeCNIConfig(t *testing.T) (string, string) {
-	cniDir, err := makeTmpDir("fakecni")
-	if err != nil {
-		t.Fatalf("Failed to create plugin config dir: %v", err)
-	}
+	cniDir := t.TempDir()
 
 	cniConfDir := path.Join(cniDir, "net.d")
-	err = os.MkdirAll(cniConfDir, 0777)
+	err := os.MkdirAll(cniConfDir, 0777)
 	if err != nil {
 		t.Fatalf("Failed to create network config dir: %v", err)
 	}
@@ -69,13 +58,6 @@ func makeFakeCNIConfig(t *testing.T) (string, string) {
 	return cniDir, cniConfDir
 }
 
-func tearDownCNIConfig(t *testing.T, confDir string) {
-	err := os.RemoveAll(confDir)
-	if err != nil {
-		t.Fatalf("Failed to cleanup CNI configs: %v", err)
-	}
-}
-
 func buildFakeConfig(t *testing.T) (string, string) {
 	conf := `
 	{
@@ -111,13 +93,10 @@ func buildFakeConfig(t *testing.T) (string, string) {
 	]
 	}`
 
-	cniDir, err := makeTmpDir("fakecni")
-	if err != nil {
-		t.Fatalf("Failed to create plugin config dir: %v", err)
-	}
+	cniDir := t.TempDir()
 
 	cniConfDir := path.Join(cniDir, "net.d")
-	err = os.MkdirAll(cniConfDir, 0777)
+	err := os.MkdirAll(cniConfDir, 0777)
 	if err != nil {
 		t.Fatalf("Failed to create network config dir: %v", err)
 	}