From 463442aa2907268f18c488ecac7ff6ab7ed48c2d Mon Sep 17 00:00:00 2001 From: "sewon.oh" Date: Mon, 21 Oct 2019 16:46:20 +0900 Subject: [PATCH] Update container hugepage limit when creating the container Unit test for updating container hugepage limit Add warning message about ignoring case. Update error handling about hugepage size requirements Signed-off-by: sewon.oh --- pkg/apis/core/v1/helper/helpers.go | 18 ++ pkg/apis/core/v1/helper/helpers_test.go | 46 ++++- pkg/kubelet/cm/cgroup_manager_linux.go | 8 +- pkg/kubelet/kuberuntime/BUILD | 16 +- .../kuberuntime_container_linux.go | 48 +++++- .../kuberuntime_container_linux_test.go | 163 +++++++++++++++++- 6 files changed, 293 insertions(+), 6 deletions(-) diff --git a/pkg/apis/core/v1/helper/helpers.go b/pkg/apis/core/v1/helper/helpers.go index 21697bf5a0f..a930520f1b1 100644 --- a/pkg/apis/core/v1/helper/helpers.go +++ b/pkg/apis/core/v1/helper/helpers.go @@ -85,6 +85,24 @@ func HugePageSizeFromResourceName(name v1.ResourceName) (resource.Quantity, erro return resource.ParseQuantity(pageSize) } +// HugePageUnitSizeFromByteSize returns hugepage size has the format. +// `size` must be guaranteed to divisible into the largest units that can be expressed. +// B (1024 = "1KB", 1048576 = "1MB", etc). +func HugePageUnitSizeFromByteSize(size int64) (string, error) { + // hugePageSizeUnitList is borrowed from opencontainers/runc/libcontainer/cgroups/utils.go + var hugePageSizeUnitList = []string{"B", "KB", "MB", "GB", "TB", "PB"} + idx := 0 + len := len(hugePageSizeUnitList) - 1 + for size%1024 == 0 && idx < len { + size /= 1024 + idx++ + } + if size > 1024 && idx < len { + return "", fmt.Errorf("size: %d%s must be guaranteed to divisible into the largest units", size, hugePageSizeUnitList[idx]) + } + return fmt.Sprintf("%d%s", size, hugePageSizeUnitList[idx]), nil +} + // IsOvercommitAllowed returns true if the resource is in the default // namespace and is not hugepages. func IsOvercommitAllowed(name v1.ResourceName) bool { diff --git a/pkg/apis/core/v1/helper/helpers_test.go b/pkg/apis/core/v1/helper/helpers_test.go index 9345be35af1..27d5b157903 100644 --- a/pkg/apis/core/v1/helper/helpers_test.go +++ b/pkg/apis/core/v1/helper/helpers_test.go @@ -21,7 +21,7 @@ import ( "reflect" "testing" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" apiequality "k8s.io/apimachinery/pkg/api/equality" "k8s.io/apimachinery/pkg/api/resource" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -1398,3 +1398,47 @@ func TestNodeSelectorRequirementKeyExistsInNodeSelectorTerms(t *testing.T) { } } } + +func TestHugePageUnitSizeFromByteSize(t *testing.T) { + tests := []struct { + size int64 + expected string + wantErr bool + }{ + { + size: 1024, + expected: "1KB", + wantErr: false, + }, + { + size: 33554432, + expected: "32MB", + wantErr: false, + }, + { + size: 3221225472, + expected: "3GB", + wantErr: false, + }, + { + size: 1024 * 1024 * 1023 * 3, + expected: "3069MB", + wantErr: true, + }, + } + for _, test := range tests { + size := test.size + result, err := HugePageUnitSizeFromByteSize(size) + if err != nil { + if test.wantErr { + t.Logf("HugePageUnitSizeFromByteSize() expected error = %v", err) + } else { + t.Errorf("HugePageUnitSizeFromByteSize() error = %v, wantErr %v", err, test.wantErr) + } + continue + } + if test.expected != result { + t.Errorf("HugePageUnitSizeFromByteSize() expected %v but got %v", test.expected, result) + } + } +} diff --git a/pkg/kubelet/cm/cgroup_manager_linux.go b/pkg/kubelet/cm/cgroup_manager_linux.go index 1bad39979cc..41dbed9567a 100644 --- a/pkg/kubelet/cm/cgroup_manager_linux.go +++ b/pkg/kubelet/cm/cgroup_manager_linux.go @@ -24,12 +24,12 @@ import ( "strings" "time" - units "github.com/docker/go-units" libcontainercgroups "github.com/opencontainers/runc/libcontainer/cgroups" cgroupfs "github.com/opencontainers/runc/libcontainer/cgroups/fs" cgroupsystemd "github.com/opencontainers/runc/libcontainer/cgroups/systemd" libcontainerconfigs "github.com/opencontainers/runc/libcontainer/configs" "k8s.io/klog" + v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" utilruntime "k8s.io/apimachinery/pkg/util/runtime" "k8s.io/apimachinery/pkg/util/sets" @@ -387,7 +387,11 @@ func (m *cgroupManagerImpl) toResources(resourceConfig *ResourceConfig) *libcont // for each page size enumerated, set that value pageSizes := sets.NewString() for pageSize, limit := range resourceConfig.HugePageLimit { - sizeString := units.CustomSize("%g%s", float64(pageSize), 1024.0, libcontainercgroups.HugePageSizeUnitList) + sizeString, err := v1helper.HugePageUnitSizeFromByteSize(pageSize) + if err != nil { + klog.Warningf("pageSize is invalid: %v", err) + continue + } resources.HugetlbLimit = append(resources.HugetlbLimit, &libcontainerconfigs.HugepageLimit{ Pagesize: sizeString, Limit: uint64(limit), diff --git a/pkg/kubelet/kuberuntime/BUILD b/pkg/kubelet/kuberuntime/BUILD index 6738b6edfd9..ba16cec3974 100644 --- a/pkg/kubelet/kuberuntime/BUILD +++ b/pkg/kubelet/kuberuntime/BUILD @@ -73,10 +73,14 @@ go_library( "//vendor/k8s.io/klog:go_default_library", ] + select({ "@io_bazel_rules_go//go/platform:android": [ + "//pkg/apis/core/v1/helper:go_default_library", "//pkg/kubelet/qos:go_default_library", + "//vendor/github.com/opencontainers/runc/libcontainer/cgroups/fs:go_default_library", ], "@io_bazel_rules_go//go/platform:linux": [ + "//pkg/apis/core/v1/helper:go_default_library", "//pkg/kubelet/qos:go_default_library", + "//vendor/github.com/opencontainers/runc/libcontainer/cgroups/fs:go_default_library", ], "@io_bazel_rules_go//go/platform:windows": [ "//pkg/kubelet/apis:go_default_library", @@ -130,7 +134,17 @@ go_test( "//vendor/github.com/stretchr/testify/assert:go_default_library", "//vendor/github.com/stretchr/testify/require:go_default_library", "//vendor/k8s.io/utils/pointer:go_default_library", - ], + ] + select({ + "@io_bazel_rules_go//go/platform:android": [ + "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", + "//vendor/github.com/opencontainers/runc/libcontainer/cgroups/fs:go_default_library", + ], + "@io_bazel_rules_go//go/platform:linux": [ + "//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library", + "//vendor/github.com/opencontainers/runc/libcontainer/cgroups/fs:go_default_library", + ], + "//conditions:default": [], + }), ) filegroup( diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go index 019f7dbdaf7..483d335b9ee 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux.go @@ -21,9 +21,12 @@ package kuberuntime import ( "time" - "k8s.io/api/core/v1" + cgroupfs "github.com/opencontainers/runc/libcontainer/cgroups/fs" + v1 "k8s.io/api/core/v1" utilfeature "k8s.io/apiserver/pkg/util/feature" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" + "k8s.io/klog" + v1helper "k8s.io/kubernetes/pkg/apis/core/v1/helper" kubefeatures "k8s.io/kubernetes/pkg/features" "k8s.io/kubernetes/pkg/kubelet/qos" ) @@ -78,5 +81,48 @@ func (m *kubeGenericRuntimeManager) generateLinuxContainerConfig(container *v1.C lc.Resources.CpuPeriod = cpuPeriod } + lc.Resources.HugepageLimits = GetHugepageLimitsFromResources(container.Resources) + return lc } + +// GetHugepageLimitsFromResources returns limits of each hugepages from resources. +func GetHugepageLimitsFromResources(resources v1.ResourceRequirements) []*runtimeapi.HugepageLimit { + var hugepageLimits []*runtimeapi.HugepageLimit + + // For each page size, limit to 0. + for _, pageSize := range cgroupfs.HugePageSizes { + hugepageLimits = append(hugepageLimits, &runtimeapi.HugepageLimit{ + PageSize: pageSize, + Limit: uint64(0), + }) + } + + requiredHugepageLimits := map[string]uint64{} + for resourceObj, amountObj := range resources.Limits { + if !v1helper.IsHugePageResourceName(resourceObj) { + continue + } + + pageSize, err := v1helper.HugePageSizeFromResourceName(resourceObj) + if err != nil { + klog.Warningf("Failed to get hugepage size from resource name: %v", err) + continue + } + + sizeString, err := v1helper.HugePageUnitSizeFromByteSize(pageSize.Value()) + if err != nil { + klog.Warningf("pageSize is invalid: %v", err) + continue + } + requiredHugepageLimits[sizeString] = uint64(amountObj.Value()) + } + + for _, hugepageLimit := range hugepageLimits { + if limit, exists := requiredHugepageLimits[hugepageLimit.PageSize]; exists { + hugepageLimit.Limit = limit + } + } + + return hugepageLimits +} diff --git a/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go b/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go index 61abbc135ab..6319477ccde 100644 --- a/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go +++ b/pkg/kubelet/kuberuntime/kuberuntime_container_linux_test.go @@ -19,12 +19,16 @@ limitations under the License. package kuberuntime import ( + "reflect" "testing" + cgroupfs "github.com/opencontainers/runc/libcontainer/cgroups/fs" "github.com/stretchr/testify/assert" - "k8s.io/api/core/v1" + v1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1alpha2" + + "k8s.io/apimachinery/pkg/api/resource" ) func makeExpectedConfig(m *kubeGenericRuntimeManager, pod *v1.Pod, containerIndex int) *runtimeapi.ContainerConfig { @@ -135,3 +139,160 @@ func TestGenerateContainerConfig(t *testing.T) { _, _, err = m.generateContainerConfig(&podWithContainerSecurityContext.Spec.Containers[0], podWithContainerSecurityContext, 0, "", podWithContainerSecurityContext.Spec.Containers[0].Image, []string{}) assert.Error(t, err, "RunAsNonRoot should fail for non-numeric username") } + +func TestGetHugepageLimitsFromResources(t *testing.T) { + var baseHugepage []*runtimeapi.HugepageLimit + + // For each page size, limit to 0. + for _, pageSize := range cgroupfs.HugePageSizes { + baseHugepage = append(baseHugepage, &runtimeapi.HugepageLimit{ + PageSize: pageSize, + Limit: uint64(0), + }) + } + + tests := []struct { + name string + resources v1.ResourceRequirements + expected []*runtimeapi.HugepageLimit + }{ + { + name: "Success2MB", + resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{ + "hugepages-2Mi": resource.MustParse("2Mi"), + }, + }, + expected: []*runtimeapi.HugepageLimit{ + { + PageSize: "2MB", + Limit: 2097152, + }, + }, + }, + { + name: "Success1GB", + resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{ + "hugepages-1Gi": resource.MustParse("2Gi"), + }, + }, + expected: []*runtimeapi.HugepageLimit{ + { + PageSize: "1GB", + Limit: 2147483648, + }, + }, + }, + { + name: "Skip2MB", + resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{ + "hugepages-2MB": resource.MustParse("2Mi"), + }, + }, + expected: []*runtimeapi.HugepageLimit{ + { + PageSize: "2MB", + Limit: 0, + }, + }, + }, + { + name: "Skip1GB", + resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{ + "hugepages-1GB": resource.MustParse("2Gi"), + }, + }, + expected: []*runtimeapi.HugepageLimit{ + { + PageSize: "1GB", + Limit: 0, + }, + }, + }, + { + name: "Success2MBand1GB", + resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{ + v1.ResourceName(v1.ResourceCPU): resource.MustParse("0"), + "hugepages-2Mi": resource.MustParse("2Mi"), + "hugepages-1Gi": resource.MustParse("2Gi"), + }, + }, + expected: []*runtimeapi.HugepageLimit{ + { + PageSize: "2MB", + Limit: 2097152, + }, + { + PageSize: "1GB", + Limit: 2147483648, + }, + }, + }, + { + name: "Skip2MBand1GB", + resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{ + v1.ResourceName(v1.ResourceCPU): resource.MustParse("0"), + "hugepages-2MB": resource.MustParse("2Mi"), + "hugepages-1GB": resource.MustParse("2Gi"), + }, + }, + expected: []*runtimeapi.HugepageLimit{ + { + PageSize: "2MB", + Limit: 0, + }, + { + PageSize: "1GB", + Limit: 0, + }, + }, + }, + } + + for _, test := range tests { + // Validate if machine supports hugepage size that used in test case. + machineHugepageSupport := true + for _, hugepageLimit := range test.expected { + hugepageSupport := false + for _, pageSize := range cgroupfs.HugePageSizes { + if pageSize == hugepageLimit.PageSize { + hugepageSupport = true + break + } + } + + if !hugepageSupport { + machineHugepageSupport = false + break + } + } + + // Case of machine can't support hugepage size + if !machineHugepageSupport { + continue + } + + expectedHugepages := baseHugepage + for _, hugepage := range test.expected { + for _, expectedHugepage := range expectedHugepages { + if expectedHugepage.PageSize == hugepage.PageSize { + expectedHugepage.Limit = hugepage.Limit + } + } + } + + results := GetHugepageLimitsFromResources(test.resources) + if !reflect.DeepEqual(expectedHugepages, results) { + t.Errorf("%s test failed. Expected %v but got %v", test.name, expectedHugepages, results) + } + + for _, hugepage := range baseHugepage { + hugepage.Limit = uint64(0) + } + } +}