Skip to content

Commit 6e411b6

Browse files
committed
Add support for pre-allocated hugepages with 2 sizes
Remove the validation for pre-allocated hugepages on node level. Validation is currently the only thing making it impossible to use pre-allocated huge pages in more than one size. We have now quite a few reports from real users that this feature is welcome.
1 parent 0255614 commit 6e411b6

File tree

5 files changed

+300
-15
lines changed

5 files changed

+300
-15
lines changed

pkg/apis/core/validation/validation.go

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4424,8 +4424,15 @@ func ValidateNodeSpecificAnnotations(annotations map[string]string, fldPath *fie
44244424
return allErrs
44254425
}
44264426

4427+
// NodeValidationOptions contains the different settings for node validation
4428+
type NodeValidationOptions struct {
4429+
// Should node a spec containing more than one huge page resource (with different sizes)
4430+
// with pre-allocated memory trigger validation errors
4431+
ValidateSingleHugePageResource bool
4432+
}
4433+
44274434
// ValidateNode tests if required fields in the node are set.
4428-
func ValidateNode(node *core.Node) field.ErrorList {
4435+
func ValidateNode(node *core.Node, opts NodeValidationOptions) field.ErrorList {
44294436
fldPath := field.NewPath("metadata")
44304437
allErrs := ValidateObjectMeta(&node.ObjectMeta, false, ValidateNodeName, fldPath)
44314438
allErrs = append(allErrs, ValidateNodeSpecificAnnotations(node.ObjectMeta.Annotations, fldPath.Child("annotations"))...)
@@ -4436,7 +4443,7 @@ func ValidateNode(node *core.Node) field.ErrorList {
44364443
// Only validate spec.
44374444
// All status fields are optional and can be updated later.
44384445
// That said, if specified, we need to ensure they are valid.
4439-
allErrs = append(allErrs, ValidateNodeResources(node)...)
4446+
allErrs = append(allErrs, ValidateNodeResources(node, opts)...)
44404447

44414448
// validate PodCIDRS only if we need to
44424449
if len(node.Spec.PodCIDRs) > 0 {
@@ -4476,13 +4483,33 @@ func ValidateNode(node *core.Node) field.ErrorList {
44764483
}
44774484

44784485
// ValidateNodeResources is used to make sure a node has valid capacity and allocatable values.
4479-
func ValidateNodeResources(node *core.Node) field.ErrorList {
4486+
func ValidateNodeResources(node *core.Node, opts NodeValidationOptions) field.ErrorList {
44804487
allErrs := field.ErrorList{}
4488+
if opts.ValidateSingleHugePageResource {
4489+
allErrs = append(allErrs, ValidateNodeSingleHugePageResources(node)...)
4490+
}
4491+
44814492
// Validate resource quantities in capacity.
4482-
hugePageSizes := sets.NewString()
44834493
for k, v := range node.Status.Capacity {
44844494
resPath := field.NewPath("status", "capacity", string(k))
44854495
allErrs = append(allErrs, ValidateResourceQuantityValue(string(k), v, resPath)...)
4496+
}
4497+
4498+
// Validate resource quantities in allocatable.
4499+
for k, v := range node.Status.Allocatable {
4500+
resPath := field.NewPath("status", "allocatable", string(k))
4501+
allErrs = append(allErrs, ValidateResourceQuantityValue(string(k), v, resPath)...)
4502+
}
4503+
return allErrs
4504+
}
4505+
4506+
// ValidateNodeHugePageResources is used to make sure a node has valid capacity and allocatable values for the huge page resources.
4507+
func ValidateNodeSingleHugePageResources(node *core.Node) field.ErrorList {
4508+
allErrs := field.ErrorList{}
4509+
// Validate resource quantities in capacity.
4510+
hugePageSizes := sets.NewString()
4511+
for k, v := range node.Status.Capacity {
4512+
resPath := field.NewPath("status", "capacity", string(k))
44864513
// track any huge page size that has a positive value
44874514
if helper.IsHugePageResourceName(k) && v.Value() > int64(0) {
44884515
hugePageSizes.Insert(string(k))
@@ -4495,7 +4522,6 @@ func ValidateNodeResources(node *core.Node) field.ErrorList {
44954522
hugePageSizes = sets.NewString()
44964523
for k, v := range node.Status.Allocatable {
44974524
resPath := field.NewPath("status", "allocatable", string(k))
4498-
allErrs = append(allErrs, ValidateResourceQuantityValue(string(k), v, resPath)...)
44994525
// track any huge page size that has a positive value
45004526
if helper.IsHugePageResourceName(k) && v.Value() > int64(0) {
45014527
hugePageSizes.Insert(string(k))
@@ -4508,7 +4534,7 @@ func ValidateNodeResources(node *core.Node) field.ErrorList {
45084534
}
45094535

45104536
// ValidateNodeUpdate tests to make sure a node update can be applied. Modifies oldNode.
4511-
func ValidateNodeUpdate(node, oldNode *core.Node) field.ErrorList {
4537+
func ValidateNodeUpdate(node, oldNode *core.Node, opts NodeValidationOptions) field.ErrorList {
45124538
fldPath := field.NewPath("metadata")
45134539
allErrs := ValidateObjectMetaUpdate(&node.ObjectMeta, &oldNode.ObjectMeta, fldPath)
45144540
allErrs = append(allErrs, ValidateNodeSpecificAnnotations(node.ObjectMeta.Annotations, fldPath.Child("annotations"))...)
@@ -4519,7 +4545,7 @@ func ValidateNodeUpdate(node, oldNode *core.Node) field.ErrorList {
45194545
// allErrs = append(allErrs, field.Invalid("status", node.Status, "must be empty"))
45204546
// }
45214547

4522-
allErrs = append(allErrs, ValidateNodeResources(node)...)
4548+
allErrs = append(allErrs, ValidateNodeResources(node, opts)...)
45234549

45244550
// Validate no duplicate addresses in node status.
45254551
addresses := make(map[core.NodeAddress]bool)

pkg/apis/core/validation/validation_test.go

Lines changed: 137 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10812,6 +10812,9 @@ func TestValidateReplicationController(t *testing.T) {
1081210812
}
1081310813

1081410814
func TestValidateNode(t *testing.T) {
10815+
opts := NodeValidationOptions{
10816+
ValidateSingleHugePageResource: true,
10817+
}
1081510818
validSelector := map[string]string{"a": "b"}
1081610819
invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"}
1081710820
successCases := []core.Node{
@@ -10918,7 +10921,7 @@ func TestValidateNode(t *testing.T) {
1091810921
},
1091910922
}
1092010923
for _, successCase := range successCases {
10921-
if errs := ValidateNode(&successCase); len(errs) != 0 {
10924+
if errs := ValidateNode(&successCase, opts); len(errs) != 0 {
1092210925
t.Errorf("expected success: %v", errs)
1092310926
}
1092410927
}
@@ -11142,7 +11145,7 @@ func TestValidateNode(t *testing.T) {
1114211145
},
1114311146
}
1114411147
for k, v := range errorCases {
11145-
errs := ValidateNode(&v)
11148+
errs := ValidateNode(&v, opts)
1114611149
if len(errs) == 0 {
1114711150
t.Errorf("expected failure for %s", k)
1114811151
}
@@ -11169,7 +11172,134 @@ func TestValidateNode(t *testing.T) {
1116911172
}
1117011173
}
1117111174

11175+
func TestNodeValidationOptions(t *testing.T) {
11176+
updateTests := []struct {
11177+
oldNode core.Node
11178+
node core.Node
11179+
opts NodeValidationOptions
11180+
valid bool
11181+
}{
11182+
{core.Node{
11183+
ObjectMeta: metav1.ObjectMeta{
11184+
Name: "validate-single-hugepages",
11185+
},
11186+
Status: core.NodeStatus{
11187+
Capacity: core.ResourceList{
11188+
core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
11189+
core.ResourceName("hugepages-1Gi"): resource.MustParse("0"),
11190+
},
11191+
},
11192+
}, core.Node{
11193+
ObjectMeta: metav1.ObjectMeta{
11194+
Name: "validate-single-hugepages",
11195+
},
11196+
Status: core.NodeStatus{
11197+
Capacity: core.ResourceList{
11198+
core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
11199+
core.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"),
11200+
},
11201+
},
11202+
}, NodeValidationOptions{ValidateSingleHugePageResource: true}, false},
11203+
{core.Node{
11204+
ObjectMeta: metav1.ObjectMeta{
11205+
Name: "validate-single-hugepages",
11206+
},
11207+
Status: core.NodeStatus{
11208+
Capacity: core.ResourceList{
11209+
core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
11210+
core.ResourceName("hugepages-1Gi"): resource.MustParse("1Gi"),
11211+
},
11212+
},
11213+
}, core.Node{
11214+
ObjectMeta: metav1.ObjectMeta{
11215+
Name: "validate-single-hugepages",
11216+
},
11217+
Status: core.NodeStatus{
11218+
Capacity: core.ResourceList{
11219+
core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
11220+
core.ResourceName("hugepages-1Gi"): resource.MustParse("1Gi"),
11221+
},
11222+
},
11223+
}, NodeValidationOptions{ValidateSingleHugePageResource: true}, false},
11224+
{core.Node{
11225+
ObjectMeta: metav1.ObjectMeta{
11226+
Name: "not-validate-single-hugepages",
11227+
},
11228+
Status: core.NodeStatus{
11229+
Capacity: core.ResourceList{
11230+
core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
11231+
core.ResourceName("hugepages-1Gi"): resource.MustParse("0"),
11232+
},
11233+
},
11234+
}, core.Node{
11235+
ObjectMeta: metav1.ObjectMeta{
11236+
Name: "not-validate-single-hugepages",
11237+
},
11238+
Status: core.NodeStatus{
11239+
Capacity: core.ResourceList{
11240+
core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
11241+
core.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"),
11242+
},
11243+
},
11244+
}, NodeValidationOptions{ValidateSingleHugePageResource: false}, true},
11245+
}
11246+
for i, test := range updateTests {
11247+
test.oldNode.ObjectMeta.ResourceVersion = "1"
11248+
test.node.ObjectMeta.ResourceVersion = "1"
11249+
errs := ValidateNodeUpdate(&test.node, &test.oldNode, test.opts)
11250+
if test.valid && len(errs) > 0 {
11251+
t.Errorf("%d: Unexpected error: %v", i, errs)
11252+
t.Logf("%#v vs %#v", test.oldNode.ObjectMeta, test.node.ObjectMeta)
11253+
}
11254+
if !test.valid && len(errs) == 0 {
11255+
t.Errorf("%d: Unexpected non-error", i)
11256+
t.Logf("%#v vs %#v", test.oldNode.ObjectMeta, test.node.ObjectMeta)
11257+
}
11258+
}
11259+
nodeTests := []struct {
11260+
node core.Node
11261+
opts NodeValidationOptions
11262+
valid bool
11263+
}{
11264+
{core.Node{
11265+
ObjectMeta: metav1.ObjectMeta{
11266+
Name: "validate-single-hugepages",
11267+
},
11268+
Status: core.NodeStatus{
11269+
Capacity: core.ResourceList{
11270+
core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
11271+
core.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"),
11272+
},
11273+
},
11274+
}, NodeValidationOptions{ValidateSingleHugePageResource: true}, false},
11275+
{core.Node{
11276+
ObjectMeta: metav1.ObjectMeta{
11277+
Name: "not-validate-single-hugepages",
11278+
},
11279+
Status: core.NodeStatus{
11280+
Capacity: core.ResourceList{
11281+
core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
11282+
core.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"),
11283+
core.ResourceName("hugepages-64Ki"): resource.MustParse("2Gi"),
11284+
},
11285+
},
11286+
}, NodeValidationOptions{ValidateSingleHugePageResource: false}, true},
11287+
}
11288+
for i, test := range nodeTests {
11289+
test.node.ObjectMeta.ResourceVersion = "1"
11290+
errs := ValidateNode(&test.node, test.opts)
11291+
if test.valid && len(errs) > 0 {
11292+
t.Errorf("%d: Unexpected error: %v", i, errs)
11293+
}
11294+
if !test.valid && len(errs) == 0 {
11295+
t.Errorf("%d: Unexpected non-error", i)
11296+
}
11297+
}
11298+
}
1117211299
func TestValidateNodeUpdate(t *testing.T) {
11300+
opts := NodeValidationOptions{
11301+
ValidateSingleHugePageResource: true,
11302+
}
1117311303
tests := []struct {
1117411304
oldNode core.Node
1117511305
node core.Node
@@ -11593,7 +11723,7 @@ func TestValidateNodeUpdate(t *testing.T) {
1159311723
for i, test := range tests {
1159411724
test.oldNode.ObjectMeta.ResourceVersion = "1"
1159511725
test.node.ObjectMeta.ResourceVersion = "1"
11596-
errs := ValidateNodeUpdate(&test.node, &test.oldNode)
11726+
errs := ValidateNodeUpdate(&test.node, &test.oldNode, opts)
1159711727
if test.valid && len(errs) > 0 {
1159811728
t.Errorf("%d: Unexpected error: %v", i, errs)
1159911729
t.Logf("%#v vs %#v", test.oldNode.ObjectMeta, test.node.ObjectMeta)
@@ -14779,6 +14909,9 @@ func makeNode(nodeName string, podCIDRs []string) core.Node {
1477914909
}
1478014910
}
1478114911
func TestValidateNodeCIDRs(t *testing.T) {
14912+
opts := NodeValidationOptions{
14913+
ValidateSingleHugePageResource: true,
14914+
}
1478214915
testCases := []struct {
1478314916
expectError bool
1478414917
node core.Node
@@ -14839,7 +14972,7 @@ func TestValidateNodeCIDRs(t *testing.T) {
1483914972
},
1484014973
}
1484114974
for _, testCase := range testCases {
14842-
errs := ValidateNode(&testCase.node)
14975+
errs := ValidateNode(&testCase.node, opts)
1484314976
if len(errs) == 0 && testCase.expectError {
1484414977
t.Errorf("expected failure for %s, but there were none", testCase.node.Name)
1484514978
return

pkg/registry/core/node/BUILD

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,8 @@ go_test(
4444
"//pkg/apis/core:go_default_library",
4545
"//pkg/apis/core/install:go_default_library",
4646
"//pkg/features:go_default_library",
47+
"//staging/src/k8s.io/apimachinery/pkg/api/resource:go_default_library",
48+
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
4749
"//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
4850
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
4951
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",

pkg/registry/core/node/strategy.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,12 @@ func nodeConfigSourceInUse(node *api.Node) bool {
124124
// Validate validates a new node.
125125
func (nodeStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
126126
node := obj.(*api.Node)
127-
return validation.ValidateNode(node)
127+
opts := validation.NodeValidationOptions{
128+
// This ensures new nodes have no more than one hugepages resource
129+
// TODO: set to false in 1.19; 1.18 servers tolerate multiple hugepages resources on update
130+
ValidateSingleHugePageResource: true,
131+
}
132+
return validation.ValidateNode(node, opts)
128133
}
129134

130135
// Canonicalize normalizes the object after validation.
@@ -133,8 +138,14 @@ func (nodeStrategy) Canonicalize(obj runtime.Object) {
133138

134139
// ValidateUpdate is the default update validation for an end user.
135140
func (nodeStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
136-
errorList := validation.ValidateNode(obj.(*api.Node))
137-
return append(errorList, validation.ValidateNodeUpdate(obj.(*api.Node), old.(*api.Node))...)
141+
oldPassesSingleHugepagesValidation := len(validation.ValidateNodeSingleHugePageResources(old.(*api.Node))) == 0
142+
opts := validation.NodeValidationOptions{
143+
// This ensures the new node has no more than one hugepages resource, if the old node did as well.
144+
// TODO: set to false in 1.19; 1.18 servers tolerate relaxed validation on update
145+
ValidateSingleHugePageResource: oldPassesSingleHugepagesValidation,
146+
}
147+
errorList := validation.ValidateNode(obj.(*api.Node), opts)
148+
return append(errorList, validation.ValidateNodeUpdate(obj.(*api.Node), old.(*api.Node), opts)...)
138149
}
139150

140151
func (nodeStrategy) AllowUnconditionalUpdate() bool {
@@ -185,7 +196,13 @@ func nodeStatusConfigInUse(node *api.Node) bool {
185196
}
186197

187198
func (nodeStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList {
188-
return validation.ValidateNodeUpdate(obj.(*api.Node), old.(*api.Node))
199+
oldPassesSingleHugepagesValidation := len(validation.ValidateNodeSingleHugePageResources(old.(*api.Node))) == 0
200+
opts := validation.NodeValidationOptions{
201+
// This ensures the new node has no more than one hugepages resource, if the old node did as well.
202+
// TODO: set to false in 1.19; 1.18 servers tolerate relaxed validation on update
203+
ValidateSingleHugePageResource: oldPassesSingleHugepagesValidation,
204+
}
205+
return validation.ValidateNodeUpdate(obj.(*api.Node), old.(*api.Node), opts)
189206
}
190207

191208
// Canonicalize normalizes the object after validation.

0 commit comments

Comments
 (0)