Merge pull request #104788 from 249043822/memorymanager-br

Fix initContainersReusableMemory delete bug in MemoryManager

pkg/kubelet/cm/memorymanager/policy_static.go

@@ -837,7 +837,7 @@ func (p *staticPolicy) updatePodReusableMemory(pod *v1.Pod, container *v1.Contai
 	// If pod entries to m.initContainersReusableMemory other than the current pod exist, delete them.
 	for uid := range p.initContainersReusableMemory {
 		if podUID != uid {
-			delete(p.initContainersReusableMemory, podUID)
+			delete(p.initContainersReusableMemory, uid)
 		}
 	}
 

pkg/kubelet/cm/memorymanager/policy_static_test.go

@@ -117,20 +117,21 @@ func areContainerMemoryAssignmentsEqual(t *testing.T, cma1, cma2 state.Container
 }
 
 type testStaticPolicy struct {
-	description           string
+	description                  string
-	assignments           state.ContainerMemoryAssignments
+	assignments                  state.ContainerMemoryAssignments
-	expectedAssignments   state.ContainerMemoryAssignments
+	expectedAssignments          state.ContainerMemoryAssignments
-	machineState          state.NUMANodeMap
+	machineState                 state.NUMANodeMap
-	expectedMachineState  state.NUMANodeMap
+	expectedMachineState         state.NUMANodeMap
-	systemReserved        systemReservedMemory
+	systemReserved               systemReservedMemory
-	expectedError         error
+	expectedError                error
-	machineInfo           *cadvisorapi.MachineInfo
+	machineInfo                  *cadvisorapi.MachineInfo
-	pod                   *v1.Pod
+	pod                          *v1.Pod
-	topologyHint          *topologymanager.TopologyHint
+	topologyHint                 *topologymanager.TopologyHint
-	expectedTopologyHints map[string][]topologymanager.TopologyHint
+	expectedTopologyHints        map[string][]topologymanager.TopologyHint
+	initContainersReusableMemory reusableMemory
 }
 
-func initTests(t *testing.T, testCase *testStaticPolicy, hint *topologymanager.TopologyHint) (Policy, state.State, error) {
+func initTests(t *testing.T, testCase *testStaticPolicy, hint *topologymanager.TopologyHint, initContainersReusableMemory reusableMemory) (Policy, state.State, error) {
 	manager := topologymanager.NewFakeManager()
 	if hint != nil {
 		manager = topologymanager.NewFakeManagerWithHint(hint)
@@ -140,6 +141,9 @@ func initTests(t *testing.T, testCase *testStaticPolicy, hint *topologymanager.T
 	if err != nil {
 		return nil, nil, err
 	}
+	if initContainersReusableMemory != nil {
+		p.(*staticPolicy).initContainersReusableMemory = initContainersReusableMemory
+	}
 	s := state.NewMemoryState()
 	s.SetMachineState(testCase.machineState)
 	s.SetMemoryAssignments(testCase.assignments)
@@ -173,7 +177,7 @@ func TestStaticPolicyNew(t *testing.T) {
 
 	for _, testCase := range testCases {
 		t.Run(testCase.description, func(t *testing.T) {
-			_, _, err := initTests(t, &testCase, nil)
+			_, _, err := initTests(t, &testCase, nil, nil)
 			if !reflect.DeepEqual(err, testCase.expectedError) {
 				t.Fatalf("The actual error: %v is different from the expected one: %v", err, testCase.expectedError)
 			}
@@ -194,7 +198,7 @@ func TestStaticPolicyName(t *testing.T) {
 	}
 	for _, testCase := range testCases {
 		t.Run(testCase.description, func(t *testing.T) {
-			p, _, err := initTests(t, &testCase, nil)
+			p, _, err := initTests(t, &testCase, nil, nil)
 			if err != nil {
 				t.Fatalf("Unexpected error: %v", err)
 			}
@@ -1022,7 +1026,7 @@ func TestStaticPolicyStart(t *testing.T) {
 	for _, testCase := range testCases {
 		t.Run(testCase.description, func(t *testing.T) {
 			t.Logf("[Start] %s", testCase.description)
-			p, s, err := initTests(t, &testCase, nil)
+			p, s, err := initTests(t, &testCase, nil, nil)
 			if err != nil {
 				t.Fatalf("Unexpected error: %v", err)
 			}
@@ -1763,7 +1767,7 @@ func TestStaticPolicyAllocate(t *testing.T) {
 	for _, testCase := range testCases {
 		t.Run(testCase.description, func(t *testing.T) {
 			t.Logf("TestStaticPolicyAllocate %s", testCase.description)
-			p, s, err := initTests(t, &testCase, testCase.topologyHint)
+			p, s, err := initTests(t, &testCase, testCase.topologyHint, nil)
 			if err != nil {
 				t.Fatalf("Unexpected error: %v", err)
 			}
@@ -2308,12 +2312,185 @@ func TestStaticPolicyAllocateWithInitContainers(t *testing.T) {
 			),
 			topologyHint: &topologymanager.TopologyHint{},
 		},
+		{
+			description:                  "should re-use init containers memory, init containers requests 7Gi and 4Gi, apps containers 5Gi and 2Gi",
+			assignments:                  state.ContainerMemoryAssignments{},
+			initContainersReusableMemory: reusableMemory{"pod0": map[string]map[v1.ResourceName]uint64{}},
+			expectedAssignments: state.ContainerMemoryAssignments{
+				"pod1": map[string][]state.Block{
+					"initContainer1": {
+						{
+							NUMAAffinity: []int{0},
+							Type:         v1.ResourceMemory,
+							Size:         0,
+						},
+						{
+							NUMAAffinity: []int{0},
+							Type:         hugepages1Gi,
+							Size:         0,
+						},
+					},
+					"initContainer2": {
+						{
+							NUMAAffinity: []int{0},
+							Type:         v1.ResourceMemory,
+							Size:         0,
+						},
+						{
+							NUMAAffinity: []int{0},
+							Type:         hugepages1Gi,
+							Size:         0,
+						},
+					},
+					"container1": {
+						{
+							NUMAAffinity: []int{0},
+							Type:         v1.ResourceMemory,
+							Size:         5 * gb,
+						},
+						{
+							NUMAAffinity: []int{0},
+							Type:         hugepages1Gi,
+							Size:         5 * gb,
+						},
+					},
+					"container2": {
+						{
+							NUMAAffinity: []int{0},
+							Type:         v1.ResourceMemory,
+							Size:         2 * gb,
+						},
+						{
+							NUMAAffinity: []int{0},
+							Type:         hugepages1Gi,
+							Size:         2 * gb,
+						},
+					},
+				},
+			},
+			machineState: state.NUMANodeMap{
+				0: &state.NUMANodeState{
+					MemoryMap: map[v1.ResourceName]*state.MemoryTable{
+						v1.ResourceMemory: {
+							Allocatable:    10240 * mb,
+							Free:           10240 * mb,
+							Reserved:       0,
+							SystemReserved: 512 * mb,
+							TotalMemSize:   10 * gb,
+						},
+						hugepages1Gi: {
+							Allocatable:    10 * gb,
+							Free:           10 * gb,
+							Reserved:       0,
+							SystemReserved: 0,
+							TotalMemSize:   10 * gb,
+						},
+					},
+					Cells: []int{0},
+				},
+			},
+			expectedMachineState: state.NUMANodeMap{
+				0: &state.NUMANodeState{
+					MemoryMap: map[v1.ResourceName]*state.MemoryTable{
+						v1.ResourceMemory: {
+							Allocatable:    10240 * mb,
+							Free:           3072 * mb,
+							Reserved:       7 * gb,
+							SystemReserved: 512 * mb,
+							TotalMemSize:   10 * gb,
+						},
+						hugepages1Gi: {
+							Allocatable:    10 * gb,
+							Free:           3 * gb,
+							Reserved:       7 * gb,
+							SystemReserved: 0,
+							TotalMemSize:   10 * gb,
+						},
+					},
+					Cells:               []int{0},
+					NumberOfAssignments: 8,
+				},
+			},
+			systemReserved: systemReservedMemory{
+				0: map[v1.ResourceName]uint64{
+					v1.ResourceMemory: 512 * mb,
+				},
+			},
+			pod: getPodWithInitContainers(
+				"pod1",
+				[]v1.Container{
+					{
+						Name: "container1",
+						Resources: v1.ResourceRequirements{
+							Limits: v1.ResourceList{
+								v1.ResourceCPU:    resource.MustParse("1000Mi"),
+								v1.ResourceMemory: resource.MustParse("5Gi"),
+								hugepages1Gi:      resource.MustParse("5Gi"),
+							},
+							Requests: v1.ResourceList{
+								v1.ResourceCPU:    resource.MustParse("1000Mi"),
+								v1.ResourceMemory: resource.MustParse("5Gi"),
+								hugepages1Gi:      resource.MustParse("5Gi"),
+							},
+						},
+					},
+					{
+						Name: "container2",
+						Resources: v1.ResourceRequirements{
+							Limits: v1.ResourceList{
+								v1.ResourceCPU:    resource.MustParse("1000Mi"),
+								v1.ResourceMemory: resource.MustParse("2Gi"),
+								hugepages1Gi:      resource.MustParse("2Gi"),
+							},
+							Requests: v1.ResourceList{
+								v1.ResourceCPU:    resource.MustParse("1000Mi"),
+								v1.ResourceMemory: resource.MustParse("2Gi"),
+								hugepages1Gi:      resource.MustParse("2Gi"),
+							},
+						},
+					},
+				},
+				[]v1.Container{
+					{
+						Name: "initContainer1",
+						Resources: v1.ResourceRequirements{
+							Limits: v1.ResourceList{
+								v1.ResourceCPU:    resource.MustParse("1000Mi"),
+								v1.ResourceMemory: resource.MustParse("7Gi"),
+								hugepages1Gi:      resource.MustParse("7Gi"),
+							},
+							Requests: v1.ResourceList{
+								v1.ResourceCPU:    resource.MustParse("1000Mi"),
+								v1.ResourceMemory: resource.MustParse("7Gi"),
+								hugepages1Gi:      resource.MustParse("7Gi"),
+							},
+						},
+					},
+					{
+						Name: "initContainer2",
+						Resources: v1.ResourceRequirements{
+							Limits: v1.ResourceList{
+								v1.ResourceCPU:    resource.MustParse("1000Mi"),
+								v1.ResourceMemory: resource.MustParse("4Gi"),
+								hugepages1Gi:      resource.MustParse("4Gi"),
+							},
+							Requests: v1.ResourceList{
+								v1.ResourceCPU:    resource.MustParse("1000Mi"),
+								v1.ResourceMemory: resource.MustParse("4Gi"),
+								hugepages1Gi:      resource.MustParse("4Gi"),
+							},
+						},
+					},
+				},
+			),
+			topologyHint: &topologymanager.TopologyHint{},
+		},
 	}
 
 	for _, testCase := range testCases {
 		t.Run(testCase.description, func(t *testing.T) {
 			klog.InfoS("TestStaticPolicyAllocateWithInitContainers", "test name", testCase.description)
-			p, s, err := initTests(t, &testCase, testCase.topologyHint)
+			p, s, err := initTests(t, &testCase, testCase.topologyHint, testCase.initContainersReusableMemory)
 			if err != nil {
 				t.Fatalf("Unexpected error: %v", err)
 			}
@@ -2583,7 +2760,7 @@ func TestStaticPolicyRemoveContainer(t *testing.T) {
 
 	for _, testCase := range testCases {
 		t.Run(testCase.description, func(t *testing.T) {
-			p, s, err := initTests(t, &testCase, nil)
+			p, s, err := initTests(t, &testCase, nil, nil)
 			if err != nil {
 				t.Fatalf("Unexpected error: %v", err)
 			}
@@ -2876,7 +3053,7 @@ func TestStaticPolicyGetTopologyHints(t *testing.T) {
 
 	for _, testCase := range testCases {
 		t.Run(testCase.description, func(t *testing.T) {
-			p, s, err := initTests(t, &testCase, nil)
+			p, s, err := initTests(t, &testCase, nil, nil)
 			if err != nil {
 				t.Fatalf("Unexpected error: %v", err)
 			}