design-proposal: Feature configurables

This design document states how features that require to have a
mechanism to change it's state, e.g., enabled/disabled should be
implemented in KubeVirt.

Signed-off-by: Javier Cano Cano <[email protected]>

Author: jcanocan

design-proposals/configurable-features.md

@@ -0,0 +1,161 @@
+# Overview
+
+With the introduction
+of [KubeVirt Feature Lifecycle](https://github.com/kubevirt/community/blob/main/design-proposals/feature-lifecycle.md)
+policy, features reaching General Availability (GA) need to drop their use of feature gates. This applies also to
+configurable features that we may want to disable.
+
+## Motivation
+
+Users may want certain features to be in a given state, for example to make the best use out of given
+resources or for compliance reasons features may expose sensitive information from the host to the virtual machines
+instances (VMI) or add additional containers to the launcher pod, which are not required by the user. 
+
+The downward metrics feature is a good example of why some clusters may want to have it enabled or disabled.
+The downward metrics feature exposes some metrics about the host node where the VMI is running to the guest. This
+information may be considered sensitive information.
+If there is no mechanism to disable the feature any VMI could request the metrics and inspect information that, in some
+cases, the admin would like to hide, creating a potential security issue.
+
+The behavior of other features might be changed by editing configurables, e.g. the maximum of CPU sockets allowed for
+each VMI can be configured.
+
+Before the introduction
+of [KubeVirt Feature Lifecycle](https://github.com/kubevirt/community/blob/main/design-proposals/feature-lifecycle.md)
+policy, many feature gates remained after feature's graduation to GA with the sole purpose of acting as a switch for the
+feature. Generally speaking, this is a bad practice, because feature gates should be reserved for controlling a feature
+until it reaches maturity. i.e., GA. Therefore, in the case that a developer wants to provide the ability to tune/change
+the state of the feature, configurables exposed in the KubeVirt CR should be provided. This should be
+accomplished while achieving [eventually consistency](https://en.wikipedia.org/wiki/Eventual_consistency). This forces
+us to avoid the feature state control checking on webhooks and moving the feature state control closer to the
+responsible code. Moreover, it has to be decided how the system should behave if a VMI is
+requiring a feature in a state different from what was configured in the KubeVirt CR, or what should happen if the
+configuration of a feature in use is changed. (see matrix below).
+
+## Goals
+
+- Establish how the features status swapping should work.
+- Describe how the system should react in these scenarios:
+    - A feature in KubeVirt is set to state A and a VMI requests the feature to be in state B.
+    - A feature in KubeVirt is set to state A, there are running VMIs using the feature in state A, and the feature is 
+      changed in KubeVirt to state B.
+    - A feature in KubeVirt is set to state A, and pending VMIs want to use it.
+    - A feature in KubeVirt is set to state A, and running VMIs using the feature in state B wants to live migrate.
+- Graduate features status swapping from features gates to configurables.
+
+## Non Goals
+
+- Describe how features protected with features gates should work.
+
+## Definition of Users
+
+Development contributors.
+
+Cluster administrators.
+
+## User Stories
+
+* As a cluster administrator, I want to be able to change the cluster-wide state of a feature by editing configurables.
+
+* As VMI owner, I want to use a given feature.
+
+## Repos
+
+Kubevirt/Kubevirt
+
+# Design
+
+In order to make a feature configurable, it must be done by adding new fields to the KubeVirt CR under
+`spec.configuration`.
+
+
+> **NOTE:** The inclusion of these new KubeVirt API fields should be carefully considered and justified. The feature
+> configurables should be avoided as much as possible.
+
+
+This is current list of GA'd features present in KubeVirt/KubeVirt which are still using feature gates and are shown as
+configurables in [HCO](https://github.com/kubevirt/hyperconverged-cluster-operator/blob/main/controllers/operands/kubevirt.go#L166-L174):
+
+- DownwardMetrics
+- Root (not sure about this one)
+- DisableMDEVConfiguration
+- PersistentReservation
+- AutoResourceLimitsGate
+- AlignCPUs
+
+This is the current list of GA'd features present in KubeVirt/KubeVirt which are still using feature gates and are always
+enabled by [HCO](https://github.com/kubevirt/hyperconverged-cluster-operator/blob/main/controllers/operands/kubevirt.go#L125-L142):
+
+- CPUManager
+- Snapshot
+- HotplugVolumes
+- GPU
+- HostDevices
+- NUMA
+- VMExport
+- DisableCustomSELinuxPolicy
+- KubevirtSeccompProfile
+- HotplugNICs
+- VMPersistentState
+- NetworkBindingPlugins
+- VMLiveUpdateFeatures
+
+Please note that only feature gates included in KubeVirt/KubeVirt are listed here.
+
+## API Examples
+The proposal configuration field, for a given feature in the KubeVirt CR, may look like:
+
+```yaml
+apiVersion: kubevirt.io/v1
+kind: KubeVirt
+[...]
+spec:
+  certificateRotateStrategy: {}
+  configuration:
+    feature-A:  {}
+[...]
+```
+The VMI object may or may not include a configuration field inside the relevant spec.
+
+## Interactions with the VMIs requests
+
+In case that, the VMI exposes a configuration field to request the feature as well as the KubeVirt CRD, the system may
+encounter some inconsistent states that should be handled in the following way:
+
+- If the feature is set to state A in the KubeVirt CR and the VMI is requesting the feature in state B, the VMIs must
+  stay in Pending state. The VMI status should be updated, showing a status message, highlighting the reason(s) for the
+  Pending state.
+- Feature status checks should only be performed during the VMI reconciliation process, not at runtime. Therefore, the
+  feature status changes in the KubeVirt CR should not affect running VMIs. Moreover, the VMI should still be able to
+  live migrate, preserving its original feature state.
+- Optionally, it could enable the possibility to reject the KubeVirt CR change request if running VMIs are using the
+  feature in a given state. However, by the default the request should be accepted.
+
+## Scalability
+
+The feature state swapping should not affect in a meaningful way the cluster resource usage.
+
+## Update/Rollback Compatibility
+
+The feature state swapping should not affect forward or backward compatibility once the feature GA. A given feature,
+after 3 releases in Beta, all feature gates must be dropped. Those features that need a configurable should define it ahead
+of time.
+
+## Functional Testing Approach
+
+The unit and functional testing frameworks should cover the relevant scenarios for each feature.
+
+# Implementation Phases
+
+The feature status check should be placed in the VMI reconciliation loop. In this way, the feature status evaluation is
+close to the VMI scheduling process, as well as allowing KubeVirt to reconcile itself if it is out of sync temporally.
+
+Regarding already existing features transitioning from feature gates as a way to set the feature status to configurable
+fields, this change is acceptable, but it should be marked as a breaking change and documented. Moreover, all feature
+gates should be evaluated to determine if they need to be dropped and transitioned to configurables.
+
+## About implementing the checking logic in the VM controller
+
+The checking in the VM controller could be added to let the user know if a VM has requested a feature in a state which 
+is different from what it is specified in the KubeVirt CR. The VM will update the VM status, showing a status message 
+highlighting the misconfiguration.