Change CPUManager containerMap to key off of (podUID, containerName)

Previously it keyed off of a pointer to the actual pod / container,
which was unnecessary, and hard to work with (especially on the
retrieval side).
This commit is contained in:
Kevin Klues 2019-11-05 19:16:46 +00:00
parent 3881e50cce
commit 0639bd0942
4 changed files with 24 additions and 40 deletions

View File

@ -5,17 +5,12 @@ go_library(
srcs = ["container_map.go"],
importpath = "k8s.io/kubernetes/pkg/kubelet/cm/cpumanager/containermap",
visibility = ["//visibility:public"],
deps = ["//staging/src/k8s.io/api/core/v1:go_default_library"],
)
go_test(
name = "go_default_test",
srcs = ["container_map_test.go"],
embed = [":go_default_library"],
deps = [
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/types:go_default_library",
],
)
filegroup(

View File

@ -18,14 +18,12 @@ package containermap
import (
"fmt"
"k8s.io/api/core/v1"
)
// ContainerMap maps (containerID)->(*v1.Pod, *v1.Container)
type ContainerMap map[string]struct {
pod *v1.Pod
container *v1.Container
podUID string
containerName string
}
// NewContainerMap creates a new ContainerMap struct
@ -33,12 +31,12 @@ func NewContainerMap() ContainerMap {
return make(ContainerMap)
}
// Add adds a mapping of (containerID)->(*v1.Pod, *v1.Container) to the ContainerMap
func (cm ContainerMap) Add(p *v1.Pod, c *v1.Container, containerID string) {
// Add adds a mapping of (containerID)->(podUID, containerName) to the ContainerMap
func (cm ContainerMap) Add(podUID, containerName, containerID string) {
cm[containerID] = struct {
pod *v1.Pod
container *v1.Container
}{p, c}
podUID string
containerName string
}{podUID, containerName}
}
// Remove removes a mapping of (containerID)->(*v1.Pod, *.v1.Container) from the ContainerMap
@ -47,19 +45,19 @@ func (cm ContainerMap) Remove(containerID string) {
}
// GetContainerID retrieves a ContainerID from the ContainerMap
func (cm ContainerMap) GetContainerID(p *v1.Pod, c *v1.Container) (string, error) {
func (cm ContainerMap) GetContainerID(podUID, containerName string) (string, error) {
for key, val := range cm {
if val.pod.UID == p.UID && val.container.Name == c.Name {
if val.podUID == podUID && val.containerName == containerName {
return key, nil
}
}
return "", fmt.Errorf("container %s not in ContainerMap for pod %s", c.Name, p.UID)
return "", fmt.Errorf("container %s not in ContainerMap for pod %s", containerName, podUID)
}
// GetContainerRef retrieves a (*v1.Pod, *v1.Container) pair from the ContainerMap
func (cm ContainerMap) GetContainerRef(containerID string) (*v1.Pod, *v1.Container, error) {
// GetContainerRef retrieves a (podUID, containerName) pair from the ContainerMap
func (cm ContainerMap) GetContainerRef(containerID string) (string, string, error) {
if _, exists := cm[containerID]; !exists {
return nil, nil, fmt.Errorf("containerID %s not in ContainerMap", containerID)
return "", "", fmt.Errorf("containerID %s not in ContainerMap", containerID)
}
return cm[containerID].pod, cm[containerID].container, nil
return cm[containerID].podUID, cm[containerID].containerName, nil
}

View File

@ -18,9 +18,6 @@ package containermap
import (
"testing"
"k8s.io/api/core/v1"
apimachinery "k8s.io/apimachinery/pkg/types"
)
func TestContainerMap(t *testing.T) {
@ -37,18 +34,13 @@ func TestContainerMap(t *testing.T) {
}
for _, tc := range testCases {
pod := v1.Pod{}
pod.UID = apimachinery.UID(tc.podUID)
// Build a new containerMap from the testCases, checking proper
// addition, retrieval along the way.
cm := NewContainerMap()
for i := range tc.containerNames {
container := v1.Container{Name: tc.containerNames[i]}
cm.Add(tc.podUID, tc.containerNames[i], tc.containerIDs[i])
cm.Add(&pod, &container, tc.containerIDs[i])
containerID, err := cm.GetContainerID(&pod, &container)
containerID, err := cm.GetContainerID(tc.podUID, tc.containerNames[i])
if err != nil {
t.Errorf("error adding and retrieving containerID: %v", err)
}
@ -56,24 +48,23 @@ func TestContainerMap(t *testing.T) {
t.Errorf("mismatched containerIDs %v, %v", containerID, tc.containerIDs[i])
}
podRef, containerRef, err := cm.GetContainerRef(containerID)
podUID, containerName, err := cm.GetContainerRef(containerID)
if err != nil {
t.Errorf("error retrieving container reference: %v", err)
}
if podRef != &pod {
t.Errorf("mismatched pod reference %v, %v", pod.UID, podRef.UID)
if podUID != tc.podUID {
t.Errorf("mismatched pod UID %v, %v", tc.podUID, podUID)
}
if containerRef != &container {
t.Errorf("mismatched container reference %v, %v", container.Name, containerRef.Name)
if containerName != tc.containerNames[i] {
t.Errorf("mismatched container Name %v, %v", tc.containerNames[i], containerName)
}
}
// Remove all entries from the containerMap, checking proper removal of
// each along the way.
for i := range tc.containerNames {
container := v1.Container{Name: tc.containerNames[i]}
cm.Remove(tc.containerIDs[i])
containerID, err := cm.GetContainerID(&pod, &container)
containerID, err := cm.GetContainerID(tc.podUID, tc.containerNames[i])
if err == nil {
t.Errorf("unexpected retrieval of containerID after removal: %v", containerID)
}

View File

@ -193,7 +193,7 @@ func (p *staticPolicy) AddContainer(s state.State, pod *v1.Pod, container *v1.Co
// add (pod, container, containerID) to the containerMap.
defer func() {
if rerr == nil {
p.containerMap.Add(pod, container, containerID)
p.containerMap.Add(string(pod.UID), container.Name, containerID)
}
}()
@ -211,7 +211,7 @@ func (p *staticPolicy) AddContainer(s state.State, pod *v1.Pod, container *v1.Co
// container is run.
for _, initContainer := range pod.Spec.InitContainers {
if container.Name != initContainer.Name {
initContainerID, err := p.containerMap.GetContainerID(pod, &initContainer)
initContainerID, err := p.containerMap.GetContainerID(string(pod.UID), initContainer.Name)
if err != nil {
continue
}