WIP: Adjust downward metrics to feature KV lfecycle

PoC of design proposal [feature
configurables](kubevirt/community#316) using the
downward metrics feature as an example. It deprecates the
`DownwardMetrics` feature gate in favor of a configurable spec
`spec.configuration.downwardMetris: {}`.

Signed-off-by: Javier Cano Cano <jcanocan@redhat.com>

api/openapi-spec/swagger.json

@@ -13538,6 +13538,10 @@
    "v1.DownwardMetrics": {
     "type": "object"
    },
+   "v1.DownwardMetricsConfiguration": {
+    "description": "DownwardMetricsConfiguration enables the downward metrics feature.",
+    "type": "object"
+   },
    "v1.DownwardMetricsVolumeSource": {
     "description": "DownwardMetricsVolumeSource adds a very small disk to VMIs which contains a limited view of host and guest metrics. The disk content is compatible with vhostmd (https://github.com/vhostmd/vhostmd) and vm-dump-metrics.",
     "type": "object"
@@ -14421,6 +14425,9 @@
      "developerConfiguration": {
       "$ref": "#/definitions/v1.DeveloperConfiguration"
      },
+     "downwardMetrics": {
+      "$ref": "#/definitions/v1.DownwardMetricsConfiguration"
+     },
      "emulatedMachines": {
       "description": "Deprecated. Use architectureConfiguration instead.",
       "type": "array",

manifests/generated/kv-resource.yaml

@@ -366,6 +366,10 @@ spec:
                           in case hardware-assisted emulation is not available. Defaults to false
                         type: boolean
                     type: object
+                  downwardMetrics:
+                    description: DownwardMetricsConfiguration enables the downward
+                      metrics feature.
+                    type: object
                   emulatedMachines:
                     description: Deprecated. Use architectureConfiguration instead.
                     items:
@@ -3593,6 +3597,10 @@ spec:
                           in case hardware-assisted emulation is not available. Defaults to false
                         type: boolean
                     type: object
+                  downwardMetrics:
+                    description: DownwardMetricsConfiguration enables the downward
+                      metrics feature.
+                    type: object
                   emulatedMachines:
                     description: Deprecated. Use architectureConfiguration instead.
                     items:

pkg/handler-launcher-com/cmd/v1/cmd.proto

@@ -98,6 +98,7 @@ message ClusterConfig{
   bool FreePageReportingDisabled = 2;
   bool BochsDisplayForEFIGuests = 3;
   bool SerialConsoleLogDisabled = 4;
+  bool DownwardMetricsEnabled = 5;
 }
 
 message InterfaceBindingMigration{

pkg/virt-api/webhooks/fuzz/fuzz_test.go

@@ -180,7 +180,7 @@ func fuzzKubeVirtConfig(seed int64) *virtconfig.ClusterConfig {
 				virtconfig.VirtIOFSGate,
 				deprecation.MacvtapGate,
 				deprecation.PasstGate,
-				virtconfig.DownwardMetricsFeatureGate,
+				deprecation.DownwardMetricsFeatureGate,
 				deprecation.NonRoot,
 				virtconfig.Root,
 				virtconfig.ClusterProfiler,

pkg/virt-api/webhooks/validating-webhook/admitters/BUILD.bazel

@@ -27,7 +27,6 @@ go_library(
     deps = [
         "//pkg/apimachinery/patch:go_default_library",
         "//pkg/controller:go_default_library",
-        "//pkg/downwardmetrics:go_default_library",
         "//pkg/hooks:go_default_library",
         "//pkg/instancetype:go_default_library",
         "//pkg/liveupdate/memory:go_default_library",

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

@@ -42,7 +42,6 @@ import (
 
 	v1 "kubevirt.io/api/core/v1"
 
-	"kubevirt.io/kubevirt/pkg/downwardmetrics"
 	"kubevirt.io/kubevirt/pkg/hooks"
 	netadmitter "kubevirt.io/kubevirt/pkg/network/admitter"
 	"kubevirt.io/kubevirt/pkg/storage/reservation"
@@ -211,22 +210,6 @@ func ValidateVirtualMachineInstanceSpec(field *k8sfield.Path, spec *v1.VirtualMa
 	causes = append(causes, validateVSOCK(field, spec, config)...)
 	causes = append(causes, validatePersistentReservation(field, spec, config)...)
 	causes = append(causes, validatePersistentState(field, spec, config)...)
-	causes = append(causes, validateDownwardMetrics(field, spec, config)...)
-
-	return causes
-}
-
-func validateDownwardMetrics(field *k8sfield.Path, spec *v1.VirtualMachineInstanceSpec, config *virtconfig.ClusterConfig) []metav1.StatusCause {
-	var causes []metav1.StatusCause
-
-	// Check if serial and feature gate is enabled
-	if downwardmetrics.HasDevice(spec) && !config.DownwardMetricsEnabled() {
-		causes = append(causes, metav1.StatusCause{
-			Type:    metav1.CauseTypeFieldValueInvalid,
-			Message: "downwardMetrics virtio serial is not allowed: DownwardMetrics feature gate is not enabled",
-			Field:   field.Child("domain", "devices", "downwardMetrics").String(),
-		})
-	}
 
 	return causes
 }
@@ -1745,14 +1728,6 @@ func validateVolumes(field *k8sfield.Path, volumes []v1.Volume, config *virtconf
 			}
 		}
 
-		if volume.DownwardMetrics != nil && !config.DownwardMetricsEnabled() {
-			causes = append(causes, metav1.StatusCause{
-				Type:    metav1.CauseTypeFieldValueInvalid,
-				Message: "downwardMetrics disks are not allowed: DownwardMetrics feature gate is not enabled.",
-				Field:   field.Index(idx).String(),
-			})
-		}
-
 		// validate HostDisk data
 		if hostDisk := volume.HostDisk; hostDisk != nil {
 			if !config.HostDiskEnabled() {

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

@@ -2525,83 +2525,8 @@ var _ = Describe("Validating VMICreate Admitter", func() {
 			})
 		})
 	})
-	Context("with downwardmetrics virtio serial", func() {
-		var vmi *v1.VirtualMachineInstance
-		validate := func() []metav1.StatusCause {
-			return validateDownwardMetrics(k8sfield.NewPath("fake"), &vmi.Spec, config)
-		}
-
-		BeforeEach(func() {
-			vmi = api.NewMinimalVMI("testvmi")
-			vmi.Spec.Domain.Devices.DownwardMetrics = &v1.DownwardMetrics{}
-		})
-
-		It("should accept a single virtio serial", func() {
-			enableFeatureGate(virtconfig.DownwardMetricsFeatureGate)
-			causes := validate()
-			Expect(causes).To(BeEmpty())
-		})
-
-		It("should reject if feature gate is not enabled", func() {
-			causes := validate()
-			Expect(causes).To(HaveLen(1))
-			Expect(causes).To(ContainElement(metav1.StatusCause{Type: metav1.CauseTypeFieldValueInvalid,
-				Field:   "fake.domain.devices.downwardMetrics",
-				Message: "downwardMetrics virtio serial is not allowed: DownwardMetrics feature gate is not enabled"}))
-		})
-	})
 
 	Context("with volume", func() {
-		It("should accept a single downwardmetrics volume", func() {
-			enableFeatureGate(virtconfig.DownwardMetricsFeatureGate)
-			vmi := api.NewMinimalVMI("testvmi")
-
-			vmi.Spec.Volumes = append(vmi.Spec.Volumes, v1.Volume{
-				Name: "testDownwardMetrics",
-				VolumeSource: v1.VolumeSource{
-					DownwardMetrics: &v1.DownwardMetricsVolumeSource{},
-				},
-			})
-
-			causes := validateVolumes(k8sfield.NewPath("fake"), vmi.Spec.Volumes, config)
-			Expect(causes).To(BeEmpty())
-		})
-		It("should reject downwardMetrics volumes if the feature gate is not enabled", func() {
-			vmi := api.NewMinimalVMI("testvmi")
-
-			vmi.Spec.Volumes = append(vmi.Spec.Volumes, v1.Volume{
-				Name: "testDownwardMetrics",
-				VolumeSource: v1.VolumeSource{
-					DownwardMetrics: &v1.DownwardMetricsVolumeSource{},
-				},
-			})
-
-			causes := validateVolumes(k8sfield.NewPath("fake"), vmi.Spec.Volumes, config)
-			Expect(causes).To(HaveLen(1))
-			Expect(causes[0].Message).To(ContainSubstring("downwardMetrics disks are not allowed: DownwardMetrics feature gate is not enabled."))
-		})
-		It("should reject downwardMetrics volumes if more than one exist", func() {
-			enableFeatureGate(virtconfig.DownwardMetricsFeatureGate)
-			vmi := api.NewMinimalVMI("testvmi")
-
-			vmi.Spec.Volumes = append(vmi.Spec.Volumes,
-				v1.Volume{
-					Name: "testDownwardMetrics",
-					VolumeSource: v1.VolumeSource{
-						DownwardMetrics: &v1.DownwardMetricsVolumeSource{},
-					},
-				},
-				v1.Volume{
-					Name: "testDownwardMetrics1",
-					VolumeSource: v1.VolumeSource{
-						DownwardMetrics: &v1.DownwardMetricsVolumeSource{},
-					},
-				},
-			)
-			causes := validateVolumes(k8sfield.NewPath("fake"), vmi.Spec.Volumes, config)
-			Expect(causes).To(HaveLen(1))
-			Expect(causes[0].Message).To(ContainSubstring("fake must have max one downwardMetric volume set"))
-		})
 		It("should reject hostDisk volumes if the feature gate is not enabled", func() {
 			vmi := api.NewMinimalVMI("testvmi")
 

pkg/virt-config/configuration_test.go

@@ -789,5 +789,9 @@ var _ = Describe("test configuration", func() {
 		It("SR-IOV live migration feature gate", func() {
 			Expect(clusterConfig.SRIOVLiveMigrationEnabled()).To(BeTrue())
 		})
+
+		It("DownwardMetrics feature gate", func() {
+			Expect(clusterConfig.DownwardMetricsEnabled()).To(BeTrue())
+		})
 	})
 })

pkg/virt-config/deprecation/feature-gates.go

@@ -38,13 +38,14 @@ const (
 )
 
 const (
-	LiveMigrationGate      = "LiveMigration"      // GA
-	SRIOVLiveMigrationGate = "SRIOVLiveMigration" // GA
-	NonRoot                = "NonRoot"            // GA
-	PSA                    = "PSA"                // GA
-	CPUNodeDiscoveryGate   = "CPUNodeDiscovery"   // GA
-	PasstGate              = "Passt"              // Deprecated
-	MacvtapGate            = "Macvtap"            // Deprecated
+	LiveMigrationGate          = "LiveMigration"      // GA
+	SRIOVLiveMigrationGate     = "SRIOVLiveMigration" // GA
+	NonRoot                    = "NonRoot"            // GA
+	PSA                        = "PSA"                // GA
+	CPUNodeDiscoveryGate       = "CPUNodeDiscovery"   // GA
+	DownwardMetricsFeatureGate = "DownwardMetrics"    // GA
+	PasstGate                  = "Passt"              // Deprecated
+	MacvtapGate                = "Macvtap"            // Deprecated
 )
 
 type FeatureGate struct {
@@ -62,6 +63,7 @@ func init() {
 	RegisterFeatureGate(FeatureGate{Name: NonRoot, State: GA})
 	RegisterFeatureGate(FeatureGate{Name: PSA, State: GA})
 	RegisterFeatureGate(FeatureGate{Name: CPUNodeDiscoveryGate, State: GA})
+	RegisterFeatureGate(FeatureGate{Name: DownwardMetricsFeatureGate, State: GA})
 
 	RegisterFeatureGate(FeatureGate{Name: PasstGate, State: Discontinued, Message: PasstDiscontinueMessage, VmiSpecUsed: passtApiUsed})
 	RegisterFeatureGate(FeatureGate{Name: MacvtapGate, State: Discontinued, Message: MacvtapDiscontinueMessage, VmiSpecUsed: macvtapApiUsed})

pkg/virt-config/feature-gates.go

@@ -40,10 +40,9 @@ const (
 	HostDiskGate          = "HostDisk"
 	VirtIOFSGate          = "ExperimentalVirtiofsSupport"
 
-	DownwardMetricsFeatureGate = "DownwardMetrics"
-	Root                       = "Root"
-	ClusterProfiler            = "ClusterProfiler"
-	WorkloadEncryptionSEV      = "WorkloadEncryptionSEV"
+	Root                  = "Root"
+	ClusterProfiler       = "ClusterProfiler"
+	WorkloadEncryptionSEV = "WorkloadEncryptionSEV"
 	// DockerSELinuxMCSWorkaround sets the SELinux level of all the non-compute virt-launcher containers to "s0".
 	DockerSELinuxMCSWorkaround = "DockerSELinuxMCSWorkaround"
 	VSOCKGate                  = "VSOCK"
@@ -122,7 +121,7 @@ func (config *ClusterConfig) NUMAEnabled() bool {
 }
 
 func (config *ClusterConfig) DownwardMetricsEnabled() bool {
-	return config.isFeatureGateEnabled(DownwardMetricsFeatureGate)
+	return config.isFeatureGateEnabled(deprecation.DownwardMetricsFeatureGate)
 }
 
 func (config *ClusterConfig) IgnitionEnabled() bool {

pkg/virt-config/virt-config.go

@@ -107,6 +107,13 @@ func IsS390X(arch string) bool {
 	return arch == "s390x"
 }
 
+func (c *ClusterConfig) IsDownwardMetricsEnabled() bool {
+	if c.GetConfig().DownwardMetrics != nil {
+		return true
+	}
+	return false
+}
+
 func (c *ClusterConfig) GetMemBalloonStatsPeriod() uint32 {
 	return *c.GetConfig().MemBalloonStatsPeriod
 }

pkg/virt-controller/watch/BUILD.bazel

@@ -20,6 +20,7 @@ go_library(
         "//pkg/certificates/bootstrap:go_default_library",
         "//pkg/container-disk:go_default_library",
         "//pkg/controller:go_default_library",
+        "//pkg/downwardmetrics:go_default_library",
         "//pkg/healthz:go_default_library",
         "//pkg/hooks:go_default_library",
         "//pkg/instancetype:go_default_library",

pkg/virt-controller/watch/vmi.go

@@ -27,6 +27,8 @@ import (
 	"strings"
 	"time"
 
+	"kubevirt.io/kubevirt/pkg/downwardmetrics"
+
 	"kubevirt.io/kubevirt/pkg/virt-controller/network"
 
 	"kubevirt.io/kubevirt/pkg/virt-controller/watch/topology"
@@ -1060,6 +1062,10 @@ func (c *VMIController) sync(vmi *virtv1.VirtualMachineInstance, pod *k8sv1.Pod,
 		if validateErr := errors.Join(validateErrors...); validateErrors != nil {
 			return &syncErrorImpl{fmt.Errorf("failed create validation: %v", validateErr), "FailedCreateValidation"}
 		}
+		if (downwardmetrics.HasDownwardMetricDisk(vmi) || downwardmetrics.HasDevice(&vmi.Spec)) && !c.clusterConfig.IsDownwardMetricsEnabled() {
+			c.recorder.Eventf(vmi, k8sv1.EventTypeWarning, "FeatureNotEnabled", "DownwardMetrics feature not enabled")
+			return &syncErrorImpl{fmt.Errorf("virtual machine is requesting a disabled feature %s", "FeatureNotEnabled"), "FeatureNotEnabled"}
+		}
 
 		vmiKey := controller.VirtualMachineInstanceKey(vmi)
 		c.podExpectations.ExpectCreations(vmiKey, 1)

pkg/virt-handler/options.go

@@ -41,6 +41,7 @@ func virtualMachineOptions(
 			FreePageReportingDisabled: clusterConfig.IsFreePageReportingDisabled(),
 			BochsDisplayForEFIGuests:  clusterConfig.BochsDisplayForEFIGuestsEnabled(),
 			SerialConsoleLogDisabled:  clusterConfig.IsSerialConsoleLogDisabled(),
+			DownwardMetricsEnabled:    clusterConfig.IsDownwardMetricsEnabled(),
 		}
 	}
 

pkg/virt-operator/resource/generate/components/validations_generated.go

@@ -987,6 +987,10 @@ var CRDsValidation map[string]string = map[string]string{
                     in case hardware-assisted emulation is not available. Defaults to false
                   type: boolean
               type: object
+            downwardMetrics:
+              description: DownwardMetricsConfiguration enables the downward metrics
+                feature.
+              type: object
             emulatedMachines:
               description: Deprecated. Use architectureConfiguration instead.
               items:

pkg/virt-operator/webhooks/kubevirt-update-admitter_test.go

@@ -280,7 +280,7 @@ var _ = Describe("Validating KubeVirtUpdate Admitter", func() {
 			Entry("should not warn if configuration nil", warnNotExpected, nil),
 		)
 
-		DescribeTable("should raise warning when a deprecated feature-gate is enabled", func(featureGate, expectedWarning string) {
+		FDescribeTable("should raise warning when a deprecated feature-gate is enabled", func(featureGate, expectedWarning string) {
 			kv := v1.KubeVirt{}
 			kvBytes, err := json.Marshal(kv)
 			Expect(err).ToNot(HaveOccurred())
@@ -312,6 +312,7 @@ var _ = Describe("Validating KubeVirtUpdate Admitter", func() {
 			Entry("with CPUNodeDiscoveryGate", deprecation.CPUNodeDiscoveryGate, fmt.Sprintf(deprecation.WarningPattern, deprecation.CPUNodeDiscoveryGate, deprecation.GA)),
 			Entry("with Passt", deprecation.PasstGate, deprecation.PasstDiscontinueMessage),
 			Entry("with MacvtapGate", deprecation.MacvtapGate, deprecation.MacvtapDiscontinueMessage),
+			Entry("with DownwardMetrics", deprecation.DownwardMetricsFeatureGate, fmt.Sprintf(deprecation.WarningPattern, deprecation.DownwardMetricsFeatureGate, deprecation.GA)),
 		)
 	})
 })

staging/src/kubevirt.io/api/apitesting/testdata/HEAD/kubevirt.io.v1.KubeVirt.json

@@ -82,6 +82,7 @@
     "productName": "productNameValue",
     "productComponent": "productComponentValue",
     "configuration": {
+      "downwardMetrics": {},
       "cpuModel": "cpuModelValue",
       "cpuRequest": "0",
       "developerConfiguration": {

staging/src/kubevirt.io/api/apitesting/testdata/HEAD/kubevirt.io.v1.KubeVirt.yaml

@@ -107,6 +107,7 @@ spec:
         nodeSelectorsKey: nodeSelectorsValue
       pvcTolerateLessSpaceUpToPercent: -31
       useEmulation: true
+    downwardMetrics: {}
     emulatedMachines:
     - emulatedMachinesValue
     evictionStrategy: evictionStrategyValue