diff --git a/pkg/kubectl/cmd/drain.go b/pkg/kubectl/cmd/drain.go index ba5ae1f1a72..710088ac2f8 100644 --- a/pkg/kubectl/cmd/drain.go +++ b/pkg/kubectl/cmd/drain.go @@ -446,7 +446,7 @@ func (o *DrainOptions) getPodsForDeletion(nodeInfo *resource.Info) (pods []corev for _, pod := range podList.Items { podOk := true - for _, filt := range []podFilter{mirrorPodFilter, o.localStorageFilter, o.unreplicatedFilter, o.daemonsetFilter} { + for _, filt := range []podFilter{o.daemonsetFilter, mirrorPodFilter, o.localStorageFilter, o.unreplicatedFilter} { filterOk, w, f := filt(pod) podOk = podOk && filterOk @@ -456,6 +456,13 @@ func (o *DrainOptions) getPodsForDeletion(nodeInfo *resource.Info) (pods []corev if f != nil { fs[f.string] = append(fs[f.string], pod.Name) } + + // short-circuit as soon as pod not ok + // at that point, there is no reason to run pod + // through any additional filters + if !podOk { + break + } } if podOk { pods = append(pods, pod) diff --git a/pkg/kubectl/cmd/drain_test.go b/pkg/kubectl/cmd/drain_test.go index 5200e95cd89..f9996314406 100644 --- a/pkg/kubectl/cmd/drain_test.go +++ b/pkg/kubectl/cmd/drain_test.go @@ -304,6 +304,34 @@ func TestDrain(t *testing.T) { }, } + ds_pod_with_emptyDir := corev1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Name: "bar", + Namespace: "default", + CreationTimestamp: metav1.Time{Time: time.Now()}, + Labels: labels, + SelfLink: testapi.Default.SelfLink("pods", "bar"), + OwnerReferences: []metav1.OwnerReference{ + { + APIVersion: "extensions/v1beta1", + Kind: "DaemonSet", + Name: "ds", + BlockOwnerDeletion: boolptr(true), + Controller: boolptr(true), + }, + }, + }, + Spec: corev1.PodSpec{ + NodeName: "node", + Volumes: []corev1.Volume{ + { + Name: "scratch", + VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{Medium: ""}}, + }, + }, + }, + } + orphaned_ds_pod := corev1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "bar", @@ -414,15 +442,16 @@ func TestDrain(t *testing.T) { } tests := []struct { - description string - node *corev1.Node - expected *corev1.Node - pods []corev1.Pod - rcs []api.ReplicationController - replicaSets []extensions.ReplicaSet - args []string - expectFatal bool - expectDelete bool + description string + node *corev1.Node + expected *corev1.Node + pods []corev1.Pod + rcs []api.ReplicationController + replicaSets []extensions.ReplicaSet + args []string + expectWarning string + expectFatal bool + expectDelete bool }{ { description: "RC-managed pod", @@ -474,6 +503,17 @@ func TestDrain(t *testing.T) { expectFatal: false, expectDelete: false, }, + { + description: "DS-managed pod with emptyDir with --ignore-daemonsets", + node: node, + expected: cordoned_node, + pods: []corev1.Pod{ds_pod_with_emptyDir}, + rcs: []api.ReplicationController{rc}, + args: []string{"node", "--ignore-daemonsets"}, + expectWarning: "WARNING: Ignoring DaemonSet-managed pods: bar\n", + expectFatal: false, + expectDelete: false, + }, { description: "Job-managed pod", node: node, @@ -661,6 +701,7 @@ func TestDrain(t *testing.T) { cmd := NewCmdDrain(f, buf, errBuf) saw_fatal := false + fatal_msg := "" func() { defer func() { // Recover from the panic below. @@ -668,7 +709,7 @@ func TestDrain(t *testing.T) { // Restore cmdutil behavior cmdutil.DefaultBehaviorOnFatal() }() - cmdutil.BehaviorOnFatal(func(e string, code int) { saw_fatal = true; panic(e) }) + cmdutil.BehaviorOnFatal(func(e string, code int) { saw_fatal = true; fatal_msg = e; panic(e) }) cmd.SetArgs(test.args) cmd.Execute() }() @@ -676,6 +717,11 @@ func TestDrain(t *testing.T) { if !saw_fatal { t.Fatalf("%s: unexpected non-error when using %s", test.description, currMethod) } + } else { + if saw_fatal { + t.Fatalf("%s: unexpected error when using %s: %s", test.description, currMethod, fatal_msg) + + } } if test.expectDelete { @@ -693,6 +739,16 @@ func TestDrain(t *testing.T) { t.Fatalf("%s: unexpected delete when using %s", test.description, currMethod) } } + + if len(test.expectWarning) > 0 { + if len(errBuf.String()) == 0 { + t.Fatalf("%s: expected warning, but found no stderr output", test.description) + } + + if errBuf.String() != test.expectWarning { + t.Fatalf("%s: actual warning message did not match expected warning message.\n Expecting: %s\n Got: %s", test.description, test.expectWarning, errBuf.String()) + } + } } } }