From 82de12dfb6801ec386e3a3893a3408c130623e6c Mon Sep 17 00:00:00 2001 From: Orel Misan Date: Wed, 3 Apr 2024 06:23:40 +0300 Subject: [PATCH] Eviction admitter, tests: Refactor regular pod admission Currently, it is not possible to filter eviction requests based on pod labels [1][2]. For this reason, the pod eviction admitter has to catch all eviction requests in the cluster and only handle eviction requests on virt-launcher pods. Eviction requests on non virt-launcher pods should be approved. Since the existing tests have global variables set by a global BeforeEach function, it is difficult to refactor in place. Create a new Describe clause in order to hold the refactored test. Add new helper functions. For additional details please see [3]. [1] https://github.com/kubernetes/kubernetes/issues/110169#issuecomment-1140512056 [2] https://kubernetes.slack.com/archives/C0EG7JC6T/p1707054818877809 [3] https://github.com/kubevirt/kubevirt/pull/11286 Signed-off-by: Orel Misan --- .../validating-webhook/admitters/BUILD.bazel | 1 + .../admitters/pod-eviction-admitter_test.go | 149 ++++++++++++++---- 2 files changed, 118 insertions(+), 32 deletions(-) diff --git a/pkg/virt-api/webhooks/validating-webhook/admitters/BUILD.bazel b/pkg/virt-api/webhooks/validating-webhook/admitters/BUILD.bazel index a95e0246fa8b..6f0ef7a73640 100644 --- a/pkg/virt-api/webhooks/validating-webhook/admitters/BUILD.bazel +++ b/pkg/virt-api/webhooks/validating-webhook/admitters/BUILD.bazel @@ -139,6 +139,7 @@ go_test( "//vendor/k8s.io/api/authentication/v1:go_default_library", "//vendor/k8s.io/api/authorization/v1:go_default_library", "//vendor/k8s.io/api/core/v1:go_default_library", + "//vendor/k8s.io/api/policy/v1:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/equality:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library", "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library", diff --git a/pkg/virt-api/webhooks/validating-webhook/admitters/pod-eviction-admitter_test.go b/pkg/virt-api/webhooks/validating-webhook/admitters/pod-eviction-admitter_test.go index 486cafc97a4d..7e9c93ae4606 100644 --- a/pkg/virt-api/webhooks/validating-webhook/admitters/pod-eviction-admitter_test.go +++ b/pkg/virt-api/webhooks/validating-webhook/admitters/pod-eviction-admitter_test.go @@ -24,18 +24,26 @@ import ( "fmt" "net/http" + "kubevirt.io/kubevirt/pkg/pointer" + "github.com/golang/mock/gomock" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + admissionv1 "k8s.io/api/admission/v1" k8sv1 "k8s.io/api/core/v1" + policyv1 "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" + "k8s.io/client-go/kubernetes/fake" "k8s.io/client-go/testing" virtv1 "kubevirt.io/api/core/v1" + "kubevirt.io/client-go/kubecli" "kubevirt.io/kubevirt/pkg/testutils" @@ -357,38 +365,6 @@ var _ = Describe("Pod eviction admitter", func() { }) }) - Context("Not a virt launcher pod", func() { - - It("Should allow any review requests", func() { - - By("Composing a dummy admission request on a virt-launcher pod") - pod := &k8sv1.Pod{ - ObjectMeta: metav1.ObjectMeta{ - Namespace: testns, - Name: "foo", - }, - } - - ar := &admissionv1.AdmissionReview{ - Request: &admissionv1.AdmissionRequest{ - Name: pod.Name, - Namespace: pod.Namespace, - }, - } - - kubeClient.Fake.PrependReactor("get", "pods", func(action testing.Action) (handled bool, obj runtime.Object, err error) { - get, ok := action.(testing.GetAction) - Expect(ok).To(BeTrue()) - Expect(pod.Namespace).To(Equal(get.GetNamespace())) - Expect(pod.Name).To(Equal(get.GetName())) - return true, pod, nil - }) - - resp := podEvictionAdmitter.Admit(ar) - Expect(resp.Allowed).To(BeTrue()) - }) - }) - prepareAdmissionReview := func(vmi *virtv1.VirtualMachineInstance, nodeName string, migratable bool) *admissionv1.AdmissionReview { dryRun := false pod := &k8sv1.Pod{ @@ -508,3 +484,112 @@ var _ = Describe("Pod eviction admitter", func() { }) }) }) + +var _ = Describe("Pod eviction admitter", func() { + const ( + testNamespace = "test-ns" + testNodeName = "node01" + ) + + When("an AdmissionReview request for the eviction of a regular pod is admitted", func() { + It("should be allowed", func() { + const evictedPodName = "my-pod" + + evictedPod := newPod(testNamespace, evictedPodName, testNodeName) + kubeClient := fake.NewSimpleClientset(evictedPod) + + virtClient := kubecli.NewMockKubevirtClient(gomock.NewController(GinkgoT())) + virtClient.EXPECT().CoreV1().Return(kubeClient.CoreV1()).AnyTimes() + + admitter := admitters.PodEvictionAdmitter{ + ClusterConfig: newClusterConfig(nil), + VirtClient: virtClient, + } + + actualAdmissionResponse := admitter.Admit( + newAdmissionReview(evictedPod.Namespace, evictedPod.Name, false), + ) + + Expect(actualAdmissionResponse).To(Equal(allowedAdmissionResponse())) + Expect(kubeClient.Fake.Actions()).To(HaveLen(1)) + }) + }) +}) + +func newClusterConfig(clusterWideEvictionStrategy *virtv1.EvictionStrategy) *virtconfig.ClusterConfig { + const ( + kubevirtCRName = "kubevirt" + kubevirtNamespace = "kubevirt" + ) + + kv := kubecli.NewMinimalKubeVirt(kubevirtCRName) + kv.Namespace = kubevirtNamespace + + kv.Spec.Configuration.EvictionStrategy = clusterWideEvictionStrategy + + clusterConfig, _, _ := testutils.NewFakeClusterConfigUsingKV(kv) + return clusterConfig +} + +func newPod(namespace, name, nodeName string) *k8sv1.Pod { + return &k8sv1.Pod{ + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + Spec: k8sv1.PodSpec{ + NodeName: nodeName, + }, + } +} + +func newAdmissionReview(evictedPodNamespace, evictedPodName string, isDryRun bool) *admissionv1.AdmissionReview { + return &admissionv1.AdmissionReview{ + Request: &admissionv1.AdmissionRequest{ + Namespace: evictedPodNamespace, + Name: evictedPodName, + DryRun: pointer.P(isDryRun), + Kind: metav1.GroupVersionKind{ + Group: "policy", + Version: "v1", + Kind: "Eviction", + }, + Resource: metav1.GroupVersionResource{ + Group: "", + Version: "v1", + Resource: "pods", + }, + SubResource: "eviction", + RequestKind: &metav1.GroupVersionKind{ + Group: "policy", + Version: "v1", + Kind: "Eviction", + }, + RequestResource: &metav1.GroupVersionResource{ + Group: "", + Version: "v1", + Resource: "pods", + }, + RequestSubResource: "eviction", + Operation: "CREATE", + Object: runtime.RawExtension{ + Object: &policyv1.Eviction{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "policy/v1", + Kind: "Eviction", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: evictedPodNamespace, + Name: evictedPodName, + }, + }, + }, + }, + } +} + +func allowedAdmissionResponse() *admissionv1.AdmissionResponse { + return &admissionv1.AdmissionResponse{ + Allowed: true, + } +}