use informer for VirtualMachineRestores in restore webhook

Signed-off-by: Michael Henriksen <[email protected]>
bk201 · Sep 10, 2020 · 98eedd7 · 98eedd7
1 parent b82a13a
commit 98eedd7
Show file tree

Hide file tree

Showing 9 changed files with 29 additions and 24 deletions.
diff --git a/api/openapi-spec/swagger.json b/api/openapi-spec/swagger.json
@@ -8779,21 +8779,8 @@
     }
    },
    "v1.Patch": {
-    "type": "object",
-    "properties": {
-     "patch": {
-      "type": "string"
-     },
-     "resourceName": {
-      "type": "string"
-     },
-     "resourceType": {
-      "type": "string"
-     },
-     "type": {
-      "type": "string"
-     }
-    }
+    "description": "Patch is provided to give a concrete name and type to the Kubernetes PATCH request body.",
+    "type": "object"
    },
    "v1.PersistentVolumeClaim": {
     "description": "PersistentVolumeClaim is a user's request for and claim to a persistent volume",

diff --git a/pkg/controller/virtinformers.go b/pkg/controller/virtinformers.go
@@ -401,6 +401,7 @@ func (f *kubeInformerFactory) VirtualMachineRestore() cache.SharedIndexInformer
 	return f.getInformer("vmRestoreInformer", func() cache.SharedIndexInformer {
 		lw := cache.NewListWatchFromClient(f.clientSet.GeneratedKubeVirtClient().SnapshotV1alpha1().RESTClient(), "virtualmachinerestores", k8sv1.NamespaceAll, fields.Everything())
 		return cache.NewSharedIndexInformer(lw, &snapshotv1.VirtualMachineRestore{}, f.defaultResync, cache.Indexers{
+			cache.NamespaceIndex: cache.MetaNamespaceIndexFunc,
 			"vm": func(obj interface{}) ([]string, error) {
 				vmr, ok := obj.(*snapshotv1.VirtualMachineRestore)
 				if !ok {

diff --git a/pkg/virt-api/api.go b/pkg/virt-api/api.go
@@ -669,6 +669,7 @@ func (app *virtAPIApp) Run() {
 	go webhookInformers.VMIInformer.Run(stopChan)
 	go webhookInformers.VMIPresetInformer.Run(stopChan)
 	go webhookInformers.NamespaceLimitsInformer.Run(stopChan)
+	go webhookInformers.VMRestoreInformer.Run(stopChan)
 	go kubeVirtInformer.Run(stopChan)
 	go configMapInformer.Run(stopChan)
 	go crdInformer.Run(stopChan)

diff --git a/pkg/virt-api/webhooks/utils.go b/pkg/virt-api/webhooks/utils.go
@@ -77,6 +77,7 @@ type Informers struct {
 	VMIPresetInformer       cache.SharedIndexInformer
 	NamespaceLimitsInformer cache.SharedIndexInformer
 	VMIInformer             cache.SharedIndexInformer
+	VMRestoreInformer       cache.SharedIndexInformer
 }
 
 // XXX fix this, this is a huge mess. Move informers to Admitter and Mutator structs.
@@ -112,6 +113,7 @@ func newInformers() *Informers {
 		VMIInformer:             kubeInformerFactory.VMI(),
 		VMIPresetInformer:       kubeInformerFactory.VirtualMachinePreset(),
 		NamespaceLimitsInformer: kubeInformerFactory.LimitRanges(),
+		VMRestoreInformer:       kubeInformerFactory.VirtualMachineRestore(),
 	}
 }
 

diff --git a/pkg/virt-api/webhooks/validating-webhook/admitters/BUILD.bazel b/pkg/virt-api/webhooks/validating-webhook/admitters/BUILD.bazel
@@ -36,6 +36,7 @@ go_library(
         "//vendor/k8s.io/apimachinery/pkg/types:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/validation/field:go_default_library",
+        "//vendor/k8s.io/client-go/tools/cache:go_default_library",
         "//vendor/kubevirt.io/containerized-data-importer/pkg/clone:go_default_library",
     ],
 )

diff --git a/pkg/virt-api/webhooks/validating-webhook/admitters/vmrestore-admitter.go b/pkg/virt-api/webhooks/validating-webhook/admitters/vmrestore-admitter.go
@@ -29,11 +29,13 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	k8sfield "k8s.io/apimachinery/pkg/util/validation/field"
+	"k8s.io/client-go/tools/cache"
 
 	v1 "kubevirt.io/client-go/api/v1"
 	snapshotv1 "kubevirt.io/client-go/apis/snapshot/v1alpha1"
 	"kubevirt.io/client-go/kubecli"
 	webhookutils "kubevirt.io/kubevirt/pkg/util/webhooks"
+	"kubevirt.io/kubevirt/pkg/virt-api/webhooks"
 	virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
 )
 
@@ -123,12 +125,14 @@ func (admitter *VMRestoreAdmitter) Admit(ar *v1beta1.AdmissionReview) *v1beta1.A
 			return webhookutils.ToAdmissionResponseError(err)
 		}
 
-		restores, err := admitter.Client.VirtualMachineRestore(ar.Request.Namespace).List(metav1.ListOptions{})
+		informers := webhooks.GetInformers()
+		objects, err := informers.VMRestoreInformer.GetIndexer().ByIndex(cache.NamespaceIndex, ar.Request.Namespace)
 		if err != nil {
 			return webhookutils.ToAdmissionResponseError(err)
 		}
 
-		for _, r := range restores.Items {
+		for _, obj := range objects {
+			r := obj.(*snapshotv1.VirtualMachineRestore)
 			if reflect.DeepEqual(r.Spec.Target, vmRestore.Spec.Target) &&
 				(r.Status == nil || r.Status.Complete == nil || !*r.Status.Complete) {
 				cause := metav1.StatusCause{

diff --git a/pkg/virt-api/webhooks/validating-webhook/admitters/vmrestore-admitter_test.go b/pkg/virt-api/webhooks/validating-webhook/admitters/vmrestore-admitter_test.go
@@ -113,7 +113,7 @@ var _ = Describe("Validating VirtualMachineRestore Admitter", func() {
 
 			resp := createTestVMRestoreAdmitter(config, nil).Admit(ar)
 			Expect(resp.Allowed).To(BeFalse())
-			Expect(resp.Result.Message).Should(ContainSubstring("Unexpected Resource"))
+			Expect(resp.Result.Message).Should(ContainSubstring("unexpected resource"))
 		})
 
 		It("should reject missing apigroup", func() {
@@ -540,11 +540,20 @@ func createTestVMRestoreAdmitter(
 	virtClient := kubecli.NewMockKubevirtClient(ctrl)
 	vmInterface := kubecli.NewMockVirtualMachineInterface(ctrl)
 	kubevirtClient := kubevirtfake.NewSimpleClientset(objs...)
+
 	virtClient.EXPECT().VirtualMachineSnapshot("default").
 		Return(kubevirtClient.SnapshotV1alpha1().VirtualMachineSnapshots("default"))
-	virtClient.EXPECT().VirtualMachineRestore("default").
-		Return(kubevirtClient.SnapshotV1alpha1().VirtualMachineRestores("default"))
 	virtClient.EXPECT().VirtualMachine(gomock.Any()).Return(vmInterface).AnyTimes()
+
+	restoreInformer, _ := testutils.NewFakeInformerFor(&snapshotv1.VirtualMachineRestore{})
+	webhooks.GetInformers().VMRestoreInformer = restoreInformer
+	for _, obj := range objs {
+		r, ok := obj.(*snapshotv1.VirtualMachineRestore)
+		if ok {
+			restoreInformer.GetIndexer().Add(r)
+		}
+	}
+
 	if vm == nil {
 		err := errors.NewNotFound(schema.GroupResource{Group: "kubevirt.io", Resource: "virtualmachines"}, "foo")
 		vmInterface.EXPECT().Get(gomock.Any(), gomock.Any()).Return(nil, err)

diff --git a/pkg/virt-api/webhooks/validating-webhook/admitters/vmsnapshot-admitter_test.go b/pkg/virt-api/webhooks/validating-webhook/admitters/vmsnapshot-admitter_test.go
@@ -56,7 +56,7 @@ var _ = Describe("Validating VirtualMachineSnapshot Admitter", func() {
 			ar := createSnapshotAdmissionReview(snapshot)
 			resp := createTestVMSnapshotAdmitter(config, nil).Admit(ar)
 			Expect(resp.Allowed).To(BeFalse())
-			Expect(resp.Result.Message).Should(Equal("Snapshot feature gate not enabled"))
+			Expect(resp.Result.Message).Should(Equal("snapshot feature gate not enabled"))
 		})
 	})
 
@@ -88,7 +88,7 @@ var _ = Describe("Validating VirtualMachineSnapshot Admitter", func() {
 
 			resp := createTestVMSnapshotAdmitter(config, nil).Admit(ar)
 			Expect(resp.Allowed).To(BeFalse())
-			Expect(resp.Result.Message).Should(ContainSubstring("Unexpected Resource"))
+			Expect(resp.Result.Message).Should(ContainSubstring("unexpected resource"))
 		})
 
 		It("should reject missing apigroup", func() {

diff --git a/tests/restore_test.go b/tests/restore_test.go
@@ -281,8 +281,8 @@ var _ = Describe("VirtualMachineRestore Tests", func() {
 									admissionregistrationv1beta1.Update,
 								},
 								Rule: admissionregistrationv1beta1.Rule{
-									APIGroups:   []string{"kubevirt.io"},
-									APIVersions: []string{"v1alpha3", "v1"},
+									APIGroups:   []string{v1.GroupName},
+									APIVersions: v1.ApiSupportedWebhookVersions,
 									Resources:   []string{"virtualmachines"},
 								},
 							}},