diff --git a/pkg/kubelet/dockertools/docker.go b/pkg/kubelet/dockertools/docker.go index c54ba80d1f2..8d499c4db70 100644 --- a/pkg/kubelet/dockertools/docker.go +++ b/pkg/kubelet/dockertools/docker.go @@ -212,19 +212,15 @@ func (p throttledDockerPuller) IsImagePresent(name string) (bool, error) { const containerNamePrefix = "k8s" // Creates a name which can be reversed to identify both full pod name and container name. -// This function returns stable name, unique name and an unique id. -// Although rand.Uint32() is not really unique, but it's enough for us because error will -// only occur when instances of the same container in the same pod have the same UID. The -// chance is really slim. -func BuildDockerName(dockerName KubeletContainerName, container *api.Container) (string, string, string) { +func BuildDockerName(dockerName KubeletContainerName, container *api.Container) (string, string) { containerName := dockerName.ContainerName + "." + strconv.FormatUint(kubecontainer.HashContainer(container), 16) stableName := fmt.Sprintf("%s_%s_%s_%s", containerNamePrefix, containerName, dockerName.PodFullName, dockerName.PodUID) - UID := fmt.Sprintf("%08x", rand.Uint32()) - return stableName, fmt.Sprintf("%s_%s", stableName, UID), UID + + return stableName, fmt.Sprintf("%s_%08x", stableName, rand.Uint32()) } // Unpacks a container name, returning the pod full name and container name we would have used to diff --git a/pkg/kubelet/dockertools/docker_test.go b/pkg/kubelet/dockertools/docker_test.go index 3edf47ab8ad..569e40ac436 100644 --- a/pkg/kubelet/dockertools/docker_test.go +++ b/pkg/kubelet/dockertools/docker_test.go @@ -119,7 +119,7 @@ func verifyPackUnpack(t *testing.T, podNamespace, podUID, podName, containerName hashutil.DeepHashObject(hasher, *container) computedHash := uint64(hasher.Sum32()) podFullName := fmt.Sprintf("%s_%s", podName, podNamespace) - _, name, _ := BuildDockerName(KubeletContainerName{podFullName, types.UID(podUID), container.Name}, container) + _, name := BuildDockerName(KubeletContainerName{podFullName, types.UID(podUID), container.Name}, container) returned, hash, err := ParseDockerName(name) if err != nil { t.Errorf("Failed to parse Docker container name %q: %v", name, err) diff --git a/pkg/kubelet/dockertools/fake_docker_client.go b/pkg/kubelet/dockertools/fake_docker_client.go index 7aa1545df3e..2b7364ed9cb 100644 --- a/pkg/kubelet/dockertools/fake_docker_client.go +++ b/pkg/kubelet/dockertools/fake_docker_client.go @@ -87,6 +87,9 @@ func (f *FakeDockerClient) SetFakeContainers(containers []*docker.Container) { if c.Config == nil { c.Config = &docker.Config{} } + if c.HostConfig == nil { + c.HostConfig = &docker.HostConfig{} + } f.ContainerMap[c.ID] = c apiContainer := docker.APIContainers{ Names: []string{c.Name}, @@ -251,7 +254,7 @@ func (f *FakeDockerClient) CreateContainer(c docker.CreateContainerOptions) (*do f.ContainerList = append([]docker.APIContainers{ {ID: name, Names: []string{name}, Image: c.Config.Image, Labels: c.Config.Labels}, }, f.ContainerList...) - container := docker.Container{ID: name, Name: name, Config: c.Config, HostConfig: c.HostConfig} + container := docker.Container{ID: name, Name: name, Config: c.Config} containerCopy := container f.ContainerMap[name] = &containerCopy f.normalSleep(100, 25, 25) @@ -260,11 +263,7 @@ func (f *FakeDockerClient) CreateContainer(c docker.CreateContainerOptions) (*do // StartContainer is a test-spy implementation of DockerInterface.StartContainer. // It adds an entry "start" to the internal method call record. -// The HostConfig at StartContainer will be deprecated from docker 1.10. Now in -// docker manager the HostConfig is set when CreateContainer(). -// TODO(random-liu): Remove the HostConfig here when it is completely removed in -// docker 1.12. -func (f *FakeDockerClient) StartContainer(id string, _ *docker.HostConfig) error { +func (f *FakeDockerClient) StartContainer(id string, hostConfig *docker.HostConfig) error { f.Lock() defer f.Unlock() f.called = append(f.called, "start") @@ -275,6 +274,7 @@ func (f *FakeDockerClient) StartContainer(id string, _ *docker.HostConfig) error if !ok { container = &docker.Container{ID: id, Name: id} } + container.HostConfig = hostConfig container.State = docker.State{ Running: true, Pid: os.Getpid(), diff --git a/pkg/kubelet/dockertools/manager.go b/pkg/kubelet/dockertools/manager.go index 1a0a779e6cf..90abf0f8e3e 100644 --- a/pkg/kubelet/dockertools/manager.go +++ b/pkg/kubelet/dockertools/manager.go @@ -500,17 +500,51 @@ func (dm *DockerManager) runContainer( cpuShares = milliCPUToShares(cpuRequest.MilliValue()) } + _, containerName := BuildDockerName(dockerName, container) + dockerOpts := docker.CreateContainerOptions{ + Name: containerName, + Config: &docker.Config{ + Env: makeEnvList(opts.Envs), + ExposedPorts: exposedPorts, + Hostname: containerHostname, + Image: container.Image, + // Memory and CPU are set here for older versions of Docker (pre-1.6). + Memory: memoryLimit, + MemorySwap: -1, + CPUShares: cpuShares, + WorkingDir: container.WorkingDir, + Labels: labels, + // Interactive containers: + OpenStdin: container.Stdin, + StdinOnce: container.StdinOnce, + Tty: container.TTY, + }, + } + + setEntrypointAndCommand(container, opts, &dockerOpts) + + glog.V(3).Infof("Container %v/%v/%v: setting entrypoint \"%v\" and command \"%v\"", pod.Namespace, pod.Name, container.Name, dockerOpts.Config.Entrypoint, dockerOpts.Config.Cmd) + + securityContextProvider := securitycontext.NewSimpleSecurityContextProvider() + securityContextProvider.ModifyContainerConfig(pod, container, dockerOpts.Config) + dockerContainer, err := dm.client.CreateContainer(dockerOpts) + if err != nil { + dm.recorder.Eventf(ref, api.EventTypeWarning, kubecontainer.FailedToCreateContainer, "Failed to create docker container with error: %v", err) + return kubecontainer.ContainerID{}, err + } + + dm.recorder.Eventf(ref, api.EventTypeNormal, kubecontainer.CreatedContainer, "Created container with docker id %v", utilstrings.ShortenString(dockerContainer.ID, 12)) + podHasSELinuxLabel := pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.SELinuxOptions != nil binds := makeMountBindings(opts.Mounts, podHasSELinuxLabel) - _, containerName, cid := BuildDockerName(dockerName, container) + // The reason we create and mount the log file in here (not in kubelet) is because + // the file's location depends on the ID of the container, and we need to create and + // mount the file before actually starting the container. + // TODO(yifan): Consider to pull this logic out since we might need to reuse it in + // other container runtime. if opts.PodContainerDir != "" && len(container.TerminationMessagePath) != 0 { - // Because the PodContainerDir contains pod uid and container name which is unique enough, - // here we just add an unique container id to make the path unique for different instances - // of the same container. - // Notice that the "container id" is just a unique number generated in BuildDockerName(), - // it is not the real docker container id. - containerLogPath := path.Join(opts.PodContainerDir, cid) + containerLogPath := path.Join(opts.PodContainerDir, dockerContainer.ID) fs, err := os.Create(containerLogPath) if err != nil { // TODO: Clean up the previouly created dir? return the error? @@ -521,6 +555,7 @@ func (dm *DockerManager) runContainer( binds = append(binds, b) } } + hc := &docker.HostConfig{ PortBindings: portBindings, Binds: binds, @@ -551,43 +586,9 @@ func (dm *DockerManager) runContainer( if len(opts.CgroupParent) > 0 { hc.CgroupParent = opts.CgroupParent } + securityContextProvider.ModifyHostConfig(pod, container, hc) - dockerOpts := docker.CreateContainerOptions{ - Name: containerName, - Config: &docker.Config{ - Env: makeEnvList(opts.Envs), - ExposedPorts: exposedPorts, - Hostname: containerHostname, - Image: container.Image, - // Memory and CPU are set here for older versions of Docker (pre-1.6). - Memory: memoryLimit, - MemorySwap: -1, - CPUShares: cpuShares, - WorkingDir: container.WorkingDir, - Labels: labels, - // Interactive containers: - OpenStdin: container.Stdin, - StdinOnce: container.StdinOnce, - Tty: container.TTY, - }, - HostConfig: hc, - } - - setEntrypointAndCommand(container, opts, &dockerOpts) - - glog.V(3).Infof("Container %v/%v/%v: setting entrypoint \"%v\" and command \"%v\"", pod.Namespace, pod.Name, container.Name, dockerOpts.Config.Entrypoint, dockerOpts.Config.Cmd) - - securityContextProvider := securitycontext.NewSimpleSecurityContextProvider() - securityContextProvider.ModifyContainerConfig(pod, container, dockerOpts.Config) - securityContextProvider.ModifyHostConfig(pod, container, dockerOpts.HostConfig) - dockerContainer, err := dm.client.CreateContainer(dockerOpts) - if err != nil { - dm.recorder.Eventf(ref, api.EventTypeWarning, kubecontainer.FailedToCreateContainer, "Failed to create docker container with error: %v", err) - return kubecontainer.ContainerID{}, err - } - dm.recorder.Eventf(ref, api.EventTypeNormal, kubecontainer.CreatedContainer, "Created container with docker id %v", utilstrings.ShortenString(dockerContainer.ID, 12)) - - if err = dm.client.StartContainer(dockerContainer.ID, nil); err != nil { + if err = dm.client.StartContainer(dockerContainer.ID, hc); err != nil { dm.recorder.Eventf(ref, api.EventTypeWarning, kubecontainer.FailedToStartContainer, "Failed to start container with docker id %v with error: %v", utilstrings.ShortenString(dockerContainer.ID, 12), err) return kubecontainer.ContainerID{}, err @@ -1879,7 +1880,7 @@ func (dm *DockerManager) doBackOff(pod *api.Pod, container *api.Container, podSt PodUID: pod.UID, ContainerName: container.Name, } - stableName, _, _ := BuildDockerName(dockerName, container) + stableName, _ := BuildDockerName(dockerName, container) if backOff.IsInBackOffSince(stableName, ts) { if ref, err := kubecontainer.GenerateContainerRef(pod, container); err == nil { dm.recorder.Eventf(ref, api.EventTypeWarning, kubecontainer.BackOffStartContainer, "Back-off restarting failed docker container") diff --git a/pkg/kubelet/dockertools/manager_test.go b/pkg/kubelet/dockertools/manager_test.go index 233cbb6b8f9..1dce60ee792 100644 --- a/pkg/kubelet/dockertools/manager_test.go +++ b/pkg/kubelet/dockertools/manager_test.go @@ -1387,7 +1387,7 @@ func TestSyncPodWithTerminationLog(t *testing.T) { t.Fatalf("unexpected error %v", err) } parts := strings.Split(newContainer.HostConfig.Binds[0], ":") - if !matchString(t, testPodContainerDir+"/[a-f0-9]", parts[0]) { + if !matchString(t, testPodContainerDir+"/k8s_bar\\.[a-f0-9]", parts[0]) { t.Errorf("Unexpected host path: %s", parts[0]) } if parts[1] != "/dev/somepath" { diff --git a/pkg/securitycontext/types.go b/pkg/securitycontext/types.go index 61549cc00eb..0e262ba3807 100644 --- a/pkg/securitycontext/types.go +++ b/pkg/securitycontext/types.go @@ -28,7 +28,7 @@ type SecurityContextProvider interface { // the container is created. ModifyContainerConfig(pod *api.Pod, container *api.Container, config *docker.Config) - // ModifyHostConfig is called before the Docker createContainer call. + // ModifyHostConfig is called before the Docker runContainer call. // The security context provider can make changes to the HostConfig, affecting // security options, whether the container is privileged, volume binds, etc. // An error is returned if it's not possible to secure the container as requested