diff --git a/pkg/kubelet/disk_manager.go b/pkg/kubelet/disk_manager.go index 15d63ab47b2..5127b2acf74 100644 --- a/pkg/kubelet/disk_manager.go +++ b/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 diff --git a/pkg/kubelet/disk_manager_test.go b/pkg/kubelet/disk_manager_test.go index 30474fd7920..664855cb137 100644 --- a/pkg/kubelet/disk_manager_test.go +++ b/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) - ok, err := dm.IsDockerDiskSpaceAvailable() - require.NoError(t, err) - require.True(t, ok) - ok, err = dm.IsRootDiskSpaceAvailable() - require.NoError(t, err) - require.False(t, ok) + + dm.Unfreeze() + + ok, err := dm.IsRootDiskSpaceAvailable() + assert.NoError(err) + assert.False(ok) } -func testFsInfoError(t *testing.T) { - assert := assert.New(t) - policy := testPolicy() - policy.RootFreeDiskMB = 10 - mockCadvisor := new(cadvisor.Mock) +// TestCache verifies that caching works properly with DiskSpaceAvailable calls +func TestCache(t *testing.T) { + assert, policy, mockCadvisor := setUp(t) dm, err := newDiskSpaceManager(mockCadvisor, policy) - require.NoError(t, err) + assert.NoError(err) - 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) - ok, err = dm.IsRootDiskSpaceAvailable() - assert.Error(err) - require.True(t, ok) -} + dm.Unfreeze() -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, }, nil).Once() mockCadvisor.On("RootFsInfo").Return(cadvisorApi.FsInfo{ - Usage: 9 * mb, - Capacity: 10 * mb, - Available: 500, + Usage: 500 * mb, + Capacity: 1000 * mb, + Available: 500 * mb, }, nil).Once() + + // Initial calls which should be recorded in mockCadvisor ok, err := dm.IsDockerDiskSpaceAvailable() + assert.NoError(err) + assert.True(ok) + ok, err = dm.IsRootDiskSpaceAvailable() + 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() - 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.True(ok) + + // Ensure no more calls to the mockCadvisor occured + assert.Equal(cadvisorCallCount, len(mockCadvisor.Calls)) +} + +// 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 + dm, err := newDiskSpaceManager(mockCadvisor, policy) + 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) + assert.True(ok) + ok, err = dm.IsRootDiskSpaceAvailable() + assert.Error(err) + assert.True(ok) +} + +// 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 * mb, + }, nil).Once() + + 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) + } }