Merge pull request #89252 from odinuge/hugepages-relax-node-validation

Relax huge page node validation
2025-07-29 14:37:00 +00:00 · 2020-03-23 13:52:47 -07:00 · 2020-03-23 13:52:47 -07:00 · d20b6cd44d
commit d20b6cd44d
parent 0641e0c6d8 19f0519b2d
4 changed files with 33 additions and 223 deletions
--- a/pkg/apis/core/validation/validation.go
+++ b/pkg/apis/core/validation/validation.go
@ -4479,15 +4479,8 @@ func ValidateNodeSpecificAnnotations(annotations map[string]string, fldPath *fie
 	return allErrs
 }

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

 	// validate PodCIDRS only if we need to
 	if len(node.Spec.PodCIDRs) > 0 {
@ -4538,11 +4531,8 @@ func ValidateNode(node *core.Node, opts NodeValidationOptions) field.ErrorList {
 }

 // ValidateNodeResources is used to make sure a node has valid capacity and allocatable values.
-func ValidateNodeResources(node *core.Node, opts NodeValidationOptions) field.ErrorList {
+func ValidateNodeResources(node *core.Node) field.ErrorList {
 	allErrs := field.ErrorList{}
-	if opts.ValidateSingleHugePageResource {
-		allErrs = append(allErrs, ValidateNodeSingleHugePageResources(node)...)
-	}

 	// Validate resource quantities in capacity.
 	for k, v := range node.Status.Capacity {
@ -4558,38 +4548,8 @@ func ValidateNodeResources(node *core.Node, opts NodeValidationOptions) field.Er
 	return allErrs
 }

-// ValidateNodeHugePageResources is used to make sure a node has valid capacity and allocatable values for the huge page resources.
-func ValidateNodeSingleHugePageResources(node *core.Node) field.ErrorList {
-	allErrs := field.ErrorList{}
-	// Validate resource quantities in capacity.
-	hugePageSizes := sets.NewString()
-	for k, v := range node.Status.Capacity {
-		resPath := field.NewPath("status", "capacity", string(k))
-		// track any huge page size that has a positive value
-		if helper.IsHugePageResourceName(k) && v.Value() > int64(0) {
-			hugePageSizes.Insert(string(k))
-		}
-		if len(hugePageSizes) > 1 {
-			allErrs = append(allErrs, field.Invalid(resPath, v, "may not have pre-allocated hugepages for multiple page sizes"))
-		}
-	}
-	// Validate resource quantities in allocatable.
-	hugePageSizes = sets.NewString()
-	for k, v := range node.Status.Allocatable {
-		resPath := field.NewPath("status", "allocatable", string(k))
-		// track any huge page size that has a positive value
-		if helper.IsHugePageResourceName(k) && v.Value() > int64(0) {
-			hugePageSizes.Insert(string(k))
-		}
-		if len(hugePageSizes) > 1 {
-			allErrs = append(allErrs, field.Invalid(resPath, v, "may not have pre-allocated hugepages for multiple page sizes"))
-		}
-	}
-	return allErrs
-}
-
 // ValidateNodeUpdate tests to make sure a node update can be applied.  Modifies oldNode.
-func ValidateNodeUpdate(node, oldNode *core.Node, opts NodeValidationOptions) field.ErrorList {
+func ValidateNodeUpdate(node, oldNode *core.Node) field.ErrorList {
 	fldPath := field.NewPath("metadata")
 	allErrs := ValidateObjectMetaUpdate(&node.ObjectMeta, &oldNode.ObjectMeta, fldPath)
 	allErrs = append(allErrs, ValidateNodeSpecificAnnotations(node.ObjectMeta.Annotations, fldPath.Child("annotations"))...)
@ -4600,7 +4560,7 @@ func ValidateNodeUpdate(node, oldNode *core.Node, opts NodeValidationOptions) fi
 	// 	allErrs = append(allErrs, field.Invalid("status", node.Status, "must be empty"))
 	// }

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

 	// Validate no duplicate addresses in node status.
 	addresses := make(map[core.NodeAddress]bool)
--- a/pkg/apis/core/validation/validation_test.go
+++ b/pkg/apis/core/validation/validation_test.go
@ -10941,9 +10941,6 @@ func TestValidateReplicationController(t *testing.T) {
 }

 func TestValidateNode(t *testing.T) {
-	opts := NodeValidationOptions{
-		ValidateSingleHugePageResource: true,
-	}
 	validSelector := map[string]string{"a": "b"}
 	invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"}
 	successCases := []core.Node{
@ -10979,6 +10976,24 @@ func TestValidateNode(t *testing.T) {
 				},
 			},
 		},
+		{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:   "abc",
+				Labels: validSelector,
+			},
+			Status: core.NodeStatus{
+				Addresses: []core.NodeAddress{
+					{Type: core.NodeExternalIP, Address: "something"},
+				},
+				Capacity: core.ResourceList{
+					core.ResourceName(core.ResourceCPU):    resource.MustParse("10"),
+					core.ResourceName(core.ResourceMemory): resource.MustParse("10G"),
+					core.ResourceName("my.org/gpu"):        resource.MustParse("10"),
+					core.ResourceName("hugepages-2Mi"):     resource.MustParse("10Gi"),
+					core.ResourceName("hugepages-1Gi"):     resource.MustParse("10Gi"),
+				},
+			},
+		},
 		{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "dedicated-node1",
@ -11050,7 +11065,7 @@ func TestValidateNode(t *testing.T) {
 		},
 	}
 	for _, successCase := range successCases {
-		if errs := ValidateNode(&successCase, opts); len(errs) != 0 {
+		if errs := ValidateNode(&successCase); len(errs) != 0 {
 			t.Errorf("expected success: %v", errs)
 		}
 	}
@ -11220,24 +11235,6 @@ func TestValidateNode(t *testing.T) {
 				},
 			},
 		},
-		"multiple-pre-allocated-hugepages": {
-			ObjectMeta: metav1.ObjectMeta{
-				Name:   "abc",
-				Labels: validSelector,
-			},
-			Status: core.NodeStatus{
-				Addresses: []core.NodeAddress{
-					{Type: core.NodeExternalIP, Address: "something"},
-				},
-				Capacity: core.ResourceList{
-					core.ResourceName(core.ResourceCPU):    resource.MustParse("10"),
-					core.ResourceName(core.ResourceMemory): resource.MustParse("10G"),
-					core.ResourceName("my.org/gpu"):        resource.MustParse("10"),
-					core.ResourceName("hugepages-2Mi"):     resource.MustParse("10Gi"),
-					core.ResourceName("hugepages-1Gi"):     resource.MustParse("10Gi"),
-				},
-			},
-		},
 		"invalid-pod-cidr": {
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "abc",
@ -11274,7 +11271,7 @@ func TestValidateNode(t *testing.T) {
 		},
 	}
 	for k, v := range errorCases {
-		errs := ValidateNode(&v, opts)
+		errs := ValidateNode(&v)
 		if len(errs) == 0 {
 			t.Errorf("expected failure for %s", k)
 		}
@ -11301,134 +11298,7 @@ func TestValidateNode(t *testing.T) {
 	}
 }

-func TestNodeValidationOptions(t *testing.T) {
-	updateTests := []struct {
-		oldNode core.Node
-		node    core.Node
-		opts    NodeValidationOptions
-		valid   bool
-	}{
-		{core.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "validate-single-hugepages",
-			},
-			Status: core.NodeStatus{
-				Capacity: core.ResourceList{
-					core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
-					core.ResourceName("hugepages-1Gi"): resource.MustParse("0"),
-				},
-			},
-		}, core.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "validate-single-hugepages",
-			},
-			Status: core.NodeStatus{
-				Capacity: core.ResourceList{
-					core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
-					core.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"),
-				},
-			},
-		}, NodeValidationOptions{ValidateSingleHugePageResource: true}, false},
-		{core.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "validate-single-hugepages",
-			},
-			Status: core.NodeStatus{
-				Capacity: core.ResourceList{
-					core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
-					core.ResourceName("hugepages-1Gi"): resource.MustParse("1Gi"),
-				},
-			},
-		}, core.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "validate-single-hugepages",
-			},
-			Status: core.NodeStatus{
-				Capacity: core.ResourceList{
-					core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
-					core.ResourceName("hugepages-1Gi"): resource.MustParse("1Gi"),
-				},
-			},
-		}, NodeValidationOptions{ValidateSingleHugePageResource: true}, false},
-		{core.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "not-validate-single-hugepages",
-			},
-			Status: core.NodeStatus{
-				Capacity: core.ResourceList{
-					core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
-					core.ResourceName("hugepages-1Gi"): resource.MustParse("0"),
-				},
-			},
-		}, core.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "not-validate-single-hugepages",
-			},
-			Status: core.NodeStatus{
-				Capacity: core.ResourceList{
-					core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
-					core.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"),
-				},
-			},
-		}, NodeValidationOptions{ValidateSingleHugePageResource: false}, true},
-	}
-	for i, test := range updateTests {
-		test.oldNode.ObjectMeta.ResourceVersion = "1"
-		test.node.ObjectMeta.ResourceVersion = "1"
-		errs := ValidateNodeUpdate(&test.node, &test.oldNode, test.opts)
-		if test.valid && len(errs) > 0 {
-			t.Errorf("%d: Unexpected error: %v", i, errs)
-			t.Logf("%#v vs %#v", test.oldNode.ObjectMeta, test.node.ObjectMeta)
-		}
-		if !test.valid && len(errs) == 0 {
-			t.Errorf("%d: Unexpected non-error", i)
-			t.Logf("%#v vs %#v", test.oldNode.ObjectMeta, test.node.ObjectMeta)
-		}
-	}
-	nodeTests := []struct {
-		node  core.Node
-		opts  NodeValidationOptions
-		valid bool
-	}{
-		{core.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "validate-single-hugepages",
-			},
-			Status: core.NodeStatus{
-				Capacity: core.ResourceList{
-					core.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"),
-					core.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"),
-				},
-			},
-		}, NodeValidationOptions{ValidateSingleHugePageResource: true}, false},
-		{core.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Name: "not-validate-single-hugepages",
-			},
-			Status: core.NodeStatus{
-				Capacity: core.ResourceList{
-					core.ResourceName("hugepages-2Mi"):  resource.MustParse("1Gi"),
-					core.ResourceName("hugepages-1Gi"):  resource.MustParse("2Gi"),
-					core.ResourceName("hugepages-64Ki"): resource.MustParse("2Gi"),
-				},
-			},
-		}, NodeValidationOptions{ValidateSingleHugePageResource: false}, true},
-	}
-	for i, test := range nodeTests {
-		test.node.ObjectMeta.ResourceVersion = "1"
-		errs := ValidateNode(&test.node, test.opts)
-		if test.valid && len(errs) > 0 {
-			t.Errorf("%d: Unexpected error: %v", i, errs)
-		}
-		if !test.valid && len(errs) == 0 {
-			t.Errorf("%d: Unexpected non-error", i)
-		}
-	}
-}
 func TestValidateNodeUpdate(t *testing.T) {
-	opts := NodeValidationOptions{
-		ValidateSingleHugePageResource: true,
-	}
 	tests := []struct {
 		oldNode core.Node
 		node    core.Node
@ -11852,7 +11722,7 @@ func TestValidateNodeUpdate(t *testing.T) {
 	for i, test := range tests {
 		test.oldNode.ObjectMeta.ResourceVersion = "1"
 		test.node.ObjectMeta.ResourceVersion = "1"
-		errs := ValidateNodeUpdate(&test.node, &test.oldNode, opts)
+		errs := ValidateNodeUpdate(&test.node, &test.oldNode)
 		if test.valid && len(errs) > 0 {
 			t.Errorf("%d: Unexpected error: %v", i, errs)
 			t.Logf("%#v vs %#v", test.oldNode.ObjectMeta, test.node.ObjectMeta)
@ -15198,9 +15068,6 @@ func makeNode(nodeName string, podCIDRs []string) core.Node {
 	}
 }
 func TestValidateNodeCIDRs(t *testing.T) {
-	opts := NodeValidationOptions{
-		ValidateSingleHugePageResource: true,
-	}
 	testCases := []struct {
 		expectError bool
 		node        core.Node
@ -15261,7 +15128,7 @@ func TestValidateNodeCIDRs(t *testing.T) {
 		},
 	}
 	for _, testCase := range testCases {
-		errs := ValidateNode(&testCase.node, opts)
+		errs := ValidateNode(&testCase.node)
 		if len(errs) == 0 && testCase.expectError {
 			t.Errorf("expected failure for %s, but there were none", testCase.node.Name)
 			return
--- a/pkg/registry/core/node/strategy.go
+++ b/pkg/registry/core/node/strategy.go
@ -124,12 +124,7 @@ func nodeConfigSourceInUse(node *api.Node) bool {
 // Validate validates a new node.
 func (nodeStrategy) Validate(ctx context.Context, obj runtime.Object) field.ErrorList {
 	node := obj.(*api.Node)
-	opts := validation.NodeValidationOptions{
-		// This ensures new nodes have no more than one hugepages resource
-		// TODO: set to false in 1.19; 1.18 servers tolerate multiple hugepages resources on update
-		ValidateSingleHugePageResource: true,
-	}
-	return validation.ValidateNode(node, opts)
+	return validation.ValidateNode(node)
 }

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

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

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

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

 // Canonicalize normalizes the object after validation.
--- a/pkg/registry/core/node/strategy_test.go
+++ b/pkg/registry/core/node/strategy_test.go
@ -263,7 +263,7 @@ func TestValidateUpdate(t *testing.T) {
 					api.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"),
 				},
 			},
-		}, false},
+		}, true},
 		{api.Node{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: "hugepage-change-values",
@ -328,7 +328,7 @@ func TestValidate(t *testing.T) {
 					api.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"),
 				},
 			},
-		}, false},
+		}, true},
 	}
 	for i, test := range tests {
 		test.node.ObjectMeta.ResourceVersion = "1"