Skip to content

Commit

Permalink
memory manager: improve the reserved memory validation logic
Browse files Browse the repository at this point in the history
We will have two layers of the validation.

- the first part of the validation logic will be implemented under the
`ValidateKubeletConfiguration` method
- the second one that requires knowledge about machine topology and
node allocatable resources will be implemented under the memory manager.

Signed-off-by: Artyom Lukianov <alukiano@redhat.com>
  • Loading branch information
Artyom Lukianov committed Feb 8, 2021
1 parent 9321340 commit 1021244
Show file tree
Hide file tree
Showing 6 changed files with 213 additions and 19 deletions.
10 changes: 9 additions & 1 deletion pkg/kubelet/apis/config/validation/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,17 @@ go_library(
srcs = [
"validation.go",
"validation_others.go",
"validation_reserved_memory.go",
"validation_windows.go",
],
importpath = "k8s.io/kubernetes/pkg/kubelet/apis/config/validation",
deps = [
"//pkg/apis/core/v1/helper:go_default_library",
"//pkg/features:go_default_library",
"//pkg/kubelet/apis/config:go_default_library",
"//pkg/kubelet/cm/cpuset:go_default_library",
"//pkg/kubelet/types:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/validation:go_default_library",
Expand All @@ -43,10 +46,15 @@ filegroup(

go_test(
name = "go_default_test",
srcs = ["validation_test.go"],
srcs = [
"validation_reserved_memory_test.go",
"validation_test.go",
],
embed = [":go_default_library"],
deps = [
"//pkg/kubelet/apis/config:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/errors:go_default_library",
],
Expand Down
2 changes: 2 additions & 0 deletions pkg/kubelet/apis/config/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,8 @@ func ValidateKubeletConfiguration(kc *kubeletconfig.KubeletConfiguration) error
}
}

allErrors = append(allErrors, validateReservedMemoryConfiguration(kc)...)

if err := validateKubeletOSConfiguration(kc); err != nil {
allErrors = append(allErrors, err)
}
Expand Down
64 changes: 64 additions & 0 deletions pkg/kubelet/apis/config/validation/validation_reserved_memory.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/*
Copyright 2018 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package validation

import (
"fmt"

v1 "k8s.io/api/core/v1"
corev1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper"
kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"
)

// validateReservedMemory validates the reserved memory configuration and returns an error if it is invalid.
func validateReservedMemoryConfiguration(kc *kubeletconfig.KubeletConfiguration) []error {
if len(kc.ReservedMemory) == 0 {
return nil
}

var errors []error

numaTypeDuplicates := map[int32]map[v1.ResourceName]bool{}
for _, reservedMemory := range kc.ReservedMemory {
numaNode := reservedMemory.NumaNode
if _, ok := numaTypeDuplicates[numaNode]; !ok {
numaTypeDuplicates[numaNode] = map[v1.ResourceName]bool{}
}

for resourceName, q := range reservedMemory.Limits {
if !reservedMemorySupportedLimit(resourceName) {
errors = append(errors, fmt.Errorf("the limit type %q for NUMA node %d is not supported, only %v is accepted", resourceName, numaNode, []v1.ResourceName{v1.ResourceMemory, v1.ResourceHugePagesPrefix + "<HugePageSize>"}))
}

// validates that the limit has non-zero value
if q.IsZero() {
errors = append(errors, fmt.Errorf("reserved memory may not be zero for NUMA node %d and resource %q", numaNode, resourceName))
}

// validates that no duplication for NUMA node and limit type occurred
if _, ok := numaTypeDuplicates[numaNode][resourceName]; ok {
errors = append(errors, fmt.Errorf("the reserved memory has a duplicate value for NUMA node %d and resource %q", numaNode, resourceName))
}
numaTypeDuplicates[numaNode][resourceName] = true
}
}
return errors
}

func reservedMemorySupportedLimit(resourceName v1.ResourceName) bool {
return corev1helper.IsHugePageResourceName(resourceName) || resourceName == v1.ResourceMemory
}
120 changes: 120 additions & 0 deletions pkg/kubelet/apis/config/validation/validation_reserved_memory_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/*
Copyright 2020 The Kubernetes Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package validation

import (
"fmt"
"testing"

v1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
kubeletconfig "k8s.io/kubernetes/pkg/kubelet/apis/config"
)

func TestValidateReservedMemoryConfiguration(t *testing.T) {
testCases := []struct {
description string
kubeletConfiguration *kubeletconfig.KubeletConfiguration
expectedError error
}{
{
description: "The kubelet configuration does not have reserved memory parameter",
kubeletConfiguration: &kubeletconfig.KubeletConfiguration{},
expectedError: nil,
},
{
description: "The kubelet configuration has valid reserved memory parameter",
kubeletConfiguration: &kubeletconfig.KubeletConfiguration{
ReservedMemory: []kubeletconfig.MemoryReservation{
{
NumaNode: 0,
Limits: v1.ResourceList{
v1.ResourceMemory: *resource.NewQuantity(128, resource.DecimalSI),
},
},
},
},
expectedError: nil,
},
{
description: "The reserved memory has duplications for the NUMA node and limit type",
kubeletConfiguration: &kubeletconfig.KubeletConfiguration{
ReservedMemory: []kubeletconfig.MemoryReservation{
{
NumaNode: 0,
Limits: v1.ResourceList{
v1.ResourceMemory: *resource.NewQuantity(128, resource.DecimalSI),
},
},
{
NumaNode: 0,
Limits: v1.ResourceList{
v1.ResourceMemory: *resource.NewQuantity(64, resource.DecimalSI),
},
},
},
},
expectedError: fmt.Errorf("the reserved memory has a duplicate value for NUMA node %d and resource %q", 0, v1.ResourceMemory),
},
{
description: "The reserved memory has unsupported limit type",
kubeletConfiguration: &kubeletconfig.KubeletConfiguration{
ReservedMemory: []kubeletconfig.MemoryReservation{
{
NumaNode: 0,
Limits: v1.ResourceList{
"blabla": *resource.NewQuantity(128, resource.DecimalSI),
},
},
},
},
expectedError: fmt.Errorf("the limit type %q for NUMA node %d is not supported, only [memory hugepages-<HugePageSize>] is accepted", "blabla", 0),
},
{
description: "The reserved memory has limit type with zero value",
kubeletConfiguration: &kubeletconfig.KubeletConfiguration{
ReservedMemory: []kubeletconfig.MemoryReservation{
{
NumaNode: 0,
Limits: v1.ResourceList{
v1.ResourceMemory: *resource.NewQuantity(0, resource.DecimalSI),
},
},
},
},
expectedError: fmt.Errorf("reserved memory may not be zero for NUMA node %d and resource %q", 0, v1.ResourceMemory),
},
}

for _, testCase := range testCases {
errors := validateReservedMemoryConfiguration(testCase.kubeletConfiguration)

if len(errors) != 0 && testCase.expectedError == nil {
t.Errorf("expected errors %v, got %v", errors, testCase.expectedError)
}

if testCase.expectedError != nil {
if len(errors) == 0 {
t.Errorf("expected error %v, got %v", testCase.expectedError, errors)
}

if errors[0].Error() != testCase.expectedError.Error() {
t.Errorf("expected error %v, got %v", testCase.expectedError, errors[0])
}
}
}
}
18 changes: 9 additions & 9 deletions pkg/kubelet/cm/memorymanager/memory_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ func (m *manager) policyRemoveContainerByRef(podUID string, containerName string
return err
}

func getTotalMemoryTypeReserved(machineInfo *cadvisorapi.MachineInfo, reservedMemory []kubeletconfig.MemoryReservation) map[v1.ResourceName]resource.Quantity {
func getTotalMemoryTypeReserved(machineInfo *cadvisorapi.MachineInfo, reservedMemory []kubeletconfig.MemoryReservation) (map[v1.ResourceName]resource.Quantity, error) {
totalMemoryType := map[v1.ResourceName]resource.Quantity{}

numaNodes := map[int]bool{}
Expand All @@ -331,8 +331,7 @@ func getTotalMemoryTypeReserved(machineInfo *cadvisorapi.MachineInfo, reservedMe

for _, reservation := range reservedMemory {
if !numaNodes[int(reservation.NumaNode)] {
klog.Warningf("The NUMA node %d specified under --reserved-memory does not exist on the machine", reservation.NumaNode)
continue
return nil, fmt.Errorf("the reserved memory configuration references a NUMA node %d that does not exist on this machine", reservation.NumaNode)
}

for resourceName, q := range reservation.Limits {
Expand All @@ -343,19 +342,20 @@ func getTotalMemoryTypeReserved(machineInfo *cadvisorapi.MachineInfo, reservedMe
}
}

return totalMemoryType
return totalMemoryType, nil
}

func validateReservedMemory(machineInfo *cadvisorapi.MachineInfo, nodeAllocatableReservation v1.ResourceList, reservedMemory []kubeletconfig.MemoryReservation) error {
totalMemoryType := getTotalMemoryTypeReserved(machineInfo, reservedMemory)
totalMemoryType, err := getTotalMemoryTypeReserved(machineInfo, reservedMemory)
if err != nil {
return err
}

commonMemoryTypeSet := make(map[v1.ResourceName]bool)
for resourceType := range totalMemoryType {
if !(corev1helper.IsHugePageResourceName(resourceType) || resourceType == v1.ResourceMemory) {
continue
}
commonMemoryTypeSet[resourceType] = true
}

for resourceType := range nodeAllocatableReservation {
if !(corev1helper.IsHugePageResourceName(resourceType) || resourceType == v1.ResourceMemory) {
continue
Expand All @@ -375,7 +375,7 @@ func validateReservedMemory(machineInfo *cadvisorapi.MachineInfo, nodeAllocatabl
}

if !(*nodeAllocatableMemory).Equal(*reservedMemory) {
return fmt.Errorf("the total amount of memory of type \"%s\" is not equal to the value determined by Node Allocatable feature", resourceType)
return fmt.Errorf("the total amount %q of type %q is not equal to the value %q determined by Node Allocatable feature", reservedMemory.String(), resourceType, nodeAllocatableMemory.String())
}
}

Expand Down
18 changes: 9 additions & 9 deletions pkg/kubelet/cm/memorymanager/memory_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func TestValidateReservedMemory(t *testing.T) {
{Id: 1},
},
}
const msgNotEqual = "the total amount of memory of type \"%s\" is not equal to the value determined by Node Allocatable feature"
const msgNotEqual = "the total amount %q of type %q is not equal to the value %q determined by Node Allocatable feature"
testCases := []struct {
description string
nodeAllocatableReservation v1.ResourceList
Expand Down Expand Up @@ -193,14 +193,14 @@ func TestValidateReservedMemory(t *testing.T) {
},
},
},
fmt.Sprintf(msgNotEqual, v1.ResourceMemory),
fmt.Sprintf(msgNotEqual, "12", v1.ResourceMemory, "0"),
},
{
"Node Allocatable set, reserved not set",
v1.ResourceList{hugepages2M: *resource.NewQuantity(5, resource.DecimalSI)},
machineInfo,
[]kubeletconfig.MemoryReservation{},
fmt.Sprintf(msgNotEqual, hugepages2M),
fmt.Sprintf(msgNotEqual, "0", hugepages2M, "5"),
},
{
"Reserved not equal to Node Allocatable",
Expand All @@ -214,7 +214,7 @@ func TestValidateReservedMemory(t *testing.T) {
},
},
},
fmt.Sprintf(msgNotEqual, v1.ResourceMemory),
fmt.Sprintf(msgNotEqual, "12", v1.ResourceMemory, "5"),
},
{
"Reserved contains the NUMA node that does not exist under the machine",
Expand All @@ -234,7 +234,7 @@ func TestValidateReservedMemory(t *testing.T) {
},
},
},
fmt.Sprintf(msgNotEqual, v1.ResourceMemory),
"the reserved memory configuration references a NUMA node 2 that does not exist on this machine",
},
{
"Reserved total equal to Node Allocatable",
Expand Down Expand Up @@ -285,7 +285,7 @@ func TestValidateReservedMemory(t *testing.T) {
},
},

fmt.Sprintf(msgNotEqual, hugepages2M),
fmt.Sprintf(msgNotEqual, "77", hugepages2M, "14"),
},
}

Expand Down Expand Up @@ -404,7 +404,7 @@ func TestGetSystemReservedMemory(t *testing.T) {
machineInfo: machineInfo,
},
{
description: "Should return error when Allocatable reservation is not equal pre reserved memory",
description: "Should return error when Allocatable reservation is not equal to the reserved memory",
nodeAllocatableReservation: v1.ResourceList{},
systemReservedMemory: []kubeletconfig.MemoryReservation{
{
Expand All @@ -415,7 +415,7 @@ func TestGetSystemReservedMemory(t *testing.T) {
},
},
expectedReserved: nil,
expectedError: fmt.Errorf("the total amount of memory of type \"memory\" is not equal to the value determined by Node Allocatable feature"),
expectedError: fmt.Errorf("the total amount \"1Gi\" of type \"memory\" is not equal to the value \"0\" determined by Node Allocatable feature"),
machineInfo: machineInfo,
},
{
Expand Down Expand Up @@ -2171,7 +2171,7 @@ func TestNewManager(t *testing.T) {
},
},
affinity: topologymanager.NewFakeManager(),
expectedError: fmt.Errorf("the total amount of memory of type %q is not equal to the value determined by Node Allocatable feature", v1.ResourceMemory),
expectedError: fmt.Errorf("the total amount \"3Gi\" of type %q is not equal to the value \"2Gi\" determined by Node Allocatable feature", v1.ResourceMemory),
expectedReserved: expectedReserved,
},
{
Expand Down

0 comments on commit 1021244

Please sign in to comment.