From cdfb5d31704018ab2d851a0dc9a6cd0f49a3aa7f Mon Sep 17 00:00:00 2001 From: Jordan Liggitt Date: Wed, 9 Jan 2019 10:26:14 -0500 Subject: [PATCH] Make pod eviction trigger graceful deletion to match deletion via API --- pkg/registry/core/pod/storage/BUILD | 7 +- pkg/registry/core/pod/storage/eviction.go | 7 +- .../core/pod/storage/eviction_test.go | 165 ++++++++++++++++++ test/integration/auth/node_test.go | 4 + 4 files changed, 181 insertions(+), 2 deletions(-) create mode 100644 pkg/registry/core/pod/storage/eviction_test.go diff --git a/pkg/registry/core/pod/storage/BUILD b/pkg/registry/core/pod/storage/BUILD index 0b6aa46035..b02aa36550 100644 --- a/pkg/registry/core/pod/storage/BUILD +++ b/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", diff --git a/pkg/registry/core/pod/storage/eviction.go b/pkg/registry/core/pod/storage/eviction.go index 3e3827a079..fdcf2d8dbd 100644 --- a/pkg/registry/core/pod/storage/eviction.go +++ b/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 } diff --git a/pkg/registry/core/pod/storage/eviction_test.go b/pkg/registry/core/pod/storage/eviction_test.go new file mode 100644 index 0000000000..a5cc1ac762 --- /dev/null +++ b/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) + } + } + }) + } +} diff --git a/test/integration/auth/node_test.go b/test/integration/auth/node_test.go index 50490d200d..bbd4c7169b 100644 --- a/test/integration/auth/node_test.go +++ b/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}, }) } }