Merge pull request #489 from dcbw/fix-pod-namespace-name-id

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

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

@@ -126,7 +126,11 @@ func hostNetNsPath() (string, error) {
 
 type sandbox struct {
 	id        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,7 +148,6 @@ type sandbox struct {
 }
 
 const (
-	podDefaultNamespace = "default"
 	defaultShmSize  = 64 * 1024 * 1024
 	nsRunDir        = "/var/run/netns"
 	podInfraCommand = "/pause"
@@ -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
+