From ef56dca6b6d3848de80888895e90d76dc13e33c2 Mon Sep 17 00:00:00 2001 From: gmarek Date: Fri, 3 Apr 2015 15:43:51 +0200 Subject: [PATCH] Remove ConditionSchedulable --- docs/node.md | 17 ++----- pkg/api/types.go | 4 +- pkg/api/v1beta1/types.go | 1 + pkg/api/v1beta2/types.go | 1 + pkg/api/v1beta3/types.go | 2 - .../controller/nodecontroller.go | 25 ---------- .../controller/nodecontroller_test.go | 50 +------------------ pkg/kubectl/resource_printer.go | 10 +++- pkg/kubectl/resource_printer_test.go | 18 ------- plugin/pkg/scheduler/factory/factory.go | 6 +-- plugin/pkg/scheduler/factory/factory_test.go | 30 +++++------ 11 files changed, 34 insertions(+), 130 deletions(-) diff --git a/docs/node.md b/docs/node.md index f1d4a9bd721..39e2febc0e2 100644 --- a/docs/node.md +++ b/docs/node.md @@ -38,24 +38,17 @@ must have appropriate conditions, see below. ### Node Condition Node Condition describes the conditions of `Running` nodes. Current valid -conditions are `NodeReady` and `NodeSchedulable`. In the future, we plan to -add more. `NodeReady` means kubelet is healthy and ready to accept pods -`NodeSchedulable` means node is allowed to schedule any new pods and is -controlled by 'unschedulable' field in node spec. Different condition provides -different level of understanding for node health. Kubernetes will make a -comprehensive scheduling decision based on the information. Node condition -is represented as a json object. For example, the following conditions mean -the node is in sane state but not allowed to accept new pods: +condition is `NodeReady`. In the future, we plan to add more. +`NodeReady` means kubelet is healthy and ready to accept pods. Different +condition provides different level of understanding for node health. +Node condition is represented as a json object. For example, +the following conditions mean the node is in sane state: ```json "conditions": [ { "kind": "Ready", "status": "True", }, - { - "kind": "Schedulable", - "status": "False", - }, ] ``` diff --git a/pkg/api/types.go b/pkg/api/types.go index c2b603ec247..782dc202349 100644 --- a/pkg/api/types.go +++ b/pkg/api/types.go @@ -1087,12 +1087,10 @@ type NodeConditionType string // These are valid conditions of node. Currently, we don't have enough information to decide // node condition. In the future, we will add more. The proposed set of conditions are: -// NodeReachable, NodeLive, NodeReady, NodeSchedulable, NodeRunnable. +// NodeReady, NodeReachable const ( // NodeReady means kubelet is healthy and ready to accept pods. NodeReady NodeConditionType = "Ready" - // NodeSchedulable means the node is ready to accept new pods. - NodeSchedulable NodeConditionType = "Schedulable" ) type NodeCondition struct { diff --git a/pkg/api/v1beta1/types.go b/pkg/api/v1beta1/types.go index e6d4fd33174..7667bdf56b7 100644 --- a/pkg/api/v1beta1/types.go +++ b/pkg/api/v1beta1/types.go @@ -945,6 +945,7 @@ const ( // NodeReady means kubelet is healthy and ready to accept pods. NodeReady NodeConditionKind = "Ready" // NodeSchedulable means the node is ready to accept new pods. + // DEPRECATED: this kind of condition is unused and has no effect even if present. NodeSchedulable NodeConditionKind = "Schedulable" ) diff --git a/pkg/api/v1beta2/types.go b/pkg/api/v1beta2/types.go index 1cb0522e750..217d0e64881 100644 --- a/pkg/api/v1beta2/types.go +++ b/pkg/api/v1beta2/types.go @@ -954,6 +954,7 @@ const ( // NodeReady means kubelet is healthy and ready to accept pods. NodeReady NodeConditionKind = "Ready" // NodeSchedulable means the node is ready to accept new pods. + // DEPRECATED: this kind of condition is unused and has no effect even if present. NodeSchedulable NodeConditionKind = "Schedulable" ) diff --git a/pkg/api/v1beta3/types.go b/pkg/api/v1beta3/types.go index 23070a36477..4590a4e513b 100644 --- a/pkg/api/v1beta3/types.go +++ b/pkg/api/v1beta3/types.go @@ -1088,8 +1088,6 @@ type NodeConditionType string const ( // NodeReady means kubelet is healthy and ready to accept pods. NodeReady NodeConditionType = "Ready" - // NodeSchedulable means the node is ready to accept new pods. - NodeSchedulable NodeConditionType = "Schedulable" ) type NodeCondition struct { diff --git a/pkg/cloudprovider/controller/nodecontroller.go b/pkg/cloudprovider/controller/nodecontroller.go index 7901123ecf2..45aa1ac1911 100644 --- a/pkg/cloudprovider/controller/nodecontroller.go +++ b/pkg/cloudprovider/controller/nodecontroller.go @@ -339,12 +339,6 @@ func (nc *NodeController) DoCheck(node *api.Node) []api.NodeCondition { } conditions = append(conditions, *newReadyCondition) - // Check Condition: NodeSchedulable - oldSchedulableCondition := nc.getCondition(&node.Status, api.NodeSchedulable) - newSchedulableCondition := nc.checkNodeSchedulable(node) - nc.updateLastTransitionTime(oldSchedulableCondition, newSchedulableCondition) - conditions = append(conditions, *newSchedulableCondition) - return conditions } @@ -360,25 +354,6 @@ func (nc *NodeController) updateLastTransitionTime(oldCondition, newCondition *a } } -// checkNodeSchedulable checks node schedulable condition, without transition timestamp set. -func (nc *NodeController) checkNodeSchedulable(node *api.Node) *api.NodeCondition { - if node.Spec.Unschedulable { - return &api.NodeCondition{ - Type: api.NodeSchedulable, - Status: api.ConditionFalse, - Reason: "User marked unschedulable during node create/update", - LastProbeTime: nc.now(), - } - } else { - return &api.NodeCondition{ - Type: api.NodeSchedulable, - Status: api.ConditionTrue, - Reason: "Node is schedulable by default", - LastProbeTime: nc.now(), - } - } -} - // checkNodeReady checks raw node ready condition, without transition timestamp set. func (nc *NodeController) checkNodeReady(node *api.Node) *api.NodeCondition { switch status, err := nc.kubeletClient.HealthCheck(node.Name); { diff --git a/pkg/cloudprovider/controller/nodecontroller_test.go b/pkg/cloudprovider/controller/nodecontroller_test.go index 86dfb95cbf6..0c0a48bac5c 100644 --- a/pkg/cloudprovider/controller/nodecontroller_test.go +++ b/pkg/cloudprovider/controller/nodecontroller_test.go @@ -625,13 +625,6 @@ func TestNodeConditionsCheck(t *testing.T) { LastProbeTime: fakeNow, LastTransitionTime: fakeNow, }, - { - Type: api.NodeSchedulable, - Status: api.ConditionTrue, - Reason: "Node is schedulable by default", - LastProbeTime: fakeNow, - LastTransitionTime: fakeNow, - }, }, }, { @@ -650,18 +643,10 @@ func TestNodeConditionsCheck(t *testing.T) { LastProbeTime: fakeNow, LastTransitionTime: fakeNow, }, - { - Type: api.NodeSchedulable, - Status: api.ConditionTrue, - Reason: "Node is schedulable by default", - LastProbeTime: fakeNow, - LastTransitionTime: fakeNow, - }, }, }, { - // User specified node as unschedulable and kubelet /healthz probe returns failure with some error. - // Expected node condition to be not ready and marked unschedulable. + // Expected node condition to be not ready as marking Node Unschedulable does not impact Readiness. node: &api.Node{ObjectMeta: api.ObjectMeta{Name: "node0"}, Spec: api.NodeSpec{Unschedulable: true}}, fakeKubeletClient: &FakeKubeletClient{ Status: probe.Failure, @@ -675,13 +660,6 @@ func TestNodeConditionsCheck(t *testing.T) { LastProbeTime: fakeNow, LastTransitionTime: fakeNow, }, - { - Type: api.NodeSchedulable, - Status: api.ConditionFalse, - Reason: "User marked unschedulable during node create/update", - LastProbeTime: fakeNow, - LastTransitionTime: fakeNow, - }, }, }, } @@ -764,13 +742,6 @@ func TestSyncProbedNodeStatus(t *testing.T) { LastProbeTime: fakeNow, LastTransitionTime: fakeNow, }, - { - Type: api.NodeSchedulable, - Status: api.ConditionTrue, - Reason: "Node is schedulable by default", - LastProbeTime: fakeNow, - LastTransitionTime: fakeNow, - }, }, Addresses: []api.NodeAddress{ {Type: api.NodeLegacyHostIP, Address: "1.2.3.4"}, @@ -795,13 +766,6 @@ func TestSyncProbedNodeStatus(t *testing.T) { LastProbeTime: fakeNow, LastTransitionTime: fakeNow, }, - { - Type: api.NodeSchedulable, - Status: api.ConditionTrue, - Reason: "Node is schedulable by default", - LastProbeTime: fakeNow, - LastTransitionTime: fakeNow, - }, }, Addresses: []api.NodeAddress{ {Type: api.NodeLegacyHostIP, Address: "1.2.3.4"}, @@ -868,12 +832,6 @@ func TestSyncProbedNodeStatusTransitionTime(t *testing.T) { Reason: "Node health check succeeded: kubelet /healthz endpoint returns ok", LastTransitionTime: util.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), }, - { - Type: api.NodeSchedulable, - Status: api.ConditionTrue, - Reason: "Node is schedulable by default", - LastTransitionTime: util.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), - }, }, }, }, @@ -903,12 +861,6 @@ func TestSyncProbedNodeStatusTransitionTime(t *testing.T) { Reason: "Node health check succeeded: kubelet /healthz endpoint returns ok", LastTransitionTime: util.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), }, - { - Type: api.NodeSchedulable, - Status: api.ConditionTrue, - Reason: "Node is schedulable by default", - LastTransitionTime: util.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC), - }, }, }, }, diff --git a/pkg/kubectl/resource_printer.go b/pkg/kubectl/resource_printer.go index 757ed5dafea..4e9f6f6ec07 100644 --- a/pkg/kubectl/resource_printer.go +++ b/pkg/kubectl/resource_printer.go @@ -482,11 +482,17 @@ func printSecretList(list *api.SecretList, w io.Writer) error { func printNode(node *api.Node, w io.Writer) error { conditionMap := make(map[api.NodeConditionType]*api.NodeCondition) - NodeAllConditions := []api.NodeConditionType{api.NodeSchedulable, api.NodeReady} + NodeAllConditions := []api.NodeConditionType{api.NodeReady} for i := range node.Status.Conditions { cond := node.Status.Conditions[i] conditionMap[cond.Type] = &cond } + var schedulable string + if node.Spec.Unschedulable { + schedulable = "Unschedulable" + } else { + schedulable = "Schedulable" + } var status []string for _, validCondition := range NodeAllConditions { if condition, ok := conditionMap[validCondition]; ok { @@ -500,7 +506,7 @@ func printNode(node *api.Node, w io.Writer) error { if len(status) == 0 { status = append(status, "Unknown") } - _, err := fmt.Fprintf(w, "%s\t%s\t%s\n", node.Name, formatLabels(node.Labels), strings.Join(status, ",")) + _, err := fmt.Fprintf(w, "%s\t%s\t%s\t%s\n", node.Name, schedulable, formatLabels(node.Labels), strings.Join(status, ",")) return err } diff --git a/pkg/kubectl/resource_printer_test.go b/pkg/kubectl/resource_printer_test.go index c2de497a905..291a63036d7 100644 --- a/pkg/kubectl/resource_printer_test.go +++ b/pkg/kubectl/resource_printer_test.go @@ -579,24 +579,6 @@ func TestPrintMinionStatus(t *testing.T) { }, status: "Unknown", }, - { - minion: api.Node{ - ObjectMeta: api.ObjectMeta{Name: "foo7"}, - Status: api.NodeStatus{Conditions: []api.NodeCondition{ - {Type: api.NodeSchedulable, Status: api.ConditionTrue}, - {Type: api.NodeReady, Status: api.ConditionTrue}}}, - }, - status: "Schedulable,Ready", - }, - { - minion: api.Node{ - ObjectMeta: api.ObjectMeta{Name: "foo8"}, - Status: api.NodeStatus{Conditions: []api.NodeCondition{ - {Type: api.NodeSchedulable, Status: api.ConditionFalse}, - {Type: api.NodeReady, Status: api.ConditionFalse}}}, - }, - status: "NotSchedulable,NotReady", - }, } for _, test := range table { diff --git a/plugin/pkg/scheduler/factory/factory.go b/plugin/pkg/scheduler/factory/factory.go index e8ab3340bf5..96981a01411 100644 --- a/plugin/pkg/scheduler/factory/factory.go +++ b/plugin/pkg/scheduler/factory/factory.go @@ -249,10 +249,8 @@ func (factory *ConfigFactory) pollMinions() (cache.Enumerator, error) { cond := node.Status.Conditions[i] conditionMap[cond.Type] = &cond } - if condition, ok := conditionMap[api.NodeSchedulable]; ok { - if condition.Status != api.ConditionTrue { - continue - } + if node.Spec.Unschedulable { + continue } if condition, ok := conditionMap[api.NodeReady]; ok { if condition.Status == api.ConditionTrue { diff --git a/plugin/pkg/scheduler/factory/factory_test.go b/plugin/pkg/scheduler/factory/factory_test.go index 89ebadce07a..c9b1ba4ba69 100644 --- a/plugin/pkg/scheduler/factory/factory_test.go +++ b/plugin/pkg/scheduler/factory/factory_test.go @@ -148,10 +148,8 @@ func TestPollMinions(t *testing.T) { }, { ObjectMeta: api.ObjectMeta{Name: "fiz"}, - Status: api.NodeStatus{ - Conditions: []api.NodeCondition{ - {Type: api.NodeSchedulable, Status: api.ConditionTrue}, - }, + Spec: api.NodeSpec{ + Unschedulable: false, }, }, { @@ -167,28 +165,34 @@ func TestPollMinions(t *testing.T) { ObjectMeta: api.ObjectMeta{Name: "fuz"}, Status: api.NodeStatus{ Conditions: []api.NodeCondition{ - {Type: api.NodeSchedulable, Status: api.ConditionTrue}, {Type: api.NodeReady, Status: api.ConditionTrue}, }, }, + Spec: api.NodeSpec{ + Unschedulable: false, + }, }, { ObjectMeta: api.ObjectMeta{Name: "buz"}, Status: api.NodeStatus{ Conditions: []api.NodeCondition{ - {Type: api.NodeSchedulable, Status: api.ConditionFalse}, {Type: api.NodeReady, Status: api.ConditionTrue}, }, }, + Spec: api.NodeSpec{ + Unschedulable: true, + }, }, { ObjectMeta: api.ObjectMeta{Name: "foobar"}, Status: api.NodeStatus{ Conditions: []api.NodeCondition{ - {Type: api.NodeSchedulable, Status: api.ConditionTrue}, {Type: api.NodeReady, Status: api.ConditionFalse}, }, }, + Spec: api.NodeSpec{ + Unschedulable: false, + }, }, }, expectedCount: 3, @@ -218,18 +222,14 @@ func TestPollMinions(t *testing.T) { minions: []api.Node{ { ObjectMeta: api.ObjectMeta{Name: "foo"}, - Status: api.NodeStatus{ - Conditions: []api.NodeCondition{ - {Type: api.NodeSchedulable, Status: api.ConditionTrue}, - }, + Spec: api.NodeSpec{ + Unschedulable: false, }, }, { ObjectMeta: api.ObjectMeta{Name: "bar"}, - Status: api.NodeStatus{ - Conditions: []api.NodeCondition{ - {Type: api.NodeSchedulable, Status: api.ConditionFalse}, - }, + Spec: api.NodeSpec{ + Unschedulable: true, }, }, },