From 5d637f015ddc4fa63843b91641120e3fc069bfcd Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Wed, 30 Aug 2017 00:57:26 +0200 Subject: [PATCH 1/6] *: store sandbox IP Don't call into net namespace on every status call Signed-off-by: Antonio Murdaca --- libkpod/container_server.go | 3 +++ libkpod/sandbox/sandbox.go | 12 ++++++++++++ pkg/annotations/annotations.go | 3 +++ server/sandbox_run.go | 18 +++++++++++------- server/sandbox_status.go | 12 +----------- 5 files changed, 30 insertions(+), 18 deletions(-) diff --git a/libkpod/container_server.go b/libkpod/container_server.go index c51ea4b5..4a65f903 100644 --- a/libkpod/container_server.go +++ b/libkpod/container_server.go @@ -294,6 +294,8 @@ func (c *ContainerServer) LoadSandbox(id string) error { return err } + ip := m.Annotations[annotations.IP] + processLabel, mountLabel, err := label.InitLabels(label.DupSecOpt(m.Process.SelinuxLabel)) if err != nil { return err @@ -311,6 +313,7 @@ func (c *ContainerServer) LoadSandbox(id string) error { if err != nil { return err } + sb.AddIP(ip) // We add a netNS only if we can load a permanent one. // Otherwise, the sandbox will live in the host namespace. diff --git a/libkpod/sandbox/sandbox.go b/libkpod/sandbox/sandbox.go index a3ba4010..62550762 100644 --- a/libkpod/sandbox/sandbox.go +++ b/libkpod/sandbox/sandbox.go @@ -154,6 +154,8 @@ type Sandbox struct { hostname string portMappings []*hostport.PortMapping stopped bool + // ipv4 or ipv6 cache + ip string } const ( @@ -202,6 +204,16 @@ func New(id, namespace, name, kubeName, logDir string, labels, annotations map[s return sb, nil } +// AddIP stores the ip in the sandbox +func (s *Sandbox) AddIP(ip string) { + s.ip = ip +} + +// IP returns the ip of the sandbox +func (s *Sandbox) IP() string { + return s.ip +} + // ID returns the id of the sandbox func (s *Sandbox) ID() string { return s.id diff --git a/pkg/annotations/annotations.go b/pkg/annotations/annotations.go index 63cc126d..80f943c2 100644 --- a/pkg/annotations/annotations.go +++ b/pkg/annotations/annotations.go @@ -19,6 +19,9 @@ const ( // HostName is the container host name annotation HostName = "io.kubernetes.cri-o.HostName" + // IP is the container ipv4 or ipv6 address + IP = "io.kubernetes.cri-o.IP" + // Image is the container image ID annotation Image = "io.kubernetes.cri-o.Image" diff --git a/server/sandbox_run.go b/server/sandbox_run.go index 815175e2..3f8be488 100644 --- a/server/sandbox_run.go +++ b/server/sandbox_run.go @@ -449,13 +449,6 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest } g.AddAnnotation(annotations.MountPoint, mountPoint) g.SetRootPath(mountPoint) - err = g.SaveToFile(filepath.Join(podContainer.Dir, "config.json"), saveOptions) - if err != nil { - return nil, fmt.Errorf("failed to save template configuration for pod sandbox %s(%s): %v", sb.Name(), id, err) - } - if err = g.SaveToFile(filepath.Join(podContainer.RunDir, "config.json"), saveOptions); err != nil { - return nil, fmt.Errorf("failed to write runtime configuration for pod sandbox %s(%s): %v", sb.Name(), id, err) - } container, err := oci.NewContainer(id, containerName, podContainer.RunDir, logPath, sb.NetNs(), labels, kubeAnnotations, "", "", "", nil, id, false, false, false, sb.Privileged(), sb.Trusted(), podContainer.Dir, created, podContainer.Config.Config.StopSignal) if err != nil { @@ -482,6 +475,9 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest return nil, fmt.Errorf("failed to get valid ipv4 address for container %s in sandbox %s", containerName, id) } + g.AddAnnotation(annotations.IP, ip) + sb.AddIP(ip) + if err = s.hostportManager.Add(id, &hostport.PodPortMapping{ Name: name, PortMappings: portMappings, @@ -494,6 +490,14 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest } } + err = g.SaveToFile(filepath.Join(podContainer.Dir, "config.json"), saveOptions) + if err != nil { + return nil, fmt.Errorf("failed to save template configuration for pod sandbox %s(%s): %v", sb.Name(), id, err) + } + if err = g.SaveToFile(filepath.Join(podContainer.RunDir, "config.json"), saveOptions); err != nil { + return nil, fmt.Errorf("failed to write runtime configuration for pod sandbox %s(%s): %v", sb.Name(), id, err) + } + if err = s.runContainer(container, sb.CgroupParent()); err != nil { return nil, err } diff --git a/server/sandbox_status.go b/server/sandbox_status.go index db95222d..f5b6dd09 100644 --- a/server/sandbox_status.go +++ b/server/sandbox_status.go @@ -18,16 +18,6 @@ func (s *Server) PodSandboxStatus(ctx context.Context, req *pb.PodSandboxStatusR podInfraContainer := sb.InfraContainer() cState := s.Runtime().ContainerStatus(podInfraContainer) - netNsPath, err := podInfraContainer.NetNsPath() - if err != nil { - return nil, err - } - ip, err := s.netPlugin.GetContainerNetworkStatus(netNsPath, sb.Namespace(), sb.KubeName(), sb.ID()) - if err != nil { - // ignore the error on network status - ip = "" - } - rStatus := pb.PodSandboxState_SANDBOX_NOTREADY if cState.Status == oci.ContainerStateRunning { rStatus = pb.PodSandboxState_SANDBOX_READY @@ -38,7 +28,7 @@ func (s *Server) PodSandboxStatus(ctx context.Context, req *pb.PodSandboxStatusR Status: &pb.PodSandboxStatus{ Id: sandboxID, CreatedAt: podInfraContainer.CreatedAt().UnixNano(), - Network: &pb.PodSandboxNetworkStatus{Ip: ip}, + Network: &pb.PodSandboxNetworkStatus{Ip: sb.IP()}, State: rStatus, Labels: sb.Labels(), Annotations: sb.Annotations(), From 2ac2832686c0e2f6a7f0a75211ff939771061cd0 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Wed, 30 Aug 2017 01:00:49 +0200 Subject: [PATCH 2/6] server: container_create: store sandbox's ip in annotations So it can be later retrieved when needed (cadvisor) Signed-off-by: Antonio Murdaca --- cmd/kpod/logs.go | 2 +- server/container_create.go | 1 + server/sandbox_run.go | 18 ++++++++++-------- server/server.go | 8 ++++++++ test/kpod_logs.bats | 2 +- 5 files changed, 21 insertions(+), 10 deletions(-) diff --git a/cmd/kpod/logs.go b/cmd/kpod/logs.go index 995f91cd..0f5fd8fa 100644 --- a/cmd/kpod/logs.go +++ b/cmd/kpod/logs.go @@ -46,7 +46,7 @@ func logsCmd(c *cli.Context) error { if len(args) != 1 { return errors.Errorf("'kpod logs' requires exactly one container name/ID") } - container := args[0] + container := c.Args().First() var opts libkpod.LogOptions opts.Details = c.Bool("details") opts.Follow = c.Bool("follow") diff --git a/server/container_create.go b/server/container_create.go index 55a58407..55b016e1 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -622,6 +622,7 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string, specgen.AddAnnotation(annotations.ImageName, imageName) specgen.AddAnnotation(annotations.ImageRef, imageRef) + specgen.AddAnnotation(annotations.IP, sb.IP()) // bind mount the pod shm specgen.AddBindMount(sb.ShmPath(), "/dev/shm", []string{"rw"}) diff --git a/server/sandbox_run.go b/server/sandbox_run.go index 3f8be488..ab9a6ee8 100644 --- a/server/sandbox_run.go +++ b/server/sandbox_run.go @@ -458,26 +458,23 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest sb.SetInfraContainer(container) + var ip string // setup the network if !hostNetwork { 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) } - if len(portMappings) != 0 { - ip, err := s.netPlugin.GetContainerNetworkStatus(netNsPath, namespace, id, containerName) - if err != nil { - return nil, fmt.Errorf("failed to get network status for container %s in sandbox %s: %v", containerName, id, err) - } + if ip, err = s.netPlugin.GetContainerNetworkStatus(netNsPath, namespace, id, kubeName); err != nil { + return nil, fmt.Errorf("failed to get network status for container %s in sandbox %s: %v", containerName, id, err) + } + if len(portMappings) != 0 { ip4 := net.ParseIP(ip).To4() if ip4 == nil { return nil, fmt.Errorf("failed to get valid ipv4 address for container %s in sandbox %s", containerName, id) } - g.AddAnnotation(annotations.IP, ip) - sb.AddIP(ip) - if err = s.hostportManager.Add(id, &hostport.PodPortMapping{ Name: name, PortMappings: portMappings, @@ -488,8 +485,13 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest } } + } else { + ip = s.BindAddress() } + g.AddAnnotation(annotations.IP, ip) + sb.AddIP(ip) + err = g.SaveToFile(filepath.Join(podContainer.Dir, "config.json"), saveOptions) if err != nil { return nil, fmt.Errorf("failed to save template configuration for pod sandbox %s(%s): %v", sb.Name(), id, err) diff --git a/server/server.go b/server/server.go index 11bc7654..63016ab6 100644 --- a/server/server.go +++ b/server/server.go @@ -66,6 +66,8 @@ type Server struct { appArmorProfile string stream streamService + + bindAddress string } // GetExec returns exec stream request @@ -233,6 +235,7 @@ func New(config *Config) (*Server, error) { return nil, err } } + s.bindAddress = bindAddress.String() _, err = net.LookupPort("tcp", config.StreamPort) if err != nil { @@ -289,6 +292,11 @@ func (s *Server) getInfraContainer(id string) *oci.Container { return s.ContainerServer.GetInfraContainer(id) } +// BindAddress is used to retrieve host's IP +func (s *Server) BindAddress() string { + return s.bindAddress +} + // GetSandboxContainer returns the infra container for a given sandbox func (s *Server) GetSandboxContainer(id string) *oci.Container { return s.ContainerServer.GetSandboxContainer(id) diff --git a/test/kpod_logs.bats b/test/kpod_logs.bats index 8dbd36c6..d11b69c1 100644 --- a/test/kpod_logs.bats +++ b/test/kpod_logs.bats @@ -5,7 +5,7 @@ load helpers IMAGE="alpine:latest" ROOT="$TESTDIR/crio" RUNROOT="$TESTDIR/crio-run" -KPOD_OPTIONS="--root $ROOT --runroot $RUNROOT $STORAGE_OPTS" +KPOD_OPTIONS="--root $ROOT --runroot $RUNROOT ${STORAGE_OPTS}" function teardown() { cleanup_test From 59476988181aa0399086fd054ac472cbab8741b2 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Thu, 31 Aug 2017 11:19:13 +0200 Subject: [PATCH 3/6] test: replace bash CNI plugin with a custom bridge Because we need a working CNI plugin to setup a correct netns so sandbox_run can grab a working IP address. Signed-off-by: Antonio Murdaca --- Dockerfile | 19 +++++++++-- contrib/test/crio-integration-playbook.yaml | 16 ++++++--- test/helpers.bash | 14 ++++++-- test/network.bats | 2 ++ test/plugin_test_args.bash | 37 --------------------- 5 files changed, 42 insertions(+), 46 deletions(-) delete mode 100755 test/plugin_test_args.bash diff --git a/Dockerfile b/Dockerfile index ce4b4af1..7bb06579 100644 --- a/Dockerfile +++ b/Dockerfile @@ -76,6 +76,23 @@ RUN set -x \ && cp bin/* /opt/cni/bin/ \ && rm -rf "$GOPATH" +# Install custom CNI bridge test plugin +# XXX: this plugin is meant to be a replacement for the old "test_plugin_args.bash" +# we need this in testing because sandbox_run now gather IP address and the mock +# plugin wasn't able to properly setup the net ns. +# The bridge is based on the same commit as the one above. +#ENV CNI_COMMIT 6bfe036c38c8e1410f1acaa4b2ee16f1851472e4 +ENV CNI_TEST_BRANCH custom-bridge +RUN set -x \ + && export GOPATH="$(mktemp -d)" \ + && git clone https://github.com/runcom/plugins.git "$GOPATH/src/github.com/containernetworking/plugins" \ + && cd "$GOPATH/src/github.com/containernetworking/plugins" \ + && git checkout -q "$CNI_TEST_BRANCH" \ + && ./build.sh \ + && mkdir -p /opt/cni/bin \ + && cp bin/bridge /opt/cni/bin/bridge-custom \ + && rm -rf "$GOPATH" + # Install crictl ENV CRICTL_COMMIT 16e6fe4d7199c5689db4630a9330e6a8a12cecd1 RUN set -x \ @@ -87,8 +104,6 @@ RUN set -x \ && cp "$GOPATH"/bin/crictl /usr/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/contrib/test/crio-integration-playbook.yaml b/contrib/test/crio-integration-playbook.yaml index 0ad67aa4..77492cf5 100644 --- a/contrib/test/crio-integration-playbook.yaml +++ b/contrib/test/crio-integration-playbook.yaml @@ -151,8 +151,6 @@ repo: https://github.com/containernetworking/plugins dest: /root/src/github.com/containernetworking/plugins version: "{{ cni_commit }}" - async: 600 - poll: 10 - name: Git fetch the PR shell: "git fetch origin +refs/pull/{{ pullrequest }}/head:refs/remotes/origin/pr/{{ pullrequest }}" args: @@ -225,10 +223,18 @@ regexp: 'execute time bats --tap \$TESTS' state: present when: xunit - - name: Copy plugin args so tests dont hang - shell: "cp test/plugin_test_args.bash /opt/cni/bin/" + - name: git clone cni test repo + git: + repo: https://github.com/runcom/plugins + dest: /root/src/github.com/containernetworking/plugins + version: "custom-bridge" + force: yes + - name: Build cni test networking + shell: ./build.sh args: - chdir: /root/src/github.com/kubernetes-incubator/cri-o/ + chdir: /root/src/github.com/containernetworking/plugins + - name: cp custom-bridge to opt bin + shell: cp /root/src/github.com/containernetworking/plugins/bin/bridge /opt/cni/bin/bridge-custom # k8s builds with go1.8.x, rhel, fedora don't have it yet - name: install Golang upstream in Fedora/RHEL shell: | diff --git a/test/helpers.bash b/test/helpers.bash index e203283b..3cccb056 100644 --- a/test/helpers.bash +++ b/test/helpers.bash @@ -407,8 +407,18 @@ function prepare_plugin_test_args_network_conf() { cat >$CRIO_CNI_CONFIG/10-plugin-test-args.conf <<-EOF { "cniVersion": "0.2.0", - "name": "crionet", - "type": "plugin_test_args.bash" + "name": "crionet_test_args", + "type": "bridge-custom", + "bridge": "cni0", + "isGateway": true, + "ipMasq": true, + "ipam": { + "type": "host-local", + "subnet": "$1", + "routes": [ + { "dst": "0.0.0.0/0" } + ] + } } EOF diff --git a/test/network.bats b/test/network.bats index c0629cf9..8121ca48 100644 --- a/test/network.bats +++ b/test/network.bats @@ -85,6 +85,8 @@ load helpers [ "$FOUND_K8S_POD_NAMESPACE" = "redhat.test.crio" ] [ "$FOUND_K8S_POD_NAME" = "podsandbox1" ] + rm -rf /tmp/plugin_test_args.out + cleanup_pods stop_crio } diff --git a/test/plugin_test_args.bash b/test/plugin_test_args.bash deleted file mode 100755 index fad0d943..00000000 --- a/test/plugin_test_args.bash +++ /dev/null @@ -1,37 +0,0 @@ -#!/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 - From e1125af43518b9a7f60b24298ac737f3d98559b2 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Mon, 4 Sep 2017 18:11:32 +0200 Subject: [PATCH 4/6] server: expose container Name and IP Signed-off-by: Antonio Murdaca --- server/container_create.go | 5 +++++ server/inspect.go | 10 +++++++++- server/sandbox_run.go | 3 +++ 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/server/container_create.go b/server/container_create.go index 55b016e1..bd2810f0 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -396,6 +396,11 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string, specgen.AddAnnotation(k, v) } } + if labels != nil { + for k, v := range labels { + specgen.AddAnnotation(k, v) + } + } // set this container's apparmor profile if it is set by sandbox if s.appArmorEnabled { diff --git a/server/inspect.go b/server/inspect.go index 0d2a6c12..0bee7c5e 100644 --- a/server/inspect.go +++ b/server/inspect.go @@ -11,6 +11,7 @@ import ( // ContainerInfo stores information about containers type ContainerInfo struct { + Name string `json:"name"` Pid int `json:"pid"` Image string `json:"image"` CreatedTime int64 `json:"created_time"` @@ -19,6 +20,7 @@ type ContainerInfo struct { LogPath string `json:"log_path"` Root string `json:"root"` Sandbox string `json:"sandbox"` + IP string `json:"ip_address"` } // CrioInfo stores information about the crio daemon @@ -60,8 +62,13 @@ func (s *Server) GetInfoMux() *bone.Mux { http.Error(w, fmt.Sprintf("container %s state is nil", containerID), http.StatusNotFound) return } - + sb := s.getSandbox(ctr.Sandbox()) + if sb == nil { + http.Error(w, fmt.Sprintf("can't find the sandbox for container id, sandbox id %s: %s", containerID, ctr.Sandbox()), http.StatusNotFound) + return + } ci := ContainerInfo{ + Name: ctr.Name(), Pid: ctrState.Pid, Image: ctr.Image(), CreatedTime: ctrState.Created.UnixNano(), @@ -70,6 +77,7 @@ func (s *Server) GetInfoMux() *bone.Mux { Root: ctr.MountPoint(), LogPath: filepath.Dir(ctr.LogPath()), Sandbox: ctr.Sandbox(), + IP: sb.IP(), } js, err := json.Marshal(ci) if err != nil { diff --git a/server/sandbox_run.go b/server/sandbox_run.go index ab9a6ee8..ac9823c8 100644 --- a/server/sandbox_run.go +++ b/server/sandbox_run.go @@ -369,6 +369,9 @@ func (s *Server) RunPodSandbox(ctx context.Context, req *pb.RunPodSandboxRequest for k, v := range kubeAnnotations { g.AddAnnotation(k, v) } + for k, v := range labels { + g.AddAnnotation(k, v) + } // extract linux sysctls from annotations and pass down to oci runtime safe, unsafe, err := SysctlsFromPodAnnotations(kubeAnnotations) From f9bf4b15e813f565d9bc1ad7cad12c475a725b90 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Tue, 5 Sep 2017 09:58:42 +0200 Subject: [PATCH 5/6] server: inspect: send full ctr log path Signed-off-by: Antonio Murdaca --- server/inspect.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/inspect.go b/server/inspect.go index 0bee7c5e..77d218d3 100644 --- a/server/inspect.go +++ b/server/inspect.go @@ -4,7 +4,6 @@ import ( "encoding/json" "fmt" "net/http" - "path/filepath" "github.com/go-zoo/bone" ) @@ -75,7 +74,7 @@ func (s *Server) GetInfoMux() *bone.Mux { Labels: ctr.Labels(), Annotations: ctr.Annotations(), Root: ctr.MountPoint(), - LogPath: filepath.Dir(ctr.LogPath()), + LogPath: ctr.LogPath(), Sandbox: ctr.Sandbox(), IP: sb.IP(), } From a51bc9753f5e1a23f94e494441fd027243d743e2 Mon Sep 17 00:00:00 2001 From: Antonio Murdaca Date: Tue, 5 Sep 2017 17:21:17 +0200 Subject: [PATCH 6/6] oci: add a note about crio-conmon- sub-cgroup with cgroupfs Signed-off-by: Antonio Murdaca --- oci/oci.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/oci/oci.go b/oci/oci.go index ef5027c5..7f03ef0f 100644 --- a/oci/oci.go +++ b/oci/oci.go @@ -202,6 +202,10 @@ func (r *Runtime) CreateContainer(c *Container, cgroupParent string) error { if err != nil { logrus.Warnf("Failed to add conmon to cgroupfs sandbox cgroup: %v", err) } else { + // XXX: this defer does nothing as the cgroup can't be deleted cause + // it contains the conmon pid in tasks + // we need to remove this defer and delete the cgroup once conmon exits + // maybe need a conmon monitor? defer control.Delete() if err := control.Add(cgroups.Process{Pid: cmd.Process.Pid}); err != nil { logrus.Warnf("Failed to add conmon to cgroupfs sandbox cgroup: %v", err)