Merge pull request #525 from abhi/cniip

Caching IP allocated by CNI plugin

pkg/server/sandbox_run.go

@@ -119,6 +119,18 @@ func (c *criContainerdService) RunPodSandbox(ctx context.Context, r *runtime.Run
 				}
 			}
 		}()
+		ip, err := c.netPlugin.GetPodNetworkStatus(podNetwork)
+		if err != nil {
+			return nil, fmt.Errorf("failed to get network status for sandbox %q: %v", id, err)
+		}
+		// Certain VM based solutions like clear containers (Issue kubernetes-incubator/cri-containerd#524)
+		//  rely on the assumption that CRI shim will not be querying the network namespace to check the
+		// network states such as IP.
+		// In furture runtime implementation should avoid relying on CRI shim implementation details.
+		// In this case however caching the IP will add a subtle performance enhancement by avoiding
+		// calls to network namespace of the pod to query the IP of the veth interface on every
+		// SandboxStatus request.
+		sandbox.IP = ip
 	}
 
 	// Create sandbox container.

pkg/server/sandbox_status.go

@@ -23,7 +23,6 @@ import (
 
 	"github.com/containerd/containerd"
 	"github.com/containerd/containerd/errdefs"
-	"github.com/cri-o/ocicni/pkg/ocicni"
 	"github.com/golang/glog"
 	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"golang.org/x/net/context"
@@ -57,11 +56,7 @@ func (c *criContainerdService) PodSandboxStatus(ctx context.Context, r *runtime.
 		processStatus = taskStatus.Status
 	}
 
-	ip, err := c.getIP(sandbox)
+	ip := c.getIP(sandbox)
-	if err != nil {
-		return nil, fmt.Errorf("failed to get sandbox ip: %v", err)
-	}
-
 	ctrInfo, err := sandbox.Container.Info(ctx)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get sandbox container info: %v", err)
@@ -79,42 +74,21 @@ func (c *criContainerdService) PodSandboxStatus(ctx context.Context, r *runtime.
 	}, nil
 }
 
-func (c *criContainerdService) getIP(sandbox sandboxstore.Sandbox) (string, error) {
+func (c *criContainerdService) getIP(sandbox sandboxstore.Sandbox) string {
 	config := sandbox.Config
 
 	if config.GetLinux().GetSecurityContext().GetNamespaceOptions().GetHostNetwork() {
 		// For sandboxes using the host network we are not
 		// responsible for reporting the IP.
-		return "", nil
+		return ""
-	}
-
-	if err := c.netPlugin.Status(); err != nil {
-		// If the network is not ready then there is nothing to report.
-		glog.V(4).Infof("getIP: unable to get sandbox %q network status: network plugin not ready.", sandbox.ID)
-		return "", nil
 	}
 
 	// The network namespace has been closed.
 	if sandbox.NetNS == nil || sandbox.NetNS.Closed() {
-		return "", nil
+		return ""
 	}
 
-	podNetwork := ocicni.PodNetwork{
+	return sandbox.IP
-		Name:         config.GetMetadata().GetName(),
-		Namespace:    config.GetMetadata().GetNamespace(),
-		ID:           sandbox.ID,
-		NetNS:        sandbox.NetNSPath,
-		PortMappings: toCNIPortMappings(config.GetPortMappings()),
-	}
-
-	ip, err := c.netPlugin.GetPodNetworkStatus(podNetwork)
-	if err == nil {
-		return ip, nil
-	}
-
-	// Ignore the error on network status
-	glog.V(4).Infof("getIP: failed to read sandbox %q IP from plugin: %v", sandbox.ID, err)
-	return "", nil
 }
 
 // toCRISandboxStatus converts sandbox metadata into CRI pod sandbox status.

pkg/store/sandbox/sandbox.go

@@ -34,6 +34,8 @@ type Sandbox struct {
 	Container containerd.Container
 	// CNI network namespace client
 	NetNS *NetNS
+	// IP of Pod if it is attached to non host network
+	IP string
 }
 
 // Store stores all sandboxes.