Merge pull request #225 from Random-Liu/update-ocicni

Update ocicni to 73f1309d6bc5c3eac78c1382408921cd771ff22e

pkg/server/sandbox_run.go

@@ -22,7 +22,7 @@ import (
 	"strings"
 
 	"github.com/containerd/containerd"
-	"github.com/cri-o/ocicni"
+	"github.com/cri-o/ocicni/pkg/ocicni"
 	"github.com/golang/glog"
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	runtimespec "github.com/opencontainers/runtime-spec/specs-go"

pkg/server/sandbox_run_test.go

@@ -20,7 +20,7 @@ import (
 	"os"
 	"testing"
 
-	"github.com/cri-o/ocicni"
+	"github.com/cri-o/ocicni/pkg/ocicni"
 	imagespec "github.com/opencontainers/image-spec/specs-go/v1"
 	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/stretchr/testify/assert"

pkg/server/sandbox_status.go

@@ -22,6 +22,7 @@ import (
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/errdefs"
+	"github.com/cri-o/ocicni/pkg/ocicni"
 	"github.com/golang/glog"
 	"golang.org/x/net/context"
 	"k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime"
@@ -57,7 +58,15 @@ func (c *criContainerdService) PodSandboxStatus(ctx context.Context, r *runtime.
 			state = runtime.PodSandboxState_SANDBOX_READY
 		}
 	}
-	ip, err := c.netPlugin.GetPodNetworkStatus(sandbox.NetNSPath)
+	config := sandbox.Config
+	podNetwork := ocicni.PodNetwork{
+		Name:         config.GetMetadata().GetName(),
+		Namespace:    config.GetMetadata().GetNamespace(),
+		ID:           id,
+		NetNS:        sandbox.NetNSPath,
+		PortMappings: toCNIPortMappings(config.GetPortMappings()),
+	}
+	ip, err := c.netPlugin.GetPodNetworkStatus(podNetwork)
 	if err != nil {
 		// Ignore the error on network status
 		ip = ""

pkg/server/sandbox_stop.go

@@ -22,7 +22,7 @@ import (
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/errdefs"
-	"github.com/cri-o/ocicni"
+	"github.com/cri-o/ocicni/pkg/ocicni"
 	"github.com/golang/glog"
 	"golang.org/x/net/context"
 	"k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime"

pkg/server/service.go

@@ -26,7 +26,7 @@ import (
 	"github.com/containerd/containerd/api/services/tasks/v1"
 	"github.com/containerd/containerd/content"
 	"github.com/containerd/containerd/images"
-	"github.com/cri-o/ocicni"
+	"github.com/cri-o/ocicni/pkg/ocicni"
 	"github.com/golang/glog"
 	"google.golang.org/grpc"
 	"k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime"

pkg/server/testing/fake_cni_plugin.go

@@ -23,7 +23,7 @@ import (
 	"sync"
 	"time"
 
-	"github.com/cri-o/ocicni"
+	"github.com/cri-o/ocicni/pkg/ocicni"
 )
 
 // CalledDetail is the struct contains called function name and arguments.
@@ -148,14 +148,14 @@ func (f *FakeCNIPlugin) TearDownPod(podNetwork ocicni.PodNetwork) error {
 }
 
 // GetPodNetworkStatus get the status of network.
-func (f *FakeCNIPlugin) GetPodNetworkStatus(netnsPath string) (string, error) {
+func (f *FakeCNIPlugin) GetPodNetworkStatus(podNetwork ocicni.PodNetwork) (string, error) {
 	f.Lock()
 	defer f.Unlock()
-	f.appendCalled("GetPodNetworkStatus", netnsPath)
+	f.appendCalled("GetPodNetworkStatus", podNetwork)
 	if err := f.getError("GetPodNetworkStatus"); err != nil {
 		return "", err
 	}
-	ip, ok := f.IPMap[netnsPath]
+	ip, ok := f.IPMap[podNetwork.NetNS]
 	if !ok {
 		return "", fmt.Errorf("failed to find the IP")
 	}

vendor.conf

@@ -7,7 +7,7 @@ github.com/containerd/cgroups 7a5fdd8330119dc70d850260db8f3594d89d6943
 github.com/coreos/go-systemd d2196463941895ee908e13531a23a39feb9e1243
 github.com/containernetworking/cni v0.6.0
 github.com/containernetworking/plugins v0.6.0
-github.com/cri-o/ocicni 4c2bf6d5198c307f76312f8fc7ef654cfd41d303
+github.com/cri-o/ocicni 73f1309d6bc5c3eac78c1382408921cd771ff22e
 github.com/davecgh/go-spew v1.1.0
 github.com/docker/distribution b38e5838b7b2f2ad48e06ec4b500011976080621
 github.com/docker/docker cc4da8112814cdbb00dbf23370f9ed764383de1f

vendor/github.com/cri-o/ocicni/pkg/ocicni/noop.go

@@ -15,7 +15,7 @@ func (noop *cniNoOp) TearDownPod(network PodNetwork) error {
 	return nil
 }
 
-func (noop *cniNoOp) GetPodNetworkStatus(netnsPath string) (string, error) {
+func (noop *cniNoOp) GetPodNetworkStatus(network PodNetwork) (string, error) {
 	return "", nil
 }
 

vendor/github.com/cri-o/ocicni/pkg/ocicni/ocicni.go

@@ -26,6 +26,13 @@ type cniNetworkPlugin struct {
 	vendorCNIDirPrefix string
 
 	monitorNetDirChan chan struct{}
+
+	// The pod map provides synchronization for a given pod's network
+	// operations.  Each pod's setup/teardown/status operations
+	// are synchronized against each other, but network operations of other
+	// pods can proceed in parallel.
+	podsLock sync.Mutex
+	pods     map[string]*podLock
 }
 
 type cniNetwork struct {
@@ -36,6 +43,61 @@ type cniNetwork struct {
 
 var errMissingDefaultNetwork = errors.New("Missing CNI default network")
 
+type podLock struct {
+	// Count of in-flight operations for this pod; when this reaches zero
+	// the lock can be removed from the pod map
+	refcount uint
+
+	// Lock to synchronize operations for this specific pod
+	mu sync.Mutex
+}
+
+func buildFullPodName(podNetwork PodNetwork) string {
+	return podNetwork.Namespace + "_" + podNetwork.Name
+}
+
+// Lock network operations for a specific pod.  If that pod is not yet in
+// the pod map, it will be added.  The reference count for the pod will
+// be increased.
+func (plugin *cniNetworkPlugin) podLock(podNetwork PodNetwork) *sync.Mutex {
+	plugin.podsLock.Lock()
+	defer plugin.podsLock.Unlock()
+
+	fullPodName := buildFullPodName(podNetwork)
+	lock, ok := plugin.pods[fullPodName]
+	if !ok {
+		lock = &podLock{}
+		plugin.pods[fullPodName] = lock
+	}
+	lock.refcount++
+	return &lock.mu
+}
+
+// Unlock network operations for a specific pod.  The reference count for the
+// pod will be decreased.  If the reference count reaches zero, the pod will be
+// removed from the pod map.
+func (plugin *cniNetworkPlugin) podUnlock(podNetwork PodNetwork) {
+	plugin.podsLock.Lock()
+	defer plugin.podsLock.Unlock()
+
+	fullPodName := buildFullPodName(podNetwork)
+	lock, ok := plugin.pods[fullPodName]
+	if !ok {
+		logrus.Warningf("Unbalanced pod lock unref for %s", fullPodName)
+		return
+	} else if lock.refcount == 0 {
+		// This should never ever happen, but handle it anyway
+		delete(plugin.pods, fullPodName)
+		logrus.Errorf("Pod lock for %s still in map with zero refcount", fullPodName)
+		return
+	}
+	lock.refcount--
+	lock.mu.Unlock()
+	if lock.refcount == 0 {
+		delete(plugin.pods, fullPodName)
+	}
+}
+
 func (plugin *cniNetworkPlugin) monitorNetDir() {
 	watcher, err := fsnotify.NewWatcher()
 	if err != nil {
@@ -111,6 +173,7 @@ func probeNetworkPluginsWithVendorCNIDirPrefix(pluginDir string, cniDirs []strin
 		cniDirs:            cniDirs,
 		vendorCNIDirPrefix: vendorCNIDirPrefix,
 		monitorNetDirChan:  make(chan struct{}),
+		pods:               make(map[string]*podLock),
 	}
 
 	// sync NetworkConfig in best effort during probing.
@@ -250,6 +313,9 @@ func (plugin *cniNetworkPlugin) SetUpPod(podNetwork PodNetwork) error {
 		return err
 	}
 
+	plugin.podLock(podNetwork).Lock()
+	defer plugin.podUnlock(podNetwork)
+
 	_, err := plugin.loNetwork.addToNetwork(podNetwork)
 	if err != nil {
 		logrus.Errorf("Error while adding to cni lo network: %s", err)
@@ -270,13 +336,19 @@ func (plugin *cniNetworkPlugin) TearDownPod(podNetwork PodNetwork) error {
 		return err
 	}
 
+	plugin.podLock(podNetwork).Lock()
+	defer plugin.podUnlock(podNetwork)
+
 	return plugin.getDefaultNetwork().deleteFromNetwork(podNetwork)
 }
 
 // TODO: Use the addToNetwork function to obtain the IP of the Pod. That will assume idempotent ADD call to the plugin.
 // Also fix the runtime's call to Status function to be done only in the case that the IP is lost, no need to do periodic calls
-func (plugin *cniNetworkPlugin) GetPodNetworkStatus(netnsPath string) (string, error) {
+func (plugin *cniNetworkPlugin) GetPodNetworkStatus(podNetwork PodNetwork) (string, error) {
-	ip, err := getContainerIP(plugin.nsenterPath, netnsPath, DefaultInterfaceName, "-4")
+	plugin.podLock(podNetwork).Lock()
+	defer plugin.podUnlock(podNetwork)
+
+	ip, err := getContainerIP(plugin.nsenterPath, podNetwork.NetNS, DefaultInterfaceName, "-4")
 	if err != nil {
 		return "", err
 	}

vendor/github.com/cri-o/ocicni/pkg/ocicni/types.go

@@ -55,7 +55,7 @@ type CNIPlugin interface {
 	TearDownPod(network PodNetwork) error
 
 	// Status is the method called to obtain the ipv4 or ipv6 addresses of the pod sandbox
-	GetPodNetworkStatus(netnsPath string) (string, error)
+	GetPodNetworkStatus(network PodNetwork) (string, error)
 
 	// NetworkStatus returns error if the network plugin is in error state
 	Status() error