From 4ecab5ea4a826365e4e5effdc72116c28cb5e2a1 Mon Sep 17 00:00:00 2001 From: Lantao Liu Date: Fri, 3 May 2019 17:14:24 -0700 Subject: [PATCH] Remove terminated pod from summary api. Signed-off-by: Lantao Liu --- pkg/kubelet/stats/BUILD | 1 + pkg/kubelet/stats/cri_stats_provider.go | 54 +++++++++++++++-- pkg/kubelet/stats/cri_stats_provider_test.go | 60 +++++++++++++------ .../stats/log_metrics_provider_test.go | 5 ++ 4 files changed, 98 insertions(+), 22 deletions(-) diff --git a/pkg/kubelet/stats/BUILD b/pkg/kubelet/stats/BUILD index 7d24ec8cdf..149e3b839e 100644 --- a/pkg/kubelet/stats/BUILD +++ b/pkg/kubelet/stats/BUILD @@ -89,6 +89,7 @@ go_test( "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/types:go_default_library", + "//staging/src/k8s.io/apimachinery/pkg/util/uuid:go_default_library", "//vendor/github.com/golang/mock/gomock:go_default_library", "//vendor/github.com/google/cadvisor/fs:go_default_library", "//vendor/github.com/google/cadvisor/info/v1:go_default_library", diff --git a/pkg/kubelet/stats/cri_stats_provider.go b/pkg/kubelet/stats/cri_stats_provider.go index f55c02b8d2..c25cb853af 100644 --- a/pkg/kubelet/stats/cri_stats_provider.go +++ b/pkg/kubelet/stats/cri_stats_provider.go @@ -137,6 +137,7 @@ func (p *criStatsProvider) listPodStats(updateCPUNanoCoreUsage bool) ([]statsapi if err != nil { return nil, fmt.Errorf("failed to list all pod sandboxes: %v", err) } + podSandboxes = removeTerminatedPods(podSandboxes) for _, s := range podSandboxes { podSandboxMap[s.Id] = s } @@ -153,7 +154,7 @@ func (p *criStatsProvider) listPodStats(updateCPUNanoCoreUsage bool) ([]statsapi return nil, fmt.Errorf("failed to list all container stats: %v", err) } - containers = removeTerminatedContainer(containers) + containers = removeTerminatedContainers(containers) // Creates container map. containerMap := make(map[string]*runtimeapi.Container) for _, c := range containers { @@ -233,6 +234,7 @@ func (p *criStatsProvider) ListPodCPUAndMemoryStats() ([]statsapi.PodStats, erro if err != nil { return nil, fmt.Errorf("failed to list all pod sandboxes: %v", err) } + podSandboxes = removeTerminatedPods(podSandboxes) for _, s := range podSandboxes { podSandboxMap[s.Id] = s } @@ -245,7 +247,7 @@ func (p *criStatsProvider) ListPodCPUAndMemoryStats() ([]statsapi.PodStats, erro return nil, fmt.Errorf("failed to list all container stats: %v", err) } - containers = removeTerminatedContainer(containers) + containers = removeTerminatedContainers(containers) // Creates container map. containerMap := make(map[string]*runtimeapi.Container) for _, c := range containers { @@ -690,9 +692,51 @@ func (p *criStatsProvider) cleanupOutdatedCaches() { } } -// removeTerminatedContainer returns the specified container but with -// the stats of the terminated containers removed. -func removeTerminatedContainer(containers []*runtimeapi.Container) []*runtimeapi.Container { +// removeTerminatedPods returns pods with terminated ones removed. +// It only removes a terminated pod when there is a running instance +// of the pod with the same name and namespace. +// This is needed because: +// 1) PodSandbox may be recreated; +// 2) Pod may be recreated with the same name and namespace. +func removeTerminatedPods(pods []*runtimeapi.PodSandbox) []*runtimeapi.PodSandbox { + podMap := make(map[statsapi.PodReference][]*runtimeapi.PodSandbox) + // Sort order by create time + sort.Slice(pods, func(i, j int) bool { + return pods[i].CreatedAt < pods[j].CreatedAt + }) + for _, pod := range pods { + refID := statsapi.PodReference{ + Name: pod.GetMetadata().GetName(), + Namespace: pod.GetMetadata().GetNamespace(), + // UID is intentionally left empty. + } + podMap[refID] = append(podMap[refID], pod) + } + + result := make([]*runtimeapi.PodSandbox, 0) + for _, refs := range podMap { + if len(refs) == 1 { + result = append(result, refs[0]) + continue + } + found := false + for i := 0; i < len(refs); i++ { + if refs[i].State == runtimeapi.PodSandboxState_SANDBOX_READY { + found = true + result = append(result, refs[i]) + } + } + if !found { + result = append(result, refs[len(refs)-1]) + } + } + return result +} + +// removeTerminatedContainers returns containers with terminated ones. +// It only removes a terminated container when there is a running instance +// of the container. +func removeTerminatedContainers(containers []*runtimeapi.Container) []*runtimeapi.Container { containerMap := make(map[containerID][]*runtimeapi.Container) // Sort order by create time sort.Slice(containers, func(i, j int) bool { diff --git a/pkg/kubelet/stats/cri_stats_provider_test.go b/pkg/kubelet/stats/cri_stats_provider_test.go index 10f5c32789..fa4e651009 100644 --- a/pkg/kubelet/stats/cri_stats_provider_test.go +++ b/pkg/kubelet/stats/cri_stats_provider_test.go @@ -31,6 +31,7 @@ import ( "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/uuid" runtimeapi "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2" critest "k8s.io/kubernetes/pkg/kubelet/apis/cri/testing" statsapi "k8s.io/kubernetes/pkg/kubelet/apis/stats/v1alpha1" @@ -77,6 +78,8 @@ const ( cName2 = "container2-name" cName3 = "container3-name" cName5 = "container5-name" + cName6 = "container6-name" + cName7 = "container7-name" ) func TestCRIListPodStats(t *testing.T) { @@ -86,7 +89,7 @@ func TestCRIListPodStats(t *testing.T) { imageFsInfo = getTestFsInfo(2000) rootFsInfo = getTestFsInfo(1000) - sandbox0 = makeFakePodSandbox("sandbox0-name", "sandbox0-uid", "sandbox0-ns") + sandbox0 = makeFakePodSandbox("sandbox0-name", "sandbox0-uid", "sandbox0-ns", false) sandbox0Cgroup = "/" + cm.GetPodCgroupNameSuffix(types.UID(sandbox0.PodSandboxStatus.Metadata.Uid)) container0 = makeFakeContainer(sandbox0, cName0, 0, false) containerStats0 = makeFakeContainerStats(container0, imageFsMountpoint) @@ -95,13 +98,13 @@ func TestCRIListPodStats(t *testing.T) { containerStats1 = makeFakeContainerStats(container1, unknownMountpoint) containerLogStats1 = makeFakeLogStats(2000) - sandbox1 = makeFakePodSandbox("sandbox1-name", "sandbox1-uid", "sandbox1-ns") + sandbox1 = makeFakePodSandbox("sandbox1-name", "sandbox1-uid", "sandbox1-ns", false) sandbox1Cgroup = "/" + cm.GetPodCgroupNameSuffix(types.UID(sandbox1.PodSandboxStatus.Metadata.Uid)) container2 = makeFakeContainer(sandbox1, cName2, 0, false) containerStats2 = makeFakeContainerStats(container2, imageFsMountpoint) containerLogStats2 = makeFakeLogStats(3000) - sandbox2 = makeFakePodSandbox("sandbox2-name", "sandbox2-uid", "sandbox2-ns") + sandbox2 = makeFakePodSandbox("sandbox2-name", "sandbox2-uid", "sandbox2-ns", false) sandbox2Cgroup = "/" + cm.GetPodCgroupNameSuffix(types.UID(sandbox2.PodSandboxStatus.Metadata.Uid)) container3 = makeFakeContainer(sandbox2, cName3, 0, true) containerStats3 = makeFakeContainerStats(container3, imageFsMountpoint) @@ -109,11 +112,21 @@ func TestCRIListPodStats(t *testing.T) { containerStats4 = makeFakeContainerStats(container4, imageFsMountpoint) containerLogStats4 = makeFakeLogStats(4000) - sandbox3 = makeFakePodSandbox("sandbox3-name", "sandbox3-uid", "sandbox3-ns") + sandbox3 = makeFakePodSandbox("sandbox3-name", "sandbox3-uid", "sandbox3-ns", false) container5 = makeFakeContainer(sandbox3, cName5, 0, true) containerStats5 = makeFakeContainerStats(container5, imageFsMountpoint) containerLogStats5 = makeFakeLogStats(5000) + // Terminated pod sandbox + sandbox4 = makeFakePodSandbox("sandbox1-name", "sandbox1-uid", "sandbox1-ns", true) + container6 = makeFakeContainer(sandbox4, cName6, 0, true) + containerStats6 = makeFakeContainerStats(container6, imageFsMountpoint) + + // Terminated pod + sandbox5 = makeFakePodSandbox("sandbox1-name", "sandbox5-uid", "sandbox1-ns", true) + container7 = makeFakeContainer(sandbox5, cName7, 0, true) + containerStats7 = makeFakeContainerStats(container7, imageFsMountpoint) + podLogName0 = "pod-log-0" podLogName1 = "pod-log-1" podLogStats0 = makeFakeLogStats(5000) @@ -157,13 +170,13 @@ func TestCRIListPodStats(t *testing.T) { On("GetDirFsInfo", imageFsMountpoint).Return(imageFsInfo, nil). On("GetDirFsInfo", unknownMountpoint).Return(cadvisorapiv2.FsInfo{}, cadvisorfs.ErrNoSuchDevice) fakeRuntimeService.SetFakeSandboxes([]*critest.FakePodSandbox{ - sandbox0, sandbox1, sandbox2, sandbox3, + sandbox0, sandbox1, sandbox2, sandbox3, sandbox4, sandbox5, }) fakeRuntimeService.SetFakeContainers([]*critest.FakeContainer{ - container0, container1, container2, container3, container4, container5, + container0, container1, container2, container3, container4, container5, container6, container7, }) fakeRuntimeService.SetFakeContainerStats([]*runtimeapi.ContainerStats{ - containerStats0, containerStats1, containerStats2, containerStats3, containerStats4, containerStats5, + containerStats0, containerStats1, containerStats2, containerStats3, containerStats4, containerStats5, containerStats6, containerStats7, }) ephemeralVolumes := makeFakeVolumeStats([]string{"ephVolume1, ephVolumes2"}) @@ -304,28 +317,38 @@ func TestCRIListPodCPUAndMemoryStats(t *testing.T) { imageFsMountpoint = "/test/mount/point" unknownMountpoint = "/unknown/mount/point" - sandbox0 = makeFakePodSandbox("sandbox0-name", "sandbox0-uid", "sandbox0-ns") + sandbox0 = makeFakePodSandbox("sandbox0-name", "sandbox0-uid", "sandbox0-ns", false) sandbox0Cgroup = "/" + cm.GetPodCgroupNameSuffix(types.UID(sandbox0.PodSandboxStatus.Metadata.Uid)) container0 = makeFakeContainer(sandbox0, cName0, 0, false) containerStats0 = makeFakeContainerStats(container0, imageFsMountpoint) container1 = makeFakeContainer(sandbox0, cName1, 0, false) containerStats1 = makeFakeContainerStats(container1, unknownMountpoint) - sandbox1 = makeFakePodSandbox("sandbox1-name", "sandbox1-uid", "sandbox1-ns") + sandbox1 = makeFakePodSandbox("sandbox1-name", "sandbox1-uid", "sandbox1-ns", false) sandbox1Cgroup = "/" + cm.GetPodCgroupNameSuffix(types.UID(sandbox1.PodSandboxStatus.Metadata.Uid)) container2 = makeFakeContainer(sandbox1, cName2, 0, false) containerStats2 = makeFakeContainerStats(container2, imageFsMountpoint) - sandbox2 = makeFakePodSandbox("sandbox2-name", "sandbox2-uid", "sandbox2-ns") + sandbox2 = makeFakePodSandbox("sandbox2-name", "sandbox2-uid", "sandbox2-ns", false) sandbox2Cgroup = "/" + cm.GetPodCgroupNameSuffix(types.UID(sandbox2.PodSandboxStatus.Metadata.Uid)) container3 = makeFakeContainer(sandbox2, cName3, 0, true) containerStats3 = makeFakeContainerStats(container3, imageFsMountpoint) container4 = makeFakeContainer(sandbox2, cName3, 1, false) containerStats4 = makeFakeContainerStats(container4, imageFsMountpoint) - sandbox3 = makeFakePodSandbox("sandbox3-name", "sandbox3-uid", "sandbox3-ns") + sandbox3 = makeFakePodSandbox("sandbox3-name", "sandbox3-uid", "sandbox3-ns", false) container5 = makeFakeContainer(sandbox3, cName5, 0, true) containerStats5 = makeFakeContainerStats(container5, imageFsMountpoint) + + // Terminated pod sandbox + sandbox4 = makeFakePodSandbox("sandbox1-name", "sandbox1-uid", "sandbox1-ns", true) + container6 = makeFakeContainer(sandbox4, cName6, 0, true) + containerStats6 = makeFakeContainerStats(container6, imageFsMountpoint) + + // Terminated pod + sandbox5 = makeFakePodSandbox("sandbox1-name", "sandbox5-uid", "sandbox1-ns", true) + container7 = makeFakeContainer(sandbox5, cName7, 0, true) + containerStats7 = makeFakeContainerStats(container7, imageFsMountpoint) ) var ( @@ -361,13 +384,13 @@ func TestCRIListPodCPUAndMemoryStats(t *testing.T) { mockCadvisor. On("ContainerInfoV2", "/", options).Return(infos, nil) fakeRuntimeService.SetFakeSandboxes([]*critest.FakePodSandbox{ - sandbox0, sandbox1, sandbox2, sandbox3, + sandbox0, sandbox1, sandbox2, sandbox3, sandbox4, sandbox5, }) fakeRuntimeService.SetFakeContainers([]*critest.FakeContainer{ - container0, container1, container2, container3, container4, container5, + container0, container1, container2, container3, container4, container5, container6, container7, }) fakeRuntimeService.SetFakeContainerStats([]*runtimeapi.ContainerStats{ - containerStats0, containerStats1, containerStats2, containerStats3, containerStats4, containerStats5, + containerStats0, containerStats1, containerStats2, containerStats3, containerStats4, containerStats5, containerStats6, containerStats7, }) ephemeralVolumes := makeFakeVolumeStats([]string{"ephVolume1, ephVolumes2"}) @@ -523,7 +546,7 @@ func TestCRIImagesFsStats(t *testing.T) { mockCadvisor.AssertExpectations(t) } -func makeFakePodSandbox(name, uid, namespace string) *critest.FakePodSandbox { +func makeFakePodSandbox(name, uid, namespace string, terminated bool) *critest.FakePodSandbox { p := &critest.FakePodSandbox{ PodSandboxStatus: runtimeapi.PodSandboxStatus{ Metadata: &runtimeapi.PodSandboxMetadata{ @@ -535,7 +558,10 @@ func makeFakePodSandbox(name, uid, namespace string) *critest.FakePodSandbox { CreatedAt: time.Now().UnixNano(), }, } - p.PodSandboxStatus.Id = critest.BuildSandboxName(p.PodSandboxStatus.Metadata) + if terminated { + p.PodSandboxStatus.State = runtimeapi.PodSandboxState_SANDBOX_NOTREADY + } + p.PodSandboxStatus.Id = string(uuid.NewUUID()) return p } @@ -561,7 +587,7 @@ func makeFakeContainer(sandbox *critest.FakePodSandbox, name string, attempt uin } else { c.ContainerStatus.State = runtimeapi.ContainerState_CONTAINER_RUNNING } - c.ContainerStatus.Id = critest.BuildContainerName(c.ContainerStatus.Metadata, sandboxID) + c.ContainerStatus.Id = string(uuid.NewUUID()) return c } diff --git a/pkg/kubelet/stats/log_metrics_provider_test.go b/pkg/kubelet/stats/log_metrics_provider_test.go index 499d6130df..6103c07919 100644 --- a/pkg/kubelet/stats/log_metrics_provider_test.go +++ b/pkg/kubelet/stats/log_metrics_provider_test.go @@ -17,6 +17,8 @@ limitations under the License. package stats import ( + "fmt" + "k8s.io/kubernetes/pkg/volume" ) @@ -41,5 +43,8 @@ func NewFakeMetricsDu(path string, stats *volume.Metrics) volume.MetricsProvider } func (f *fakeMetricsDu) GetMetrics() (*volume.Metrics, error) { + if f.fakeStats == nil { + return nil, fmt.Errorf("no stats provided") + } return f.fakeStats, nil }