sandbox: pass correct pod Namespace/Name to network plugins and fix id/name ordering

Two issues:
1) pod Namespace was always set to "", which prevents plugins from figuring out
what the actual pod is, and from getting more info about that pod from the
runtime via out-of-band mechanisms

2) the pod Name and ID arguments were switched, further preventing #1

Signed-off-by: Dan Williams <dcbw@redhat.com>

Dockerfile

@@ -66,6 +66,8 @@ RUN set -x \
        && cp bin/* /opt/cni/bin/ \
        && rm -rf "$GOPATH"
 
+COPY test/plugin_test_args.bash /opt/cni/bin/plugin_test_args.bash
+
 # Make sure we have some policy for pulling images
 RUN mkdir -p /etc/containers
 COPY test/policy.json /etc/containers/policy.json

server/sandbox.go

@@ -125,8 +125,12 @@ func hostNetNsPath() (string, error) {
 }
 
 type sandbox struct {
-	id             string
+	id        string
-	name           string
+	namespace string
+	// OCI pod name (eg "<namespace>-<name>-<attempt>")
+	name string
+	// Kubernetes pod name (eg, "<name>")
+	kubeName       string
 	logDir         string
 	labels         fields.Set
 	annotations    map[string]string
@@ -144,10 +148,9 @@ type sandbox struct {
 }
 
 const (
-	podDefaultNamespace = "default"
+	defaultShmSize  = 64 * 1024 * 1024
-	defaultShmSize      = 64 * 1024 * 1024
+	nsRunDir        = "/var/run/netns"
-	nsRunDir            = "/var/run/netns"
+	podInfraCommand = "/pause"
-	podInfraCommand     = "/pause"
 )
 
 var (
@@ -254,7 +257,7 @@ func (s *Server) generatePodIDandName(name string, namespace string, attempt uin
 		id  = stringid.GenerateNonCryptoID()
 	)
 	if namespace == "" {
-		namespace = podDefaultNamespace
+		return "", "", fmt.Errorf("cannot generate pod ID without namespace")
 	}
 
 	if name, err = s.reservePodName(id, fmt.Sprintf("%s-%s-%v", namespace, name, attempt)); err != nil {

server/sandbox_run.go

@@ -71,15 +71,15 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 	logrus.Debugf("RunPodSandboxRequest %+v", req)
 	var processLabel, mountLabel, netNsPath, resolvPath string
 	// process req.Name
-	name := req.GetConfig().GetMetadata().Name
+	kubeName := req.GetConfig().GetMetadata().Name
-	if name == "" {
+	if kubeName == "" {
 		return nil, fmt.Errorf("PodSandboxConfig.Name should not be empty")
 	}
 
 	namespace := req.GetConfig().GetMetadata().Namespace
 	attempt := req.GetConfig().GetMetadata().Attempt
 
-	id, name, err := s.generatePodIDandName(name, namespace, attempt)
+	id, name, err := s.generatePodIDandName(kubeName, namespace, attempt)
 	if err != nil {
 		return nil, err
 	}
@@ -268,7 +268,9 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 
 	sb := &sandbox{
 		id:           id,
+		namespace:    namespace,
 		name:         name,
+		kubeName:     kubeName,
 		logDir:       logDir,
 		labels:       labels,
 		annotations:  annotations,
@@ -405,8 +407,7 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest
 
 	// setup the network
 	if !hostNetwork {
-		podNamespace := ""
+		if err = s.netPlugin.SetUpPod(netNsPath, namespace, kubeName, id); err != nil {
-		if err = s.netPlugin.SetUpPod(netNsPath, podNamespace, id, containerName); err != nil {
 			return nil, fmt.Errorf("failed to create network for container %s in sandbox %s: %v", containerName, id, err)
 		}
 	}

server/sandbox_status.go

@@ -27,8 +27,7 @@ func (s *Server) PodSandboxStatus(ctx context.Context, req *pb.PodSandboxStatusR
 	if err != nil {
 		return nil, err
 	}
-	podNamespace := ""
+	ip, err := s.netPlugin.GetContainerNetworkStatus(netNsPath, sb.namespace, sb.kubeName, sb.id)
-	ip, err := s.netPlugin.GetContainerNetworkStatus(netNsPath, podNamespace, sb.id, podInfraContainer.Name())
 	if err != nil {
 		// ignore the error on network status
 		ip = ""

server/sandbox_stop.go

@@ -19,20 +19,19 @@ func (s *Server) StopPodSandbox(ctx context.Context, req *pb.StopPodSandboxReque
 		return nil, err
 	}
 
-	podNamespace := ""
 	podInfraContainer := sb.infraContainer
 	netnsPath, err := podInfraContainer.NetNsPath()
 	if err != nil {
 		return nil, err
 	}
 	if _, err := os.Stat(netnsPath); err == nil {
-		if err2 := s.netPlugin.TearDownPod(netnsPath, podNamespace, sb.id, podInfraContainer.Name()); err2 != nil {
+		if err2 := s.netPlugin.TearDownPod(netnsPath, sb.namespace, sb.kubeName, sb.id); err2 != nil {
 			return nil, fmt.Errorf("failed to destroy network for container %s in sandbox %s: %v",
 				podInfraContainer.Name(), sb.id, err2)
 		}
 	} else if !os.IsNotExist(err) { // it's ok for netnsPath to *not* exist
 		return nil, fmt.Errorf("failed to stat netns path for container %s in sandbox %s before tearing down the network: %v",
-			podInfraContainer.Name(), sb.id, err)
+			sb.name, sb.id, err)
 	}
 
 	// Close the sandbox networking namespace.

test/helpers.bash

@@ -149,7 +149,12 @@ function start_ocid() {
 	"$OCID_BINARY" --conmon "$CONMON_BINARY" --listen "$OCID_SOCKET" --runtime "$RUNTIME_BINARY" --root "$TESTDIR/ocid" --runroot "$TESTDIR/ocid-run" $STORAGE_OPTS --seccomp-profile "$seccomp" --apparmor-profile "$apparmor" --cni-config-dir "$OCID_CNI_CONFIG" --signature-policy "$INTEGRATION_ROOT"/policy.json --config /dev/null config >$OCID_CONFIG
 
 	# Prepare the CNI configuration files, we're running with non host networking by default
-	prepare_network_conf $POD_CIDR
+	if [[ -n "$4" ]]; then
+		netfunc="$4"
+	else
+		netfunc="prepare_network_conf"
+	fi
+	${netfunc} $POD_CIDR
 
 	"$OCID_BINARY" --debug --config "$OCID_CONFIG" & OCID_PID=$!
 	wait_until_reachable
@@ -288,6 +293,19 @@ EOF
 	echo 0
 }
 
+function prepare_plugin_test_args_network_conf() {
+	mkdir -p $OCID_CNI_CONFIG
+	cat >$OCID_CNI_CONFIG/10-plugin-test-args.conf <<-EOF
+{
+    "cniVersion": "0.2.0",
+    "name": "ocidnet",
+    "type": "plugin_test_args.bash"
+}
+EOF
+
+	echo 0
+}
+
 function check_pod_cidr() {
         fullnetns=`ocic pod status --id $1 | grep namespace | cut -d ' ' -f 3`
 	netns=`basename $fullnetns`

test/network.bats

@@ -51,3 +51,19 @@ load helpers
 	cleanup_pods
 	stop_ocid
 }
+
+@test "Ensure correct CNI plugin namespace/name/container-id arguments" {
+	start_ocid "" "" "" "prepare_plugin_test_args_network_conf"
+	run ocic pod run --config "$TESTDATA"/sandbox_config.json
+	[ "$status" -eq 0 ]
+
+	. /tmp/plugin_test_args.out
+
+	[ "$FOUND_CNI_CONTAINERID" != "redhat.test.ocid" ]
+	[ "$FOUND_CNI_CONTAINERID" != "podsandbox1" ]
+	[ "$FOUND_K8S_POD_NAMESPACE" = "redhat.test.ocid" ]
+	[ "$FOUND_K8S_POD_NAME" = "podsandbox1" ]
+
+	cleanup_pods
+	stop_ocid
+}

test/plugin_test_args.bash

@@ -0,0 +1,37 @@
+#!/bin/bash
+
+if [[ -z "${CNI_ARGS}" ]]; then
+	exit 1
+fi
+
+IFS=';' read -ra array <<< "${CNI_ARGS}"
+for arg in "${array[@]}"; do
+	IFS='=' read -ra item <<< "${arg}"
+	if [[ "${item[0]}" = "K8S_POD_NAMESPACE" ]]; then
+		K8S_POD_NAMESPACE="${item[1]}"
+	elif [[ "${item[0]}" = "K8S_POD_NAME" ]]; then
+		K8S_POD_NAME="${item[1]}"
+	fi
+done
+
+if [[ -z "${CNI_CONTAINERID}" ]]; then
+	exit 1
+elif [[ -z "${K8S_POD_NAMESPACE}" ]]; then
+	exit 1
+elif [[ -z "${K8S_POD_NAME}" ]]; then
+	exit 1
+fi
+
+echo "FOUND_CNI_CONTAINERID=${CNI_CONTAINERID}" >> /tmp/plugin_test_args.out
+echo "FOUND_K8S_POD_NAMESPACE=${K8S_POD_NAMESPACE}" >> /tmp/plugin_test_args.out
+echo "FOUND_K8S_POD_NAME=${K8S_POD_NAME}" >> /tmp/plugin_test_args.out
+
+cat <<-EOF
+{
+  "cniVersion": "0.2.0",
+  "ip4": {
+    "ip": "1.1.1.1/24"
+  }
+}
+EOF
+