allow evictions subresource to accept policy/v1 and policy/v1beta1

This commit is contained in:
Jordan Liggitt 2021-03-31 16:54:55 -04:00
parent e22cd7dbc4
commit 33ad842480
12 changed files with 170 additions and 33 deletions

View File

@ -23,6 +23,7 @@ import (
"time"
policyv1 "k8s.io/api/policy/v1"
policyv1beta1 "k8s.io/api/policy/v1beta1"
"k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
@ -70,10 +71,23 @@ type EvictionREST struct {
var _ = rest.NamedCreater(&EvictionREST{})
var _ = rest.GroupVersionKindProvider(&EvictionREST{})
var _ = rest.GroupVersionAcceptor(&EvictionREST{})
var v1Eviction = schema.GroupVersionKind{Group: "policy", Version: "v1", Kind: "Eviction"}
// GroupVersionKind specifies a particular GroupVersionKind to discovery
func (r *EvictionREST) GroupVersionKind(containingGV schema.GroupVersion) schema.GroupVersionKind {
return schema.GroupVersionKind{Group: "policy", Version: "v1", Kind: "Eviction"}
return v1Eviction
}
// AcceptsGroupVersion indicates both v1 and v1beta1 Eviction objects are acceptable
func (r *EvictionREST) AcceptsGroupVersion(gv schema.GroupVersion) bool {
switch gv {
case policyv1.SchemeGroupVersion, policyv1beta1.SchemeGroupVersion:
return true
default:
return false
}
}
// New creates a new eviction resource

View File

@ -313,7 +313,7 @@ func (c LegacyRESTStorageProvider) NewLegacyRESTStorage(restOptionsGetter generi
if legacyscheme.Scheme.IsVersionRegistered(schema.GroupVersion{Group: "autoscaling", Version: "v1"}) {
restStorageMap["replicationControllers/scale"] = controllerStorage.Scale
}
if legacyscheme.Scheme.IsVersionRegistered(schema.GroupVersion{Group: "policy", Version: "v1beta1"}) {
if podStorage.Eviction != nil {
restStorageMap["pods/eviction"] = podStorage.Eviction
}
if serviceAccountStorage.Token != nil {

View File

@ -123,7 +123,7 @@ func createHandler(r rest.NamedCreater, scope *RequestScope, admit admission.Int
scope.err(err, w, req)
return
}
if gvk.GroupVersion() != gv {
if !scope.AcceptsGroupVersion(gvk.GroupVersion()) {
err = errors.NewBadRequest(fmt.Sprintf("the API version in the data (%s) does not match the expected API version (%v)", gvk.GroupVersion().String(), gv.String()))
scope.err(err, w, req)
return

View File

@ -91,6 +91,12 @@ type RequestScope struct {
Resource schema.GroupVersionResource
Kind schema.GroupVersionKind
// AcceptsGroupVersionDelegate is an optional delegate that can be queried about whether a given GVK
// can be accepted in create or update requests. If nil, only scope.Kind is accepted.
// Note that this does not enable multi-version support for reads from a single endpoint.
AcceptsGroupVersionDelegate rest.GroupVersionAcceptor
Subresource string
MetaGroupVersion schema.GroupVersion
@ -105,6 +111,17 @@ func (scope *RequestScope) err(err error, w http.ResponseWriter, req *http.Reque
responsewriters.ErrorNegotiated(err, scope.Serializer, scope.Kind.GroupVersion(), w, req)
}
// AcceptsGroupVersion returns true if the specified GroupVersion is allowed
// in create and update requests.
func (scope *RequestScope) AcceptsGroupVersion(gv schema.GroupVersion) bool {
// If there's a custom acceptor, delegate to it. This is extremely rare.
if scope.AcceptsGroupVersionDelegate != nil {
return scope.AcceptsGroupVersionDelegate.AcceptsGroupVersion(gv)
}
// Fall back to only allowing the singular Kind. This is the typical behavior.
return gv == scope.Kind.GroupVersion()
}
func (scope *RequestScope) AllowsMediaTypeTransform(mimeType, mimeSubType string, gvk *schema.GroupVersionKind) bool {
// some handlers like CRDs can't serve all the mime types that PartialObjectMetadata or Table can - if
// gvk is nil (no conversion) allow StandardSerializers to further restrict the set of mime types.

View File

@ -110,7 +110,7 @@ func UpdateResource(r rest.Updater, scope *RequestScope, admit admission.Interfa
scope.err(err, w, req)
return
}
if gvk.GroupVersion() != defaultGVK.GroupVersion() {
if !scope.AcceptsGroupVersion(gvk.GroupVersion()) {
err = errors.NewBadRequest(fmt.Sprintf("the API version in the data (%s) does not match the expected API version (%s)", gvk.GroupVersion(), defaultGVK.GroupVersion()))
scope.err(err, w, req)
return

View File

@ -251,6 +251,7 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
connecter, isConnecter := storage.(rest.Connecter)
storageMeta, isMetadata := storage.(rest.StorageMetadata)
storageVersionProvider, isStorageVersionProvider := storage.(rest.StorageVersionProvider)
gvAcceptor, _ := storage.(rest.GroupVersionAcceptor)
if !isMetadata {
storageMeta = defaultStorageMetadata{}
}
@ -587,6 +588,8 @@ func (a *APIInstaller) registerResourceHandlers(path string, storage rest.Storag
Subresource: subresource,
Kind: fqKindToRegister,
AcceptsGroupVersionDelegate: gvAcceptor,
HubGroupVersion: schema.GroupVersion{Group: fqKindToRegister.Group, Version: runtime.APIVersionInternal},
MetaGroupVersion: metav1.SchemeGroupVersion,

View File

@ -92,6 +92,13 @@ type GroupVersionKindProvider interface {
GroupVersionKind(containingGV schema.GroupVersion) schema.GroupVersionKind
}
// GroupVersionAcceptor is used to determine if a particular GroupVersion is acceptable to send to an endpoint.
// This is used for endpoints which accept multiple versions (which is extremely rare).
// The only known instance is pods/evictions which accepts policy/v1, but also policy/v1beta1 for backwards compatibility.
type GroupVersionAcceptor interface {
AcceptsGroupVersion(gv schema.GroupVersion) bool
}
// Lister is an object that can retrieve resources that match the provided field and label criteria.
type Lister interface {
// NewList returns an empty object that can be used with the List call.

View File

@ -28,7 +28,6 @@ import (
appsv1 "k8s.io/api/apps/v1"
v1 "k8s.io/api/core/v1"
policyv1 "k8s.io/api/policy/v1"
policyv1beta1 "k8s.io/api/policy/v1beta1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@ -285,7 +284,7 @@ var _ = SIGDescribe("DisruptionController", func() {
pod, err := locateRunningPod(cs, ns)
framework.ExpectNoError(err)
e := &policyv1beta1.Eviction{
e := &policyv1.Eviction{
ObjectMeta: metav1.ObjectMeta{
Name: pod.Name,
Namespace: ns,
@ -293,7 +292,7 @@ var _ = SIGDescribe("DisruptionController", func() {
}
if c.shouldDeny {
err = cs.CoreV1().Pods(ns).Evict(context.TODO(), e)
err = cs.CoreV1().Pods(ns).EvictV1(context.TODO(), e)
gomega.Expect(err).Should(gomega.MatchError("Cannot evict pod as it would violate the pod's disruption budget."))
} else {
// Only wait for running pods in the "allow" case
@ -304,7 +303,7 @@ var _ = SIGDescribe("DisruptionController", func() {
// Since disruptionAllowed starts out false, if an eviction is ever allowed,
// that means the controller is working.
err = wait.PollImmediate(framework.Poll, timeout, func() (bool, error) {
err = cs.CoreV1().Pods(ns).Evict(context.TODO(), e)
err = cs.CoreV1().Pods(ns).EvictV1(context.TODO(), e)
if err != nil {
return false, nil
}
@ -325,13 +324,13 @@ var _ = SIGDescribe("DisruptionController", func() {
pod, err := locateRunningPod(cs, ns)
framework.ExpectNoError(err)
e := &policyv1beta1.Eviction{
e := &policyv1.Eviction{
ObjectMeta: metav1.ObjectMeta{
Name: pod.Name,
Namespace: ns,
},
}
err = cs.CoreV1().Pods(ns).Evict(context.TODO(), e)
err = cs.CoreV1().Pods(ns).EvictV1(context.TODO(), e)
gomega.Expect(err).Should(gomega.MatchError("Cannot evict pod as it would violate the pod's disruption budget."))
ginkgo.By("Updating the pdb to allow a pod to be evicted")
@ -344,7 +343,7 @@ var _ = SIGDescribe("DisruptionController", func() {
ginkgo.By("Trying to evict the same pod we tried earlier which should now be evictable")
waitForPodsOrDie(cs, ns, 3)
waitForPdbToObserveHealthyPods(cs, ns, 3)
err = cs.CoreV1().Pods(ns).Evict(context.TODO(), e)
err = cs.CoreV1().Pods(ns).EvictV1(context.TODO(), e)
framework.ExpectNoError(err) // the eviction is now allowed
ginkgo.By("Patching the pdb to disallow a pod to be evicted")
@ -362,13 +361,13 @@ var _ = SIGDescribe("DisruptionController", func() {
waitForPodsOrDie(cs, ns, 3)
pod, err = locateRunningPod(cs, ns) // locate a new running pod
framework.ExpectNoError(err)
e = &policyv1beta1.Eviction{
e = &policyv1.Eviction{
ObjectMeta: metav1.ObjectMeta{
Name: pod.Name,
Namespace: ns,
},
}
err = cs.CoreV1().Pods(ns).Evict(context.TODO(), e)
err = cs.CoreV1().Pods(ns).EvictV1(context.TODO(), e)
gomega.Expect(err).Should(gomega.MatchError("Cannot evict pod as it would violate the pod's disruption budget."))
ginkgo.By("Deleting the pdb to allow a pod to be evicted")
@ -376,7 +375,7 @@ var _ = SIGDescribe("DisruptionController", func() {
ginkgo.By("Trying to evict the same pod we tried earlier which should now be evictable")
waitForPodsOrDie(cs, ns, 3)
err = cs.CoreV1().Pods(ns).Evict(context.TODO(), e)
err = cs.CoreV1().Pods(ns).EvictV1(context.TODO(), e)
framework.ExpectNoError(err) // the eviction is now allowed
})

View File

@ -30,7 +30,7 @@ import (
"github.com/aws/aws-sdk-go/service/ec2"
"github.com/onsi/ginkgo"
v1 "k8s.io/api/core/v1"
policyv1beta1 "k8s.io/api/policy/v1beta1"
policyv1 "k8s.io/api/policy/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/labels"
@ -423,7 +423,7 @@ var _ = utils.SIGDescribe("Pod Disks", func() {
framework.ExpectNoError(podClient.Delete(context.TODO(), host0Pod.Name, *metav1.NewDeleteOptions(0)), "Unable to delete host0Pod")
} else if disruptOp == evictPod {
evictTarget := &policyv1beta1.Eviction{
evictTarget := &policyv1.Eviction{
ObjectMeta: metav1.ObjectMeta{
Name: host0Pod.Name,
Namespace: ns,
@ -431,7 +431,7 @@ var _ = utils.SIGDescribe("Pod Disks", func() {
}
ginkgo.By("evicting host0Pod")
err = wait.PollImmediate(framework.Poll, podEvictTimeout, func() (bool, error) {
if err := cs.CoreV1().Pods(ns).Evict(context.TODO(), evictTarget); err != nil {
if err := cs.CoreV1().Pods(ns).EvictV1(context.TODO(), evictTarget); err != nil {
framework.Logf("Failed to evict host0Pod, ignoring error: %v", err)
return false, nil
}

View File

@ -42,7 +42,7 @@ import (
corev1 "k8s.io/api/core/v1"
v1 "k8s.io/api/core/v1"
extensionsv1beta1 "k8s.io/api/extensions/v1beta1"
policyv1beta1 "k8s.io/api/policy/v1beta1"
policyv1 "k8s.io/api/policy/v1"
apiextensionsclientset "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@ -1128,7 +1128,7 @@ func testPodBindingEviction(c *testContext) {
}).Do(context.TODO()).Error()
case gvr("", "v1", "pods/eviction"):
err = c.clientset.CoreV1().RESTClient().Post().Namespace(pod.GetNamespace()).Resource("pods").Name(pod.GetName()).SubResource("eviction").Body(&policyv1beta1.Eviction{
err = c.clientset.CoreV1().RESTClient().Post().Namespace(pod.GetNamespace()).Resource("pods").Name(pod.GetName()).SubResource("eviction").Body(&policyv1.Eviction{
ObjectMeta: metav1.ObjectMeta{Name: pod.GetName()},
DeleteOptions: &forceDelete,
}).Do(context.TODO()).Error()

View File

@ -26,7 +26,7 @@ import (
coordination "k8s.io/api/coordination/v1"
corev1 "k8s.io/api/core/v1"
policy "k8s.io/api/policy/v1beta1"
policy "k8s.io/api/policy/v1"
storagev1 "k8s.io/api/storage/v1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/api/resource"
@ -305,9 +305,9 @@ func TestNodeAuthorizer(t *testing.T) {
createNode2NormalPodEviction := func(client clientset.Interface) func() error {
return func() error {
zero := int64(0)
return client.PolicyV1beta1().Evictions("ns").Evict(context.TODO(), &policy.Eviction{
return client.PolicyV1().Evictions("ns").Evict(context.TODO(), &policy.Eviction{
TypeMeta: metav1.TypeMeta{
APIVersion: "policy/v1beta1",
APIVersion: "policy/v1",
Kind: "Eviction",
},
ObjectMeta: metav1.ObjectMeta{
@ -321,9 +321,9 @@ func TestNodeAuthorizer(t *testing.T) {
createNode2MirrorPodEviction := func(client clientset.Interface) func() error {
return func() error {
zero := int64(0)
return client.PolicyV1beta1().Evictions("ns").Evict(context.TODO(), &policy.Eviction{
return client.PolicyV1().Evictions("ns").Evict(context.TODO(), &policy.Eviction{
TypeMeta: metav1.TypeMeta{
APIVersion: "policy/v1beta1",
APIVersion: "policy/v1",
Kind: "Eviction",
},
ObjectMeta: metav1.ObjectMeta{

View File

@ -18,9 +18,11 @@ package evictions
import (
"context"
"encoding/json"
"fmt"
"net/http/httptest"
"reflect"
"strings"
"sync"
"sync/atomic"
"testing"
@ -31,6 +33,10 @@ import (
policyv1beta1 "k8s.io/api/policy/v1beta1"
apierrors "k8s.io/apimachinery/pkg/api/errors"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
"k8s.io/apimachinery/pkg/runtime"
"k8s.io/apimachinery/pkg/runtime/schema"
"k8s.io/apimachinery/pkg/types"
utilerrors "k8s.io/apimachinery/pkg/util/errors"
"k8s.io/apimachinery/pkg/util/intstr"
"k8s.io/apimachinery/pkg/util/wait"
@ -110,10 +116,10 @@ func TestConcurrentEvictionRequests(t *testing.T) {
go func(id int, errCh chan error) {
defer wg.Done()
podName := fmt.Sprintf(podNameFormat, id)
eviction := newV1beta1Eviction(ns.Name, podName, deleteOption)
eviction := newV1Eviction(ns.Name, podName, deleteOption)
err := wait.PollImmediate(5*time.Second, 60*time.Second, func() (bool, error) {
e := clientSet.PolicyV1beta1().Evictions(ns.Name).Evict(context.TODO(), eviction)
e := clientSet.PolicyV1().Evictions(ns.Name).Evict(context.TODO(), eviction)
switch {
case apierrors.IsTooManyRequests(e):
return false, nil
@ -219,9 +225,9 @@ func TestTerminalPodEviction(t *testing.T) {
t.Fatalf("Error while listing pod disruption budget")
}
oldPdb := pdbList.Items[0]
eviction := newV1beta1Eviction(ns.Name, pod.Name, deleteOption)
eviction := newV1Eviction(ns.Name, pod.Name, deleteOption)
err = wait.PollImmediate(5*time.Second, 60*time.Second, func() (bool, error) {
e := clientSet.PolicyV1beta1().Evictions(ns.Name).Evict(context.TODO(), eviction)
e := clientSet.PolicyV1().Evictions(ns.Name).Evict(context.TODO(), eviction)
switch {
case apierrors.IsTooManyRequests(e):
return false, nil
@ -251,6 +257,97 @@ func TestTerminalPodEviction(t *testing.T) {
}
}
// TestEvictionVersions ensures the eviction endpoint accepts and returns the correct API versions
func TestEvictionVersions(t *testing.T) {
s, closeFn, rm, informers, clientSet := rmSetup(t)
defer closeFn()
stopCh := make(chan struct{})
informers.Start(stopCh)
go rm.Run(stopCh)
defer close(stopCh)
config := restclient.Config{Host: s.URL}
ns := "default"
subresource := "eviction"
pod := newPod("test")
if _, err := clientSet.CoreV1().Pods(ns).Create(context.TODO(), pod, metav1.CreateOptions{}); err != nil {
t.Errorf("Failed to create pod: %v", err)
}
dynamicClient, err := dynamic.NewForConfig(&config)
if err != nil {
t.Fatalf("Failed to create clientset: %v", err)
}
podClient := dynamicClient.Resource(schema.GroupVersionResource{Group: "", Version: "v1", Resource: "pods"}).Namespace(ns)
// get should not be supported
if _, err := podClient.Get(context.TODO(), pod.Name, metav1.GetOptions{}, subresource); !apierrors.IsMethodNotSupported(err) {
t.Fatalf("expected MethodNotSupported for GET, got %v", err)
}
// patch should not be supported
for _, patchType := range []types.PatchType{types.JSONPatchType, types.MergePatchType, types.StrategicMergePatchType, types.ApplyPatchType} {
if _, err := podClient.Patch(context.TODO(), pod.Name, patchType, []byte{}, metav1.PatchOptions{}, subresource); !apierrors.IsMethodNotSupported(err) {
t.Fatalf("expected MethodNotSupported for GET, got %v", err)
}
}
allowedEvictions := []runtime.Object{
// v1beta1, no apiVersion/kind
&policyv1beta1.Eviction{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{Name: pod.Name},
DeleteOptions: &metav1.DeleteOptions{DryRun: []string{metav1.DryRunAll}},
},
// v1beta1, apiVersion/kind
&policyv1beta1.Eviction{
TypeMeta: metav1.TypeMeta{APIVersion: "policy/v1beta1", Kind: "Eviction"},
ObjectMeta: metav1.ObjectMeta{Name: pod.Name},
DeleteOptions: &metav1.DeleteOptions{DryRun: []string{metav1.DryRunAll}},
},
// v1, no apiVersion/kind
&policyv1.Eviction{
TypeMeta: metav1.TypeMeta{},
ObjectMeta: metav1.ObjectMeta{Name: pod.Name},
DeleteOptions: &metav1.DeleteOptions{DryRun: []string{metav1.DryRunAll}},
},
// v1, apiVersion/kind
&policyv1.Eviction{
TypeMeta: metav1.TypeMeta{APIVersion: "policy/v1", Kind: "Eviction"},
ObjectMeta: metav1.ObjectMeta{Name: pod.Name},
DeleteOptions: &metav1.DeleteOptions{DryRun: []string{metav1.DryRunAll}},
},
}
v1Status := schema.GroupVersionKind{Version: "v1", Kind: "Status"}
for _, allowedEviction := range allowedEvictions {
data, _ := json.Marshal(allowedEviction)
u := &unstructured.Unstructured{}
json.Unmarshal(data, u)
result, err := podClient.Create(context.TODO(), u, metav1.CreateOptions{}, subresource)
if err != nil {
t.Fatalf("error posting %s: %v", string(data), err)
}
if result.GroupVersionKind() != v1Status {
t.Fatalf("expected v1 Status, got %#v", result)
}
}
// create unknown eviction version with apiVersion/kind should fail
u := &unstructured.Unstructured{Object: map[string]interface{}{
"metadata": map[string]interface{}{"name": pod.Name},
"apiVersion": "policy/v2",
"kind": "Eviction",
}}
if _, err := podClient.Create(context.TODO(), u, metav1.CreateOptions{}, subresource); err == nil {
t.Fatal("expected error posting unknown Eviction version, got none")
} else if !strings.Contains(err.Error(), "policy/v2") {
t.Fatalf("expected error about policy/v2, got %#v", err)
}
}
func newPod(podName string) *v1.Pod {
return &v1.Pod{
ObjectMeta: metav1.ObjectMeta{
@ -309,10 +406,10 @@ func newPDB() *policyv1.PodDisruptionBudget {
}
}
func newV1beta1Eviction(ns, evictionName string, deleteOption metav1.DeleteOptions) *policyv1beta1.Eviction {
return &policyv1beta1.Eviction{
func newV1Eviction(ns, evictionName string, deleteOption metav1.DeleteOptions) *policyv1.Eviction {
return &policyv1.Eviction{
TypeMeta: metav1.TypeMeta{
APIVersion: "Policy/v1beta1",
APIVersion: "policy/v1",
Kind: "Eviction",
},
ObjectMeta: metav1.ObjectMeta{