Merge pull request #13211 from ashcrow/kubelet-disk-manager-tests

pkg/kubelet/disk_manager.go

@@ -28,6 +28,9 @@ import (
 
 // Manages policy for diskspace management for disks holding docker images and root fs.
 
+// mb is used to easily convert an int to an mb
+const mb = 1024 * 1024
+
 // Implementation is thread-safe.
 type diskSpaceManager interface {
 	// Checks the available disk space
@@ -107,8 +110,6 @@ func (dm *realDiskSpaceManager) isSpaceAvailable(fsType string, threshold int, f
 		return true, fmt.Errorf("wrong available space for %q: %+v", fsType, fsInfo)
 	}
 
-	const mb = int64(1024 * 1024)
-
 	if fsInfo.Available < int64(threshold)*mb {
 		glog.Infof("Running out of space on disk for %q: available %d MB, threshold %d MB", fsType, fsInfo.Available/mb, threshold)
 		return false, nil

pkg/kubelet/disk_manager_test.go

@@ -33,12 +33,17 @@ func testPolicy() DiskSpacePolicy {
 	}
 }
 
-func testValidPolicy(t *testing.T) {
+func setUp(t *testing.T) (*assert.Assertions, DiskSpacePolicy, *cadvisor.Mock) {
 	assert := assert.New(t)
 	policy := testPolicy()
 	c := new(cadvisor.Mock)
+	return assert, policy, c
+}
+
+func TestValidPolicy(t *testing.T) {
+	assert, policy, c := setUp(t)
 	_, err := newDiskSpaceManager(c, policy)
-	require.NoError(t, err)
+	assert.NoError(err)
 
 	policy = testPolicy()
 	policy.DockerFreeDiskMB = -1
@@ -51,98 +56,274 @@ func testValidPolicy(t *testing.T) {
 	assert.Error(err)
 }
 
-func testSpaceAvailable(t *testing.T) {
-	policy := testPolicy()
-	mockCadvisor := new(cadvisor.Mock)
+func TestSpaceAvailable(t *testing.T) {
+	assert, policy, mockCadvisor := setUp(t)
 	dm, err := newDiskSpaceManager(mockCadvisor, policy)
-	require.NoError(t, err)
-	const mb = 1024 * 1024
+	assert.NoError(err)
+
 	mockCadvisor.On("DockerImagesFsInfo").Return(cadvisorApi.FsInfo{
-		Usage:    400 * mb,
-		Capacity: 1000 * mb,
+		Usage:     400 * mb,
+		Capacity:  1000 * mb,
+		Available: 600 * mb,
 	}, nil)
 	mockCadvisor.On("RootFsInfo").Return(cadvisorApi.FsInfo{
 		Usage:    9 * mb,
 		Capacity: 10 * mb,
 	}, nil)
+
+	dm.Unfreeze()
+
 	ok, err := dm.IsDockerDiskSpaceAvailable()
-	require.NoError(t, err)
-	require.True(t, ok)
+	assert.NoError(err)
+	assert.True(ok)
+
 	ok, err = dm.IsRootDiskSpaceAvailable()
-	require.NoError(t, err)
-	require.True(t, ok)
+	assert.NoError(err)
+	assert.False(ok)
 }
 
-func testRootFsAvailable(t *testing.T) {
-	policy := testPolicy()
-	policy.RootFreeDiskMB = 10
-	mockCadvisor := new(cadvisor.Mock)
+// TestIsDockerDiskSpaceAvailableWithSpace verifies IsDockerDiskSpaceAvailable results when
+// space is available.
+func TestIsDockerDiskSpaceAvailableWithSpace(t *testing.T) {
+	assert, policy, mockCadvisor := setUp(t)
 	dm, err := newDiskSpaceManager(mockCadvisor, policy)
 	require.NoError(t, err)
 
-	const mb = 1024 * 1024
 	// 500MB available
 	mockCadvisor.On("DockerImagesFsInfo").Return(cadvisorApi.FsInfo{
 		Usage:     9500 * mb,
 		Capacity:  10000 * mb,
 		Available: 500 * mb,
 	}, nil)
-	// 10MB available
+
+	dm.Unfreeze()
+
+	ok, err := dm.IsDockerDiskSpaceAvailable()
+	assert.NoError(err)
+	assert.True(ok)
+}
+
+// TestIsDockerDiskSpaceAvailableWithoutSpace verifies IsDockerDiskSpaceAvailable results when
+// space is not available.
+func TestIsDockerDiskSpaceAvailableWithoutSpace(t *testing.T) {
+	// 1MB available
+	assert, policy, mockCadvisor := setUp(t)
+	mockCadvisor.On("DockerImagesFsInfo").Return(cadvisorApi.FsInfo{
+		Usage:     999 * mb,
+		Capacity:  1000 * mb,
+		Available: 1 * mb,
+	}, nil)
+
+	dm, err := newDiskSpaceManager(mockCadvisor, policy)
+	require.NoError(t, err)
+
+	dm.Unfreeze()
+
+	ok, err := dm.IsDockerDiskSpaceAvailable()
+	assert.NoError(err)
+	assert.False(ok)
+}
+
+// TestIsRootDiskSpaceAvailableWithSpace verifies IsRootDiskSpaceAvailable results when
+// space is available.
+func TestIsRootDiskSpaceAvailableWithSpace(t *testing.T) {
+	assert, policy, mockCadvisor := setUp(t)
+	policy.RootFreeDiskMB = 10
+	dm, err := newDiskSpaceManager(mockCadvisor, policy)
+	assert.NoError(err)
+
+	// 999MB available
+	mockCadvisor.On("RootFsInfo").Return(cadvisorApi.FsInfo{
+		Usage:     1 * mb,
+		Capacity:  1000 * mb,
+		Available: 999 * mb,
+	}, nil)
+
+	dm.Unfreeze()
+
+	ok, err := dm.IsRootDiskSpaceAvailable()
+	assert.NoError(err)
+	assert.True(ok)
+}
+
+// TestIsRootDiskSpaceAvailableWithoutSpace verifies IsRootDiskSpaceAvailable results when
+// space is not available.
+func TestIsRootDiskSpaceAvailableWithoutSpace(t *testing.T) {
+	assert, policy, mockCadvisor := setUp(t)
+	policy.RootFreeDiskMB = 10
+	dm, err := newDiskSpaceManager(mockCadvisor, policy)
+	assert.NoError(err)
+
+	// 9MB available
 	mockCadvisor.On("RootFsInfo").Return(cadvisorApi.FsInfo{
 		Usage:     990 * mb,
 		Capacity:  1000 * mb,
-		Available: 10 * mb,
+		Available: 9 * mb,
 	}, nil)
+
+	dm.Unfreeze()
+
+	ok, err := dm.IsRootDiskSpaceAvailable()
+	assert.NoError(err)
+	assert.False(ok)
+}
+
+// TestCache verifies that caching works properly with DiskSpaceAvailable calls
+func TestCache(t *testing.T) {
+	assert, policy, mockCadvisor := setUp(t)
+	dm, err := newDiskSpaceManager(mockCadvisor, policy)
+	assert.NoError(err)
+
+	dm.Unfreeze()
+
+	mockCadvisor.On("DockerImagesFsInfo").Return(cadvisorApi.FsInfo{
+		Usage:     400 * mb,
+		Capacity:  1000 * mb,
+		Available: 300 * mb,
+	}, nil).Once()
+	mockCadvisor.On("RootFsInfo").Return(cadvisorApi.FsInfo{
+		Usage:     500 * mb,
+		Capacity:  1000 * mb,
+		Available: 500 * mb,
+	}, nil).Once()
+
+	// Initial calls which should be recorded in mockCadvisor
 	ok, err := dm.IsDockerDiskSpaceAvailable()
-	require.NoError(t, err)
-	require.True(t, ok)
+	assert.NoError(err)
+	assert.True(ok)
+
 	ok, err = dm.IsRootDiskSpaceAvailable()
-	require.NoError(t, err)
-	require.False(t, ok)
+	assert.NoError(err)
+	assert.True(ok)
+
+	// Get the current count of calls to mockCadvisor
+	cadvisorCallCount := len(mockCadvisor.Calls)
+
+	// Checking for space again shouldn't need to mock as cache would serve it.
+	ok, err = dm.IsDockerDiskSpaceAvailable()
+	assert.NoError(err)
+	assert.True(ok)
+
+	ok, err = dm.IsRootDiskSpaceAvailable()
+	assert.NoError(err)
+	assert.True(ok)
+
+	// Ensure no more calls to the mockCadvisor occured
+	assert.Equal(cadvisorCallCount, len(mockCadvisor.Calls))
 }
 
-func testFsInfoError(t *testing.T) {
-	assert := assert.New(t)
-	policy := testPolicy()
+// TestFsInfoError verifies errors are returned  by DiskSpaceAvailable calls
+// when FsInfo calls return an error
+func TestFsInfoError(t *testing.T) {
+	assert, policy, mockCadvisor := setUp(t)
 	policy.RootFreeDiskMB = 10
-	mockCadvisor := new(cadvisor.Mock)
 	dm, err := newDiskSpaceManager(mockCadvisor, policy)
-	require.NoError(t, err)
+	assert.NoError(err)
 
+	dm.Unfreeze()
 	mockCadvisor.On("DockerImagesFsInfo").Return(cadvisorApi.FsInfo{}, fmt.Errorf("can't find fs"))
 	mockCadvisor.On("RootFsInfo").Return(cadvisorApi.FsInfo{}, fmt.Errorf("EBUSY"))
 	ok, err := dm.IsDockerDiskSpaceAvailable()
 	assert.Error(err)
-	require.True(t, ok)
+	assert.True(ok)
 	ok, err = dm.IsRootDiskSpaceAvailable()
 	assert.Error(err)
-	require.True(t, ok)
+	assert.True(ok)
 }
 
-func testCache(t *testing.T) {
-	policy := testPolicy()
-	mockCadvisor := new(cadvisor.Mock)
-	dm, err := newDiskSpaceManager(mockCadvisor, policy)
-	require.NoError(t, err)
-	const mb = 1024 * 1024
-	mockCadvisor.On("DockerImagesFsInfo").Return(cadvisorApi.FsInfo{
-		Usage:     400 * mb,
-		Capacity:  1000 * mb,
-		Available: 300 * mb,
+// Test_getFSInfo verifies multiple possible cases for getFsInfo.
+func Test_getFsInfo(t *testing.T) {
+	assert, policy, mockCadvisor := setUp(t)
+
+	// Sunny day case
+	mockCadvisor.On("RootFsInfo").Return(cadvisorApi.FsInfo{
+		Usage:     10 * mb,
+		Capacity:  100 * mb,
+		Available: 90 * mb,
+	}, nil).Once()
+
+	dm := &realDiskSpaceManager{
+		cadvisor:   mockCadvisor,
+		policy:     policy,
+		cachedInfo: map[string]fsInfo{},
+		frozen:     false,
+	}
+
+	available, err := dm.isSpaceAvailable("root", 10, dm.cadvisor.RootFsInfo)
+	assert.True(available)
+	assert.NoError(err)
+
+	// Threshold case
+	mockCadvisor = new(cadvisor.Mock)
+	mockCadvisor.On("RootFsInfo").Return(cadvisorApi.FsInfo{
+		Usage:     9 * mb,
+		Capacity:  100 * mb,
+		Available: 9 * mb,
 	}, nil).Once()
+
+	dm = &realDiskSpaceManager{
+		cadvisor:   mockCadvisor,
+		policy:     policy,
+		cachedInfo: map[string]fsInfo{},
+		frozen:     false,
+	}
+	available, err = dm.isSpaceAvailable("root", 10, dm.cadvisor.RootFsInfo)
+	assert.False(available)
+	assert.NoError(err)
+
+	// Frozen case
 	mockCadvisor.On("RootFsInfo").Return(cadvisorApi.FsInfo{
 		Usage:     9 * mb,
 		Capacity:  10 * mb,
-		Available: 500,
+		Available: 500 * mb,
 	}, nil).Once()
-	ok, err := dm.IsDockerDiskSpaceAvailable()
-	ok, err = dm.IsRootDiskSpaceAvailable()
 
-	// Checking for space again shouldn't need to mock as cache would serve it.
-	ok, err = dm.IsDockerDiskSpaceAvailable()
-	require.NoError(t, err)
-	require.True(t, ok)
-	ok, err = dm.IsRootDiskSpaceAvailable()
-	require.NoError(t, err)
-	require.True(t, ok)
+	dm = &realDiskSpaceManager{
+		cadvisor:   mockCadvisor,
+		policy:     policy,
+		cachedInfo: map[string]fsInfo{},
+		frozen:     true,
+	}
+	available, err = dm.isSpaceAvailable("root", 10, dm.cadvisor.RootFsInfo)
+	assert.True(available)
+	assert.NoError(err)
+
+	// Capacity error case
+	mockCadvisor = new(cadvisor.Mock)
+	mockCadvisor.On("RootFsInfo").Return(cadvisorApi.FsInfo{
+		Usage:     9 * mb,
+		Capacity:  0,
+		Available: 500 * mb,
+	}, nil).Once()
+
+	dm = &realDiskSpaceManager{
+		cadvisor:   mockCadvisor,
+		policy:     policy,
+		cachedInfo: map[string]fsInfo{},
+		frozen:     false,
+	}
+	available, err = dm.isSpaceAvailable("root", 10, dm.cadvisor.RootFsInfo)
+	assert.True(available)
+	assert.Error(err)
+	assert.Contains(fmt.Sprintf("%s", err), "could not determine capacity")
+
+	// Available error case skipped as v2.FSInfo uses uint64 and this
+	// can not be less than 0
+}
+
+// TestUnfreeze verifies that Unfreze does infact change the frozen
+// private field in master
+func TestUnfreeze(t *testing.T) {
+	dm := &realDiskSpaceManager{
+		cadvisor:   new(cadvisor.Mock),
+		policy:     testPolicy(),
+		cachedInfo: map[string]fsInfo{},
+		frozen:     true,
+	}
+
+	dm.Unfreeze()
+
+	if dm.frozen {
+		t.Errorf("DiskSpaceManager did not unfreeze: %+v", dm)
+	}
 }