diff --git a/pkg/kubelet/container/os.go b/pkg/kubelet/container/os.go index fc2ac8df0134d..18e5221e1241a 100644 --- a/pkg/kubelet/container/os.go +++ b/pkg/kubelet/container/os.go @@ -19,6 +19,7 @@ package container import ( "io/ioutil" "os" + "path/filepath" "time" ) @@ -35,6 +36,7 @@ type OSInterface interface { Chtimes(path string, atime time.Time, mtime time.Time) error Pipe() (r *os.File, w *os.File, err error) ReadDir(dirname string) ([]os.FileInfo, error) + Glob(pattern string) ([]string, error) } // RealOS is used to dispatch the real system level operations. @@ -90,3 +92,9 @@ func (RealOS) Pipe() (r *os.File, w *os.File, err error) { func (RealOS) ReadDir(dirname string) ([]os.FileInfo, error) { return ioutil.ReadDir(dirname) } + +// Glob will call filepath.Glob to return the names of all files matching +// pattern. +func (RealOS) Glob(pattern string) ([]string, error) { + return filepath.Glob(pattern) +} diff --git a/pkg/kubelet/rkt/mock_os/mockfileinfo.go b/pkg/kubelet/container/testing/mockfileinfo.go similarity index 99% rename from pkg/kubelet/rkt/mock_os/mockfileinfo.go rename to pkg/kubelet/container/testing/mockfileinfo.go index 17f8f956a32d6..1470da45a8038 100644 --- a/pkg/kubelet/rkt/mock_os/mockfileinfo.go +++ b/pkg/kubelet/container/testing/mockfileinfo.go @@ -18,7 +18,7 @@ limitations under the License. // Edited to include required boilerplate // Source: os (interfaces: FileInfo) -package mock_os +package testing import ( os "os" diff --git a/pkg/kubelet/container/testing/os.go b/pkg/kubelet/container/testing/os.go index 923daf7fe0ad1..e70c2809f8a64 100644 --- a/pkg/kubelet/container/testing/os.go +++ b/pkg/kubelet/container/testing/os.go @@ -74,6 +74,7 @@ func (f *FakeOS) Remove(path string) error { // RemoveAll is a fake call that just returns nil. func (f *FakeOS) RemoveAll(path string) error { + f.Removes = append(f.Removes, path) return nil } @@ -104,3 +105,8 @@ func (f *FakeOS) ReadDir(dirname string) ([]os.FileInfo, error) { } return nil, nil } + +// Glob is a fake call that returns nil. +func (f *FakeOS) Glob(pattern string) ([]string, error) { + return nil, nil +} diff --git a/pkg/kubelet/kuberuntime/kuberuntime_gc.go b/pkg/kubelet/kuberuntime/kuberuntime_gc.go index 59227954ef41b..7696e6dd355c2 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_gc.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_gc.go @@ -292,20 +292,21 @@ func (cgc *containerGC) evictPodLogsDirectories(allSourcesReady bool) error { return fmt.Errorf("failed to read podLogsRootDirectory %q: %v", podLogsRootDirectory, err) } for _, dir := range dirs { - podUID := types.UID(dir.Name()) + name := dir.Name() + podUID := types.UID(name) if !cgc.isPodDeleted(podUID) { continue } - err := osInterface.RemoveAll(filepath.Join(podLogsRootDirectory, dir.Name())) + err := osInterface.RemoveAll(filepath.Join(podLogsRootDirectory, name)) if err != nil { - glog.Errorf("Failed to remove pod logs directory %q: %v", dir.Name(), err) + glog.Errorf("Failed to remove pod logs directory %q: %v", name, err) } } } // Remove dead container log symlinks. // TODO(random-liu): Remove this after cluster logging supports CRI container log path. - logSymlinks, _ := filepath.Glob(filepath.Join(legacyContainerLogsDir, fmt.Sprintf("*.%s", legacyLogSuffix))) + logSymlinks, _ := osInterface.Glob(filepath.Join(legacyContainerLogsDir, fmt.Sprintf("*.%s", legacyLogSuffix))) for _, logSymlink := range logSymlinks { if _, err := osInterface.Stat(logSymlink); os.IsNotExist(err) { err := osInterface.Remove(logSymlink) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go b/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go index a56f6d42e6586..6c62f3fe51309 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_gc_test.go @@ -17,13 +17,17 @@ limitations under the License. package kuberuntime import ( + "os" + "path/filepath" "testing" "time" + "github.com/golang/mock/gomock" "github.com/stretchr/testify/assert" "k8s.io/kubernetes/pkg/api" runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" + containertest "k8s.io/kubernetes/pkg/kubelet/container/testing" ) func TestSandboxGC(t *testing.T) { @@ -290,3 +294,41 @@ func TestContainerGC(t *testing.T) { } } } + +// Notice that legacy container symlink is not tested since it may be deprecated soon. +func TestPodLogDirectoryGC(t *testing.T) { + _, _, m, err := createTestRuntimeManager() + assert.NoError(t, err) + fakeOS := m.osInterface.(*containertest.FakeOS) + fakePodGetter := m.containerGC.podGetter.(*fakePodGetter) + + // pod log directories without corresponding pods should be removed. + fakePodGetter.pods["123"] = makeTestPod("foo1", "new", "123", nil) + fakePodGetter.pods["456"] = makeTestPod("foo2", "new", "456", nil) + files := []string{"123", "456", "789", "012"} + removed := []string{filepath.Join(podLogsRootDirectory, "789"), filepath.Join(podLogsRootDirectory, "012")} + + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + fakeOS.ReadDirFn = func(string) ([]os.FileInfo, error) { + var fileInfos []os.FileInfo + for _, file := range files { + mockFI := containertest.NewMockFileInfo(ctrl) + mockFI.EXPECT().Name().Return(file) + fileInfos = append(fileInfos, mockFI) + } + return fileInfos, nil + } + + // allSourcesReady == true, pod log directories without corresponding pod should be removed. + err = m.containerGC.evictPodLogsDirectories(true) + assert.NoError(t, err) + assert.Equal(t, removed, fakeOS.Removes) + + // allSourcesReady == false, pod log directories should not be removed. + fakeOS.Removes = []string{} + err = m.containerGC.evictPodLogsDirectories(false) + assert.NoError(t, err) + assert.Empty(t, fakeOS.Removes) +} diff --git a/pkg/kubelet/rkt/rkt_test.go b/pkg/kubelet/rkt/rkt_test.go index 76611e77bb884..472ccc697f225 100644 --- a/pkg/kubelet/rkt/rkt_test.go +++ b/pkg/kubelet/rkt/rkt_test.go @@ -40,7 +40,6 @@ import ( "k8s.io/kubernetes/pkg/kubelet/network" "k8s.io/kubernetes/pkg/kubelet/network/kubenet" "k8s.io/kubernetes/pkg/kubelet/network/mock_network" - "k8s.io/kubernetes/pkg/kubelet/rkt/mock_os" "k8s.io/kubernetes/pkg/kubelet/types" kubetypes "k8s.io/kubernetes/pkg/types" "k8s.io/kubernetes/pkg/util/errors" @@ -813,7 +812,7 @@ func TestGetPodStatus(t *testing.T) { if !ok { t.Errorf("osStat called with %v, but only knew about %#v", name, podTimes) } - mockFI := mock_os.NewMockFileInfo(ctrl) + mockFI := containertesting.NewMockFileInfo(ctrl) mockFI.EXPECT().ModTime().Return(podTime) return mockFI, nil } @@ -1781,7 +1780,7 @@ func TestGarbageCollect(t *testing.T) { var fileInfos []os.FileInfo for _, name := range serviceFileNames { - mockFI := mock_os.NewMockFileInfo(ctrl) + mockFI := containertesting.NewMockFileInfo(ctrl) mockFI.EXPECT().Name().Return(name) fileInfos = append(fileInfos, mockFI) }