Merge pull request #33870 from Random-Liu/summary-api-cri

Automatic merge from submit-queue

CRI: Implement temporary ImageStats in kuberuntime_manager

For #33048 and #33189.

This PR:
1) Implement a temporary `ImageStats` in kuberuntime_manager.go
2) Add container name label on infra container to make the current summary api logic work with dockershim.

I run the summary api test locally and it passed for me. Notice that the original summary api test is not showing up on CRI testgrid because it was removed yesterday. It will be added back in https://github.com/kubernetes/kubernetes/pull/33779.

@yujuhong @feiskyer

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

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 (
@@ -207,6 +208,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
-	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
 }

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