From e0ff1392794e3b02c88242ae1abd12f04cf07c38 Mon Sep 17 00:00:00 2001 From: Rohit Jnagal Date: Wed, 1 Apr 2015 23:11:33 +0000 Subject: [PATCH] Remove validation for Capacity as it got moved from Spec to Status. Also fix breakage from ExternalID validation: Default ExternalID to node name when not specified. --- pkg/api/testing/fuzzer.go | 4 ++ pkg/api/v1beta1/conversion_test.go | 9 ++- pkg/api/v1beta1/defaults.go | 5 ++ pkg/api/v1beta1/defaults_test.go | 11 ++++ pkg/api/v1beta1/types.go | 2 +- pkg/api/v1beta2/defaults.go | 5 ++ pkg/api/v1beta2/defaults_test.go | 11 ++++ pkg/api/v1beta2/types.go | 2 +- pkg/api/v1beta3/defaults.go | 5 ++ pkg/api/v1beta3/defaults_test.go | 11 ++++ pkg/api/v1beta3/types.go | 2 +- pkg/api/validation/validation.go | 17 +----- pkg/api/validation/validation_test.go | 80 ++------------------------- pkg/kubectl/cmd/get_test.go | 3 + 14 files changed, 72 insertions(+), 95 deletions(-) diff --git a/pkg/api/testing/fuzzer.go b/pkg/api/testing/fuzzer.go index 7c7870d22f3..08b88a82209 100644 --- a/pkg/api/testing/fuzzer.go +++ b/pkg/api/testing/fuzzer.go @@ -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 } diff --git a/pkg/api/v1beta1/conversion_test.go b/pkg/api/v1beta1/conversion_test.go index f8953895207..fec3cc1e4b1 100644 --- a/pkg/api/v1beta1/conversion_test.go +++ b/pkg/api/v1beta1/conversion_test.go @@ -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"), diff --git a/pkg/api/v1beta1/defaults.go b/pkg/api/v1beta1/defaults.go index 6011272a045..f1c79530234 100644 --- a/pkg/api/v1beta1/defaults.go +++ b/pkg/api/v1beta1/defaults.go @@ -153,6 +153,11 @@ func init() { obj.Phase = NamespaceActive } }, + func(obj *Minion) { + if obj.ExternalID == "" { + obj.ExternalID = obj.ID + } + }, ) } diff --git a/pkg/api/v1beta1/defaults_test.go b/pkg/api/v1beta1/defaults_test.go index 19c27c5067b..ad79d0afd2a 100644 --- a/pkg/api/v1beta1/defaults_test.go +++ b/pkg/api/v1beta1/defaults_test.go @@ -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 := ¤t.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) + } +} diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index 75e4f3ec6d9..d97254b2838 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -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. diff --git a/pkg/api/v1beta2/defaults.go b/pkg/api/v1beta2/defaults.go index 025a2b128d2..7b3846c72d9 100644 --- a/pkg/api/v1beta2/defaults.go +++ b/pkg/api/v1beta2/defaults.go @@ -154,6 +154,11 @@ func init() { obj.Phase = NamespaceActive } }, + func(obj *Minion) { + if obj.ExternalID == "" { + obj.ExternalID = obj.ID + } + }, ) } diff --git a/pkg/api/v1beta2/defaults_test.go b/pkg/api/v1beta2/defaults_test.go index 40652ed57e0..687aafcd9d2 100644 --- a/pkg/api/v1beta2/defaults_test.go +++ b/pkg/api/v1beta2/defaults_test.go @@ -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 := ¤t.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) + } +} diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index 2e0677e815a..39660b68358 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -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. diff --git a/pkg/api/v1beta3/defaults.go b/pkg/api/v1beta3/defaults.go index 27883abe07e..166e53b738c 100644 --- a/pkg/api/v1beta3/defaults.go +++ b/pkg/api/v1beta3/defaults.go @@ -108,6 +108,11 @@ func init() { obj.Phase = NamespaceActive } }, + func(obj *Node) { + if obj.Spec.ExternalID == "" { + obj.Spec.ExternalID = obj.Name + } + }, ) } diff --git a/pkg/api/v1beta3/defaults_test.go b/pkg/api/v1beta3/defaults_test.go index d0549ee57a4..ae99f3fe220 100644 --- a/pkg/api/v1beta3/defaults_test.go +++ b/pkg/api/v1beta3/defaults_test.go @@ -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 := ¤t.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) + } +} diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index 78841dcebdb..19a4bf7c943 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -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"` } diff --git a/pkg/api/validation/validation.go b/pkg/api/validation/validation.go index 17a51271fb8..04b15017a95 100644 --- a/pkg/api/validation/validation.go +++ b/pkg/api/validation/validation.go @@ -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 { diff --git a/pkg/api/validation/validation_test.go b/pkg/api/validation/validation_test.go index db6ea861abb..82672d34dc2 100644 --- a/pkg/api/validation/validation_test.go +++ b/pkg/api/validation/validation_test.go @@ -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) @@ -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]) diff --git a/pkg/kubectl/cmd/get_test.go b/pkg/kubectl/cmd/get_test.go index 10112153af9..51257aa2b1e 100644 --- a/pkg/kubectl/cmd/get_test.go +++ b/pkg/kubectl/cmd/get_test.go @@ -319,6 +319,9 @@ func TestGetMultipleTypeObjectsWithDirectReference(t *testing.T) { ObjectMeta: api.ObjectMeta{ Name: "foo", }, + Spec: api.NodeSpec{ + ExternalID: "ext", + }, } f, tf, codec := NewAPIFactory()