diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 3daeb139d59..bafd91b3fcc 100644 --- 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) diff --git a/pkg/apis/core/validation/validation_test.go b/pkg/apis/core/validation/validation_test.go index 1c49266ee88..8e23a7e93f5 100644 --- 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 diff --git a/pkg/registry/core/node/strategy.go b/pkg/registry/core/node/strategy.go index 1ca914b6a49..aaddb69e72a 100644 --- 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. diff --git a/pkg/registry/core/node/strategy_test.go b/pkg/registry/core/node/strategy_test.go index 3300196622f..57500663737 100644 --- 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"