From 0169dce5858d2c3c81e16cd06fe1cfb51194c4f9 Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Thu, 16 Mar 2017 10:42:31 +0100 Subject: [PATCH 1/2] container: Add image ID and name to the container status Kubelet 1.6 seems to request that those fields must be present. Signed-off-by: Samuel Ortiz --- server/container_status.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/server/container_status.go b/server/container_status.go index 4afd9118..4d3dd4bf 100644 --- a/server/container_status.go +++ b/server/container_status.go @@ -16,18 +16,27 @@ func (s *Server) ContainerStatus(ctx context.Context, req *pb.ContainerStatusReq return nil, err } - if err := s.runtime.UpdateStatus(c); err != nil { + if err = s.runtime.UpdateStatus(c); err != nil { return nil, err } containerID := c.ID() + image := c.Image() resp := &pb.ContainerStatusResponse{ Status: &pb.ContainerStatus{ Id: containerID, Metadata: c.Metadata(), + Image: image, }, } + status, err := s.images.ImageStatus(s.imageContext, image.Image) + if err != nil { + return nil, err + } + + resp.Status.ImageRef = status.ID + cState := s.runtime.ContainerStatus(c) rStatus := pb.ContainerState_CONTAINER_UNKNOWN From 4ac92d73e4caae8788ae414b611dbcc0089d5f2a Mon Sep 17 00:00:00 2001 From: Samuel Ortiz Date: Thu, 16 Mar 2017 10:59:41 +0100 Subject: [PATCH 2/2] container: Fix the OCI Process Args string build The way we build the OCI Process Args slice is incorrect. With the current implementation we may for example end up building this slice with only the entry point arguments, if the kubelet passed information is missing the Command slice. We also will end up building the Args slice with the Image config process arguments, without the defined entry point, if kubelet does not tell us anything about the container process command to be run. This patch fixes that by favoring the kubelet ContainerConfig information. If that is missing, we try to complete it with the container image information. We always use ContainerConfig.Command[] or ImageConfig.EntryPoint[] as the first OCI Process Args slice entries. Signed-off-by: Samuel Ortiz --- server/container_create.go | 86 +++++++++++++++++++++++++++++--------- 1 file changed, 67 insertions(+), 19 deletions(-) diff --git a/server/container_create.go b/server/container_create.go index 06c172da..0a051f0b 100644 --- a/server/container_create.go +++ b/server/container_create.go @@ -13,6 +13,7 @@ import ( "github.com/kubernetes-incubator/cri-o/oci" "github.com/kubernetes-incubator/cri-o/server/apparmor" "github.com/kubernetes-incubator/cri-o/server/seccomp" + "github.com/opencontainers/image-spec/specs-go/v1" "github.com/opencontainers/runc/libcontainer/label" "github.com/opencontainers/runtime-tools/generate" "golang.org/x/net/context" @@ -56,6 +57,69 @@ func addOciBindMounts(sb *sandbox, containerConfig *pb.ContainerConfig, specgen return nil } +// buildOCIProcessArgs build an OCI compatible process arguments slice. +func buildOCIProcessArgs(containerKubeConfig *pb.ContainerConfig, imageOCIConfig *v1.Image) ([]string, error) { + processArgs := []string{} + + kubeCommands := containerKubeConfig.Command + kubeArgs := containerKubeConfig.Args + + if imageOCIConfig == nil { + // HACK We should error out here, not being able to get an Image config is fatal. + // When https://github.com/kubernetes-incubator/cri-o/issues/395 is fixed + // we'll remove that one and return an error here. + if containerKubeConfig.Metadata != nil { + logrus.Errorf("empty image config for %s", containerKubeConfig.Metadata.Name) + + // HACK until https://github.com/kubernetes-incubator/cri-o/issues/395 is fixed. + // If the container is kubeadm's dummy, imageOCIConfig is nil, and both + // kubeCommands and kubeArgs are empty. So we set processArgs to /pause as the + // dummy container is just a pause one. + // (See https://github.com/kubernetes/kubernetes/blob/master/cmd/kubeadm/app/master/templates.go) + if containerKubeConfig.Metadata.Name == "dummy" { + return []string{podInfraCommand}, nil + } + } else { + logrus.Errorf("empty image config for %s", containerKubeConfig.Image.Image) + } + } + + // We got an OCI Image configuration. + // We will only use it if the kubelet information is incomplete. + if kubeCommands == nil { + if imageOCIConfig != nil && imageOCIConfig.Config.Entrypoint != nil { + processArgs = append(processArgs, imageOCIConfig.Config.Entrypoint...) + } + } else { + processArgs = append(processArgs, kubeCommands...) + } + + if kubeArgs == nil { + // Our kubelet command arguments slice is empty. + // We will use the OCI Image configuration only if we already + // have found a process command, i.e. if processArgs is no + // longer empty. No doing so will have us create a process + // arguments slice starting with the OCI Image command arguments + // as the entry point. + if processArgs != nil && + imageOCIConfig != nil && + imageOCIConfig.Config.Entrypoint != nil && imageOCIConfig.Config.Cmd != nil { + processArgs = append(processArgs, imageOCIConfig.Config.Cmd...) + } + } else { + processArgs = append(processArgs, kubeArgs...) + } + + if processArgs == nil { + // Use a reasonable default if everything failed. + processArgs = []string{"/bin/sh"} + } + + logrus.Debugf("OCI process args %v", processArgs) + + return processArgs, nil +} + // CreateContainer creates a new container in specified PodSandbox func (s *Server) CreateContainer(ctx context.Context, req *pb.CreateContainerRequest) (res *pb.CreateContainerResponse, err error) { logrus.Debugf("CreateContainerRequest %+v", req) @@ -145,19 +209,6 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string, // creates a spec Generator with the default spec. specgen := generate.New() - processArgs := []string{} - commands := containerConfig.Command - args := containerConfig.Args - if commands == nil && args == nil { - processArgs = nil - } - if commands != nil { - processArgs = append(processArgs, commands...) - } - if args != nil { - processArgs = append(processArgs, args...) - } - cwd := containerConfig.WorkingDir if cwd == "" { cwd = "/" @@ -372,12 +423,9 @@ func (s *Server) createSandboxContainer(ctx context.Context, containerID string, return nil, fmt.Errorf("failed to mount container %s(%s): %v", containerName, containerID, err) } - if processArgs == nil { - if containerInfo.Config != nil && len(containerInfo.Config.Config.Cmd) > 0 { - processArgs = containerInfo.Config.Config.Cmd - } else { - processArgs = []string{"/bin/sh"} - } + processArgs, err := buildOCIProcessArgs(containerConfig, containerInfo.Config) + if err != nil { + return nil, err } specgen.SetProcessArgs(processArgs)