Fix eviction dry-run

k3s-v1.15.3
Antoine Pelisse 2019-04-23 15:16:32 -07:00
parent bd12b01387
commit 37f266349c
4 changed files with 167 additions and 9 deletions

View File

@ -20,6 +20,7 @@ go_test(
"//pkg/securitycontext:go_default_library",
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/api/policy/v1beta1:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/apitesting:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/equality:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
@ -27,7 +28,9 @@ go_test(
"//staging/src/k8s.io/apimachinery/pkg/fields:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/labels:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/runtime/serializer:go_default_library",
"//staging/src/k8s.io/apimachinery/pkg/util/diff:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/apis/example/v1:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/endpoints/request:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/registry/generic:go_default_library",
"//staging/src/k8s.io/apiserver/pkg/registry/generic/registry:go_default_library",

View File

@ -19,6 +19,7 @@ package storage
import (
"context"
"fmt"
"reflect"
"time"
policyv1beta1 "k8s.io/api/policy/v1beta1"
@ -30,6 +31,7 @@ import (
"k8s.io/apimachinery/pkg/util/wait"
genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
"k8s.io/apiserver/pkg/registry/rest"
"k8s.io/apiserver/pkg/util/dryrun"
policyclient "k8s.io/client-go/kubernetes/typed/policy/v1beta1"
"k8s.io/client-go/util/retry"
api "k8s.io/kubernetes/pkg/apis/core"
@ -78,11 +80,36 @@ func (r *EvictionREST) New() runtime.Object {
return &policy.Eviction{}
}
// Propagate dry-run takes the dry-run option from the request and pushes it into the eviction object.
// It returns an error if they have non-matching dry-run options.
func propagateDryRun(eviction *policy.Eviction, options *metav1.CreateOptions) (*metav1.DeleteOptions, error) {
if eviction.DeleteOptions == nil {
return &metav1.DeleteOptions{DryRun: options.DryRun}, nil
}
if len(eviction.DeleteOptions.DryRun) == 0 {
eviction.DeleteOptions.DryRun = options.DryRun
return eviction.DeleteOptions, nil
}
if len(options.DryRun) == 0 {
return eviction.DeleteOptions, nil
}
if !reflect.DeepEqual(options.DryRun, eviction.DeleteOptions.DryRun) {
return nil, fmt.Errorf("Non-matching dry-run options in request and content: %v and %v", options.DryRun, eviction.DeleteOptions.DryRun)
}
return eviction.DeleteOptions, nil
}
// Create attempts to create a new eviction. That is, it tries to evict a pod.
func (r *EvictionREST) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
eviction := obj.(*policy.Eviction)
obj, err := r.store.Get(ctx, eviction.Name, &metav1.GetOptions{})
deletionOptions, err := propagateDryRun(eviction, options)
if err != nil {
return nil, err
}
obj, err = r.store.Get(ctx, eviction.Name, &metav1.GetOptions{})
if err != nil {
return nil, err
}
@ -90,7 +117,7 @@ func (r *EvictionREST) Create(ctx context.Context, obj runtime.Object, createVal
// Evicting a terminal pod should result in direct deletion of pod as it already caused disruption by the time we are evicting.
// There is no need to check for pdb.
if pod.Status.Phase == api.PodSucceeded || pod.Status.Phase == api.PodFailed {
_, _, err = r.store.Delete(ctx, eviction.Name, eviction.DeleteOptions)
_, _, err = r.store.Delete(ctx, eviction.Name, deletionOptions)
if err != nil {
return nil, err
}
@ -119,7 +146,7 @@ func (r *EvictionREST) Create(ctx context.Context, obj runtime.Object, createVal
// If it was false already, or if it becomes false during the course of our retries,
// raise an error marked as a 429.
if err := r.checkAndDecrement(pod.Namespace, pod.Name, pdb); err != nil {
if err := r.checkAndDecrement(pod.Namespace, pod.Name, pdb, dryrun.IsDryRun(deletionOptions.DryRun)); err != nil {
return err
}
}
@ -139,11 +166,6 @@ 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
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
@ -154,7 +176,7 @@ func (r *EvictionREST) Create(ctx context.Context, obj runtime.Object, createVal
}
// checkAndDecrement checks if the provided PodDisruptionBudget allows any disruption.
func (r *EvictionREST) checkAndDecrement(namespace string, podName string, pdb policyv1beta1.PodDisruptionBudget) error {
func (r *EvictionREST) checkAndDecrement(namespace string, podName string, pdb policyv1beta1.PodDisruptionBudget, dryRun bool) error {
if pdb.Status.ObservedGeneration < pdb.Generation {
// TODO(mml): Add a Retry-After header. Once there are time-based
// budgets, we can sometimes compute a sensible suggested value. But
@ -180,6 +202,12 @@ func (r *EvictionREST) checkAndDecrement(namespace string, podName string, pdb p
if pdb.Status.DisruptedPods == nil {
pdb.Status.DisruptedPods = make(map[string]metav1.Time)
}
// If this is a dry-run, we don't need to go any further than that.
if dryRun == true {
return nil
}
// Eviction handler needs to inform the PDB controller that it is about to delete a pod
// so it should not consider it as available in calculations when updating PodDisruptions allowed.
// If the pod is not deleted within a reasonable time limit PDB controller will assume that it won't

View File

@ -17,16 +17,25 @@ limitations under the License.
package storage
import (
"context"
"testing"
policyv1beta1 "k8s.io/api/policy/v1beta1"
"k8s.io/apimachinery/pkg/api/apitesting"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/serializer"
examplev1 "k8s.io/apiserver/pkg/apis/example/v1"
genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
"k8s.io/apiserver/pkg/registry/generic"
genericregistry "k8s.io/apiserver/pkg/registry/generic/registry"
"k8s.io/apiserver/pkg/storage"
etcdtesting "k8s.io/apiserver/pkg/storage/etcd/testing"
"k8s.io/client-go/kubernetes/fake"
api "k8s.io/kubernetes/pkg/apis/core"
"k8s.io/kubernetes/pkg/apis/policy"
"k8s.io/kubernetes/pkg/registry/registrytest"
)
func TestEviction(t *testing.T) {
@ -136,3 +145,119 @@ func TestEviction(t *testing.T) {
})
}
}
type FailDeleteUpdateStorage struct {
storage.Interface
}
func (f FailDeleteUpdateStorage) Delete(ctx context.Context, key string, out runtime.Object, precondition *storage.Preconditions) error {
return storage.NewKeyNotFoundError(key, 0)
}
func (f FailDeleteUpdateStorage) GuaranteedUpdate(ctx context.Context, key string, ptrToType runtime.Object, ignoreNotFound bool,
preconditions *storage.Preconditions, tryUpdate storage.UpdateFunc, suggestion ...runtime.Object) error {
return storage.NewKeyNotFoundError(key, 0)
}
var scheme = runtime.NewScheme()
var codecs = serializer.NewCodecFactory(scheme)
func newFailDeleteUpdateStorage(t *testing.T) (*REST, *etcdtesting.EtcdTestServer) {
etcdStorage, server := registrytest.NewEtcdStorage(t, "")
restOptions := generic.RESTOptions{
StorageConfig: etcdStorage,
Decorator: generic.UndecoratedStorage,
DeleteCollectionWorkers: 3,
ResourcePrefix: "pods",
}
storage := NewStorage(restOptions, nil, nil, nil)
storage.Pod.Store.Storage = genericregistry.DryRunnableStorage{
Storage: FailDeleteUpdateStorage{storage.Pod.Store.Storage.Storage},
Codec: apitesting.TestStorageCodec(codecs, examplev1.SchemeGroupVersion),
}
return storage.Pod, server
}
func TestEvictionDryRun(t *testing.T) {
testcases := []struct {
name string
evictionOptions *metav1.DeleteOptions
requestOptions *metav1.CreateOptions
pod *api.Pod
pdbs []runtime.Object
}{
{
name: "just request-options",
requestOptions: &metav1.CreateOptions{DryRun: []string{"All"}},
evictionOptions: &metav1.DeleteOptions{},
pod: func() *api.Pod {
pod := validNewPod()
pod.Labels = map[string]string{"a": "true"}
pod.Spec.NodeName = "foo"
return pod
}(),
},
{
name: "just eviction-options",
requestOptions: &metav1.CreateOptions{},
evictionOptions: &metav1.DeleteOptions{DryRun: []string{"All"}},
pod: func() *api.Pod {
pod := validNewPod()
pod.Labels = map[string]string{"a": "true"}
pod.Spec.NodeName = "foo"
return pod
}(),
},
{
name: "both options",
evictionOptions: &metav1.DeleteOptions{DryRun: []string{"All"}},
requestOptions: &metav1.CreateOptions{DryRun: []string{"All"}},
pod: func() *api.Pod {
pod := validNewPod()
pod.Labels = map[string]string{"a": "true"}
pod.Spec.NodeName = "foo"
return pod
}(),
},
{
name: "with pdbs",
evictionOptions: &metav1.DeleteOptions{DryRun: []string{"All"}},
requestOptions: &metav1.CreateOptions{DryRun: []string{"All"}},
pod: func() *api.Pod {
pod := validNewPod()
pod.Labels = map[string]string{"a": "true"}
pod.Spec.NodeName = "foo"
return pod
}(),
pdbs: []runtime.Object{&policyv1beta1.PodDisruptionBudget{
ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"},
Spec: policyv1beta1.PodDisruptionBudgetSpec{Selector: &metav1.LabelSelector{MatchLabels: map[string]string{"a": "true"}}},
Status: policyv1beta1.PodDisruptionBudgetStatus{PodDisruptionsAllowed: 1},
}},
},
}
for _, tc := range testcases {
t.Run(tc.name, func(t *testing.T) {
testContext := genericapirequest.WithNamespace(genericapirequest.NewContext(), metav1.NamespaceDefault)
storage, server := newFailDeleteUpdateStorage(t)
defer server.Terminate(t)
defer storage.Store.DestroyFunc()
pod := validNewPod()
pod.Labels = map[string]string{"a": "true"}
pod.Spec.NodeName = "foo"
if _, err := storage.Create(testContext, pod, nil, &metav1.CreateOptions{}); err != nil {
t.Error(err)
}
client := fake.NewSimpleClientset()
evictionRest := newEvictionStorage(storage.Store, client.PolicyV1beta1())
eviction := &policy.Eviction{ObjectMeta: metav1.ObjectMeta{Name: "foo", Namespace: "default"}, DeleteOptions: tc.evictionOptions}
_, err := evictionRest.Create(testContext, eviction, nil, tc.requestOptions)
if err != nil {
t.Fatalf("Failed to run eviction: %v", err)
}
})
}
}

2
vendor/modules.txt vendored
View File

@ -1202,6 +1202,8 @@ k8s.io/apiserver/pkg/apis/audit/v1beta1
k8s.io/apiserver/pkg/apis/audit/validation
k8s.io/apiserver/pkg/apis/config
k8s.io/apiserver/pkg/apis/config/v1
k8s.io/apiserver/pkg/apis/example
k8s.io/apiserver/pkg/apis/example/v1
k8s.io/apiserver/pkg/audit
k8s.io/apiserver/pkg/audit/event
k8s.io/apiserver/pkg/audit/policy