From 13f6e956855304816fdb79568ae7633307d65a58 Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Thu, 4 May 2017 11:41:15 -0500 Subject: [PATCH] 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 --- Dockerfile | 2 ++ server/sandbox.go | 17 ++++++++++------- server/sandbox_run.go | 11 ++++++----- server/sandbox_status.go | 3 +-- server/sandbox_stop.go | 5 ++--- test/helpers.bash | 20 +++++++++++++++++++- test/network.bats | 16 ++++++++++++++++ test/plugin_test_args.bash | 37 +++++++++++++++++++++++++++++++++++++ 8 files changed, 93 insertions(+), 18 deletions(-) create mode 100755 test/plugin_test_args.bash diff --git a/Dockerfile b/Dockerfile index d24b4040..b93728c1 100644 --- a/Dockerfile +++ b/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 diff --git a/server/sandbox.go b/server/sandbox.go index 4fdf7491..0f57f557 100644 --- a/server/sandbox.go +++ b/server/sandbox.go @@ -125,8 +125,12 @@ func hostNetNsPath() (string, error) { } type sandbox struct { - id string - name string + id string + namespace string + // OCI pod name (eg "--") + name string + // Kubernetes pod name (eg, "") + 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 - nsRunDir = "/var/run/netns" - podInfraCommand = "/pause" + defaultShmSize = 64 * 1024 * 1024 + nsRunDir = "/var/run/netns" + 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 { diff --git a/server/sandbox_run.go b/server/sandbox_run.go index d8dbff12..88797343 100644 --- a/server/sandbox_run.go +++ b/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 - if name == "" { + kubeName := req.GetConfig().GetMetadata().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, podNamespace, id, containerName); err != nil { + if err = s.netPlugin.SetUpPod(netNsPath, namespace, kubeName, id); err != nil { return nil, fmt.Errorf("failed to create network for container %s in sandbox %s: %v", containerName, id, err) } } diff --git a/server/sandbox_status.go b/server/sandbox_status.go index 7f8b241f..15d35260 100644 --- a/server/sandbox_status.go +++ b/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, podNamespace, sb.id, podInfraContainer.Name()) + ip, err := s.netPlugin.GetContainerNetworkStatus(netNsPath, sb.namespace, sb.kubeName, sb.id) if err != nil { // ignore the error on network status ip = "" diff --git a/server/sandbox_stop.go b/server/sandbox_stop.go index 002dbb24..a6f8d32b 100644 --- a/server/sandbox_stop.go +++ b/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. diff --git a/test/helpers.bash b/test/helpers.bash index 7844916b..4a244c16 100644 --- a/test/helpers.bash +++ b/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` diff --git a/test/network.bats b/test/network.bats index a044b6fb..39480d40 100644 --- a/test/network.bats +++ b/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 +} diff --git a/test/plugin_test_args.bash b/test/plugin_test_args.bash new file mode 100755 index 00000000..fad0d943 --- /dev/null +++ b/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 +