From 1c0385a6508343d2f1ece66304d1a8581e2f1b3e Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Thu, 7 Sep 2017 00:14:01 +0000 Subject: [PATCH 1/2] Update ocicni to 73f1309d6bc5c3eac78c1382408921cd771ff22e Signed-off-by: Lantao Liu --- vendor.conf | 2 +- .../cri-o/ocicni/{ => pkg/ocicni}/noop.go | 2 +- .../cri-o/ocicni/{ => pkg/ocicni}/ocicni.go | 76 ++++++++++++++++++- .../cri-o/ocicni/{ => pkg/ocicni}/types.go | 2 +- .../cri-o/ocicni/{ => pkg/ocicni}/util.go | 0 5 files changed, 77 insertions(+), 5 deletions(-) rename vendor/github.com/cri-o/ocicni/{ => pkg/ocicni}/noop.go (80%) rename vendor/github.com/cri-o/ocicni/{ => pkg/ocicni}/ocicni.go (79%) rename vendor/github.com/cri-o/ocicni/{ => pkg/ocicni}/types.go (97%) rename vendor/github.com/cri-o/ocicni/{ => pkg/ocicni}/util.go (100%) diff --git a/vendor.conf b/vendor.conf index 615d18b91..afa55c589 100644 --- a/vendor.conf +++ b/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 diff --git a/vendor/github.com/cri-o/ocicni/noop.go b/vendor/github.com/cri-o/ocicni/pkg/ocicni/noop.go similarity index 80% rename from vendor/github.com/cri-o/ocicni/noop.go rename to vendor/github.com/cri-o/ocicni/pkg/ocicni/noop.go index fa2e0ee43..9f315a7c6 100644 --- a/vendor/github.com/cri-o/ocicni/noop.go +++ b/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 } diff --git a/vendor/github.com/cri-o/ocicni/ocicni.go b/vendor/github.com/cri-o/ocicni/pkg/ocicni/ocicni.go similarity index 79% rename from vendor/github.com/cri-o/ocicni/ocicni.go rename to vendor/github.com/cri-o/ocicni/pkg/ocicni/ocicni.go index 3b8b62c6a..03918bfa4 100644 --- a/vendor/github.com/cri-o/ocicni/ocicni.go +++ b/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) { - ip, err := getContainerIP(plugin.nsenterPath, netnsPath, DefaultInterfaceName, "-4") +func (plugin *cniNetworkPlugin) GetPodNetworkStatus(podNetwork PodNetwork) (string, error) { + plugin.podLock(podNetwork).Lock() + defer plugin.podUnlock(podNetwork) + + ip, err := getContainerIP(plugin.nsenterPath, podNetwork.NetNS, DefaultInterfaceName, "-4") if err != nil { return "", err } diff --git a/vendor/github.com/cri-o/ocicni/types.go b/vendor/github.com/cri-o/ocicni/pkg/ocicni/types.go similarity index 97% rename from vendor/github.com/cri-o/ocicni/types.go rename to vendor/github.com/cri-o/ocicni/pkg/ocicni/types.go index 00ad00ff9..a272e92e7 100644 --- a/vendor/github.com/cri-o/ocicni/types.go +++ b/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 diff --git a/vendor/github.com/cri-o/ocicni/util.go b/vendor/github.com/cri-o/ocicni/pkg/ocicni/util.go similarity index 100% rename from vendor/github.com/cri-o/ocicni/util.go rename to vendor/github.com/cri-o/ocicni/pkg/ocicni/util.go From f36ef46b35461b8e9e6b6ec496c0471dfa7a6ba6 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Thu, 7 Sep 2017 00:14:12 +0000 Subject: [PATCH 2/2] Use new ocicni. Signed-off-by: Lantao Liu --- pkg/server/sandbox_run.go | 2 +- pkg/server/sandbox_run_test.go | 2 +- pkg/server/sandbox_status.go | 11 ++++++++++- pkg/server/sandbox_stop.go | 2 +- pkg/server/service.go | 2 +- pkg/server/testing/fake_cni_plugin.go | 8 ++++---- 6 files changed, 18 insertions(+), 9 deletions(-) diff --git a/pkg/server/sandbox_run.go b/pkg/server/sandbox_run.go index 8bc8ffddb..833188eae 100644 --- a/pkg/server/sandbox_run.go +++ b/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" diff --git a/pkg/server/sandbox_run_test.go b/pkg/server/sandbox_run_test.go index 0a1996bf6..d3126c225 100644 --- a/pkg/server/sandbox_run_test.go +++ b/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" diff --git a/pkg/server/sandbox_status.go b/pkg/server/sandbox_status.go index c2cd1c825..2e84cedc7 100644 --- a/pkg/server/sandbox_status.go +++ b/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 = "" diff --git a/pkg/server/sandbox_stop.go b/pkg/server/sandbox_stop.go index a19ea1454..cd56aebe7 100644 --- a/pkg/server/sandbox_stop.go +++ b/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" diff --git a/pkg/server/service.go b/pkg/server/service.go index fca4e1070..f64655e0f 100644 --- a/pkg/server/service.go +++ b/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" diff --git a/pkg/server/testing/fake_cni_plugin.go b/pkg/server/testing/fake_cni_plugin.go index c3f212032..ec964cc1a 100644 --- a/pkg/server/testing/fake_cni_plugin.go +++ b/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") }