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,13 +24,10 @@ import (
 	"k8s.io/kubernetes/pkg/kubelet/util/sliceutils"
 )
 
-var (
-	fakeImageSize uint64 = 1
-)
-
 type FakeImageService struct {
 	sync.Mutex
 
+	FakeImageSize uint64
 	Called        []string
 	Images        map[string]*runtimeApi.Image
 }
@@ -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

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)

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{

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)

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

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 = "_"
 )

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
+	allImages, err := m.imageService.ListImages(nil)
-	return &kubecontainer.ImageStats{TotalStorageBytes: usageBytes}, 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
 }

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)
+}