Refactored status manager code of updatePodFromAllocation

Author: vivzbansal

pkg/kubelet/status/status_manager.go

@@ -262,69 +262,40 @@ func (m *manager) UpdatePodFromAllocation(pod *v1.Pod) (*v1.Pod, bool) {
 	return updatePodFromAllocation(pod, allocs)
 }
 
-/* func updatePodFromAllocation(pod *v1.Pod, allocs state.PodResourceAllocation) (*v1.Pod, bool) {
+func updatePodFromAllocation(pod *v1.Pod, allocs state.PodResourceAllocation) (*v1.Pod, bool) {
 	allocated, found := allocs[string(pod.UID)]
 	if !found {
 		return pod, false
 	}
 
 	updated := false
-	updateContainerResources := func(c *v1.Container) {
+	containerAlloc := func(c v1.Container) (v1.ResourceRequirements, bool) {
 		if cAlloc, ok := allocated[c.Name]; ok {
 			if !apiequality.Semantic.DeepEqual(c.Resources, cAlloc) {
+				// Allocation differs from pod spec, retrieve the allocation
 				if !updated {
+					// If this is the first update to be performed, copy the pod
 					pod = pod.DeepCopy()
 					updated = true
 				}
-				c.Resources = *cAlloc.DeepCopy()
+				return cAlloc, true
 			}
 		}
+		return v1.ResourceRequirements{}, false
 	}
 
-	for i := range pod.Spec.Containers {
-		updateContainerResources(&pod.Spec.Containers[i])
-	}
-	for i := range pod.Spec.InitContainers {
-		updateContainerResources(&pod.Spec.InitContainers[i])
-	}
-	return pod, updated
-} */
-
-// TODO(vibansal): Refactor this function to something above commented code.
-func updatePodFromAllocation(pod *v1.Pod, allocs state.PodResourceAllocation) (*v1.Pod, bool) {
-	allocated, found := allocs[string(pod.UID)]
-	if !found {
-		return pod, false
-	}
-	updated := false
 	for i, c := range pod.Spec.Containers {
-		if cAlloc, ok := allocated[c.Name]; ok {
-			if !apiequality.Semantic.DeepEqual(c.Resources, cAlloc) {
-				// Allocation differs from pod spec, update
-				if !updated {
-					// If this is the first update, copy the pod
-					pod = pod.DeepCopy()
-					updated = true
-				}
-				pod.Spec.Containers[i].Resources = cAlloc
-			}
+		if cAlloc, found := containerAlloc(c); found {
+			// Allocation differs from pod spec, update
+			pod.Spec.Containers[i].Resources = cAlloc
 		}
 	}
-
 	for i, c := range pod.Spec.InitContainers {
-		if cAlloc, ok := allocated[c.Name]; ok {
-			if !apiequality.Semantic.DeepEqual(c.Resources, cAlloc) {
-				// Allocation differs from pod spec, update
-				if !updated {
-					// If this is the first update, copy the pod
-					pod = pod.DeepCopy()
-					updated = true
-				}
-				pod.Spec.InitContainers[i].Resources = cAlloc
-			}
+		if cAlloc, found := containerAlloc(c); found {
+			// Allocation differs from pod spec, update
+			pod.Spec.InitContainers[i].Resources = cAlloc
 		}
 	}
-
 	return pod, updated
 }
 

pkg/kubelet/status/status_manager_test.go

@@ -36,14 +36,11 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
-	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	clientset "k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/kubernetes/fake"
 	core "k8s.io/client-go/testing"
-	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	podutil "k8s.io/kubernetes/pkg/api/v1/pod"
 	api "k8s.io/kubernetes/pkg/apis/core"
-	"k8s.io/kubernetes/pkg/features"
 	kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
 	kubepod "k8s.io/kubernetes/pkg/kubelet/pod"
 	"k8s.io/kubernetes/pkg/kubelet/status/state"
@@ -2095,12 +2092,12 @@ func TestUpdatePodFromAllocation(t *testing.T) {
 					Name: "c1-init",
 					Resources: v1.ResourceRequirements{
 						Requests: v1.ResourceList{
-							v1.ResourceCPU:    *resource.NewMilliQuantity(200, resource.DecimalSI),
-							v1.ResourceMemory: *resource.NewQuantity(300, resource.DecimalSI),
+							v1.ResourceCPU:    *resource.NewMilliQuantity(500, resource.DecimalSI),
+							v1.ResourceMemory: *resource.NewQuantity(600, resource.DecimalSI),
 						},
 						Limits: v1.ResourceList{
-							v1.ResourceCPU:    *resource.NewMilliQuantity(400, resource.DecimalSI),
-							v1.ResourceMemory: *resource.NewQuantity(500, resource.DecimalSI),
+							v1.ResourceCPU:    *resource.NewMilliQuantity(700, resource.DecimalSI),
+							v1.ResourceMemory: *resource.NewQuantity(800, resource.DecimalSI),
 						},
 					},
 				},
@@ -2161,7 +2158,6 @@ func TestUpdatePodFromAllocation(t *testing.T) {
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.SidecarContainers, true)
 			pod := test.pod.DeepCopy()
 			allocatedPod, updated := updatePodFromAllocation(pod, test.allocs)