Merge pull request #2410 from haircommander/cleanup/allocation-manager-4.20

OCPBUGS-59641: cleanup: fetch individual PodResourceInfo from allocated resource state

Author: sdodson

pkg/kubelet/allocation/allocation_manager.go

@@ -109,14 +109,20 @@ func (m *manager) GetContainerResourceAllocation(podUID types.UID, containerName
 // UpdatePodFromAllocation overwrites the pod spec with the allocation.
 // This function does a deep copy only if updates are needed.
 func (m *manager) UpdatePodFromAllocation(pod *v1.Pod) (*v1.Pod, bool) {
-	// TODO(tallclair): This clones the whole cache, but we only need 1 pod.
-	allocs := m.allocated.GetPodResourceInfoMap()
-	return updatePodFromAllocation(pod, allocs)
+	if pod == nil {
+		return pod, false
+	}
+
+	allocated, ok := m.allocated.GetPodResourceInfo(pod.UID)
+	if !ok {
+		return pod, false
+	}
+
+	return updatePodFromAllocation(pod, allocated)
 }
 
-func updatePodFromAllocation(pod *v1.Pod, allocs state.PodResourceInfoMap) (*v1.Pod, bool) {
-	allocated, found := allocs[pod.UID]
-	if !found {
+func updatePodFromAllocation(pod *v1.Pod, allocated state.PodResourceInfo) (*v1.Pod, bool) {
+	if pod == nil {
 		return pod, false
 	}
 

pkg/kubelet/allocation/allocation_manager_test.go

@@ -103,50 +103,44 @@ func TestUpdatePodFromAllocation(t *testing.T) {
 	tests := []struct {
 		name         string
 		pod          *v1.Pod
-		allocs       state.PodResourceInfoMap
+		allocated    state.PodResourceInfo
 		expectPod    *v1.Pod
 		expectUpdate bool
 	}{{
 		name: "steady state",
 		pod:  pod,
-		allocs: state.PodResourceInfoMap{
-			pod.UID: state.PodResourceInfo{
-				ContainerResources: map[string]v1.ResourceRequirements{
-					"c1":                  *pod.Spec.Containers[0].Resources.DeepCopy(),
-					"c2":                  *pod.Spec.Containers[1].Resources.DeepCopy(),
-					"c1-restartable-init": *pod.Spec.InitContainers[0].Resources.DeepCopy(),
-					"c1-init":             *pod.Spec.InitContainers[1].Resources.DeepCopy(),
-				},
+		allocated: state.PodResourceInfo{
+			ContainerResources: map[string]v1.ResourceRequirements{
+				"c1":                  *pod.Spec.Containers[0].Resources.DeepCopy(),
+				"c2":                  *pod.Spec.Containers[1].Resources.DeepCopy(),
+				"c1-restartable-init": *pod.Spec.InitContainers[0].Resources.DeepCopy(),
+				"c1-init":             *pod.Spec.InitContainers[1].Resources.DeepCopy(),
 			},
 		},
 		expectUpdate: false,
 	}, {
 		name:         "no allocations",
 		pod:          pod,
-		allocs:       state.PodResourceInfoMap{},
+		allocated:    state.PodResourceInfo{},
 		expectUpdate: false,
 	}, {
 		name: "missing container allocation",
 		pod:  pod,
-		allocs: state.PodResourceInfoMap{
-			pod.UID: state.PodResourceInfo{
-				ContainerResources: map[string]v1.ResourceRequirements{
-					"c2": *pod.Spec.Containers[1].Resources.DeepCopy(),
-				},
+		allocated: state.PodResourceInfo{
+			ContainerResources: map[string]v1.ResourceRequirements{
+				"c2": *pod.Spec.Containers[1].Resources.DeepCopy(),
 			},
 		},
 		expectUpdate: false,
 	}, {
 		name: "resized container",
 		pod:  pod,
-		allocs: state.PodResourceInfoMap{
-			pod.UID: state.PodResourceInfo{
-				ContainerResources: map[string]v1.ResourceRequirements{
-					"c1":                  *resizedPod.Spec.Containers[0].Resources.DeepCopy(),
-					"c2":                  *resizedPod.Spec.Containers[1].Resources.DeepCopy(),
-					"c1-restartable-init": *resizedPod.Spec.InitContainers[0].Resources.DeepCopy(),
-					"c1-init":             *resizedPod.Spec.InitContainers[1].Resources.DeepCopy(),
-				},
+		allocated: state.PodResourceInfo{
+			ContainerResources: map[string]v1.ResourceRequirements{
+				"c1":                  *resizedPod.Spec.Containers[0].Resources.DeepCopy(),
+				"c2":                  *resizedPod.Spec.Containers[1].Resources.DeepCopy(),
+				"c1-restartable-init": *resizedPod.Spec.InitContainers[0].Resources.DeepCopy(),
+				"c1-init":             *resizedPod.Spec.InitContainers[1].Resources.DeepCopy(),
 			},
 		},
 		expectUpdate: true,
@@ -156,7 +150,7 @@ func TestUpdatePodFromAllocation(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			pod := test.pod.DeepCopy()
-			allocatedPod, updated := updatePodFromAllocation(pod, test.allocs)
+			allocatedPod, updated := updatePodFromAllocation(pod, test.allocated)
 
 			if test.expectUpdate {
 				assert.True(t, updated, "updated")

pkg/kubelet/allocation/state/state.go

@@ -50,6 +50,7 @@ func (pr PodResourceInfoMap) Clone() PodResourceInfoMap {
 type Reader interface {
 	GetContainerResources(podUID types.UID, containerName string) (v1.ResourceRequirements, bool)
 	GetPodResourceInfoMap() PodResourceInfoMap
+	GetPodResourceInfo(podUID types.UID) (PodResourceInfo, bool)
 }
 
 type writer interface {

pkg/kubelet/allocation/state/state_checkpoint.go

@@ -112,13 +112,20 @@ func (sc *stateCheckpoint) GetContainerResources(podUID types.UID, containerName
 	return sc.cache.GetContainerResources(podUID, containerName)
 }
 
-// GetPodResourceInfoMap returns current pod resource information
+// GetPodResourceInfoMap returns current pod resource information map
 func (sc *stateCheckpoint) GetPodResourceInfoMap() PodResourceInfoMap {
 	sc.mux.RLock()
 	defer sc.mux.RUnlock()
 	return sc.cache.GetPodResourceInfoMap()
 }
 
+// GetPodResourceInfo returns current pod resource information
+func (sc *stateCheckpoint) GetPodResourceInfo(podUID types.UID) (PodResourceInfo, bool) {
+	sc.mux.RLock()
+	defer sc.mux.RUnlock()
+	return sc.cache.GetPodResourceInfo(podUID)
+}
+
 // SetContainerResoruces sets resources information for a pod's container
 func (sc *stateCheckpoint) SetContainerResources(podUID types.UID, containerName string, resources v1.ResourceRequirements) error {
 	sc.mux.Lock()
@@ -172,6 +179,10 @@ func (sc *noopStateCheckpoint) GetPodResourceInfoMap() PodResourceInfoMap {
 	return nil
 }
 
+func (sc *noopStateCheckpoint) GetPodResourceInfo(_ types.UID) (PodResourceInfo, bool) {
+	return PodResourceInfo{}, false
+}
+
 func (sc *noopStateCheckpoint) SetContainerResources(_ types.UID, _ string, _ v1.ResourceRequirements) error {
 	return nil
 }

pkg/kubelet/allocation/state/state_mem.go

@@ -65,6 +65,14 @@ func (s *stateMemory) GetPodResourceInfoMap() PodResourceInfoMap {
 	return s.podResources.Clone()
 }
 
+func (s *stateMemory) GetPodResourceInfo(podUID types.UID) (PodResourceInfo, bool) {
+	s.RLock()
+	defer s.RUnlock()
+
+	resourceInfo, ok := s.podResources[podUID]
+	return resourceInfo, ok
+}
+
 func (s *stateMemory) SetContainerResources(podUID types.UID, containerName string, resources v1.ResourceRequirements) error {
 	s.Lock()
 	defer s.Unlock()