Merge pull request #56713 from juanvallejo/jvallejo/handle-ds-pod-drain-local-storage

Automatic merge from submit-queue. If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

Allow kubectl drain to continue w ds-managed pods with local storage

**Release note**:
```release-note
NONE
```

Prevents oadm drain from failing if it encounters DaemonSet-managed pods
that have local storage, when the option to ignore DaemonSet-managed
pods has been specified.

Will add a test

cc @kubernetes/sig-cli-misc @deads2k @fabianofranz @dustymabe
pull/6/head
Kubernetes Submit Queue 2018-01-12 23:05:46 -08:00 committed by GitHub
commit 4bc93609ea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 74 additions and 11 deletions

View File

@ -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)

View File

@ -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())
}
}
}
}
}