Remove DataVolume feature gate in favor of dynamic cdi api detection

Signed-off-by: David Vossel <[email protected]>

cmd/virt-handler/virt-handler.go

@@ -254,7 +254,7 @@ func (app *virtHandlerApp) Run() {
 		gracefulShutdownInformer,
 		int(app.WatchdogTimeoutDuration.Seconds()),
 		app.MaxDevices,
-		virtconfig.NewClusterConfig(factory.ConfigMap(), app.namespace),
+		virtconfig.NewClusterConfig(factory.ConfigMap(), factory.CRD(), app.namespace),
 		app.migrationTLSConfig,
 	)
 

manifests/generated/rbac-kubevirt.authorization.k8s.yaml.in

@@ -77,6 +77,14 @@ rules:
   verbs:
   - watch
   - list
+- apiGroups:
+  - apiextensions.k8s.io
+  resources:
+  - customresourcedefinitions
+  verbs:
+  - get
+  - list
+  - watch
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
@@ -242,6 +250,14 @@ rules:
   - get
   - list
   - watch
+- apiGroups:
+  - apiextensions.k8s.io
+  resources:
+  - customresourcedefinitions
+  verbs:
+  - get
+  - list
+  - watch
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding
@@ -301,6 +317,14 @@ rules:
   verbs:
   - create
   - patch
+- apiGroups:
+  - apiextensions.k8s.io
+  resources:
+  - customresourcedefinitions
+  verbs:
+  - get
+  - list
+  - watch
 ---
 apiVersion: rbac.authorization.k8s.io/v1
 kind: ClusterRoleBinding

manifests/generated/rbac-operator.authorization.k8s.yaml.in

@@ -187,6 +187,14 @@ rules:
   verbs:
   - watch
   - list
+- apiGroups:
+  - apiextensions.k8s.io
+  resources:
+  - customresourcedefinitions
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - ""
   resources:
@@ -281,6 +289,14 @@ rules:
   - get
   - list
   - watch
+- apiGroups:
+  - apiextensions.k8s.io
+  resources:
+  - customresourcedefinitions
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - kubevirt.io
   resources:
@@ -309,6 +325,14 @@ rules:
   verbs:
   - create
   - patch
+- apiGroups:
+  - apiextensions.k8s.io
+  resources:
+  - customresourcedefinitions
+  verbs:
+  - get
+  - list
+  - watch
 - apiGroups:
   - ""
   resources:

pkg/controller/virtinformers.go

@@ -99,6 +99,9 @@ type KubeInformerFactory interface {
 	// Fake CDI DataVolume informer used when feature gate is disabled
 	DummyDataVolume() cache.SharedIndexInformer
 
+	// CRD
+	CRD() cache.SharedIndexInformer
+
 	// Wachtes for KubeVirt objects
 	KubeVirt() cache.SharedIndexInformer
 
@@ -526,3 +529,19 @@ func (f *kubeInformerFactory) OperatorPodDisruptionBudget() cache.SharedIndexInf
 func (f *kubeInformerFactory) K8SInformerFactory() informers.SharedInformerFactory {
 	return f.k8sInformers
 }
+
+func (f *kubeInformerFactory) CRD() cache.SharedIndexInformer {
+	return f.getInformer("CRDInformer", func() cache.SharedIndexInformer {
+
+		ext, err := extclient.NewForConfig(f.clientSet.Config())
+		if err != nil {
+			panic(err)
+		}
+
+		restClient := ext.ApiextensionsV1beta1().RESTClient()
+
+		lw := cache.NewListWatchFromClient(restClient, "customresourcedefinitions", k8sv1.NamespaceAll, fields.Everything())
+
+		return cache.NewSharedIndexInformer(lw, &extv1beta1.CustomResourceDefinition{}, f.defaultResync, cache.Indexers{})
+	})
+}

pkg/testutils/BUILD.bazel

@@ -19,6 +19,7 @@ go_library(
         "//vendor/github.com/onsi/gomega/types:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
         "//vendor/k8s.io/api/policy/v1beta1:go_default_library",
+        "//vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/runtime:go_default_library",

pkg/testutils/mock_config.go

@@ -2,6 +2,7 @@ package testutils
 
 import (
 	v1 "k8s.io/api/core/v1"
+	extv1beta1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1"
 	v12 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/rand"
 	"k8s.io/client-go/tools/cache"
@@ -14,11 +15,34 @@ const (
 	namespace     = "kubevirt"
 )
 
-func NewFakeClusterConfig(cfgMap *v1.ConfigMap) (*virtconfig.ClusterConfig, cache.SharedIndexInformer) {
+func NewFakeClusterConfig(cfgMap *v1.ConfigMap) (*virtconfig.ClusterConfig, cache.SharedIndexInformer, cache.SharedIndexInformer) {
 	informer, _ := NewFakeInformerFor(&v1.ConfigMap{})
+	crdInformer, _ := NewFakeInformerFor(&extv1beta1.CustomResourceDefinition{})
+
 	copy := copy(cfgMap)
 	informer.GetStore().Add(copy)
-	return virtconfig.NewClusterConfig(informer, namespace), informer
+	crdInformer.GetStore().Add(&extv1beta1.CustomResourceDefinition{
+		Spec: extv1beta1.CustomResourceDefinitionSpec{
+			Names: extv1beta1.CustomResourceDefinitionNames{
+				Kind: "DataVolume",
+			},
+		},
+	})
+	return virtconfig.NewClusterConfig(informer, crdInformer, namespace), informer, crdInformer
+}
+
+func RemoveDataVolumeAPI(crdInformer cache.SharedIndexInformer) {
+	crdInformer.GetStore().Replace(nil, "")
+}
+
+func AddDataVolumeAPI(crdInformer cache.SharedIndexInformer) {
+	crdInformer.GetStore().Add(&extv1beta1.CustomResourceDefinition{
+		Spec: extv1beta1.CustomResourceDefinitionSpec{
+			Names: extv1beta1.CustomResourceDefinitionNames{
+				Kind: "DataVolume",
+			},
+		},
+	})
 }
 
 func UpdateFakeClusterConfig(informer cache.SharedIndexInformer, cfgMap *v1.ConfigMap) {

pkg/virt-api/api.go

@@ -1021,21 +1021,23 @@ func (app *virtAPIApp) Run() {
 	webhookInformers := webhooks.GetInformers()
 	kubeInformerFactory := controller.NewKubeInformerFactory(app.virtCli.RestClient(), app.virtCli, app.namespace)
 	configMapInformer := kubeInformerFactory.ConfigMap()
+	crdInformer := kubeInformerFactory.CRD()
 
 	stopChan := make(chan struct{}, 1)
 	defer close(stopChan)
 	go webhookInformers.VMIInformer.Run(stopChan)
 	go webhookInformers.VMIPresetInformer.Run(stopChan)
 	go webhookInformers.NamespaceLimitsInformer.Run(stopChan)
 	go configMapInformer.Run(stopChan)
+	go crdInformer.Run(stopChan)
 
 	cache.WaitForCacheSync(stopChan,
 		webhookInformers.VMIInformer.HasSynced,
 		webhookInformers.VMIPresetInformer.HasSynced,
 		webhookInformers.NamespaceLimitsInformer.HasSynced,
 		configMapInformer.HasSynced)
 
-	app.clusterConfig = virtconfig.NewClusterConfig(configMapInformer, app.namespace)
+	app.clusterConfig = virtconfig.NewClusterConfig(configMapInformer, crdInformer, app.namespace)
 
 	// Verify/create webhook endpoint.
 	err = app.createWebhook()

pkg/virt-api/rest/subresource_test.go

@@ -92,7 +92,7 @@ var _ = Describe("VirtualMachineInstance Subresources", func() {
 			vmi.Status.Phase = v1.Running
 			vmi.ObjectMeta.SetUID(uuid.NewUUID())
 
-			config, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
+			config, _, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
 			templateService := services.NewTemplateService("whatever", "whatever", "whatever", "whatever", pvcCache, app.VirtCli, config)
 
 			pod, err := templateService.RenderLaunchManifest(vmi)
@@ -265,7 +265,7 @@ var _ = Describe("VirtualMachineInstance Subresources", func() {
 			vmi.Status.Phase = v1.Running
 			vmi.ObjectMeta.SetUID(uuid.NewUUID())
 
-			config, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
+			config, _, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
 			templateService := services.NewTemplateService("whatever", "whatever", "whatever", "whatever", pvcCache, app.VirtCli, config)
 
 			pod, err := templateService.RenderLaunchManifest(vmi)

pkg/virt-api/webhooks/mutating-webhook/mutators/vmi-mutator_test.go

@@ -135,7 +135,7 @@ var _ = Describe("VirtualMachineInstance Mutator", func() {
 		)
 
 		mutator = &VMIsMutator{}
-		mutator.ClusterConfig, configMapInformer = testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
+		mutator.ClusterConfig, configMapInformer, _ = testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
 	})
 
 	It("should apply presets on VMI create", func() {

pkg/virt-api/webhooks/validating-webhook/admitters/migration-create-admitter_test.go

@@ -37,7 +37,7 @@ import (
 )
 
 var _ = Describe("Validating MigrationCreate Admitter", func() {
-	config, configMapInformer := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
+	config, configMapInformer, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
 	migrationCreateAdmitter := &MigrationCreateAdmitter{ClusterConfig: config}
 
 	enableFeatureGate := func(featureGate string) {

pkg/virt-api/webhooks/validating-webhook/admitters/migration-update-admitter_test.go

@@ -37,7 +37,7 @@ import (
 
 var _ = Describe("Validating MigrationUpdate Admitter", func() {
 	migrationUpdateAdmitter := &MigrationUpdateAdmitter{}
-	_, configMapInformer := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
+	_, configMapInformer, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
 
 	enableFeatureGate := func(featureGate string) {
 		testutils.UpdateFakeClusterConfig(configMapInformer, &k8sv1.ConfigMap{

pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter.go

@@ -1245,10 +1245,10 @@ func validateVolumes(field *k8sfield.Path, volumes []v1.Volume, config *virtconf
 			volumeSourceSetCount++
 		}
 		if volume.DataVolume != nil {
-			if !config.DataVolumesEnabled() {
+			if !config.HasDataVolumeAPI() {
 				causes = append(causes, metav1.StatusCause{
 					Type:    metav1.CauseTypeFieldValueInvalid,
-					Message: "DataVolume feature gate is not enabled",
+					Message: "DataVolume api is not present in cluster. CDI must be installed for DataVolume support.",
 					Field:   field.Index(idx).String(),
 				})
 			}

pkg/virt-api/webhooks/validating-webhook/admitters/vmi-create-admitter_test.go

@@ -45,7 +45,7 @@ import (
 )
 
 var _ = Describe("Validating VMICreate Admitter", func() {
-	config, configMapInformer := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
+	config, configMapInformer, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
 	vmiCreateAdmitter := &VMICreateAdmitter{ClusterConfig: config}
 
 	dnsConfigTestOption := "test"
@@ -2330,7 +2330,7 @@ var _ = Describe("Validating VMICreate Admitter", func() {
 })
 
 var _ = Describe("Function getNumberOfPodInterfaces()", func() {
-	config, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
+	config, _, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
 
 	It("should work for empty network list", func() {
 		spec := &v1.VirtualMachineInstanceSpec{}

pkg/virt-api/webhooks/validating-webhook/admitters/vmirs-admitter_test.go

@@ -36,7 +36,7 @@ import (
 )
 
 var _ = Describe("Validating VMIRS Admitter", func() {
-	config, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
+	config, _, _ := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
 	vmirsAdmitter := &VMIRSAdmitter{ClusterConfig: config}
 
 	table.DescribeTable("should reject documents containing unknown or missing fields for", func(data string, validationResult string, gvr metav1.GroupVersionResource, review func(ar *v1beta1.AdmissionReview) *v1beta1.AdmissionResponse) {

pkg/virt-api/webhooks/validating-webhook/admitters/vms-admitter_test.go

@@ -39,26 +39,13 @@ import (
 	cdiv1 "kubevirt.io/containerized-data-importer/pkg/apis/core/v1alpha1"
 	"kubevirt.io/kubevirt/pkg/testutils"
 	"kubevirt.io/kubevirt/pkg/virt-api/webhooks"
-	virtconfig "kubevirt.io/kubevirt/pkg/virt-config"
 )
 
 var _ = Describe("Validating VM Admitter", func() {
-	config, configMapInformer := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
+	config, _, crdInformer := testutils.NewFakeClusterConfig(&k8sv1.ConfigMap{})
 	vmsAdmitter := &VMsAdmitter{ClusterConfig: config}
 
 	notRunning := false
-	enableFeatureGate := func(featureGate string) {
-		testutils.UpdateFakeClusterConfig(configMapInformer, &k8sv1.ConfigMap{
-			Data: map[string]string{virtconfig.FeatureGatesKey: featureGate},
-		})
-	}
-	disableFeatureGates := func() {
-		testutils.UpdateFakeClusterConfig(configMapInformer, &k8sv1.ConfigMap{})
-	}
-
-	AfterEach(func() {
-		disableFeatureGates()
-	})
 
 	It("reject invalid VirtualMachineInstance spec", func() {
 		vmi := v1.NewMinimalVMI("testvmi")
@@ -164,7 +151,7 @@ var _ = Describe("Validating VM Admitter", func() {
 			},
 		}
 
-		enableFeatureGate("DataVolumes")
+		testutils.AddDataVolumeAPI(crdInformer)
 		resp := vmsAdmitter.Admit(ar)
 		Expect(resp.Allowed).To(BeTrue())
 	})
@@ -209,7 +196,7 @@ var _ = Describe("Validating VM Admitter", func() {
 			},
 		}
 
-		enableFeatureGate("DataVolumes")
+		testutils.AddDataVolumeAPI(crdInformer)
 		resp := vmsAdmitter.Admit(ar)
 		Expect(resp.Allowed).To(BeFalse())
 		Expect(len(resp.Result.Details.Causes)).To(Equal(1))
@@ -225,7 +212,7 @@ var _ = Describe("Validating VM Admitter", func() {
 					VolumeSource: volumeSource,
 				})
 
-				enableFeatureGate("DataVolumes")
+				testutils.AddDataVolumeAPI(crdInformer)
 				causes := validateVolumes(k8sfield.NewPath("fake"), vmi.Spec.Volumes, config)
 				Expect(len(causes)).To(Equal(0))
 			},
@@ -248,7 +235,7 @@ var _ = Describe("Validating VM Admitter", func() {
 				VolumeSource: v1.VolumeSource{DataVolume: &v1.DataVolumeSource{Name: "fake"}},
 			})
 
-			disableFeatureGates()
+			testutils.RemoveDataVolumeAPI(crdInformer)
 			causes := validateVolumes(k8sfield.NewPath("fake"), vmi.Spec.Volumes, config)
 			Expect(len(causes)).To(Equal(1))
 			Expect(causes[0].Field).To(Equal("fake[0]"))
@@ -261,7 +248,7 @@ var _ = Describe("Validating VM Admitter", func() {
 				VolumeSource: v1.VolumeSource{DataVolume: &v1.DataVolumeSource{Name: ""}},
 			})
 
-			enableFeatureGate("DataVolumes")
+			testutils.AddDataVolumeAPI(crdInformer)
 			causes := validateVolumes(k8sfield.NewPath("fake"), vmi.Spec.Volumes, config)
 			Expect(len(causes)).To(Equal(1))
 			Expect(string(causes[0].Type)).To(Equal("FieldValueRequired"))

pkg/virt-config/BUILD.bazel

@@ -15,6 +15,7 @@ go_library(
         "//staging/src/kubevirt.io/client-go/kubecli:go_default_library",
         "//staging/src/kubevirt.io/client-go/log:go_default_library",
         "//vendor/k8s.io/api/core/v1:go_default_library",
+        "//vendor/k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1beta1:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/api/resource:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",