Add pod log garbage collection unit test.

dantrevino · Oct 24, 2016 · 0655ae5 · 0655ae5
1 parent e87fa5e
commit 0655ae5
Show file tree

Hide file tree

Showing 6 changed files with 64 additions and 8 deletions.
diff --git 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 → ...kubelet/container/testing/mockfileinfo.go b/pkg/kubelet/rkt/mock_os/mockfileinfo.go → ...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
@@ -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
@@ -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
@@ -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
@@ -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)
 			}