diff --git a/pkg/api/helpers.go b/pkg/api/helpers.go index 1c64fb02a1c..9dd97719372 100644 --- a/pkg/api/helpers.go +++ b/pkg/api/helpers.go @@ -113,3 +113,20 @@ var standardFinalizers = util.NewStringSet( func IsStandardFinalizerName(str string) bool { return standardFinalizers.Has(str) } + +// AddToNodeAddresses appends the NodeAddresses to the passed-by-pointer slice, +// only if they do not already exist +func AddToNodeAddresses(addresses *[]NodeAddress, addAddresses ...NodeAddress) { + for _, add := range addAddresses { + exists := false + for _, existing := range *addresses { + if existing.Address == add.Address && existing.Type == add.Type { + exists = true + break + } + } + if !exists { + *addresses = append(*addresses, add) + } + } +} diff --git a/pkg/api/helpers_test.go b/pkg/api/helpers_test.go index b065b00ec91..d9b38dc5557 100644 --- a/pkg/api/helpers_test.go +++ b/pkg/api/helpers_test.go @@ -85,3 +85,60 @@ func TestIsStandardResource(t *testing.T) { } } } + +func TestAddToNodeAddresses(t *testing.T) { + testCases := []struct { + existing []NodeAddress + toAdd []NodeAddress + expected []NodeAddress + }{ + { + existing: []NodeAddress{}, + toAdd: []NodeAddress{}, + expected: []NodeAddress{}, + }, + { + existing: []NodeAddress{}, + toAdd: []NodeAddress{ + {Type: NodeExternalIP, Address: "1.1.1.1"}, + {Type: NodeHostName, Address: "localhost"}, + }, + expected: []NodeAddress{ + {Type: NodeExternalIP, Address: "1.1.1.1"}, + {Type: NodeHostName, Address: "localhost"}, + }, + }, + { + existing: []NodeAddress{}, + toAdd: []NodeAddress{ + {Type: NodeExternalIP, Address: "1.1.1.1"}, + {Type: NodeExternalIP, Address: "1.1.1.1"}, + }, + expected: []NodeAddress{ + {Type: NodeExternalIP, Address: "1.1.1.1"}, + }, + }, + { + existing: []NodeAddress{ + {Type: NodeExternalIP, Address: "1.1.1.1"}, + {Type: NodeInternalIP, Address: "10.1.1.1"}, + }, + toAdd: []NodeAddress{ + {Type: NodeExternalIP, Address: "1.1.1.1"}, + {Type: NodeHostName, Address: "localhost"}, + }, + expected: []NodeAddress{ + {Type: NodeExternalIP, Address: "1.1.1.1"}, + {Type: NodeInternalIP, Address: "10.1.1.1"}, + {Type: NodeHostName, Address: "localhost"}, + }, + }, + } + + for i, tc := range testCases { + AddToNodeAddresses(&tc.existing, tc.toAdd...) + if !Semantic.DeepEqual(tc.expected, tc.existing) { + t.Error("case[%d], expected: %v, got: %v", i, tc.expected, tc.existing) + } + } +} diff --git a/pkg/api/types.go b/pkg/api/types.go index 998640cbbc4..ccbd8d2957d 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1166,7 +1166,6 @@ type NodeAddress struct { // NodeResources is an object for conveying resource information about a node. // see http://docs.k8s.io/resources.md for more details. -// TODO: Use ResourceList instead? type NodeResources struct { // Capacity represents the available resources of a node Capacity ResourceList `json:"capacity,omitempty"` @@ -1816,22 +1815,6 @@ const ( StrategicMergePatchType PatchType = "application/strategic-merge-patch+json" ) -// Appends the NodeAddresses to the passed-by-pointer slice, only if they do not already exist -func AddToNodeAddresses(addresses *[]NodeAddress, addAddresses ...NodeAddress) { - for _, add := range addAddresses { - exists := false - for _, existing := range *addresses { - if existing.Address == add.Address && existing.Type == add.Type { - exists = true - break - } - } - if !exists { - *addresses = append(*addresses, add) - } - } -} - // Type and constants for component health validation. type ComponentConditionType string diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index d2535dc1676..d047164588c 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -1074,8 +1074,8 @@ func ValidateReadOnlyPersistentDisks(volumes []api.Volume) errs.ValidationErrorL return allErrs } -// ValidateMinion tests if required fields in the node are set. -func ValidateMinion(node *api.Node) errs.ValidationErrorList { +// ValidateNode tests if required fields in the node are set. +func ValidateNode(node *api.Node) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} allErrs = append(allErrs, ValidateObjectMeta(&node.ObjectMeta, false, ValidateNodeName).Prefix("metadata")...) @@ -1090,34 +1090,42 @@ func ValidateMinion(node *api.Node) errs.ValidationErrorList { return allErrs } -// ValidateMinionUpdate tests to make sure a minion update can be applied. Modifies oldMinion. -func ValidateMinionUpdate(oldMinion *api.Node, minion *api.Node) errs.ValidationErrorList { +// ValidateNodeUpdate tests to make sure a node update can be applied. Modifies oldNode. +func ValidateNodeUpdate(oldNode *api.Node, node *api.Node) errs.ValidationErrorList { allErrs := errs.ValidationErrorList{} - allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldMinion.ObjectMeta, &minion.ObjectMeta).Prefix("metadata")...) + allErrs = append(allErrs, ValidateObjectMetaUpdate(&oldNode.ObjectMeta, &node.ObjectMeta).Prefix("metadata")...) // TODO: Enable the code once we have better api object.status update model. Currently, // anyone can update node status. - // if !api.Semantic.DeepEqual(minion.Status, api.NodeStatus{}) { - // allErrs = append(allErrs, errs.NewFieldInvalid("status", minion.Status, "status must be empty")) + // if !api.Semantic.DeepEqual(node.Status, api.NodeStatus{}) { + // allErrs = append(allErrs, errs.NewFieldInvalid("status", node.Status, "status must be empty")) // } + // Validte no duplicate addresses in node status. + addresses := make(map[api.NodeAddress]bool) + for _, address := range node.Status.Addresses { + if _, ok := addresses[address]; ok { + allErrs = append(allErrs, fmt.Errorf("duplicate node addresses found")) + } + addresses[address] = true + } + // TODO: move reset function to its own location // Ignore metadata changes now that they have been tested - oldMinion.ObjectMeta = minion.ObjectMeta + oldNode.ObjectMeta = node.ObjectMeta // Allow users to update capacity - oldMinion.Status.Capacity = minion.Status.Capacity + oldNode.Status.Capacity = node.Status.Capacity // Allow users to unschedule node - oldMinion.Spec.Unschedulable = minion.Spec.Unschedulable + oldNode.Spec.Unschedulable = node.Spec.Unschedulable // Clear status - oldMinion.Status = minion.Status + oldNode.Status = node.Status // TODO: Add a 'real' ValidationError type for this error and provide print actual diffs. - if !api.Semantic.DeepEqual(oldMinion, minion) { - glog.V(4).Infof("Update failed validation %#v vs %#v", oldMinion, minion) + if !api.Semantic.DeepEqual(oldNode, node) { + glog.V(4).Infof("Update failed validation %#v vs %#v", oldNode, node) allErrs = append(allErrs, fmt.Errorf("update contains more than labels or capacity changes")) } - // TODO: validate Spec.Capacity return allErrs } diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index ca10ea7b83a..46e11fa048d 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -2055,7 +2055,7 @@ func TestValidateReplicationController(t *testing.T) { } } -func TestValidateMinion(t *testing.T) { +func TestValidateNode(t *testing.T) { validSelector := map[string]string{"a": "b"} invalidSelector := map[string]string{"NoUppercaseOrSpecialCharsLike=Equals": "b"} successCases := []api.Node{ @@ -2097,7 +2097,7 @@ func TestValidateMinion(t *testing.T) { }, } for _, successCase := range successCases { - if errs := ValidateMinion(&successCase); len(errs) != 0 { + if errs := ValidateNode(&successCase); len(errs) != 0 { t.Errorf("expected success: %v", errs) } } @@ -2148,7 +2148,7 @@ func TestValidateMinion(t *testing.T) { }, } for k, v := range errorCases { - errs := ValidateMinion(&v) + errs := ValidateNode(&v) if len(errs) == 0 { t.Errorf("expected failure for %s", k) } @@ -2168,11 +2168,11 @@ func TestValidateMinion(t *testing.T) { } } -func TestValidateMinionUpdate(t *testing.T) { +func TestValidateNodeUpdate(t *testing.T) { tests := []struct { - oldMinion api.Node - minion api.Node - valid bool + oldNode api.Node + node api.Node + valid bool }{ {api.Node{}, api.Node{}, true}, {api.Node{ @@ -2300,14 +2300,50 @@ func TestValidateMinionUpdate(t *testing.T) { Unschedulable: true, }, }, true}, + {api.Node{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + }, + Spec: api.NodeSpec{ + Unschedulable: false, + }, + }, api.Node{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + }, + Status: api.NodeStatus{ + Addresses: []api.NodeAddress{ + {Type: api.NodeExternalIP, Address: "1.1.1.1"}, + {Type: api.NodeExternalIP, Address: "1.1.1.1"}, + }, + }, + }, false}, + {api.Node{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + }, + Spec: api.NodeSpec{ + Unschedulable: false, + }, + }, api.Node{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + }, + Status: api.NodeStatus{ + Addresses: []api.NodeAddress{ + {Type: api.NodeExternalIP, Address: "1.1.1.1"}, + {Type: api.NodeInternalIP, Address: "10.1.1.1"}, + }, + }, + }, true}, } for i, test := range tests { - test.oldMinion.ObjectMeta.ResourceVersion = "1" - test.minion.ObjectMeta.ResourceVersion = "1" - errs := ValidateMinionUpdate(&test.oldMinion, &test.minion) + test.oldNode.ObjectMeta.ResourceVersion = "1" + test.node.ObjectMeta.ResourceVersion = "1" + errs := ValidateNodeUpdate(&test.oldNode, &test.node) if test.valid && len(errs) > 0 { t.Errorf("%d: Unexpected error: %v", i, errs) - t.Logf("%#v vs %#v", test.oldMinion.ObjectMeta, test.minion.ObjectMeta) + t.Logf("%#v vs %#v", test.oldNode.ObjectMeta, test.node.ObjectMeta) } if !test.valid && len(errs) == 0 { t.Errorf("%d: Unexpected non-error", i) diff --git a/pkg/registry/minion/rest.go b/pkg/registry/minion/rest.go index be64f5d7074..8f3ddd9a4eb 100644 --- a/pkg/registry/minion/rest.go +++ b/pkg/registry/minion/rest.go @@ -71,13 +71,13 @@ func (nodeStrategy) PrepareForUpdate(obj, old runtime.Object) { // Validate validates a new node. func (nodeStrategy) Validate(ctx api.Context, obj runtime.Object) fielderrors.ValidationErrorList { node := obj.(*api.Node) - return validation.ValidateMinion(node) + return validation.ValidateNode(node) } // ValidateUpdate is the default update validation for an end user. func (nodeStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Object) fielderrors.ValidationErrorList { - errorList := validation.ValidateMinion(obj.(*api.Node)) - return append(errorList, validation.ValidateMinionUpdate(old.(*api.Node), obj.(*api.Node))...) + errorList := validation.ValidateNode(obj.(*api.Node)) + return append(errorList, validation.ValidateNodeUpdate(old.(*api.Node), obj.(*api.Node))...) } type nodeStatusStrategy struct { @@ -98,7 +98,7 @@ func (nodeStatusStrategy) PrepareForUpdate(obj, old runtime.Object) { } func (nodeStatusStrategy) ValidateUpdate(ctx api.Context, obj, old runtime.Object) fielderrors.ValidationErrorList { - return validation.ValidateMinionUpdate(old.(*api.Node), obj.(*api.Node)) + return validation.ValidateNodeUpdate(old.(*api.Node), obj.(*api.Node)) } // ResourceGetter is an interface for retrieving resources by ResourceLocation.