From ce7003262f8414955aa5e31e05534efe57e9c575 Mon Sep 17 00:00:00 2001 From: WanLinghao Date: Mon, 7 Jan 2019 12:52:27 +0800 Subject: [PATCH] Improve pkg/kubelet/cadvisor package: 1. Perfect unit test code 2. Clean unused function&&test file --- pkg/kubelet/cadvisor/BUILD | 11 +--- pkg/kubelet/cadvisor/cadvisor_linux.go | 30 ---------- pkg/kubelet/cadvisor/cadvisor_linux_test.go | 65 --------------------- pkg/kubelet/cadvisor/util_test.go | 55 ++++++++++++----- 4 files changed, 43 insertions(+), 118 deletions(-) delete mode 100644 pkg/kubelet/cadvisor/cadvisor_linux_test.go diff --git a/pkg/kubelet/cadvisor/BUILD b/pkg/kubelet/cadvisor/BUILD index f8421a3977c..cf30e5aa3ff 100644 --- a/pkg/kubelet/cadvisor/BUILD +++ b/pkg/kubelet/cadvisor/BUILD @@ -35,7 +35,6 @@ go_library( "//vendor/github.com/google/cadvisor/container:go_default_library", "//vendor/github.com/google/cadvisor/fs:go_default_library", "//vendor/github.com/google/cadvisor/manager:go_default_library", - "//vendor/github.com/google/cadvisor/metrics:go_default_library", "//vendor/github.com/google/cadvisor/utils/sysfs:go_default_library", "//vendor/k8s.io/klog:go_default_library", ], @@ -48,21 +47,17 @@ go_library( go_test( name = "go_default_test", - srcs = [ - "cadvisor_linux_test.go", - "util_test.go", - ], + srcs = ["util_test.go"], embed = [":go_default_library"], deps = select({ "@io_bazel_rules_go//go/platform:linux": [ - "//pkg/apis/core/v1/helper:go_default_library", "//pkg/features:go_default_library", - "//pkg/kubelet/types:go_default_library", + "//staging/src/k8s.io/api/core/v1:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature:go_default_library", "//staging/src/k8s.io/apiserver/pkg/util/feature/testing:go_default_library", + "//vendor/github.com/google/cadvisor/container/crio:go_default_library", "//vendor/github.com/google/cadvisor/info/v1:go_default_library", - "//vendor/github.com/google/cadvisor/metrics:go_default_library", "//vendor/github.com/stretchr/testify/assert:go_default_library", ], "//conditions:default": [], diff --git a/pkg/kubelet/cadvisor/cadvisor_linux.go b/pkg/kubelet/cadvisor/cadvisor_linux.go index 0f02a811426..41d057c42e0 100644 --- a/pkg/kubelet/cadvisor/cadvisor_linux.go +++ b/pkg/kubelet/cadvisor/cadvisor_linux.go @@ -32,10 +32,8 @@ import ( cadvisorapi "github.com/google/cadvisor/info/v1" cadvisorapiv2 "github.com/google/cadvisor/info/v2" "github.com/google/cadvisor/manager" - "github.com/google/cadvisor/metrics" "github.com/google/cadvisor/utils/sysfs" "k8s.io/klog" - "k8s.io/kubernetes/pkg/kubelet/types" ) type cadvisorClient struct { @@ -72,34 +70,6 @@ func init() { } } -func containerLabels(c *cadvisorapi.ContainerInfo) map[string]string { - // Prometheus requires that all metrics in the same family have the same labels, - // so we arrange to supply blank strings for missing labels - var name, image, podName, namespace, containerName string - if len(c.Aliases) > 0 { - name = c.Aliases[0] - } - image = c.Spec.Image - if v, ok := c.Spec.Labels[types.KubernetesPodNameLabel]; ok { - podName = v - } - if v, ok := c.Spec.Labels[types.KubernetesPodNamespaceLabel]; ok { - namespace = v - } - if v, ok := c.Spec.Labels[types.KubernetesContainerNameLabel]; ok { - containerName = v - } - set := map[string]string{ - metrics.LabelID: c.Name, - metrics.LabelName: name, - metrics.LabelImage: image, - "pod_name": podName, - "namespace": namespace, - "container_name": containerName, - } - return set -} - func New(imageFsInfoProvider ImageFsInfoProvider, rootPath string, usingLegacyStats bool) (Interface, error) { sysFs := sysfs.NewRealSysFs() diff --git a/pkg/kubelet/cadvisor/cadvisor_linux_test.go b/pkg/kubelet/cadvisor/cadvisor_linux_test.go deleted file mode 100644 index 48f06e026c1..00000000000 --- a/pkg/kubelet/cadvisor/cadvisor_linux_test.go +++ /dev/null @@ -1,65 +0,0 @@ -// +build cgo,linux - -/* -Copyright 2016 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package cadvisor - -import ( - "reflect" - "testing" - - info "github.com/google/cadvisor/info/v1" - "github.com/google/cadvisor/metrics" - "k8s.io/kubernetes/pkg/kubelet/types" -) - -func TestContainerLabels(t *testing.T) { - container := &info.ContainerInfo{ - ContainerReference: info.ContainerReference{ - Name: "/docker/f81ad5335d390944e454ea19ab0924037d57337c19731524ad96eb26e74b6c6d", - Aliases: []string{"k8s_POD.639b2af2_foo-web-315473031-e40e2_foobar_a369ace2-5fa9-11e6-b10f-c81f66e5e84d_851a97fd"}, - }, - Spec: info.ContainerSpec{ - Image: "qux/foo:latest", - Labels: map[string]string{ - "io.kubernetes.container.hash": "639b2af2", - types.KubernetesContainerNameLabel: "POD", - "io.kubernetes.container.restartCount": "0", - "io.kubernetes.container.terminationMessagePath": "", - types.KubernetesPodNameLabel: "foo-web-315473031-e40e2", - types.KubernetesPodNamespaceLabel: "foobar", - "io.kubernetes.pod.terminationGracePeriod": "30", - types.KubernetesPodUIDLabel: "a369ace2-5fa9-11e6-b10f-c81f66e5e84d", - }, - Envs: map[string]string{ - "foo+env": "prod", - }, - }, - } - want := map[string]string{ - metrics.LabelID: "/docker/f81ad5335d390944e454ea19ab0924037d57337c19731524ad96eb26e74b6c6d", - metrics.LabelName: "k8s_POD.639b2af2_foo-web-315473031-e40e2_foobar_a369ace2-5fa9-11e6-b10f-c81f66e5e84d_851a97fd", - metrics.LabelImage: "qux/foo:latest", - "namespace": "foobar", - "container_name": "POD", - "pod_name": "foo-web-315473031-e40e2", - } - - if have := containerLabels(container); !reflect.DeepEqual(want, have) { - t.Errorf("want %v, have %v", want, have) - } -} diff --git a/pkg/kubelet/cadvisor/util_test.go b/pkg/kubelet/cadvisor/util_test.go index 5d83e861cd2..bb65b9acea0 100644 --- a/pkg/kubelet/cadvisor/util_test.go +++ b/pkg/kubelet/cadvisor/util_test.go @@ -19,18 +19,20 @@ limitations under the License. package cadvisor import ( + "reflect" "testing" + "github.com/google/cadvisor/container/crio" info "github.com/google/cadvisor/info/v1" "github.com/stretchr/testify/assert" + "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" utilfeature "k8s.io/apiserver/pkg/util/feature" utilfeaturetesting "k8s.io/apiserver/pkg/util/feature/testing" - v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" "k8s.io/kubernetes/pkg/features" ) -func TestCapacityFromMachineInfo(t *testing.T) { +func TestCapacityFromMachineInfoWithHugePagesEnable(t *testing.T) { machineInfo := &info.MachineInfo{ NumCores: 2, MemoryCapacity: 2048, @@ -42,18 +44,41 @@ func TestCapacityFromMachineInfo(t *testing.T) { }, } - // enable the features.HugePages + expected := v1.ResourceList{ + v1.ResourceCPU: *resource.NewMilliQuantity(int64(2000), resource.DecimalSI), + v1.ResourceMemory: *resource.NewQuantity(int64(2048), resource.BinarySI), + "hugepages-5Ki": *resource.NewQuantity(int64(51200), resource.BinarySI), + } defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HugePages, true)() - - resourceList := CapacityFromMachineInfo(machineInfo) - - // assert the cpu and memory - assert.EqualValues(t, machineInfo.NumCores*1000, resourceList.Cpu().MilliValue(), "unexpected CPU value") - assert.EqualValues(t, machineInfo.MemoryCapacity, resourceList.Memory().Value(), "unexpected memory value") - - // assert the hugepage - hugePageKey := int64(5 * 1024) - value, found := resourceList[v1helper.HugePageResourceName(*resource.NewQuantity(hugePageKey, resource.BinarySI))] - assert.True(t, found, "hugepage not found") - assert.EqualValues(t, hugePageKey*10, value.Value(), "unexpected hugepage value") + actual := CapacityFromMachineInfo(machineInfo) + if !reflect.DeepEqual(actual, expected) { + t.Errorf("when set hugepages true, got resource list %v, want %v", actual, expected) + } +} + +func TestCapacityFromMachineInfoWithHugePagesDisable(t *testing.T) { + machineInfo := &info.MachineInfo{ + NumCores: 2, + MemoryCapacity: 2048, + HugePages: []info.HugePagesInfo{ + { + PageSize: 5, + NumPages: 10, + }, + }, + } + + expected := v1.ResourceList{ + v1.ResourceCPU: *resource.NewMilliQuantity(int64(2000), resource.DecimalSI), + v1.ResourceMemory: *resource.NewQuantity(int64(2048), resource.BinarySI), + } + defer utilfeaturetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.HugePages, false)() + actual := CapacityFromMachineInfo(machineInfo) + if !reflect.DeepEqual(actual, expected) { + t.Errorf("when set hugepages false, got resource list %v, want %v", actual, expected) + } +} + +func TestCrioSocket(t *testing.T) { + assert.EqualValues(t, CrioSocket, crio.CrioSocket, "CrioSocket in this package must equal the one in github.com/google/cadvisor/container/crio/client.go") }