Merge pull request #100724 from liggitt/eviction-v1beta1

Add policy/v1 Eviction support

Kubernetes-commit: 27a625cf8921007eaf115425b6d61587eb253e92

Godeps/Godeps.json

@@ -1036,7 +1036,7 @@
 		},
 		{
 			"ImportPath": "k8s.io/api",
-			"Rev": "e55a4a108a76"
+			"Rev": "86cef11b7287"
 		},
 		{
 			"ImportPath": "k8s.io/apimachinery",
@@ -1048,7 +1048,7 @@
 		},
 		{
 			"ImportPath": "k8s.io/client-go",
-			"Rev": "0c029ff945dd"
+			"Rev": "4e267f6cae31"
 		},
 		{
 			"ImportPath": "k8s.io/code-generator",

go.mod

@@ -33,10 +33,10 @@ require (
 	github.com/stretchr/testify v1.6.1
 	golang.org/x/sys v0.0.0-20210225134936-a50acf3fe073
 	gopkg.in/yaml.v2 v2.4.0
-	k8s.io/api v0.0.0-20210415154719-e55a4a108a76
+	k8s.io/api v0.0.0-20210416194706-86cef11b7287
 	k8s.io/apimachinery v0.0.0-20210415154527-1ba67c107540
 	k8s.io/cli-runtime v0.0.0-20210416000535-a3112a1a9f25
-	k8s.io/client-go v0.0.0-20210415235004-0c029ff945dd
+	k8s.io/client-go v0.0.0-20210416194931-4e267f6cae31
 	k8s.io/component-base v0.0.0-20210412032905-a57cc3fac704
 	k8s.io/component-helpers v0.0.0-20210411032753-ddd2d94034ce
 	k8s.io/klog/v2 v2.8.0
@@ -49,10 +49,10 @@ require (
 )
 
 replace (
-	k8s.io/api => k8s.io/api v0.0.0-20210415154719-e55a4a108a76
+	k8s.io/api => k8s.io/api v0.0.0-20210416194706-86cef11b7287
 	k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20210415154527-1ba67c107540
 	k8s.io/cli-runtime => k8s.io/cli-runtime v0.0.0-20210416000535-a3112a1a9f25
-	k8s.io/client-go => k8s.io/client-go v0.0.0-20210415235004-0c029ff945dd
+	k8s.io/client-go => k8s.io/client-go v0.0.0-20210416194931-4e267f6cae31
 	k8s.io/code-generator => k8s.io/code-generator v0.0.0-20210411031433-bdc239664504
 	k8s.io/component-base => k8s.io/component-base v0.0.0-20210412032905-a57cc3fac704
 	k8s.io/component-helpers => k8s.io/component-helpers v0.0.0-20210411032753-ddd2d94034ce

go.sum

@@ -727,10 +727,10 @@ honnef.co/go/tools v0.0.0-20190418001031-e561f6794a2a/go.mod h1:rf3lG4BRIbNafJWh
 honnef.co/go/tools v0.0.0-20190523083050-ea95bdfd59fc/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
 honnef.co/go/tools v0.0.1-2019.2.3/go.mod h1:a3bituU0lyd329TUQxRnasdCoJDkEUEAqEt0JzvZhAg=
 honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
-k8s.io/api v0.0.0-20210415154719-e55a4a108a76/go.mod h1:DHqxvzZf7mMtXnGgywYgvLoyfA2SGsMd1vdGAEcDjbA=
+k8s.io/api v0.0.0-20210416194706-86cef11b7287/go.mod h1:DHqxvzZf7mMtXnGgywYgvLoyfA2SGsMd1vdGAEcDjbA=
 k8s.io/apimachinery v0.0.0-20210415154527-1ba67c107540/go.mod h1:CRK4uy8GKqwLtMa4Gj9i/B2EBjcqOsuvwP2xZJWUwmk=
 k8s.io/cli-runtime v0.0.0-20210416000535-a3112a1a9f25/go.mod h1:bY35daqhQG2nFzNoaAYiUXOO1sQPRqOZxq1Q6MReXlQ=
-k8s.io/client-go v0.0.0-20210415235004-0c029ff945dd/go.mod h1:gZDn8z9shVCQQs7oeSaWAS3c3gp2/YAq4WY5npomrUw=
+k8s.io/client-go v0.0.0-20210416194931-4e267f6cae31/go.mod h1:RbsIgkosH5M8OkVp5z3OCH5jtynzbv79nvpmcUh94EM=
 k8s.io/code-generator v0.0.0-20210411031433-bdc239664504/go.mod h1:PXvzWuKkvLN0uh/kV/NghowjknTgxfcJ0SaRzqi6vok=
 k8s.io/component-base v0.0.0-20210412032905-a57cc3fac704/go.mod h1:zEytEW+Y696JL4g97mo/bP6hzeY26r9zDRw2IFbc7fE=
 k8s.io/component-helpers v0.0.0-20210411032753-ddd2d94034ce/go.mod h1:dm5Cz9BuwaSuCnj7vUkH178djdfCI5TkDJ+lPJZYToI=

pkg/cmd/drain/drain_test.go

@@ -34,7 +34,6 @@ import (
 	appsv1 "k8s.io/api/apps/v1"
 	batchv1 "k8s.io/api/batch/v1"
 	corev1 "k8s.io/api/core/v1"
-	policyv1beta1 "k8s.io/api/policy/v1beta1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
@@ -767,7 +766,7 @@ func TestDrain(t *testing.T) {
 									{
 										Name: "policy",
 										PreferredVersion: metav1.GroupVersionForDiscovery{
-											GroupVersion: "policy/v1beta1",
+											GroupVersion: "policy/v1",
 										},
 									},
 								},
@@ -780,8 +779,10 @@ func TestDrain(t *testing.T) {
 							if testEviction {
 								resourceList.APIResources = []metav1.APIResource{
 									{
-										Name: drain.EvictionSubresource,
+										Name:    drain.EvictionSubresource,
-										Kind: drain.EvictionKind,
+										Kind:    drain.EvictionKind,
+										Group:   "policy",
+										Version: "v1",
 									},
 								}
 							}
@@ -849,7 +850,7 @@ func TestDrain(t *testing.T) {
 							if test.failUponEvictionOrDeletion {
 								return nil, errors.New("request failed")
 							}
-							return &http.Response{StatusCode: http.StatusCreated, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, &policyv1beta1.Eviction{})}, nil
+							return &http.Response{StatusCode: http.StatusCreated, Header: cmdtesting.DefaultHeader(), Body: cmdtesting.ObjBody(codec, &metav1.Status{})}, nil
 						default:
 							t.Fatalf("%s: unexpected request: %v %#v\n%#v", test.description, req.Method, req.URL, req)
 							return nil, nil

pkg/cmd/testing/fake.go

@@ -565,6 +565,7 @@ func (f *TestFactory) KubernetesClientSet() (*kubernetes.Clientset, error) {
 	clientset.AppsV1beta2().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client
 	clientset.AppsV1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client
 	clientset.PolicyV1beta1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client
+	clientset.PolicyV1().RESTClient().(*restclient.RESTClient).Client = fakeClient.Client
 	clientset.DiscoveryClient.RESTClient().(*restclient.RESTClient).Client = fakeClient.Client
 
 	return clientset, nil

pkg/drain/drain.go

@@ -24,12 +24,14 @@ import (
 	"time"
 
 	corev1 "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/fields"
 	"k8s.io/apimachinery/pkg/labels"
 	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
 	utilerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/wait"
 	"k8s.io/cli-runtime/pkg/resource"
@@ -103,35 +105,21 @@ type waitForDeleteParams struct {
 
 // CheckEvictionSupport uses Discovery API to find out if the server support
 // eviction subresource If support, it will return its groupVersion; Otherwise,
-// it will return an empty string
+// it will return an empty GroupVersion
-func CheckEvictionSupport(clientset kubernetes.Interface) (string, error) {
+func CheckEvictionSupport(clientset kubernetes.Interface) (schema.GroupVersion, error) {
 	discoveryClient := clientset.Discovery()
-	groupList, err := discoveryClient.ServerGroups()
+
-	if err != nil {
+	// version info available in subresources since v1.8.0 in https://github.com/kubernetes/kubernetes/pull/49971
-		return "", err
-	}
-	foundPolicyGroup := false
-	var policyGroupVersion string
-	for _, group := range groupList.Groups {
-		if group.Name == "policy" {
-			foundPolicyGroup = true
-			policyGroupVersion = group.PreferredVersion.GroupVersion
-			break
-		}
-	}
-	if !foundPolicyGroup {
-		return "", nil
-	}
 	resourceList, err := discoveryClient.ServerResourcesForGroupVersion("v1")
 	if err != nil {
-		return "", err
+		return schema.GroupVersion{}, err
 	}
 	for _, resource := range resourceList.APIResources {
-		if resource.Name == EvictionSubresource && resource.Kind == EvictionKind {
+		if resource.Name == EvictionSubresource && resource.Kind == EvictionKind && len(resource.Group) > 0 && len(resource.Version) > 0 {
-			return policyGroupVersion, nil
+			return schema.GroupVersion{Group: resource.Group, Version: resource.Version}, nil
 		}
 	}
-	return "", nil
+	return schema.GroupVersion{}, nil
 }
 
 func (d *Helper) makeDeleteOptions() metav1.DeleteOptions {
@@ -157,7 +145,7 @@ func (d *Helper) DeletePod(pod corev1.Pod) error {
 }
 
 // EvictPod will evict the give pod, or return an error if it couldn't
-func (d *Helper) EvictPod(pod corev1.Pod, policyGroupVersion string) error {
+func (d *Helper) EvictPod(pod corev1.Pod, evictionGroupVersion schema.GroupVersion) error {
 	if d.DryRunStrategy == cmdutil.DryRunServer {
 		if err := d.DryRunVerifier.HasSupport(pod.GroupVersionKind()); err != nil {
 			return err
@@ -165,20 +153,30 @@ func (d *Helper) EvictPod(pod corev1.Pod, policyGroupVersion string) error {
 	}
 
 	delOpts := d.makeDeleteOptions()
-	eviction := &policyv1beta1.Eviction{
-		TypeMeta: metav1.TypeMeta{
-			APIVersion: policyGroupVersion,
-			Kind:       EvictionKind,
-		},
-		ObjectMeta: metav1.ObjectMeta{
-			Name:      pod.Name,
-			Namespace: pod.Namespace,
-		},
-		DeleteOptions: &delOpts,
-	}
 
-	// Remember to change change the URL manipulation func when Eviction's version change
+	switch evictionGroupVersion {
-	return d.Client.PolicyV1beta1().Evictions(eviction.Namespace).Evict(d.getContext(), eviction)
+	case policyv1.SchemeGroupVersion:
+		// send policy/v1 if the server supports it
+		eviction := &policyv1.Eviction{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      pod.Name,
+				Namespace: pod.Namespace,
+			},
+			DeleteOptions: &delOpts,
+		}
+		return d.Client.PolicyV1().Evictions(eviction.Namespace).Evict(context.TODO(), eviction)
+
+	default:
+		// otherwise, fall back to policy/v1beta1, supported by all servers that support the eviction subresource
+		eviction := &policyv1beta1.Eviction{
+			ObjectMeta: metav1.ObjectMeta{
+				Name:      pod.Name,
+				Namespace: pod.Namespace,
+			},
+			DeleteOptions: &delOpts,
+		}
+		return d.Client.PolicyV1beta1().Evictions(eviction.Namespace).Evict(context.TODO(), eviction)
+	}
 }
 
 // GetPodsForDeletion receives resource info for a node, and returns those pods as PodDeleteList,
@@ -259,20 +257,20 @@ func (d *Helper) DeleteOrEvictPods(pods []corev1.Pod) error {
 	}
 
 	if !d.DisableEviction {
-		policyGroupVersion, err := CheckEvictionSupport(d.Client)
+		evictionGroupVersion, err := CheckEvictionSupport(d.Client)
 		if err != nil {
 			return err
 		}
 
-		if len(policyGroupVersion) > 0 {
+		if !evictionGroupVersion.Empty() {
-			return d.evictPods(pods, policyGroupVersion, getPodFn)
+			return d.evictPods(pods, evictionGroupVersion, getPodFn)
 		}
 	}
 
 	return d.deletePods(pods, getPodFn)
 }
 
-func (d *Helper) evictPods(pods []corev1.Pod, policyGroupVersion string, getPodFn func(namespace, name string) (*corev1.Pod, error)) error {
+func (d *Helper) evictPods(pods []corev1.Pod, evictionGroupVersion schema.GroupVersion, getPodFn func(namespace, name string) (*corev1.Pod, error)) error {
 	returnCh := make(chan error, 1)
 	// 0 timeout means infinite, we use MaxInt64 to represent it.
 	var globalTimeout time.Duration
@@ -313,7 +311,7 @@ func (d *Helper) evictPods(pods []corev1.Pod, policyGroupVersion string, getPodF
 					refreshPod = false
 				}
 
-				err := d.EvictPod(activePod, policyGroupVersion)
+				err := d.EvictPod(activePod, evictionGroupVersion)
 				if err == nil {
 					break
 				} else if apierrors.IsNotFound(err) {

pkg/drain/drain_test.go

@@ -29,6 +29,7 @@ import (
 	"time"
 
 	corev1 "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"
@@ -214,13 +215,20 @@ func createPods(ifCreateNewPods bool) (map[string]corev1.Pod, []corev1.Pod) {
 	return podMap, podSlice
 }
 
+func addCoreNonEvictionSupport(t *testing.T, k *fake.Clientset) {
+	coreResources := &metav1.APIResourceList{
+		GroupVersion: "v1",
+	}
+	k.Resources = append(k.Resources, coreResources)
+}
+
 // addEvictionSupport implements simple fake eviction support on the fake.Clientset
-func addEvictionSupport(t *testing.T, k *fake.Clientset) {
+func addEvictionSupport(t *testing.T, k *fake.Clientset, version string) {
 	podsEviction := metav1.APIResource{
 		Name:    "pods/eviction",
 		Kind:    "Eviction",
-		Group:   "",
+		Group:   "policy",
-		Version: "v1",
+		Version: version,
 	}
 	coreResources := &metav1.APIResourceList{
 		GroupVersion: "v1",
@@ -238,13 +246,26 @@ func addEvictionSupport(t *testing.T, k *fake.Clientset) {
 			return false, nil, nil
 		}
 
-		eviction := *action.(ktest.CreateAction).GetObject().(*policyv1beta1.Eviction)
+		namespace := ""
+		name := ""
+		switch version {
+		case "v1":
+			eviction := *action.(ktest.CreateAction).GetObject().(*policyv1.Eviction)
+			namespace = eviction.Namespace
+			name = eviction.Name
+		case "v1beta1":
+			eviction := *action.(ktest.CreateAction).GetObject().(*policyv1beta1.Eviction)
+			namespace = eviction.Namespace
+			name = eviction.Name
+		default:
+			t.Errorf("unknown version %s", version)
+		}
 		// Avoid the lock
 		go func() {
-			err := k.CoreV1().Pods(eviction.Namespace).Delete(context.TODO(), eviction.Name, metav1.DeleteOptions{})
+			err := k.CoreV1().Pods(namespace).Delete(context.TODO(), name, metav1.DeleteOptions{})
 			if err != nil {
 				// Errorf because we can't call Fatalf from another goroutine
-				t.Errorf("failed to delete pod: %s/%s", eviction.Namespace, eviction.Name)
+				t.Errorf("failed to delete pod: %s/%s", namespace, name)
 			}
 		}()
 
@@ -253,22 +274,23 @@ func addEvictionSupport(t *testing.T, k *fake.Clientset) {
 }
 
 func TestCheckEvictionSupport(t *testing.T) {
-	for _, evictionSupported := range []bool{true, false} {
+	for _, evictionVersion := range []string{"", "v1", "v1beta1"} {
-		evictionSupported := evictionSupported
+		t.Run(fmt.Sprintf("evictionVersion=%v", evictionVersion),
-		t.Run(fmt.Sprintf("evictionSupported=%v", evictionSupported),
 			func(t *testing.T) {
 				k := fake.NewSimpleClientset()
-				if evictionSupported {
+				if len(evictionVersion) > 0 {
-					addEvictionSupport(t, k)
+					addEvictionSupport(t, k, evictionVersion)
+				} else {
+					addCoreNonEvictionSupport(t, k)
 				}
 
 				apiGroup, err := CheckEvictionSupport(k)
 				if err != nil {
 					t.Fatalf("unexpected error: %v", err)
 				}
-				expectedAPIGroup := ""
+				expectedAPIGroup := schema.GroupVersion{}
-				if evictionSupported {
+				if len(evictionVersion) > 0 {
-					expectedAPIGroup = "policy/v1"
+					expectedAPIGroup = schema.GroupVersion{Group: "policy", Version: evictionVersion}
 				}
 				if apiGroup != expectedAPIGroup {
 					t.Fatalf("expected apigroup %q, actual=%q", expectedAPIGroup, apiGroup)
@@ -312,7 +334,7 @@ func TestDeleteOrEvict(t *testing.T) {
 			}
 
 			// Create 4 pods, and try to remove the first 2
-			var expectedEvictions []policyv1beta1.Eviction
+			var expectedEvictions []policyv1.Eviction
 			var create []runtime.Object
 			deletePods := []corev1.Pod{}
 			for i := 1; i <= 4; i++ {
@@ -325,9 +347,7 @@ func TestDeleteOrEvict(t *testing.T) {
 					deletePods = append(deletePods, *pod)
 
 					if tc.evictionSupported && !tc.disableEviction {
-						eviction := policyv1beta1.Eviction{}
+						eviction := policyv1.Eviction{}
-						eviction.Kind = "Eviction"
-						eviction.APIVersion = "policy/v1"
 						eviction.Namespace = pod.Namespace
 						eviction.Name = pod.Name
 
@@ -344,7 +364,9 @@ func TestDeleteOrEvict(t *testing.T) {
 			// Build the fake client
 			k := fake.NewSimpleClientset(create...)
 			if tc.evictionSupported {
-				addEvictionSupport(t, k)
+				addEvictionSupport(t, k, "v1")
+			} else {
+				addCoreNonEvictionSupport(t, k)
 			}
 			h.Client = k
 			h.DisableEviction = tc.disableEviction
@@ -372,19 +394,19 @@ func TestDeleteOrEvict(t *testing.T) {
 			}
 
 			// Test that pods were evicted as expected
-			var actualEvictions []policyv1beta1.Eviction
+			var actualEvictions []policyv1.Eviction
 			for _, action := range k.Actions() {
 				if action.GetVerb() != "create" || action.GetResource().Resource != "pods" || action.GetSubresource() != "eviction" {
 					continue
 				}
-				eviction := *action.(ktest.CreateAction).GetObject().(*policyv1beta1.Eviction)
+				eviction := *action.(ktest.CreateAction).GetObject().(*policyv1.Eviction)
 				actualEvictions = append(actualEvictions, eviction)
 			}
 			sort.Slice(actualEvictions, func(i, j int) bool {
 				return actualEvictions[i].Name < actualEvictions[j].Name
 			})
 			if !reflect.DeepEqual(actualEvictions, expectedEvictions) {
-				t.Errorf("%s: unexpected evictions; actual %v; expected %v", tc.description, actualEvictions, expectedEvictions)
+				t.Errorf("%s: unexpected evictions; actual\n\t%v\nexpected\n\t%v", tc.description, actualEvictions, expectedEvictions)
 			}
 		})
 	}