Merge pull request #28627 from ronnielai/disk-gc2

pkg/kubelet/container/runtime.go

@@ -119,6 +119,8 @@ type Runtime interface {
 	// stream the log. Set 'follow' to false and specify the number of lines (e.g.
 	// "100" or "all") to tail the log.
 	GetContainerLogs(pod *api.Pod, containerID ContainerID, logOptions *api.PodLogOptions, stdout, stderr io.Writer) (err error)
+	// Delete a container. If the container is still running, an error is returned.
+	DeleteContainer(containerID ContainerID) error
 	// ContainerCommandRunner encapsulates the command runner interfaces for testability.
 	ContainerCommandRunner
 	// ContainerAttach encapsulates the attaching to containers for testability

pkg/kubelet/container/testing/fake_runtime.go

@@ -384,6 +384,14 @@ func (f *FakeRuntime) GarbageCollect(gcPolicy ContainerGCPolicy, ready bool) err
 	return f.Err
 }
 
+func (f *FakeRuntime) DeleteContainer(containerID ContainerID) error {
+	f.Lock()
+	defer f.Unlock()
+
+	f.CalledFunctions = append(f.CalledFunctions, "DeleteContainer")
+	return f.Err
+}
+
 func (f *FakeRuntime) ImageStats() (*ImageStats, error) {
 	f.Lock()
 	defer f.Unlock()

pkg/kubelet/container/testing/runtime_mock.go

@@ -143,6 +143,11 @@ func (r *Mock) GarbageCollect(gcPolicy ContainerGCPolicy, ready bool) error {
 	return args.Error(0)
 }
 
+func (r *Mock) DeleteContainer(containerID ContainerID) error {
+	args := r.Called(containerID)
+	return args.Error(0)
+}
+
 func (r *Mock) ImageStats() (*ImageStats, error) {
 	args := r.Called()
 	return args.Get(0).(*ImageStats), args.Error(1)

pkg/kubelet/dockertools/container_gc.go

@@ -111,15 +111,7 @@ 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++ {
-		err := cgc.client.RemoveContainer(containers[i].id, dockertypes.ContainerRemoveOptions{RemoveVolumes: true})
-		if err != nil {
-			glog.Warningf("Failed to remove dead container %q: %v", containers[i].name, err)
-		}
-		symlinkPath := LogSymlink(cgc.containerLogsDir, containers[i].podNameWithNamespace, containers[i].containerName, containers[i].id)
-		err = os.Remove(symlinkPath)
-		if err != nil && !os.IsNotExist(err) {
-			glog.Warningf("Failed to remove container %q log symlink %q: %v", containers[i].name, symlinkPath, err)
-		}
+		cgc.removeContainer(containers[i].id, containers[i].podNameWithNamespace, containers[i].containerName)
 	}
 
 	// Assume we removed the containers so that we're not too aggressive.
@@ -253,6 +245,38 @@ func (cgc *containerGC) GarbageCollect(gcPolicy kubecontainer.ContainerGCPolicy,
 	return nil
 }
 
+func (cgc *containerGC) removeContainer(id string, podNameWithNamespace string, containerName string) {
+	glog.V(4).Infof("Removing container %q name %q", id, containerName)
+	err := cgc.client.RemoveContainer(id, dockertypes.ContainerRemoveOptions{RemoveVolumes: true})
+	if err != nil {
+		glog.Warningf("Failed to remove container %q: %v", id, err)
+	}
+	symlinkPath := LogSymlink(cgc.containerLogsDir, podNameWithNamespace, containerName, id)
+	err = os.Remove(symlinkPath)
+	if err != nil && !os.IsNotExist(err) {
+		glog.Warningf("Failed to remove container %q log symlink %q: %v", id, symlinkPath, err)
+	}
+}
+
+func (cgc *containerGC) deleteContainer(id string) error {
+	containerInfo, err := cgc.client.InspectContainer(id)
+	if err != nil {
+		glog.Warningf("Failed to inspect container %q: %v", id, err)
+		return err
+	}
+	if containerInfo.State.Running {
+		return fmt.Errorf("container %q is still running", id)
+	}
+
+	containerName, _, err := ParseDockerName(containerInfo.Name)
+	if err != nil {
+		return err
+	}
+
+	cgc.removeContainer(id, containerName.PodFullName, containerName.ContainerName)
+	return nil
+}
+
 func (cgc *containerGC) isPodDeleted(podUID types.UID) bool {
 	_, found := cgc.podGetter.GetPodByUID(podUID)
 	return !found

pkg/kubelet/dockertools/container_gc_test.go

@@ -89,6 +89,28 @@ func verifyStringArrayEqualsAnyOrder(t *testing.T, actual, expected []string) {
 	}
 }
 
+func TestDeleteContainerSkipRunningContainer(t *testing.T) {
+	gc, fakeDocker := newTestContainerGC(t)
+	fakeDocker.SetFakeContainers([]*FakeContainer{
+		makeContainer("1876", "foo", "POD", true, makeTime(0)),
+	})
+	addPods(gc.podGetter, "foo")
+
+	assert.Error(t, gc.deleteContainer("1876"))
+	assert.Len(t, fakeDocker.Removed, 0)
+}
+
+func TestDeleteContainerRemoveDeadContainer(t *testing.T) {
+	gc, fakeDocker := newTestContainerGC(t)
+	fakeDocker.SetFakeContainers([]*FakeContainer{
+		makeContainer("1876", "foo", "POD", false, makeTime(0)),
+	})
+	addPods(gc.podGetter, "foo")
+
+	assert.Nil(t, gc.deleteContainer("1876"))
+	assert.Len(t, fakeDocker.Removed, 1)
+}
+
 func TestGarbageCollectZeroMaxContainers(t *testing.T) {
 	gc, fakeDocker := newTestContainerGC(t)
 	fakeDocker.SetFakeContainers([]*FakeContainer{

pkg/kubelet/dockertools/docker_manager.go

@@ -2354,6 +2354,10 @@ func getIPCMode(pod *api.Pod) string {
 	return ipcMode
 }
 
+func (dm *DockerManager) DeleteContainer(containerID kubecontainer.ContainerID) error {
+	return dm.containerGC.deleteContainer(containerID.ID)
+}
+
 // GetNetNS returns the network namespace path for the given container
 func (dm *DockerManager) GetNetNS(containerID kubecontainer.ContainerID) (string, error) {
 	inspectResult, err := dm.client.InspectContainer(containerID.ID)

pkg/kubelet/rkt/rkt.go

@@ -1837,6 +1837,10 @@ func podDetailsFromServiceFile(serviceFilePath string) (string, string, string,
 	return "", "", "", false, fmt.Errorf("failed to parse pod from file %s", serviceFilePath)
 }
 
+func (r *Runtime) DeleteContainer(containerID kubecontainer.ContainerID) error {
+	return fmt.Errorf("unimplemented")
+}
+
 // GarbageCollect collects the pods/containers.
 // After one GC iteration:
 // - The deleted pods will be removed.