diff --git a/pkg/kubelet/container/helpers.go b/pkg/kubelet/container/helpers.go index 76eab41bc03..b0b9d0c84bb 100644 --- a/pkg/kubelet/container/helpers.go +++ b/pkg/kubelet/container/helpers.go @@ -95,11 +95,12 @@ func ConvertPodStatusToRunningPod(podStatus *PodStatus) Pod { continue } container := &Container{ - ID: containerStatus.ID, - Name: containerStatus.Name, - Image: containerStatus.Image, - Hash: containerStatus.Hash, - State: containerStatus.State, + ID: containerStatus.ID, + Name: containerStatus.Name, + Image: containerStatus.Image, + ImageID: containerStatus.ImageID, + Hash: containerStatus.Hash, + State: containerStatus.State, } runningPod.Containers = append(runningPod.Containers, container) } diff --git a/pkg/kubelet/container/runtime.go b/pkg/kubelet/container/runtime.go index ea316b7821a..c407bd97828 100644 --- a/pkg/kubelet/container/runtime.go +++ b/pkg/kubelet/container/runtime.go @@ -245,6 +245,8 @@ type Container struct { // The image name of the container, this also includes the tag of the image, // the expected form is "NAME:TAG". Image string + // The id of the image used by the container. + ImageID string // Hash of the container, used for comparison. Optional for containers // not managed by kubelet. Hash uint64 diff --git a/pkg/kubelet/dockertools/convert.go b/pkg/kubelet/dockertools/convert.go index ba440baa053..2bbf3758dce 100644 --- a/pkg/kubelet/dockertools/convert.go +++ b/pkg/kubelet/dockertools/convert.go @@ -56,10 +56,11 @@ func toRuntimeContainer(c *dockertypes.Container) (*kubecontainer.Container, err } return &kubecontainer.Container{ - ID: kubecontainer.DockerID(c.ID).ContainerID(), - Name: dockerName.ContainerName, - Image: c.Image, - Hash: hash, + ID: kubecontainer.DockerID(c.ID).ContainerID(), + Name: dockerName.ContainerName, + Image: c.Image, + ImageID: c.ImageID, + Hash: hash, // (random-liu) docker uses status to indicate whether a container is running or exited. // However, in kubernetes we usually use state to indicate whether a container is running or exited, // while use status to indicate the comprehensive status of the container. So we have different naming diff --git a/pkg/kubelet/image_manager.go b/pkg/kubelet/image_manager.go index 0b1ffa9377c..7d21892dd36 100644 --- a/pkg/kubelet/image_manager.go +++ b/pkg/kubelet/image_manager.go @@ -170,8 +170,8 @@ func (im *realImageManager) detectImages(detectTime time.Time) error { imagesInUse := sets.NewString() for _, pod := range pods { for _, container := range pod.Containers { - glog.V(5).Infof("Pod %s/%s, container %s uses image %s", pod.Namespace, pod.Name, container.Name, container.Image) - imagesInUse.Insert(container.Image) + glog.V(5).Infof("Pod %s/%s, container %s uses image %s(%s)", pod.Namespace, pod.Name, container.Name, container.Image, container.ImageID) + imagesInUse.Insert(container.ImageID) } } @@ -341,14 +341,9 @@ func (ev byLastUsedAndDetected) Less(i, j int) bool { } func isImageUsed(image container.Image, imagesInUse sets.String) bool { - // Check the image ID and all the RepoTags and RepoDigests. + // Check the image ID. if _, ok := imagesInUse[image.ID]; ok { return true } - for _, tag := range append(image.RepoTags, image.RepoDigests...) { - if _, ok := imagesInUse[tag]; ok { - return true - } - } return false } diff --git a/pkg/kubelet/image_manager_test.go b/pkg/kubelet/image_manager_test.go index 5dd43896720..40276db8069 100644 --- a/pkg/kubelet/image_manager_test.go +++ b/pkg/kubelet/image_manager_test.go @@ -59,15 +59,20 @@ func (im *realImageManager) getImageRecord(name string) (*imageRecord, bool) { return &vCopy, ok } +// Returns the id of the image with the given ID. +func imageID(id int) string { + return fmt.Sprintf("image-%d", id) +} + // Returns the name of the image with the given ID. func imageName(id int) string { - return fmt.Sprintf("image-%d", id) + return imageID(id) + "-name" } // Make an image with the specified ID. func makeImage(id int, size int64) container.Image { return container.Image{ - ID: imageName(id), + ID: imageID(id), Size: size, } } @@ -75,8 +80,9 @@ func makeImage(id int, size int64) container.Image { // Make a container with the specified ID. It will use the image with the same ID. func makeContainer(id int) *container.Container { return &container.Container{ - ID: container.ContainerID{Type: "test", ID: fmt.Sprintf("container-%d", id)}, - Image: imageName(id), + ID: container.ContainerID{Type: "test", ID: fmt.Sprintf("container-%d", id)}, + Image: imageName(id), + ImageID: imageID(id), } } @@ -85,11 +91,21 @@ func TestDetectImagesInitialDetect(t *testing.T) { fakeRuntime.ImageList = []container.Image{ makeImage(0, 1024), makeImage(1, 2048), + makeImage(2, 2048), } fakeRuntime.AllPodList = []*containertest.FakePod{ {Pod: &container.Pod{ Containers: []*container.Container{ - makeContainer(1), + { + ID: container.ContainerID{Type: "test", ID: fmt.Sprintf("container-%d", 1)}, + ImageID: imageID(1), + // The image filed is not set to simulate a no-name image + }, + { + ID: container.ContainerID{Type: "test", ID: fmt.Sprintf("container-%d", 2)}, + Image: imageName(2), + ImageID: imageID(2), + }, }, }}, } @@ -98,12 +114,16 @@ func TestDetectImagesInitialDetect(t *testing.T) { err := manager.detectImages(zero) assert := assert.New(t) require.NoError(t, err) - assert.Equal(manager.imageRecordsLen(), 2) - noContainer, ok := manager.getImageRecord(imageName(0)) + assert.Equal(manager.imageRecordsLen(), 3) + noContainer, ok := manager.getImageRecord(imageID(0)) require.True(t, ok) assert.Equal(zero, noContainer.firstDetected) assert.Equal(zero, noContainer.lastUsed) - withContainer, ok := manager.getImageRecord(imageName(1)) + withContainerUsingNoNameImage, ok := manager.getImageRecord(imageID(1)) + require.True(t, ok) + assert.Equal(zero, withContainerUsingNoNameImage.firstDetected) + assert.True(withContainerUsingNoNameImage.lastUsed.After(startTime)) + withContainer, ok := manager.getImageRecord(imageID(2)) require.True(t, ok) assert.Equal(zero, withContainer.firstDetected) assert.True(withContainer.lastUsed.After(startTime)) @@ -141,15 +161,15 @@ func TestDetectImagesWithNewImage(t *testing.T) { err = manager.detectImages(detectedTime) require.NoError(t, err) assert.Equal(manager.imageRecordsLen(), 3) - noContainer, ok := manager.getImageRecord(imageName(0)) + noContainer, ok := manager.getImageRecord(imageID(0)) require.True(t, ok) assert.Equal(zero, noContainer.firstDetected) assert.Equal(zero, noContainer.lastUsed) - withContainer, ok := manager.getImageRecord(imageName(1)) + withContainer, ok := manager.getImageRecord(imageID(1)) require.True(t, ok) assert.Equal(zero, withContainer.firstDetected) assert.True(withContainer.lastUsed.After(startTime)) - newContainer, ok := manager.getImageRecord(imageName(2)) + newContainer, ok := manager.getImageRecord(imageID(2)) require.True(t, ok) assert.Equal(detectedTime, newContainer.firstDetected) assert.Equal(zero, noContainer.lastUsed) @@ -173,7 +193,7 @@ func TestDetectImagesContainerStopped(t *testing.T) { assert := assert.New(t) require.NoError(t, err) assert.Equal(manager.imageRecordsLen(), 2) - withContainer, ok := manager.getImageRecord(imageName(1)) + withContainer, ok := manager.getImageRecord(imageID(1)) require.True(t, ok) // Simulate container being stopped. @@ -181,11 +201,11 @@ func TestDetectImagesContainerStopped(t *testing.T) { err = manager.detectImages(time.Now()) require.NoError(t, err) assert.Equal(manager.imageRecordsLen(), 2) - container1, ok := manager.getImageRecord(imageName(0)) + container1, ok := manager.getImageRecord(imageID(0)) require.True(t, ok) assert.Equal(zero, container1.firstDetected) assert.Equal(zero, container1.lastUsed) - container2, ok := manager.getImageRecord(imageName(1)) + container2, ok := manager.getImageRecord(imageID(1)) require.True(t, ok) assert.Equal(zero, container2.firstDetected) assert.True(container2.lastUsed.Equal(withContainer.lastUsed)) @@ -331,62 +351,6 @@ func TestFreeSpaceTiesBrokenByDetectedTime(t *testing.T) { assert.Len(fakeRuntime.ImageList, 1) } -func TestFreeSpaceImagesAlsoDoesLookupByRepoTags(t *testing.T) { - manager, fakeRuntime, _ := newRealImageManager(ImageGCPolicy{}) - fakeRuntime.ImageList = []container.Image{ - makeImage(0, 1024), - { - ID: "5678", - RepoTags: []string{"potato", "salad"}, - Size: 2048, - }, - } - fakeRuntime.AllPodList = []*containertest.FakePod{ - {Pod: &container.Pod{ - Containers: []*container.Container{ - { - ID: container.ContainerID{Type: "test", ID: "c5678"}, - Image: "salad", - }, - }, - }}, - } - - spaceFreed, err := manager.freeSpace(1024, time.Now()) - assert := assert.New(t) - require.NoError(t, err) - assert.EqualValues(1024, spaceFreed) - assert.Len(fakeRuntime.ImageList, 1) -} - -func TestFreeSpaceImagesAlsoDoesLookupByRepoDigests(t *testing.T) { - manager, fakeRuntime, _ := newRealImageManager(ImageGCPolicy{}) - fakeRuntime.ImageList = []container.Image{ - makeImage(0, 1024), - { - ID: "5678", - RepoDigests: []string{"potato", "salad"}, - Size: 2048, - }, - } - fakeRuntime.AllPodList = []*containertest.FakePod{ - {Pod: &container.Pod{ - Containers: []*container.Container{ - { - ID: container.ContainerID{Type: "test", ID: "c5678"}, - Image: "salad", - }, - }, - }}, - } - - spaceFreed, err := manager.freeSpace(1024, time.Now()) - assert := assert.New(t) - require.NoError(t, err) - assert.EqualValues(1024, spaceFreed) - assert.Len(fakeRuntime.ImageList, 1) -} - func TestGarbageCollectBelowLowThreshold(t *testing.T) { policy := ImageGCPolicy{ HighThresholdPercent: 90, diff --git a/pkg/kubelet/rkt/rkt.go b/pkg/kubelet/rkt/rkt.go index 980d076167f..b71eb2c9b20 100644 --- a/pkg/kubelet/rkt/rkt.go +++ b/pkg/kubelet/rkt/rkt.go @@ -1528,9 +1528,10 @@ func (r *Runtime) convertRktPod(rktpod *rktapi.Pod) (*kubecontainer.Pod, error) ID: buildContainerID(&containerID{rktpod.Id, app.Name}), Name: app.Name, // By default, the version returned by rkt API service will be "latest" if not specified. - Image: fmt.Sprintf("%s:%s", app.Image.Name, app.Image.Version), - Hash: containerHash, - State: appStateToContainerState(app.State), + Image: fmt.Sprintf("%s:%s", app.Image.Name, app.Image.Version), + ImageID: app.Image.Id, + Hash: containerHash, + State: appStateToContainerState(app.State), }) } diff --git a/pkg/kubelet/rkt/rkt_test.go b/pkg/kubelet/rkt/rkt_test.go index bab2eb431dc..a75fe2d2ddc 100644 --- a/pkg/kubelet/rkt/rkt_test.go +++ b/pkg/kubelet/rkt/rkt_test.go @@ -374,18 +374,20 @@ func TestGetPods(t *testing.T) { Namespace: "default", Containers: []*kubecontainer.Container{ { - ID: kubecontainer.BuildContainerID("rkt", "uuid-4002:app-1"), - Name: "app-1", - Image: "img-name-1:latest", - Hash: 1001, - State: "running", + ID: kubecontainer.BuildContainerID("rkt", "uuid-4002:app-1"), + Name: "app-1", + Image: "img-name-1:latest", + ImageID: "img-id-1", + Hash: 1001, + State: "running", }, { - ID: kubecontainer.BuildContainerID("rkt", "uuid-4002:app-2"), - Name: "app-2", - Image: "img-name-2:latest", - Hash: 1002, - State: "exited", + ID: kubecontainer.BuildContainerID("rkt", "uuid-4002:app-2"), + Name: "app-2", + Image: "img-name-2:latest", + ImageID: "img-id-2", + Hash: 1002, + State: "exited", }, }, }, @@ -435,18 +437,20 @@ func TestGetPods(t *testing.T) { Namespace: "default", Containers: []*kubecontainer.Container{ { - ID: kubecontainer.BuildContainerID("rkt", "uuid-4002:app-1"), - Name: "app-1", - Image: "img-name-1:latest", - Hash: 1001, - State: "running", + ID: kubecontainer.BuildContainerID("rkt", "uuid-4002:app-1"), + Name: "app-1", + Image: "img-name-1:latest", + ImageID: "img-id-1", + Hash: 1001, + State: "running", }, { - ID: kubecontainer.BuildContainerID("rkt", "uuid-4002:app-2"), - Name: "app-2", - Image: "img-name-2:latest", - Hash: 1002, - State: "exited", + ID: kubecontainer.BuildContainerID("rkt", "uuid-4002:app-2"), + Name: "app-2", + Image: "img-name-2:latest", + ImageID: "img-id-2", + Hash: 1002, + State: "exited", }, }, }, @@ -456,32 +460,36 @@ func TestGetPods(t *testing.T) { Namespace: "default", Containers: []*kubecontainer.Container{ { - ID: kubecontainer.BuildContainerID("rkt", "uuid-4003:app-11"), - Name: "app-11", - Image: "img-name-11:latest", - Hash: 10011, - State: "exited", + ID: kubecontainer.BuildContainerID("rkt", "uuid-4003:app-11"), + Name: "app-11", + Image: "img-name-11:latest", + ImageID: "img-id-11", + Hash: 10011, + State: "exited", }, { - ID: kubecontainer.BuildContainerID("rkt", "uuid-4003:app-22"), - Name: "app-22", - Image: "img-name-22:latest", - Hash: 10022, - State: "exited", + ID: kubecontainer.BuildContainerID("rkt", "uuid-4003:app-22"), + Name: "app-22", + Image: "img-name-22:latest", + ImageID: "img-id-22", + Hash: 10022, + State: "exited", }, { - ID: kubecontainer.BuildContainerID("rkt", "uuid-4004:app-11"), - Name: "app-11", - Image: "img-name-11:latest", - Hash: 10011, - State: "running", + ID: kubecontainer.BuildContainerID("rkt", "uuid-4004:app-11"), + Name: "app-11", + Image: "img-name-11:latest", + ImageID: "img-id-11", + Hash: 10011, + State: "running", }, { - ID: kubecontainer.BuildContainerID("rkt", "uuid-4004:app-22"), - Name: "app-22", - Image: "img-name-22:latest", - Hash: 10022, - State: "running", + ID: kubecontainer.BuildContainerID("rkt", "uuid-4004:app-22"), + Name: "app-22", + Image: "img-name-22:latest", + ImageID: "img-id-22", + Hash: 10022, + State: "running", }, }, },