Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove validation for Capacity. Add default for ExternalID #6325

Merged
merged 1 commit into from
Apr 2, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkg/api/testing/fuzzer.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,10 @@ func FuzzerFor(t *testing.T, version string, src rand.Source) *fuzz.Fuzzer {
}
}
},
func(n *api.Node, c fuzz.Continue) {
c.FuzzNoCustom(n)
n.Spec.ExternalID = "external"
},
)
return f
}
9 changes: 7 additions & 2 deletions pkg/api/v1beta1/conversion_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,15 @@ func TestVolumeMountConversionToNew(t *testing.T) {

func TestMinionListConversionToNew(t *testing.T) {
oldMinion := func(id string) current.Minion {
return current.Minion{TypeMeta: current.TypeMeta{ID: id}}
return current.Minion{
TypeMeta: current.TypeMeta{ID: id},
ExternalID: id}
}
newNode := func(id string) newer.Node {
return newer.Node{ObjectMeta: newer.ObjectMeta{Name: id}}
return newer.Node{
ObjectMeta: newer.ObjectMeta{Name: id},
Spec: newer.NodeSpec{ExternalID: id},
}
}
oldMinions := []current.Minion{
oldMinion("foo"),
Expand Down
5 changes: 5 additions & 0 deletions pkg/api/v1beta1/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,11 @@ func init() {
obj.Phase = NamespaceActive
}
},
func(obj *Minion) {
if obj.ExternalID == "" {
obj.ExternalID = obj.ID
}
},
)
}

Expand Down
11 changes: 11 additions & 0 deletions pkg/api/v1beta1/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,3 +187,14 @@ func TestSetDefaultServicePort(t *testing.T) {
t.Errorf("Expected port %d, got %v", in.Ports[0].Port, out.Ports[0].ContainerPort)
}
}

func TestSetDefaultMinionExternalID(t *testing.T) {
name := "node0"
m := &current.Minion{}
m.ID = name
obj2 := roundTrip(t, runtime.Object(m))
m2 := obj2.(*current.Minion)
if m2.ExternalID != name {
t.Errorf("Expected default External ID: %s, got: %s", name, m2.ExternalID)
}
}
2 changes: 1 addition & 1 deletion pkg/api/v1beta1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -986,7 +986,7 @@ type Minion struct {
// Labels for the node
Labels map[string]string `json:"labels,omitempty" description:"map of string keys and values that can be used to organize and categorize minions; labels of a minion assigned by the scheduler must match the scheduled pod's nodeSelector"`
// External ID of the node
ExternalID string `json:"externalID,omitempty" description:"external id of the node assigned by some machine database (e.g. a cloud provider)"`
ExternalID string `json:"externalID,omitempty" description:"external id of the node assigned by some machine database (e.g. a cloud provider). Defaults to node name when empty."`
}

// MinionList is a list of minions.
Expand Down
5 changes: 5 additions & 0 deletions pkg/api/v1beta2/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,11 @@ func init() {
obj.Phase = NamespaceActive
}
},
func(obj *Minion) {
if obj.ExternalID == "" {
obj.ExternalID = obj.ID
}
},
)
}

Expand Down
11 changes: 11 additions & 0 deletions pkg/api/v1beta2/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,3 +186,14 @@ func TestSetDefaultServicePort(t *testing.T) {
t.Errorf("Expected port %d, got %v", in.Ports[0].Port, out.Ports[0].ContainerPort)
}
}

func TestSetDefaultMinionExternalID(t *testing.T) {
name := "node0"
m := &current.Minion{}
m.ID = name
obj2 := roundTrip(t, runtime.Object(m))
m2 := obj2.(*current.Minion)
if m2.ExternalID != name {
t.Errorf("Expected default External ID: %s, got: %s", name, m2.ExternalID)
}
}
2 changes: 1 addition & 1 deletion pkg/api/v1beta2/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -1001,7 +1001,7 @@ type Minion struct {
// Labels for the node
Labels map[string]string `json:"labels,omitempty" description:"map of string keys and values that can be used to organize and categorize minions; labels of a minion assigned by the scheduler must match the scheduled pod's nodeSelector"`
// External ID of the node
ExternalID string `json:"externalID,omitempty" description:"external id of the node assigned by some machine database (e.g. a cloud provider)"`
ExternalID string `json:"externalID,omitempty" description:"external id of the node assigned by some machine database (e.g. a cloud provider). Defaults to node name when empty."`
}

// MinionList is a list of minions.
Expand Down
5 changes: 5 additions & 0 deletions pkg/api/v1beta3/defaults.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ func init() {
obj.Phase = NamespaceActive
}
},
func(obj *Node) {
if obj.Spec.ExternalID == "" {
obj.Spec.ExternalID = obj.Name
}
},
)
}

Expand Down
11 changes: 11 additions & 0 deletions pkg/api/v1beta3/defaults_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,3 +182,14 @@ func TestSetDefaultPodSpecHostNetwork(t *testing.T) {
t.Errorf("Expected container port to be defaulted, was made %d instead of %d", hostPortNum, portNum)
}
}

func TestSetDefaultNodeExternalID(t *testing.T) {
name := "node0"
n := &current.Node{}
n.Name = name
obj2 := roundTrip(t, runtime.Object(n))
n2 := obj2.(*current.Node)
if n2.Spec.ExternalID != name {
t.Errorf("Expected default External ID: %s, got: %s", name, n2.Spec.ExternalID)
}
}
2 changes: 1 addition & 1 deletion pkg/api/v1beta3/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -999,7 +999,7 @@ type NodeSpec struct {
// PodCIDR represents the pod IP range assigned to the node
PodCIDR string `json:"podCIDR,omitempty" description:"pod IP range assigned to the node"`
// External ID of the node assigned by some machine database (e.g. a cloud provider)
ExternalID string `json:"externalID,omitempty" description:"external ID assigned to the node by some machine database (e.g. a cloud provider)"`
ExternalID string `json:"externalID,omitempty" description:"external ID assigned to the node by some machine database (e.g. a cloud provider). Defaults to node name when empty."`
// Unschedulable controls node schedulability of new pods. By default node is schedulable.
Unschedulable bool `json:"unschedulable,omitempty" description:"disable pod scheduling on the node"`
}
Expand Down
17 changes: 2 additions & 15 deletions pkg/api/validation/validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -942,21 +942,8 @@ func ValidateReadOnlyPersistentDisks(volumes []api.Volume) errs.ValidationErrorL
func ValidateMinion(node *api.Node) errs.ValidationErrorList {
allErrs := errs.ValidationErrorList{}
allErrs = append(allErrs, ValidateObjectMeta(&node.ObjectMeta, false, ValidateNodeName).Prefix("metadata")...)
// Capacity is required. Within capacity, memory and cpu resources are required.
if len(node.Status.Capacity) == 0 {
allErrs = append(allErrs, errs.NewFieldRequired("status.Capacity"))
} else {
if val, ok := node.Status.Capacity[api.ResourceMemory]; !ok {
allErrs = append(allErrs, errs.NewFieldRequired("status.Capacity[memory]"))
} else if val.Value() < 0 {
allErrs = append(allErrs, errs.NewFieldInvalid("status.Capacity[memory]", val, "memory capacity cannot be negative"))
}
if val, ok := node.Status.Capacity[api.ResourceCPU]; !ok {
allErrs = append(allErrs, errs.NewFieldRequired("status.Capacity[cpu]"))
} else if val.Value() < 0 {
allErrs = append(allErrs, errs.NewFieldInvalid("status.Capacity[cpu]", val, "cpu capacity cannot be negative"))
}
}

// Only validate spec. All status fields are optional and can be updated later.

// external ID is required.
if len(node.Spec.ExternalID) == 0 {
Expand Down
80 changes: 5 additions & 75 deletions pkg/api/validation/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1938,73 +1938,6 @@ func TestValidateMinion(t *testing.T) {
},
},
},
"missing-capacity": {
ObjectMeta: api.ObjectMeta{
Name: "abc-123",
Labels: validSelector,
},
Spec: api.NodeSpec{
ExternalID: "external",
},
},
"missing-memory": {
ObjectMeta: api.ObjectMeta{
Name: "abc-123",
Labels: validSelector,
},
Status: api.NodeStatus{
Capacity: api.ResourceList{
api.ResourceName(api.ResourceCPU): resource.MustParse("10"),
},
},
Spec: api.NodeSpec{
ExternalID: "external",
},
},
"missing-cpu": {
ObjectMeta: api.ObjectMeta{
Name: "abc-123",
Labels: validSelector,
},
Status: api.NodeStatus{
Capacity: api.ResourceList{
api.ResourceName(api.ResourceMemory): resource.MustParse("10G"),
},
},
Spec: api.NodeSpec{
ExternalID: "external",
},
},
"invalid-memory": {
ObjectMeta: api.ObjectMeta{
Name: "abc-123",
Labels: validSelector,
},
Status: api.NodeStatus{
Capacity: api.ResourceList{
api.ResourceName(api.ResourceCPU): resource.MustParse("10"),
api.ResourceName(api.ResourceMemory): resource.MustParse("-10G"),
},
},
Spec: api.NodeSpec{
ExternalID: "external",
},
},
"invalid-cpu": {
ObjectMeta: api.ObjectMeta{
Name: "abc-123",
Labels: validSelector,
},
Status: api.NodeStatus{
Capacity: api.ResourceList{
api.ResourceName(api.ResourceCPU): resource.MustParse("-10"),
api.ResourceName(api.ResourceMemory): resource.MustParse("10G"),
},
},
Spec: api.NodeSpec{
ExternalID: "external",
},
},
}
for k, v := range errorCases {
errs := ValidateMinion(&v)
Expand All @@ -2014,14 +1947,11 @@ func TestValidateMinion(t *testing.T) {
for i := range errs {
field := errs[i].(*errors.ValidationError).Field
expectedFields := map[string]bool{
"metadata.name": true,
"metadata.labels": true,
"metadata.annotations": true,
"metadata.namespace": true,
"status.Capacity": true,
"status.Capacity[memory]": true,
"status.Capacity[cpu]": true,
"spec.ExternalID": true,
"metadata.name": true,
"metadata.labels": true,
"metadata.annotations": true,
"metadata.namespace": true,
"spec.ExternalID": true,
}
if expectedFields[field] == false {
t.Errorf("%s: missing prefix for: %v", k, errs[i])
Expand Down
3 changes: 3 additions & 0 deletions pkg/kubectl/cmd/get_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -319,6 +319,9 @@ func TestGetMultipleTypeObjectsWithDirectReference(t *testing.T) {
ObjectMeta: api.ObjectMeta{
Name: "foo",
},
Spec: api.NodeSpec{
ExternalID: "ext",
},
}

f, tf, codec := NewAPIFactory()
Expand Down