diff --git a/pkg/kubelet/api/testing/fake_runtime_service.go b/pkg/kubelet/api/testing/fake_runtime_service.go index 64cad8518b2..887129d7b03 100644 --- a/pkg/kubelet/api/testing/fake_runtime_service.go +++ b/pkg/kubelet/api/testing/fake_runtime_service.go @@ -19,6 +19,7 @@ package testing import ( "fmt" "io" + "reflect" "sync" "time" @@ -77,6 +78,16 @@ func (r *FakeRuntimeService) SetFakeContainers(containers []*FakeContainer) { } +func (r *FakeRuntimeService) AssertCalls(calls []string) error { + r.Lock() + defer r.Unlock() + + if !reflect.DeepEqual(calls, r.Called) { + return fmt.Errorf("expected %#v, got %#v", calls, r.Called) + } + return nil +} + func NewFakeRuntimeService() *FakeRuntimeService { return &FakeRuntimeService{ Called: make([]string, 0), diff --git a/pkg/kubelet/container/os.go b/pkg/kubelet/container/os.go index 8a07391998a..fc2ac8df013 100644 --- a/pkg/kubelet/container/os.go +++ b/pkg/kubelet/container/os.go @@ -29,6 +29,7 @@ type OSInterface interface { Symlink(oldname string, newname string) error Stat(path string) (os.FileInfo, error) Remove(path string) error + RemoveAll(path string) error Create(path string) (*os.File, error) Hostname() (name string, err error) Chtimes(path string, atime time.Time, mtime time.Time) error @@ -59,6 +60,11 @@ func (RealOS) Remove(path string) error { return os.Remove(path) } +// RemoveAll will call os.RemoveAll to remove the path and its children. +func (RealOS) RemoveAll(path string) error { + return os.RemoveAll(path) +} + // Create will call os.Create to create and return a file // at path. func (RealOS) Create(path string) (*os.File, error) { diff --git a/pkg/kubelet/container/testing/os.go b/pkg/kubelet/container/testing/os.go index 74c884d68fb..923daf7fe0a 100644 --- a/pkg/kubelet/container/testing/os.go +++ b/pkg/kubelet/container/testing/os.go @@ -26,11 +26,13 @@ import ( // If a member of the form `*Fn` is set, that function will be called in place // of the real call. type FakeOS struct { - StatFn func(string) (os.FileInfo, error) - ReadDirFn func(string) ([]os.FileInfo, error) - HostName string - Removes []string - Files map[string][]*os.FileInfo + StatFn func(string) (os.FileInfo, error) + ReadDirFn func(string) ([]os.FileInfo, error) + MkdirAllFn func(string, os.FileMode) error + SymlinkFn func(string, string) error + HostName string + Removes []string + Files map[string][]*os.FileInfo } func NewFakeOS() *FakeOS { @@ -41,12 +43,18 @@ func NewFakeOS() *FakeOS { } // Mkdir is a fake call that just returns nil. -func (FakeOS) MkdirAll(path string, perm os.FileMode) error { +func (f *FakeOS) MkdirAll(path string, perm os.FileMode) error { + if f.MkdirAllFn != nil { + return f.MkdirAllFn(path, perm) + } return nil } // Symlink is a fake call that just returns nil. -func (FakeOS) Symlink(oldname string, newname string) error { +func (f *FakeOS) Symlink(oldname string, newname string) error { + if f.SymlinkFn != nil { + return f.SymlinkFn(oldname, newname) + } return nil } @@ -64,6 +72,11 @@ func (f *FakeOS) Remove(path string) error { return nil } +// RemoveAll is a fake call that just returns nil. +func (f *FakeOS) RemoveAll(path string) error { + return nil +} + // Create is a fake call that returns nil. func (FakeOS) Create(path string) (*os.File, error) { return nil, nil @@ -89,5 +102,5 @@ func (f *FakeOS) ReadDir(dirname string) ([]os.FileInfo, error) { if f.ReadDirFn != nil { return f.ReadDirFn(dirname) } - return nil, errors.New("unimplemented testing mock") + return nil, nil } diff --git a/pkg/kubelet/dockershim/docker_container.go b/pkg/kubelet/dockershim/docker_container.go index 5c204685994..36d91009419 100644 --- a/pkg/kubelet/dockershim/docker_container.go +++ b/pkg/kubelet/dockershim/docker_container.go @@ -19,6 +19,8 @@ package dockershim import ( "fmt" "io" + "os" + "path/filepath" "time" dockertypes "github.com/docker/engine-api/types" @@ -91,6 +93,8 @@ func (ds *dockerService) CreateContainer(podSandboxID string, config *runtimeApi labels := makeLabels(config.GetLabels(), config.GetAnnotations()) // Apply a the container type label. labels[containerTypeLabelKey] = containerTypeLabelContainer + // Write the container log path in the labels. + labels[containerLogPathLabelKey] = filepath.Join(sandboxConfig.GetLogDirectory(), config.GetLogPath()) // Write the sandbox ID in the labels. labels[sandboxIDLabelKey] = podSandboxID @@ -181,9 +185,63 @@ func (ds *dockerService) CreateContainer(podSandboxID string, config *runtimeApi return "", err } +// getContainerLogPath returns the container log path specified by kubelet and the real +// path where docker stores the container log. +func (ds *dockerService) getContainerLogPath(containerID string) (string, string, error) { + info, err := ds.client.InspectContainer(containerID) + if err != nil { + return "", "", fmt.Errorf("failed to inspect container %q: %v", containerID, err) + } + return info.Config.Labels[containerLogPathLabelKey], info.LogPath, nil +} + +// createContainerLogSymlink creates the symlink for docker container log. +func (ds *dockerService) createContainerLogSymlink(containerID string) error { + path, realPath, err := ds.getContainerLogPath(containerID) + if err != nil { + return fmt.Errorf("failed to get container %q log path: %v", containerID, err) + } + if path != "" { + // Only create the symlink when container log path is specified. + if err = ds.os.Symlink(realPath, path); err != nil { + return fmt.Errorf("failed to create symbolic link %q to the container log file %q for container %q: %v", + path, realPath, containerID, err) + } + } + return nil +} + +// removeContainerLogSymlink removes the symlink for docker container log. +func (ds *dockerService) removeContainerLogSymlink(containerID string) error { + path, _, err := ds.getContainerLogPath(containerID) + if err != nil { + return fmt.Errorf("failed to get container %q log path: %v", containerID, err) + } + if path != "" { + // Only remove the symlink when container log path is specified. + err := ds.os.Remove(path) + if err != nil && !os.IsNotExist(err) { + return fmt.Errorf("failed to remove container %q log symlink %q: %v", containerID, path, err) + } + } + return nil +} + // StartContainer starts the container. func (ds *dockerService) StartContainer(containerID string) error { - return ds.client.StartContainer(containerID) + err := ds.client.StartContainer(containerID) + if err != nil { + return fmt.Errorf("failed to start container %q: %v", containerID, err) + } + // Create container log symlink. + if err := ds.createContainerLogSymlink(containerID); err != nil { + // Do not stop the container if fail to create symlink, because: + // 1. This is not a critical failure. + // 2. We don't have enough information to properly stop container here. + // Kubelet will surface this error to user with event. + return err + } + return nil } // StopContainer stops a running container with a grace period (i.e., timeout). @@ -194,7 +252,18 @@ func (ds *dockerService) StopContainer(containerID string, timeout int64) error // RemoveContainer removes the container. // TODO: If a container is still running, should we forcibly remove it? func (ds *dockerService) RemoveContainer(containerID string) error { - return ds.client.RemoveContainer(containerID, dockertypes.ContainerRemoveOptions{RemoveVolumes: true}) + // Ideally, log lifecycle should be independent of container lifecycle. + // However, docker will remove container log after container is removed, + // we can't prevent that now, so we also cleanup the symlink here. + err := ds.removeContainerLogSymlink(containerID) + if err != nil { + return err + } + err = ds.client.RemoveContainer(containerID, dockertypes.ContainerRemoveOptions{RemoveVolumes: true}) + if err != nil { + return fmt.Errorf("failed to remove container %q: %v", containerID, err) + } + return nil } func getContainerTimestamps(r *dockertypes.ContainerJSON) (time.Time, time.Time, time.Time, error) { diff --git a/pkg/kubelet/dockershim/docker_container_test.go b/pkg/kubelet/dockershim/docker_container_test.go index e5595860d80..6f672dd7453 100644 --- a/pkg/kubelet/dockershim/docker_container_test.go +++ b/pkg/kubelet/dockershim/docker_container_test.go @@ -18,12 +18,14 @@ package dockershim import ( "fmt" + "path/filepath" "testing" "time" "github.com/stretchr/testify/assert" runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" + containertest "k8s.io/kubernetes/pkg/kubelet/container/testing" ) // A helper to create a basic config. @@ -171,3 +173,45 @@ func TestContainerStatus(t *testing.T) { status, err = ds.ContainerStatus(id) assert.Error(t, err, fmt.Sprintf("status of container: %+v", status)) } + +// TestContainerLogPath tests the container log creation logic. +func TestContainerLogPath(t *testing.T) { + ds, fDocker, _ := newTestDockerService() + podLogPath := "/pod/1" + containerLogPath := "0" + kubeletContainerLogPath := filepath.Join(podLogPath, containerLogPath) + sConfig := makeSandboxConfig("foo", "bar", "1", 0) + sConfig.LogDirectory = &podLogPath + config := makeContainerConfig(sConfig, "pause", "iamimage", 0, nil, nil) + config.LogPath = &containerLogPath + + const sandboxId = "sandboxid" + id, err := ds.CreateContainer(sandboxId, config, sConfig) + + // Check internal container log label + c, err := fDocker.InspectContainer(id) + assert.NoError(t, err) + assert.Equal(t, c.Config.Labels[containerLogPathLabelKey], kubeletContainerLogPath) + + // Set docker container log path + dockerContainerLogPath := "/docker/container/log" + c.LogPath = dockerContainerLogPath + + // Verify container log symlink creation + fakeOS := ds.os.(*containertest.FakeOS) + fakeOS.SymlinkFn = func(oldname, newname string) error { + assert.Equal(t, dockerContainerLogPath, oldname) + assert.Equal(t, kubeletContainerLogPath, newname) + return nil + } + err = ds.StartContainer(id) + assert.NoError(t, err) + + err = ds.StopContainer(id, 0) + assert.NoError(t, err) + + // Verify container log symlink deletion + err = ds.RemoveContainer(id) + assert.NoError(t, err) + assert.Equal(t, fakeOS.Removes, []string{kubeletContainerLogPath}) +} diff --git a/pkg/kubelet/dockershim/docker_sandbox.go b/pkg/kubelet/dockershim/docker_sandbox.go index 76f3d56923c..a0e3ab33672 100644 --- a/pkg/kubelet/dockershim/docker_sandbox.go +++ b/pkg/kubelet/dockershim/docker_sandbox.go @@ -71,7 +71,7 @@ func (ds *dockerService) RunPodSandbox(config *runtimeApi.PodSandboxConfig) (str // Step 3: Start the sandbox container. // Assume kubelet's garbage collector would remove the sandbox later, if // startContainer failed. - err = ds.StartContainer(createResp.ID) + err = ds.client.StartContainer(createResp.ID) return createResp.ID, err } diff --git a/pkg/kubelet/dockershim/docker_service.go b/pkg/kubelet/dockershim/docker_service.go index e998e559471..832bd01dac2 100644 --- a/pkg/kubelet/dockershim/docker_service.go +++ b/pkg/kubelet/dockershim/docker_service.go @@ -48,16 +48,18 @@ const ( containerTypeLabelKey = "io.kubernetes.docker.type" containerTypeLabelSandbox = "podsandbox" containerTypeLabelContainer = "container" + containerLogPathLabelKey = "io.kubernetes.container.logpath" sandboxIDLabelKey = "io.kubernetes.sandbox.id" ) -var internalLabelKeys []string = []string{containerTypeLabelKey, sandboxIDLabelKey} +var internalLabelKeys []string = []string{containerTypeLabelKey, containerLogPathLabelKey, sandboxIDLabelKey} // NOTE: Anything passed to DockerService should be eventually handled in another way when we switch to running the shim as a different process. func NewDockerService(client dockertools.DockerInterface, seccompProfileRoot string, podSandboxImage string) DockerLegacyService { return &dockerService{ seccompProfileRoot: seccompProfileRoot, client: dockertools.NewInstrumentedDockerInterface(client), + os: kubecontainer.RealOS{}, podSandboxImage: podSandboxImage, } } @@ -81,6 +83,7 @@ type DockerLegacyService interface { type dockerService struct { seccompProfileRoot string client dockertools.DockerInterface + os kubecontainer.OSInterface podSandboxImage string } diff --git a/pkg/kubelet/dockershim/docker_service_test.go b/pkg/kubelet/dockershim/docker_service_test.go index deba2de337d..2784f998482 100644 --- a/pkg/kubelet/dockershim/docker_service_test.go +++ b/pkg/kubelet/dockershim/docker_service_test.go @@ -19,6 +19,7 @@ package dockershim import ( "time" + containertest "k8s.io/kubernetes/pkg/kubelet/container/testing" "k8s.io/kubernetes/pkg/kubelet/dockertools" "k8s.io/kubernetes/pkg/util/clock" ) @@ -26,5 +27,5 @@ import ( func newTestDockerService() (*dockerService, *dockertools.FakeDockerClient, *clock.FakeClock) { fakeClock := clock.NewFakeClock(time.Time{}) c := dockertools.NewFakeDockerClientWithClock(fakeClock) - return &dockerService{client: c}, c, fakeClock + return &dockerService{client: c, os: &containertest.FakeOS{}}, c, fakeClock } diff --git a/pkg/kubelet/kuberuntime/helpers.go b/pkg/kubelet/kuberuntime/helpers.go index cfb7cff9714..34cb93f45c7 100644 --- a/pkg/kubelet/kuberuntime/helpers.go +++ b/pkg/kubelet/kuberuntime/helpers.go @@ -18,12 +18,14 @@ package kuberuntime import ( "fmt" + "path/filepath" "strconv" "github.com/golang/glog" "k8s.io/kubernetes/pkg/api" runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" + "k8s.io/kubernetes/pkg/types" ) const ( @@ -199,3 +201,13 @@ func getStableKey(pod *api.Pod, container *api.Container) string { hash := strconv.FormatUint(kubecontainer.HashContainer(container), 16) return fmt.Sprintf("%s_%s_%s_%s_%s", pod.Name, pod.Namespace, string(pod.UID), container.Name, hash) } + +// buildContainerLogsPath builds log path for container relative to pod logs directory. +func buildContainerLogsPath(containerName string, restartCount int) string { + return fmt.Sprintf("%s_%d.log", containerName, restartCount) +} + +// buildPodLogsDirectory builds absolute log directory path for a pod sandbox. +func buildPodLogsDirectory(podUID types.UID) string { + return filepath.Join(podLogsRootDirectory, string(podUID)) +} diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container.go b/pkg/kubelet/kuberuntime/kuberuntime_container.go index 889af2a5551..43ee831a476 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container.go @@ -22,7 +22,7 @@ import ( "io/ioutil" "math/rand" "os" - "path" + "path/filepath" "sort" "sync" "time" @@ -112,12 +112,9 @@ func (m *kubeGenericRuntimeManager) startContainer(podSandboxID string, podSandb } } - return "", nil -} + // TODO(random-liu): Add legacy container log location support. -// getContainerLogsPath gets log path for container. -func getContainerLogsPath(containerName string, podUID kubetypes.UID) string { - return path.Join(podLogsRootDirectory, string(podUID), fmt.Sprintf("%s.log", containerName)) + return "", nil } // generateContainerConfig generates container config for kubelet runtime api. @@ -128,7 +125,7 @@ func (m *kubeGenericRuntimeManager) generateContainerConfig(container *api.Conta } command, args := kubecontainer.ExpandContainerCommandAndArgs(container, opts.Envs) - containerLogsPath := getContainerLogsPath(container.Name, pod.UID) + containerLogsPath := buildContainerLogsPath(container.Name, restartCount) podHasSELinuxLabel := pod.Spec.SecurityContext != nil && pod.Spec.SecurityContext.SELinuxOptions != nil restartCountUint32 := uint32(restartCount) config := &runtimeApi.ContainerConfig{ @@ -269,7 +266,8 @@ func makeMounts(opts *kubecontainer.RunContainerOptions, container *api.Containe // here we just add a random id to make the path unique for different instances // of the same container. cid := makeUID() - containerLogPath := path.Join(opts.PodContainerDir, cid) + containerLogPath := filepath.Join(opts.PodContainerDir, cid) + // TODO: We should try to use os interface here. fs, err := os.Create(containerLogPath) if err != nil { glog.Errorf("Error on creating termination-log file %q: %v", containerLogPath, err) @@ -690,7 +688,33 @@ func (m *kubeGenericRuntimeManager) ExecInContainer(containerID kubecontainer.Co return fmt.Errorf("not implemented") } +// removeContainer removes the container and the container logs. +// Notice that we remove the container logs first, so that container will not be removed if +// container logs are failed to be removed, and kubelet will retry this later. This guarantees +// that container logs to be removed with the container. +// Notice that we assume that the container should only be removed in non-running state, and +// it will not write container logs anymore in that state. +func (m *kubeGenericRuntimeManager) removeContainer(containerID string) error { + glog.V(4).Infof("Removing container %q", containerID) + // Cleanup the container log. + status, err := m.runtimeService.ContainerStatus(containerID) + if err != nil { + glog.Errorf("ContainerStatus for %q error: %v", containerID, err) + return err + } + labeledInfo := getContainerInfoFromLabels(status.Labels) + annotatedInfo := getContainerInfoFromAnnotations(status.Annotations) + path := filepath.Join(buildPodLogsDirectory(labeledInfo.PodUID), + buildContainerLogsPath(labeledInfo.ContainerName, annotatedInfo.RestartCount)) + if err := m.osInterface.Remove(path); err != nil && !os.IsNotExist(err) { + glog.Errorf("Failed to remove container %q log %q: %v", containerID, path, err) + return err + } + // Remove the container. + return m.runtimeService.RemoveContainer(containerID) +} + // DeleteContainer removes a container. func (m *kubeGenericRuntimeManager) DeleteContainer(containerID kubecontainer.ContainerID) error { - return m.runtimeService.RemoveContainer(containerID.ID) + return m.removeContainer(containerID.ID) } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go new file mode 100644 index 00000000000..a8858286c43 --- /dev/null +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_test.go @@ -0,0 +1,66 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kuberuntime + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/kubernetes/pkg/api" + runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" + containertest "k8s.io/kubernetes/pkg/kubelet/container/testing" +) + +// TestRemoveContainer tests removing the container and its corresponding container logs. +func TestRemoveContainer(t *testing.T) { + fakeRuntime, _, m, err := createTestRuntimeManager() + pod := &api.Pod{ + ObjectMeta: api.ObjectMeta{ + UID: "12345678", + Name: "bar", + Namespace: "new", + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "foo", + Image: "busybox", + ImagePullPolicy: api.PullIfNotPresent, + }, + }, + }, + } + + // Create fake sandbox and container + _, fakeContainers, err := makeAndSetFakePod(m, fakeRuntime, pod) + assert.NoError(t, err) + assert.Equal(t, len(fakeContainers), 1) + + containerId := fakeContainers[0].GetId() + fakeOS := m.osInterface.(*containertest.FakeOS) + err = m.removeContainer(containerId) + assert.NoError(t, err) + // Verify container log is removed + expectedContainerLogPath := filepath.Join(podLogsRootDirectory, "12345678", "foo_0.log") + assert.Equal(t, fakeOS.Removes, []string{expectedContainerLogPath}) + // Verify container is removed + fakeRuntime.AssertCalls([]string{"RemoveContainer"}) + containers, err := fakeRuntime.ListContainers(&runtimeApi.ContainerFilter{Id: &containerId}) + assert.NoError(t, err) + assert.Empty(t, containers) +} diff --git a/pkg/kubelet/kuberuntime/kuberuntime_gc.go b/pkg/kubelet/kuberuntime/kuberuntime_gc.go index 726d4005f41..ed28610501c 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_gc.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_gc.go @@ -17,6 +17,8 @@ limitations under the License. package kuberuntime import ( + "fmt" + "path/filepath" "sort" "time" @@ -116,26 +118,20 @@ func (cgc *containerGC) removeOldestN(containers []containerGCInfo, toRemove int // Remove from oldest to newest (last to first). numToKeep := len(containers) - toRemove for i := numToKeep; i < len(containers); i++ { - cgc.removeContainer(containers[i].id, containers[i].name) + if err := cgc.manager.removeContainer(containers[i].id); err != nil { + glog.Errorf("Failed to remove container %q: %v", containers[i].id, err) + } } // Assume we removed the containers so that we're not too aggressive. return containers[:numToKeep] } -// removeContainer removes the container by containerID. -func (cgc *containerGC) removeContainer(containerID, containerName string) { - glog.V(4).Infof("Removing container %q name %q", containerID, containerName) - if err := cgc.client.RemoveContainer(containerID); err != nil { - glog.Warningf("Failed to remove container %q: %v", containerID, err) - } -} - // removeSandbox removes the sandbox by sandboxID. func (cgc *containerGC) removeSandbox(sandboxID string) { glog.V(4).Infof("Removing sandbox %q", sandboxID) if err := cgc.client.RemovePodSandbox(sandboxID); err != nil { - glog.Warningf("Failed to remove sandbox %q: %v", sandboxID, err) + glog.Errorf("Failed to remove sandbox %q: %v", sandboxID, err) } } @@ -184,7 +180,7 @@ func (cgc *containerGC) evictableContainers(minAge time.Duration) (containersByE // evictableSandboxes gets all sandboxes that are evictable. Evictable sandboxes are: not running // and contains no containers at all. -func (cgc *containerGC) evictableSandboxes() ([]string, error) { +func (cgc *containerGC) evictableSandboxes(minAge time.Duration) ([]string, error) { containers, err := cgc.manager.getKubeletContainers(true) if err != nil { return nil, err @@ -196,7 +192,7 @@ func (cgc *containerGC) evictableSandboxes() ([]string, error) { } evictSandboxes := make([]string, 0) - newestGCTime := time.Now().Add(-sandboxMinGCAge) + newestGCTime := time.Now().Add(-minAge) for _, sandbox := range sandboxes { // Prune out ready sandboxes. if sandbox.GetState() == runtimeApi.PodSandBoxState_READY { @@ -234,6 +230,30 @@ func (cgc *containerGC) isPodDeleted(podUID types.UID) bool { return !found } +// evictPodLogsDirectories evicts all evictable pod logs directories. Pod logs directories +// are evictable if there are no corresponding pods. +func (cgc *containerGC) evictPodLogsDirectories(allSourcesReady bool) error { + osInterface := cgc.manager.osInterface + if allSourcesReady { + // Only remove pod logs directories when all sources are ready. + dirs, err := osInterface.ReadDir(podLogsRootDirectory) + if err != nil { + return fmt.Errorf("failed to read podLogsRootDirectory %q: %v", podLogsRootDirectory, err) + } + for _, dir := range dirs { + podUID := types.UID(dir.Name()) + if !cgc.isPodDeleted(podUID) { + continue + } + err := osInterface.RemoveAll(filepath.Join(podLogsRootDirectory, dir.Name())) + if err != nil { + glog.Errorf("Failed to remove pod logs directory %q: %v", dir.Name(), err) + } + } + } + return nil +} + // GarbageCollect removes dead containers using the specified container gc policy. // Note that gc policy is not applied to sandboxes. Sandboxes are only removed when they are // not ready and containing no containers. @@ -289,7 +309,7 @@ func (cgc *containerGC) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy, } // Remove sandboxes with zero containers - evictSandboxes, err := cgc.evictableSandboxes() + evictSandboxes, err := cgc.evictableSandboxes(sandboxMinGCAge) if err != nil { return err } @@ -297,5 +317,7 @@ func (cgc *containerGC) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy, cgc.removeSandbox(sandbox) } - return nil + // Remove pod sandbox log directory + // TODO(random-liu): Add legacy container log localtion cleanup. + return cgc.evictPodLogsDirectories(allSourcesReady) } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go b/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go index 9ddf114b92a..e10cf156a21 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_sandbox.go @@ -38,6 +38,14 @@ func (m *kubeGenericRuntimeManager) createPodSandbox(pod *api.Pod, attempt uint3 return "", message, err } + // Create pod logs directory + err = m.osInterface.MkdirAll(podSandboxConfig.GetLogDirectory(), 0755) + if err != nil { + message := fmt.Sprintf("Create pod log directory for pod %q failed: %v", format.Pod(pod), err) + glog.Errorf(message) + return "", message, err + } + podSandBoxID, err := m.runtimeService.RunPodSandbox(podSandboxConfig) if err != nil { message := fmt.Sprintf("CreatePodSandbox for pod %q failed: %v", format.Pod(pod), err) @@ -82,6 +90,9 @@ func (m *kubeGenericRuntimeManager) generatePodSandboxConfig(pod *api.Pod, attem podSandboxConfig.Hostname = &hostname } + logDir := buildPodLogsDirectory(pod.UID) + podSandboxConfig.LogDirectory = &logDir + cgroupParent := "" portMappings := []*runtimeApi.PortMapping{} for _, c := range pod.Spec.Containers { diff --git a/pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go b/pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go new file mode 100644 index 00000000000..b9d9e1c924f --- /dev/null +++ b/pkg/kubelet/kuberuntime/kuberuntime_sandbox_test.go @@ -0,0 +1,64 @@ +/* +Copyright 2016 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kuberuntime + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "k8s.io/kubernetes/pkg/api" + runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" + containertest "k8s.io/kubernetes/pkg/kubelet/container/testing" +) + +// TestCreatePodSandbox tests creating sandbox and its corresponding pod log directory. +func TestCreatePodSandbox(t *testing.T) { + fakeRuntime, _, m, err := createTestRuntimeManager() + pod := &api.Pod{ + ObjectMeta: api.ObjectMeta{ + UID: "12345678", + Name: "bar", + Namespace: "new", + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "foo", + Image: "busybox", + ImagePullPolicy: api.PullIfNotPresent, + }, + }, + }, + } + + fakeOS := m.osInterface.(*containertest.FakeOS) + fakeOS.MkdirAllFn = func(path string, perm os.FileMode) error { + // Check pod logs root directory is created. + assert.Equal(t, filepath.Join(podLogsRootDirectory, "12345678"), path) + assert.Equal(t, os.FileMode(0755), perm) + return nil + } + id, _, err := m.createPodSandbox(pod, 1) + assert.NoError(t, err) + fakeRuntime.AssertCalls([]string{"RunPodSandbox"}) + sandboxes, err := fakeRuntime.ListPodSandbox(&runtimeApi.PodSandboxFilter{Id: &id}) + assert.NoError(t, err) + assert.Equal(t, len(sandboxes), 1) + // TODO Check pod sandbox configuration +}