diff --git a/pkg/apis/core/validation/validation.go b/pkg/apis/core/validation/validation.go index 5ae3d7084b4..cec7ebf1d32 100644 --- a/pkg/apis/core/validation/validation.go +++ b/pkg/apis/core/validation/validation.go @@ -4424,8 +4424,15 @@ 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) field.ErrorList { +func ValidateNode(node *core.Node, opts NodeValidationOptions) field.ErrorList { fldPath := field.NewPath("metadata") allErrs := ValidateObjectMeta(&node.ObjectMeta, false, ValidateNodeName, fldPath) allErrs = append(allErrs, ValidateNodeSpecificAnnotations(node.ObjectMeta.Annotations, fldPath.Child("annotations"))...) @@ -4436,7 +4443,7 @@ func ValidateNode(node *core.Node) 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)...) + allErrs = append(allErrs, ValidateNodeResources(node, opts)...) // validate PodCIDRS only if we need to if len(node.Spec.PodCIDRs) > 0 { @@ -4476,13 +4483,33 @@ func ValidateNode(node *core.Node) field.ErrorList { } // ValidateNodeResources is used to make sure a node has valid capacity and allocatable values. -func ValidateNodeResources(node *core.Node) field.ErrorList { +func ValidateNodeResources(node *core.Node, opts NodeValidationOptions) 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 { + resPath := field.NewPath("status", "capacity", string(k)) + allErrs = append(allErrs, ValidateResourceQuantityValue(string(k), v, resPath)...) + } + + // Validate resource quantities in allocatable. + for k, v := range node.Status.Allocatable { + resPath := field.NewPath("status", "allocatable", string(k)) + allErrs = append(allErrs, ValidateResourceQuantityValue(string(k), v, resPath)...) + } + 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)) - allErrs = append(allErrs, ValidateResourceQuantityValue(string(k), v, resPath)...) // track any huge page size that has a positive value if helper.IsHugePageResourceName(k) && v.Value() > int64(0) { hugePageSizes.Insert(string(k)) @@ -4495,7 +4522,6 @@ func ValidateNodeResources(node *core.Node) field.ErrorList { hugePageSizes = sets.NewString() for k, v := range node.Status.Allocatable { resPath := field.NewPath("status", "allocatable", string(k)) - allErrs = append(allErrs, ValidateResourceQuantityValue(string(k), v, resPath)...) // track any huge page size that has a positive value if helper.IsHugePageResourceName(k) && v.Value() > int64(0) { hugePageSizes.Insert(string(k)) @@ -4508,7 +4534,7 @@ func ValidateNodeResources(node *core.Node) field.ErrorList { } // ValidateNodeUpdate tests to make sure a node update can be applied. Modifies oldNode. -func ValidateNodeUpdate(node, oldNode *core.Node) field.ErrorList { +func ValidateNodeUpdate(node, oldNode *core.Node, opts NodeValidationOptions) field.ErrorList { fldPath := field.NewPath("metadata") allErrs := ValidateObjectMetaUpdate(&node.ObjectMeta, &oldNode.ObjectMeta, fldPath) allErrs = append(allErrs, ValidateNodeSpecificAnnotations(node.ObjectMeta.Annotations, fldPath.Child("annotations"))...) @@ -4519,7 +4545,7 @@ func ValidateNodeUpdate(node, oldNode *core.Node) field.ErrorList { // allErrs = append(allErrs, field.Invalid("status", node.Status, "must be empty")) // } - allErrs = append(allErrs, ValidateNodeResources(node)...) + allErrs = append(allErrs, ValidateNodeResources(node, opts)...) // 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 de8c1d49fc1..1e44306a4d9 100644 --- a/pkg/apis/core/validation/validation_test.go +++ b/pkg/apis/core/validation/validation_test.go @@ -10812,6 +10812,9 @@ 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{ @@ -10918,7 +10921,7 @@ func TestValidateNode(t *testing.T) { }, } for _, successCase := range successCases { - if errs := ValidateNode(&successCase); len(errs) != 0 { + if errs := ValidateNode(&successCase, opts); len(errs) != 0 { t.Errorf("expected success: %v", errs) } } @@ -11142,7 +11145,7 @@ func TestValidateNode(t *testing.T) { }, } for k, v := range errorCases { - errs := ValidateNode(&v) + errs := ValidateNode(&v, opts) if len(errs) == 0 { t.Errorf("expected failure for %s", k) } @@ -11169,7 +11172,134 @@ 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 @@ -11593,7 +11723,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) + errs := ValidateNodeUpdate(&test.node, &test.oldNode, 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) @@ -14779,6 +14909,9 @@ func makeNode(nodeName string, podCIDRs []string) core.Node { } } func TestValidateNodeCIDRs(t *testing.T) { + opts := NodeValidationOptions{ + ValidateSingleHugePageResource: true, + } testCases := []struct { expectError bool node core.Node @@ -14839,7 +14972,7 @@ func TestValidateNodeCIDRs(t *testing.T) { }, } for _, testCase := range testCases { - errs := ValidateNode(&testCase.node) + errs := ValidateNode(&testCase.node, opts) 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/BUILD b/pkg/registry/core/node/BUILD index d12439d40c0..0a76925b89b 100644 --- a/pkg/registry/core/node/BUILD +++ b/pkg/registry/core/node/BUILD @@ -44,6 +44,8 @@ go_test( "//pkg/apis/core:go_default_library", "//pkg/apis/core/install:go_default_library", "//pkg/features: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/fields:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library", "//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library", diff --git a/pkg/registry/core/node/strategy.go b/pkg/registry/core/node/strategy.go index aaddb69e72a..1ca914b6a49 100644 --- a/pkg/registry/core/node/strategy.go +++ b/pkg/registry/core/node/strategy.go @@ -124,7 +124,12 @@ 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) - return validation.ValidateNode(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) } // Canonicalize normalizes the object after validation. @@ -133,8 +138,14 @@ 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 { - errorList := validation.ValidateNode(obj.(*api.Node)) - return append(errorList, validation.ValidateNodeUpdate(obj.(*api.Node), old.(*api.Node))...) + 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)...) } func (nodeStrategy) AllowUnconditionalUpdate() bool { @@ -185,7 +196,13 @@ func nodeStatusConfigInUse(node *api.Node) bool { } func (nodeStatusStrategy) ValidateUpdate(ctx context.Context, obj, old runtime.Object) field.ErrorList { - return validation.ValidateNodeUpdate(obj.(*api.Node), old.(*api.Node)) + 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) } // 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 198ea59b183..3300196622f 100644 --- a/pkg/registry/core/node/strategy_test.go +++ b/pkg/registry/core/node/strategy_test.go @@ -17,9 +17,12 @@ limitations under the License. package node import ( + "context" "reflect" "testing" + "k8s.io/apimachinery/pkg/api/resource" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/util/diff" @@ -234,3 +237,107 @@ func TestDropFields(t *testing.T) { }() } } +func TestValidateUpdate(t *testing.T) { + tests := []struct { + oldNode api.Node + node api.Node + valid bool + }{ + {api.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hugepage-change-values-from-0", + }, + Status: api.NodeStatus{ + Capacity: api.ResourceList{ + api.ResourceName("hugepages-2Mi"): resource.MustParse("0"), + api.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, api.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hugepage-change-values-from-0", + }, + Status: api.NodeStatus{ + Capacity: api.ResourceList{ + api.ResourceName("hugepages-2Mi"): resource.MustParse("2Gi"), + api.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, false}, + {api.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hugepage-change-values", + }, + Status: api.NodeStatus{ + Capacity: api.ResourceList{ + api.ResourceName("hugepages-2Mi"): resource.MustParse("1Gi"), + api.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, api.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "hugepage-change-values", + }, + Status: api.NodeStatus{ + Capacity: api.ResourceList{ + api.ResourceName("hugepages-2Mi"): resource.MustParse("2Gi"), + api.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, true}, + } + for i, test := range tests { + test.node.ObjectMeta.ResourceVersion = "1" + errs := (nodeStrategy{}).ValidateUpdate(context.Background(), &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) + } + if !test.valid && len(errs) == 0 { + t.Errorf("%d: Unexpected non-error", i) + } + } +} +func TestValidate(t *testing.T) { + tests := []struct { + node api.Node + valid bool + }{ + {api.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "one-hugepage-size", + }, + Status: api.NodeStatus{ + Capacity: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + api.ResourceMemory: resource.MustParse("10000"), + api.ResourceName("hugepages-2Mi"): resource.MustParse("0"), + api.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, true}, + {api.Node{ + ObjectMeta: metav1.ObjectMeta{ + Name: "multiple-hugepage-sizes", + }, + Status: api.NodeStatus{ + Capacity: api.ResourceList{ + api.ResourceCPU: resource.MustParse("100"), + api.ResourceMemory: resource.MustParse("10000"), + api.ResourceName("hugepages-2Mi"): resource.MustParse("2Gi"), + api.ResourceName("hugepages-1Gi"): resource.MustParse("2Gi"), + }, + }, + }, false}, + } + for i, test := range tests { + test.node.ObjectMeta.ResourceVersion = "1" + errs := (nodeStrategy{}).Validate(context.Background(), &test.node) + 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) + } + } +}