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 OutOfDisk from controllers #51294

Merged
merged 1 commit into from Sep 5, 2017
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
2 changes: 0 additions & 2 deletions pkg/controller/daemon/BUILD
Expand Up @@ -74,8 +74,6 @@ go_test(
"//pkg/securitycontext:go_default_library",
"//pkg/util/labels:go_default_library",
"//plugin/pkg/scheduler/algorithm:go_default_library",
"//plugin/pkg/scheduler/algorithm/predicates:go_default_library",
"//plugin/pkg/scheduler/schedulercache:go_default_library",
"//vendor/k8s.io/api/core/v1:go_default_library",
"//vendor/k8s.io/api/extensions/v1beta1:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
Expand Down
31 changes: 2 additions & 29 deletions pkg/controller/daemon/daemon_controller.go
Expand Up @@ -1180,6 +1180,7 @@ func (dsc *DaemonSetsController) simulate(newPod *v1.Pod, node *v1.Node, ds *ext
Effect: v1.TaintEffectNoSchedule,
})

// TODO(#48843) OutOfDisk taints will be removed in 1.10
if utilfeature.DefaultFeatureGate.Enabled(features.ExperimentalCriticalPodAnnotation) &&
kubelettypes.IsCriticalPod(newPod) {
v1helper.AddOrUpdateTolerationInPod(newPod, &v1.Toleration{
Expand Down Expand Up @@ -1221,7 +1222,7 @@ func (dsc *DaemonSetsController) simulate(newPod *v1.Pod, node *v1.Node, ds *ext
// summary. Returned booleans are:
// * wantToRun:
// Returns true when a user would expect a pod to run on this node and ignores conditions
// such as OutOfDisk or insufficient resource that would cause a daemonset pod not to schedule.
// such as DiskPressure or insufficient resource that would cause a daemonset pod not to schedule.
// This is primarily used to populate daemonset status.
// * shouldSchedule:
// Returns true when a daemonset should be scheduled to a node if a daemonset pod is not already
Expand Down Expand Up @@ -1257,11 +1258,6 @@ func (dsc *DaemonSetsController) nodeShouldRunDaemonPod(node *v1.Node, ds *exten
var emitEvent bool
// we try to partition predicates into two partitions here: intentional on the part of the operator and not.
switch reason {
case predicates.ErrNodeOutOfDisk:
// the kubelet will evict this pod if it needs to. Let kubelet
// decide whether to continue running this pod so leave shouldContinueRunning
// set to true
shouldSchedule = false
// intentional
case
predicates.ErrNodeSelectorNotMatch,
Expand Down Expand Up @@ -1344,9 +1340,6 @@ func Predicates(pod *v1.Pod, nodeInfo *schedulercache.NodeInfo) (bool, []algorit
fit, reasons, err = predicates.EssentialPredicates(pod, nil, nodeInfo)
} else {
fit, reasons, err = predicates.GeneralPredicates(pod, nil, nodeInfo)
ncFit, ncReasons := NodeConditionPredicates(nodeInfo)
fit = ncFit && fit
reasons = append(reasons, ncReasons...)
}
if err != nil {
return false, predicateFails, err
Expand All @@ -1358,26 +1351,6 @@ func Predicates(pod *v1.Pod, nodeInfo *schedulercache.NodeInfo) (bool, []algorit
return len(predicateFails) == 0, predicateFails, nil
}

func NodeConditionPredicates(nodeInfo *schedulercache.NodeInfo) (bool, []algorithm.PredicateFailureReason) {
reasons := []algorithm.PredicateFailureReason{}

// If TaintNodesByCondition feature was enabled, account PodToleratesNodeTaints predicates.
if utilfeature.DefaultFeatureGate.Enabled(features.TaintNodesByCondition) {
return true, nil
}

for _, c := range nodeInfo.Node().Status.Conditions {
// TODO: There are other node status that the DaemonSet should ideally respect too,
// e.g. MemoryPressure, and DiskPressure
if c.Type == v1.NodeOutOfDisk && c.Status == v1.ConditionTrue {
reasons = append(reasons, predicates.ErrNodeOutOfDisk)
break
}
}

return len(reasons) == 0, reasons
}

// byCreationTimestamp sorts a list by creation timestamp, using their names as a tie breaker.
type byCreationTimestamp []*extensions.DaemonSet

Expand Down
146 changes: 1 addition & 145 deletions pkg/controller/daemon/daemon_controller_test.go
Expand Up @@ -47,8 +47,6 @@ import (
"k8s.io/kubernetes/pkg/securitycontext"
labelsutil "k8s.io/kubernetes/pkg/util/labels"
"k8s.io/kubernetes/plugin/pkg/scheduler/algorithm"
"k8s.io/kubernetes/plugin/pkg/scheduler/algorithm/predicates"
"k8s.io/kubernetes/plugin/pkg/scheduler/schedulercache"
)

var (
Expand Down Expand Up @@ -503,20 +501,6 @@ func TestNotReadNodeDaemonDoesNotLaunchPod(t *testing.T) {
}
}

// DaemonSets should not place onto OutOfDisk nodes
func TestOutOfDiskNodeDaemonDoesNotLaunchPod(t *testing.T) {
for _, strategy := range updateStrategies() {
ds := newDaemonSet("foo")
ds.Spec.UpdateStrategy = *strategy
manager, podControl, _ := newTestController(ds)
node := newNode("not-enough-disk", nil)
node.Status.Conditions = []v1.NodeCondition{{Type: v1.NodeOutOfDisk, Status: v1.ConditionTrue}}
manager.nodeStore.Add(node)
manager.dsStore.Add(ds)
syncAndValidateDaemonSets(t, manager, ds, podControl, 0, 0, 0)
}
}

func resourcePodSpec(nodeName, memory, cpu string) v1.PodSpec {
return v1.PodSpec{
NodeName: nodeName,
Expand Down Expand Up @@ -1267,30 +1251,8 @@ func setDaemonSetToleration(ds *extensions.DaemonSet, tolerations []v1.Toleratio
ds.Spec.Template.Spec.Tolerations = tolerations
}

// DaemonSet should launch a critical pod even when the node is OutOfDisk.
func TestOutOfDiskNodeDaemonLaunchesCriticalPod(t *testing.T) {
for _, strategy := range updateStrategies() {
ds := newDaemonSet("critical")
ds.Spec.UpdateStrategy = *strategy
setDaemonSetCritical(ds)
manager, podControl, _ := newTestController(ds)

node := newNode("not-enough-disk", nil)
node.Status.Conditions = []v1.NodeCondition{{Type: v1.NodeOutOfDisk, Status: v1.ConditionTrue}}
manager.nodeStore.Add(node)

// Without enabling critical pod annotation feature gate, we shouldn't create critical pod
utilfeature.DefaultFeatureGate.Set("ExperimentalCriticalPodAnnotation=False")
manager.dsStore.Add(ds)
syncAndValidateDaemonSets(t, manager, ds, podControl, 0, 0, 0)

// Enabling critical pod annotation feature gate should create critical pod
utilfeature.DefaultFeatureGate.Set("ExperimentalCriticalPodAnnotation=True")
syncAndValidateDaemonSets(t, manager, ds, podControl, 1, 0, 0)
}
}

// DaemonSet should launch a critical pod even when the node with OutOfDisk taints.
// TODO(#48843) OutOfDisk taints will be removed in 1.10
func TestTaintOutOfDiskNodeDaemonLaunchesCriticalPod(t *testing.T) {
for _, strategy := range updateStrategies() {
ds := newDaemonSet("critical")
Expand Down Expand Up @@ -1454,23 +1416,6 @@ func TestNodeShouldRunDaemonPod(t *testing.T) {
shouldSchedule: true,
shouldContinueRunning: true,
},
{
ds: &extensions.DaemonSet{
Spec: extensions.DaemonSetSpec{
Selector: &metav1.LabelSelector{MatchLabels: simpleDaemonSetLabel},
Template: v1.PodTemplateSpec{
ObjectMeta: metav1.ObjectMeta{
Labels: simpleDaemonSetLabel,
},
Spec: resourcePodSpec("", "50M", "0.5"),
},
},
},
nodeCondition: []v1.NodeCondition{{Type: v1.NodeOutOfDisk, Status: v1.ConditionTrue}},
wantToRun: true,
shouldSchedule: false,
shouldContinueRunning: true,
},
{
ds: &extensions.DaemonSet{
Spec: extensions.DaemonSetSpec{
Expand Down Expand Up @@ -1612,41 +1557,6 @@ func TestUpdateNode(t *testing.T) {
ds: newDaemonSet("ds"),
shouldEnqueue: true,
},
{
test: "Node conditions changed",
oldNode: func() *v1.Node {
node := newNode("node1", nil)
node.Status.Conditions = []v1.NodeCondition{
{Type: v1.NodeOutOfDisk, Status: v1.ConditionTrue},
}
return node
}(),
newNode: newNode("node1", nil),
ds: newDaemonSet("ds"),
shouldEnqueue: true,
},
{
test: "Node conditions not changed",
oldNode: func() *v1.Node {
node := newNode("node1", nil)
node.Status.Conditions = []v1.NodeCondition{
{Type: v1.NodeOutOfDisk, Status: v1.ConditionTrue},
{Type: v1.NodeMemoryPressure, Status: v1.ConditionFalse},
{Type: v1.NodeDiskPressure, Status: v1.ConditionFalse},
{Type: v1.NodeNetworkUnavailable, Status: v1.ConditionFalse},
}
return node
}(),
newNode: func() *v1.Node {
node := newNode("node1", nil)
node.Status.Conditions = []v1.NodeCondition{
{Type: v1.NodeOutOfDisk, Status: v1.ConditionTrue},
}
return node
}(),
ds: newDaemonSet("ds"),
shouldEnqueue: false,
},
}
for _, c := range cases {
for _, strategy := range updateStrategies() {
Expand Down Expand Up @@ -2205,57 +2115,3 @@ func getQueuedKeys(queue workqueue.RateLimitingInterface) []string {
sort.Strings(keys)
return keys
}

func TestPredicates(t *testing.T) {
type args struct {
pod *v1.Pod
node *v1.Node
}
tests := []struct {
name string
args args
want bool
wantRes []algorithm.PredicateFailureReason
wantErr bool
}{
{
name: "retrun OutOfDiskErr if outOfDisk",
args: args{
pod: newPod("pod1-", "node-0", nil, nil),
node: &v1.Node{
ObjectMeta: metav1.ObjectMeta{
Name: "node-0",
},
Status: v1.NodeStatus{
Conditions: []v1.NodeCondition{
{Type: v1.NodeOutOfDisk, Status: v1.ConditionTrue},
},
Allocatable: v1.ResourceList{
v1.ResourcePods: resource.MustParse("100"),
},
},
},
},
want: false,
wantRes: []algorithm.PredicateFailureReason{predicates.ErrNodeOutOfDisk},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
nodeInfo := schedulercache.NewNodeInfo(tt.args.pod)
nodeInfo.SetNode(tt.args.node)

got, res, err := Predicates(tt.args.pod, nodeInfo)
if (err != nil) != tt.wantErr {
t.Errorf("%s (error): error = %v, wantErr %v", tt.name, err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("%s (fit): got = %v, want %v", tt.name, got, tt.want)
}
if !reflect.DeepEqual(res, tt.wantRes) {
t.Errorf("%s (reasons): got = %v, want %v", tt.name, res, tt.wantRes)
}
})
}
}
1 change: 1 addition & 0 deletions pkg/controller/daemon/util/daemonset_util.go
Expand Up @@ -71,6 +71,7 @@ func CreatePodTemplate(template v1.PodTemplateSpec, generation int64, hash strin
Effect: v1.TaintEffectNoSchedule,
})

// TODO(#48843) OutOfDisk taints will be removed in 1.10
if utilfeature.DefaultFeatureGate.Enabled(features.ExperimentalCriticalPodAnnotation) &&
kubelettypes.IsCritical(newTemplate.Namespace, newTemplate.Annotations) {
v1helper.AddOrUpdateTolerationInPodSpec(&newTemplate.Spec, &v1.Toleration{
Expand Down
1 change: 0 additions & 1 deletion pkg/controller/node/node_controller.go
Expand Up @@ -985,7 +985,6 @@ func (nc *Controller) tryUpdateNodeStatus(node *v1.Node) (time.Duration, v1.Node

// remaining node conditions should also be set to Unknown
remainingNodeConditionTypes := []v1.NodeConditionType{
v1.NodeOutOfDisk,
v1.NodeMemoryPressure,
v1.NodeDiskPressure,
// We don't change 'NodeNetworkUnavailable' condition, as it's managed on a control plane level.
Expand Down
58 changes: 0 additions & 58 deletions pkg/controller/node/nodecontroller_test.go
Expand Up @@ -1455,14 +1455,6 @@ func TestMonitorNodeStatusUpdateStatus(t *testing.T) {
LastHeartbeatTime: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
LastTransitionTime: fakeNow,
},
{
Type: v1.NodeOutOfDisk,
Status: v1.ConditionUnknown,
Reason: "NodeStatusNeverUpdated",
Message: "Kubelet never posted node status.",
LastHeartbeatTime: metav1.Date(2012, 1, 1, 0, 0, 0, 0, time.UTC),
LastTransitionTime: fakeNow,
},
{
Type: v1.NodeMemoryPressure,
Status: v1.ConditionUnknown,
Expand Down Expand Up @@ -1520,13 +1512,6 @@ func TestMonitorNodeStatusUpdateStatus(t *testing.T) {
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
{
Type: v1.NodeOutOfDisk,
Status: v1.ConditionFalse,
// Node status hasn't been updated for 1hr.
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
},
Capacity: v1.ResourceList{
v1.ResourceName(v1.ResourceCPU): resource.MustParse("10"),
Expand All @@ -1551,13 +1536,6 @@ func TestMonitorNodeStatusUpdateStatus(t *testing.T) {
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
{
Type: v1.NodeOutOfDisk,
Status: v1.ConditionFalse,
// Node status hasn't been updated for 1hr.
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
},
Capacity: v1.ResourceList{
v1.ResourceName(v1.ResourceCPU): resource.MustParse("10"),
Expand All @@ -1580,14 +1558,6 @@ func TestMonitorNodeStatusUpdateStatus(t *testing.T) {
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Time{Time: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC).Add(time.Hour)},
},
{
Type: v1.NodeOutOfDisk,
Status: v1.ConditionUnknown,
Reason: "NodeStatusUnknown",
Message: "Kubelet stopped posting node status.",
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Time{Time: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC).Add(time.Hour)},
},
{
Type: v1.NodeMemoryPressure,
Status: v1.ConditionUnknown,
Expand Down Expand Up @@ -1767,13 +1737,6 @@ func TestMonitorNodeStatusMarkPodsNotReady(t *testing.T) {
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
{
Type: v1.NodeOutOfDisk,
Status: v1.ConditionFalse,
// Node status hasn't been updated for 1hr.
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
},
Capacity: v1.ResourceList{
v1.ResourceName(v1.ResourceCPU): resource.MustParse("10"),
Expand All @@ -1800,13 +1763,6 @@ func TestMonitorNodeStatusMarkPodsNotReady(t *testing.T) {
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
{
Type: v1.NodeOutOfDisk,
Status: v1.ConditionFalse,
// Node status hasn't been updated for 1hr.
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
},
Capacity: v1.ResourceList{
v1.ResourceName(v1.ResourceCPU): resource.MustParse("10"),
Expand Down Expand Up @@ -1837,13 +1793,6 @@ func TestMonitorNodeStatusMarkPodsNotReady(t *testing.T) {
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
{
Type: v1.NodeOutOfDisk,
Status: v1.ConditionFalse,
// Node status hasn't been updated for 1hr.
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
},
Capacity: v1.ResourceList{
v1.ResourceName(v1.ResourceCPU): resource.MustParse("10"),
Expand All @@ -1870,13 +1819,6 @@ func TestMonitorNodeStatusMarkPodsNotReady(t *testing.T) {
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
{
Type: v1.NodeOutOfDisk,
Status: v1.ConditionFalse,
// Node status hasn't been updated for 1hr.
LastHeartbeatTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
LastTransitionTime: metav1.Date(2015, 1, 1, 12, 0, 0, 0, time.UTC),
},
},
Capacity: v1.ResourceList{
v1.ResourceName(v1.ResourceCPU): resource.MustParse("10"),
Expand Down