Merge pull request #72730 from liggitt/eviction-graceful-deletion

Make pod eviction trigger graceful deletion to match deletion via API

pkg/registry/core/pod/storage/BUILD

@@ -8,10 +8,15 @@ load(
 
 go_test(
     name = "go_default_test",
-    srcs = ["storage_test.go"],
+    srcs = [
+        "eviction_test.go",
+        "storage_test.go",
+    ],
     embed = [":go_default_library"],
     deps = [
         "//pkg/apis/core:go_default_library",
+        "//pkg/apis/policy:go_default_library",
+        "//pkg/client/clientset_generated/internalclientset/fake:go_default_library",
         "//pkg/registry/registrytest:go_default_library",
         "//pkg/securitycontext:go_default_library",
         "//staging/src/k8s.io/api/core/v1:go_default_library",

pkg/registry/core/pod/storage/eviction.go

@@ -138,7 +138,12 @@ func (r *EvictionREST) Create(ctx context.Context, obj runtime.Object, createVal
 	// At this point there was either no PDB or we succeeded in decrementing
 
 	// Try the delete
-	_, _, err = r.store.Delete(ctx, eviction.Name, eviction.DeleteOptions)
+	deletionOptions := eviction.DeleteOptions
+	if deletionOptions == nil {
+		// default to non-nil to trigger graceful deletion
+		deletionOptions = &metav1.DeleteOptions{}
+	}
+	_, _, err = r.store.Delete(ctx, eviction.Name, deletionOptions)
 	if err != nil {
 		return nil, err
 	}

pkg/registry/core/pod/storage/eviction_test.go

@@ -0,0 +1,165 @@
+/*
+Copyright 2019 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package storage
+
+import (
+	"testing"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
+	api "k8s.io/kubernetes/pkg/apis/core"
+	"k8s.io/kubernetes/pkg/apis/policy"
+	"k8s.io/kubernetes/pkg/client/clientset_generated/internalclientset/fake"
+)
+
+func TestEviction(t *testing.T) {
+	testcases := []struct {
+		name     string
+		pdbs     []runtime.Object
+		pod      *api.Pod
+		eviction *policy.Eviction
+
+		expectError   bool
+		expectDeleted bool
+	}{
+		{
+			name:          "no pdbs, unscheduled pod, nil delete options, deletes immediately",
+			pdbs:          nil,
+			pod:           validNewPod(),
+			eviction:      &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}},
+			expectDeleted: true,
+		},
+		{
+			name:          "no pdbs, scheduled pod, nil delete options, deletes gracefully",
+			pdbs:          nil,
+			pod:           func() *api.Pod { pod := validNewPod(); pod.Spec.NodeName = "foo"; return pod }(),
+			eviction:      &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}},
+			expectDeleted: false, // not deleted immediately because of graceful deletion
+		},
+		{
+			name:          "no pdbs, scheduled pod, empty delete options, deletes gracefully",
+			pdbs:          nil,
+			pod:           func() *api.Pod { pod := validNewPod(); pod.Spec.NodeName = "foo"; return pod }(),
+			eviction:      &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, DeleteOptions: &metav1.DeleteOptions{}},
+			expectDeleted: false, // not deleted immediately because of graceful deletion
+		},
+		{
+			name:          "no pdbs, scheduled pod, graceless delete options, deletes immediately",
+			pdbs:          nil,
+			pod:           func() *api.Pod { pod := validNewPod(); pod.Spec.NodeName = "foo"; return pod }(),
+			eviction:      &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
+			expectDeleted: true,
+		},
+		{
+			name: "matching pdbs with no disruptions allowed",
+			pdbs: []runtime.Object{&policy.PodDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
+				Spec:       policy.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
+				Status:     policy.PodDisruptionBudgetStatus{PodDisruptionsAllowed: 0},
+			}},
+			pod: func() *api.Pod {
+				pod := validNewPod()
+				pod.Labels = map[string]string{"a": "true"}
+				pod.Spec.NodeName = "foo"
+				return pod
+			}(),
+			eviction:    &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
+			expectError: true,
+		},
+		{
+			name: "matching pdbs with disruptions allowed",
+			pdbs: []runtime.Object{&policy.PodDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
+				Spec:       policy.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
+				Status:     policy.PodDisruptionBudgetStatus{PodDisruptionsAllowed: 1},
+			}},
+			pod: func() *api.Pod {
+				pod := validNewPod()
+				pod.Labels = map[string]string{"a": "true"}
+				pod.Spec.NodeName = "foo"
+				return pod
+			}(),
+			eviction:      &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
+			expectDeleted: true,
+		},
+		{
+			name: "non-matching pdbs",
+			pdbs: []runtime.Object{&policy.PodDisruptionBudget{
+				ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
+				Spec:       policy.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"b": "true"}}},
+				Status:     policy.PodDisruptionBudgetStatus{PodDisruptionsAllowed: 0},
+			}},
+			pod: func() *api.Pod {
+				pod := validNewPod()
+				pod.Labels = map[string]string{"a": "true"}
+				pod.Spec.NodeName = "foo"
+				return pod
+			}(),
+			eviction:      &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, DeleteOptions: metav1.NewDeleteOptions(0)},
+			expectDeleted: true,
+		},
+	}
+
+	for _, tc := range testcases {
+		t.Run(tc.name, func(t *testing.T) {
+			testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceDefault)
+			storage, _, _, server := newStorage(t)
+			defer server.Terminate(t)
+			defer storage.Store.DestroyFunc()
+			if tc.pod != nil {
+				if _, err := storage.Create(testContext, tc.pod, nil, &metav1.CreateOptions{}); err != nil {
+					t.Error(err)
+				}
+			}
+
+			client := fake.NewSimpleClientset(tc.pdbs...)
+			evictionRest := newEvictionStorage(storage.Store, client.Policy())
+			_, err := evictionRest.Create(testContext, tc.eviction, nil, &metav1.CreateOptions{})
+			if (err != nil) != tc.expectError {
+				t.Errorf("expected error=%v, got %v", tc.expectError, err)
+				return
+			}
+			if tc.expectError {
+				return
+			}
+
+			if tc.pod != nil {
+				existingPod, err := storage.Get(testContext, tc.pod.Name, &metav1.GetOptions{})
+				if tc.expectDeleted {
+					if !apierrors.IsNotFound(err) {
+						t.Errorf("expected to be deleted, lookup returned %#v", existingPod)
+					}
+					return
+				} else if apierrors.IsNotFound(err) {
+					t.Errorf("expected graceful deletion, got %v", err)
+					return
+				}
+
+				if err != nil {
+					t.Errorf("%#v", err)
+					return
+				}
+
+				if existingPod.(*api.Pod).DeletionTimestamp == nil {
+					t.Errorf("expected gracefully deleted pod with deletionTimestamp set, got %#v", existingPod)
+				}
+			}
+		})
+	}
+}

test/integration/auth/node_test.go

@@ -314,6 +314,7 @@ func TestNodeAuthorizer(t *testing.T) {
 	}
 	createNode2NormalPodEviction := func(client clientset.Interface) func() error {
 		return func() error {
+			zero := int64(0)
 			return client.Policy().Evictions("ns").Evict(&policy.Eviction{
 				TypeMeta: metav1.TypeMeta{
 					APIVersion: "policy/v1beta1",
@@ -323,11 +324,13 @@ func TestNodeAuthorizer(t *testing.T) {
 					Name:      "node2normalpod",
 					Namespace: "ns",
 				},
+				DeleteOptions: &metav1.DeleteOptions{GracePeriodSeconds: &zero},
 			})
 		}
 	}
 	createNode2MirrorPodEviction := func(client clientset.Interface) func() error {
 		return func() error {
+			zero := int64(0)
 			return client.Policy().Evictions("ns").Evict(&policy.Eviction{
 				TypeMeta: metav1.TypeMeta{
 					APIVersion: "policy/v1beta1",
@@ -337,6 +340,7 @@ func TestNodeAuthorizer(t *testing.T) {
 					Name:      "node2mirrorpod",
 					Namespace: "ns",
 				},
+				DeleteOptions: &metav1.DeleteOptions{GracePeriodSeconds: &zero},
 			})
 		}
 	}