From c3ce58b934fb3b47f38b47d98d65071bf737c9f4 Mon Sep 17 00:00:00 2001 From: Random-Liu Date: Fri, 30 Sep 2016 17:08:32 -0700 Subject: [PATCH] Implement temporary ImageStats in kuberuntime_manager, and fix a bug in dockershim which causes summary api not working properly. --- pkg/kubelet/api/testing/fake_image_service.go | 24 +++++++++++-------- .../dockershim/docker_container_test.go | 12 ++++++++-- pkg/kubelet/dockershim/docker_sandbox.go | 4 ++++ pkg/kubelet/dockershim/docker_sandbox_test.go | 10 +++++++- pkg/kubelet/dockershim/helpers.go | 10 ++++++-- pkg/kubelet/dockershim/naming.go | 3 ++- pkg/kubelet/kuberuntime/kuberuntime_image.go | 16 ++++++++++--- .../kuberuntime/kuberuntime_image_test.go | 15 ++++++++++++ 8 files changed, 75 insertions(+), 19 deletions(-) diff --git a/pkg/kubelet/api/testing/fake_image_service.go b/pkg/kubelet/api/testing/fake_image_service.go index cde7ee77ea2..d53fec9e638 100644 --- a/pkg/kubelet/api/testing/fake_image_service.go +++ b/pkg/kubelet/api/testing/fake_image_service.go @@ -24,15 +24,12 @@ import ( "k8s.io/kubernetes/pkg/kubelet/util/sliceutils" ) -var ( - fakeImageSize uint64 = 1 -) - type FakeImageService struct { sync.Mutex - Called []string - Images map[string]*runtimeApi.Image + FakeImageSize uint64 + Called []string + Images map[string]*runtimeApi.Image } func (r *FakeImageService) SetFakeImages(images []string) { @@ -41,10 +38,17 @@ func (r *FakeImageService) SetFakeImages(images []string) { r.Images = make(map[string]*runtimeApi.Image) for _, image := range images { - r.Images[image] = makeFakeImage(image) + r.Images[image] = r.makeFakeImage(image) } } +func (r *FakeImageService) SetFakeImageSize(size uint64) { + r.Lock() + defer r.Unlock() + + r.FakeImageSize = size +} + func NewFakeImageService() *FakeImageService { return &FakeImageService{ Called: make([]string, 0), @@ -52,10 +56,10 @@ func NewFakeImageService() *FakeImageService { } } -func makeFakeImage(image string) *runtimeApi.Image { +func (r *FakeImageService) makeFakeImage(image string) *runtimeApi.Image { return &runtimeApi.Image{ Id: &image, - Size_: &fakeImageSize, + Size_: &r.FakeImageSize, RepoTags: []string{image}, } } @@ -102,7 +106,7 @@ func (r *FakeImageService) PullImage(image *runtimeApi.ImageSpec, auth *runtimeA // image's name for easily making fake images. imageID := image.GetImage() if _, ok := r.Images[imageID]; !ok { - r.Images[imageID] = makeFakeImage(image.GetImage()) + r.Images[imageID] = r.makeFakeImage(image.GetImage()) } return nil diff --git a/pkg/kubelet/dockershim/docker_container_test.go b/pkg/kubelet/dockershim/docker_container_test.go index 110275fd232..d30b1957a59 100644 --- a/pkg/kubelet/dockershim/docker_container_test.go +++ b/pkg/kubelet/dockershim/docker_container_test.go @@ -92,7 +92,7 @@ func TestListContainers(t *testing.T) { // TestContainerStatus tests the basic lifecycle operations and verify that // the status returned reflects the operations performed. func TestContainerStatus(t *testing.T) { - ds, _, fClock := newTestDockerService() + ds, fDocker, fClock := newTestDockerService() sConfig := makeSandboxConfig("foo", "bar", "1", 0) labels := map[string]string{"abc.xyz": "foo"} annotations := map[string]string{"foo.bar.baz": "abc"} @@ -126,7 +126,15 @@ func TestContainerStatus(t *testing.T) { // Create the container. fClock.SetTime(time.Now().Add(-1 * time.Hour)) *expected.CreatedAt = fClock.Now().Unix() - id, err := ds.CreateContainer("sandboxid", config, sConfig) + const sandboxId = "sandboxid" + id, err := ds.CreateContainer(sandboxId, config, sConfig) + + // Check internal labels + c, err := fDocker.InspectContainer(id) + assert.NoError(t, err) + assert.Equal(t, c.Config.Labels[containerTypeLabelKey], containerTypeLabelContainer) + assert.Equal(t, c.Config.Labels[sandboxIDLabelKey], sandboxId) + // Set the id manually since we don't know the id until it's created. expected.Id = &id assert.NoError(t, err) diff --git a/pkg/kubelet/dockershim/docker_sandbox.go b/pkg/kubelet/dockershim/docker_sandbox.go index 125eef34f7e..1658fe6a7a1 100644 --- a/pkg/kubelet/dockershim/docker_sandbox.go +++ b/pkg/kubelet/dockershim/docker_sandbox.go @@ -26,6 +26,7 @@ import ( runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" "k8s.io/kubernetes/pkg/kubelet/qos" + "k8s.io/kubernetes/pkg/kubelet/types" ) const ( @@ -202,6 +203,9 @@ func (ds *dockerService) makeSandboxDockerConfig(c *runtimeApi.PodSandboxConfig, labels := makeLabels(c.GetLabels(), c.GetAnnotations()) // Apply a label to distinguish sandboxes from regular containers. labels[containerTypeLabelKey] = containerTypeLabelSandbox + // Apply a container name label for infra container. This is used in summary api. + // TODO(random-liu): Deprecate this label once container metrics is directly got from CRI. + labels[types.KubernetesContainerNameLabel] = sandboxContainerName hc := &dockercontainer.HostConfig{} createConfig := &dockertypes.ContainerCreateConfig{ diff --git a/pkg/kubelet/dockershim/docker_sandbox_test.go b/pkg/kubelet/dockershim/docker_sandbox_test.go index 8fcf1f0c6d0..5595ee2158f 100644 --- a/pkg/kubelet/dockershim/docker_sandbox_test.go +++ b/pkg/kubelet/dockershim/docker_sandbox_test.go @@ -25,6 +25,7 @@ import ( "github.com/stretchr/testify/assert" runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" + "k8s.io/kubernetes/pkg/kubelet/types" ) // A helper to create a basic config. @@ -86,7 +87,7 @@ func TestListSandboxes(t *testing.T) { // TestSandboxStatus tests the basic lifecycle operations and verify that // the status returned reflects the operations performed. func TestSandboxStatus(t *testing.T) { - ds, _, fClock := newTestDockerService() + ds, fDocker, fClock := newTestDockerService() labels := map[string]string{"label": "foobar1"} annotations := map[string]string{"annotation": "abc"} config := makeSandboxConfigWithLabelsAndAnnotations("foo", "bar", "1", 0, labels, annotations) @@ -112,6 +113,13 @@ func TestSandboxStatus(t *testing.T) { fClock.SetTime(time.Now()) *expected.CreatedAt = fClock.Now().Unix() id, err := ds.RunPodSandbox(config) + + // Check internal labels + c, err := fDocker.InspectContainer(id) + assert.NoError(t, err) + assert.Equal(t, c.Config.Labels[containerTypeLabelKey], containerTypeLabelSandbox) + assert.Equal(t, c.Config.Labels[types.KubernetesContainerNameLabel], sandboxContainerName) + expected.Id = &id // ID is only known after the creation. status, err := ds.PodSandboxStatus(id) assert.NoError(t, err) diff --git a/pkg/kubelet/dockershim/helpers.go b/pkg/kubelet/dockershim/helpers.go index 1a444b3aa4a..83fa2cc907e 100644 --- a/pkg/kubelet/dockershim/helpers.go +++ b/pkg/kubelet/dockershim/helpers.go @@ -29,6 +29,7 @@ import ( runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" "k8s.io/kubernetes/pkg/kubelet/dockertools" + "k8s.io/kubernetes/pkg/kubelet/types" ) const ( @@ -87,8 +88,6 @@ func extractLabels(input map[string]string) (map[string]string, map[string]strin // Check if the key is used internally by the shim. internal := false for _, internalKey := range internalLabelKeys { - // TODO: containerTypeLabelKey is the only internal label the shim uses - // right now. Expand this to a list later. if k == internalKey { internal = true break @@ -98,6 +97,13 @@ func extractLabels(input map[string]string) (map[string]string, map[string]strin continue } + // Delete the container name label for the sandbox. It is added in the shim, + // should not be exposed via CRI. + if k == types.KubernetesContainerNameLabel && + input[containerTypeLabelKey] == containerTypeLabelSandbox { + continue + } + // Check if the label should be treated as an annotation. if strings.HasPrefix(k, annotationPrefix) { annotations[strings.TrimPrefix(k, annotationPrefix)] = v diff --git a/pkg/kubelet/dockershim/naming.go b/pkg/kubelet/dockershim/naming.go index a5c1cf0c8cf..86147645064 100644 --- a/pkg/kubelet/dockershim/naming.go +++ b/pkg/kubelet/dockershim/naming.go @@ -22,6 +22,7 @@ import ( "strings" runtimeApi "k8s.io/kubernetes/pkg/kubelet/api/v1alpha1/runtime" + "k8s.io/kubernetes/pkg/kubelet/leaky" ) // Container "names" are implementation details that do not concern @@ -44,7 +45,7 @@ const ( kubePrefix = "k8s" // sandboxContainerName is a string to include in the docker container so // that users can easily identify the sandboxes. - sandboxContainerName = "POD" + sandboxContainerName = leaky.PodInfraContainerName // Delimiter used to construct docker container names. nameDelimiter = "_" ) diff --git a/pkg/kubelet/kuberuntime/kuberuntime_image.go b/pkg/kubelet/kuberuntime/kuberuntime_image.go index d093c8bbc97..b9b9cb1f463 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_image.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_image.go @@ -127,8 +127,18 @@ func (m *kubeGenericRuntimeManager) RemoveImage(image kubecontainer.ImageSpec) e } // ImageStats returns the statistics of the image. -// TODO: Implement this function. +// Notice that current logic doesn't really work for images which share layers (e.g. docker image), +// this is a known issue, and we'll address this by getting imagefs stats directly from CRI. +// TODO: Get imagefs stats directly from CRI. func (m *kubeGenericRuntimeManager) ImageStats() (*kubecontainer.ImageStats, error) { - var usageBytes uint64 = 0 - return &kubecontainer.ImageStats{TotalStorageBytes: usageBytes}, nil + allImages, err := m.imageService.ListImages(nil) + if err != nil { + glog.Errorf("ListImages failed: %v", err) + return nil, err + } + stats := &kubecontainer.ImageStats{} + for _, img := range allImages { + stats.TotalStorageBytes += img.GetSize_() + } + return stats, nil } diff --git a/pkg/kubelet/kuberuntime/kuberuntime_image_test.go b/pkg/kubelet/kuberuntime/kuberuntime_image_test.go index 429daf26a63..9124de9cc9b 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_image_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_image_test.go @@ -78,3 +78,18 @@ func TestRemoveImage(t *testing.T) { assert.NoError(t, err) assert.Equal(t, 0, len(fakeImageService.Images)) } + +func TestImageStats(t *testing.T) { + _, fakeImageService, fakeManager, err := createTestRuntimeManager() + assert.NoError(t, err) + + const imageSize = 64 + fakeImageService.SetFakeImageSize(imageSize) + images := []string{"1111", "2222", "3333"} + fakeImageService.SetFakeImages(images) + + actualStats, err := fakeManager.ImageStats() + assert.NoError(t, err) + expectedStats := &kubecontainer.ImageStats{TotalStorageBytes: imageSize * uint64(len(images))} + assert.Equal(t, expectedStats, actualStats) +}