Validate Node IPs; clean up validation code

This commit is contained in:
Deyuan Deng 2015-04-22 13:55:05 -04:00 committed by Deyuan Deng
parent 7505bed054
commit c73ce1db34
6 changed files with 147 additions and 46 deletions

View File

@ -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)
}
}
}

View File

@ -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)
}
}
}

View File

@ -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

View File

@ -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
}

View File

@ -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)

View File

@ -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.