Skip to content

Commit

Permalink
Make downwardMetric feature gate opt-in
Browse files Browse the repository at this point in the history
The feature gate `downwardMetric` is an opt-out feature gate. This is
due to backward compatibility reasons. Before this feature gate was
introduced, it was hardcoded. It makes this feature gate to be
disabled by default, leaving this decision to the final user.

Signed-off-by: Javier Cano Cano <[email protected]>
  • Loading branch information
jcanocan committed Apr 1, 2024
1 parent 52e5462 commit 8399021
Show file tree
Hide file tree
Showing 17 changed files with 53 additions and 30 deletions.
6 changes: 4 additions & 2 deletions api/v1beta1/hyperconverged_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ type HyperConvergedSpec struct {

// featureGates is a map of feature gate flags. Setting a flag to `true` will enable
// the feature. Setting `false` or removing the feature gate, disables the feature.
// +kubebuilder:default={"withHostPassthroughCPU": false, "enableCommonBootImageImport": true, "deployTektonTaskResources": false, "deployVmConsoleProxy": false, "deployKubeSecondaryDNS": false, "nonRoot": true, "disableMDevConfiguration": false, "persistentReservation": false, "enableManagedTenantQuota": false,"autoResourceLimits": false, "enableApplicationAwareQuota": false}
// +kubebuilder:default={"downwardMetrics": false, "withHostPassthroughCPU": false, "enableCommonBootImageImport": true, "deployTektonTaskResources": false, "deployVmConsoleProxy": false, "deployKubeSecondaryDNS": false, "nonRoot": true, "disableMDevConfiguration": false, "persistentReservation": false, "enableManagedTenantQuota": false,"autoResourceLimits": false, "enableApplicationAwareQuota": false}
// +optional
FeatureGates HyperConvergedFeatureGates `json:"featureGates,omitempty"`

Expand Down Expand Up @@ -395,6 +395,8 @@ type VirtualMachineOptions struct {
type HyperConvergedFeatureGates struct {
// Allow to expose a limited set of host metrics to guests.
// +optional
// +kubebuilder:default=false
// +default=false
DownwardMetrics *bool `json:"downwardMetrics,omitempty"`

// Allow migrating a virtual machine with CPU host-passthrough mode. This should be
Expand Down Expand Up @@ -819,7 +821,7 @@ type HyperConverged struct {
metav1.TypeMeta `json:",inline"`
metav1.ObjectMeta `json:"metadata,omitempty"`

// +kubebuilder:default={"certConfig": {"ca": {"duration": "48h0m0s", "renewBefore": "24h0m0s"}, "server": {"duration": "24h0m0s", "renewBefore": "12h0m0s"}},"featureGates": {"withHostPassthroughCPU": false, "enableCommonBootImageImport": true, "deployTektonTaskResources": false, "deployVmConsoleProxy": false, "deployKubeSecondaryDNS": false, "nonRoot": true, "disableMDevConfiguration": false, "persistentReservation": false, "enableManagedTenantQuota": false, "autoResourceLimits": false, "enableApplicationAwareQuota": false}, "liveMigrationConfig": {"completionTimeoutPerGiB": 800, "parallelMigrationsPerCluster": 5, "parallelOutboundMigrationsPerNode": 2, "progressTimeout": 150, "allowAutoConverge": false, "allowPostCopy": false}, "resourceRequirements": {"vmiCPUAllocationRatio": 10}, "uninstallStrategy": "BlockUninstallIfWorkloadsExist", "virtualMachineOptions": {"disableFreePageReporting": false, "disableSerialConsoleLog": true}}
// +kubebuilder:default={"certConfig": {"ca": {"duration": "48h0m0s", "renewBefore": "24h0m0s"}, "server": {"duration": "24h0m0s", "renewBefore": "12h0m0s"}},"featureGates": {"downwardMetrics": false, "withHostPassthroughCPU": false, "enableCommonBootImageImport": true, "deployTektonTaskResources": false, "deployVmConsoleProxy": false, "deployKubeSecondaryDNS": false, "nonRoot": true, "disableMDevConfiguration": false, "persistentReservation": false, "enableManagedTenantQuota": false, "autoResourceLimits": false, "enableApplicationAwareQuota": false}, "liveMigrationConfig": {"completionTimeoutPerGiB": 800, "parallelMigrationsPerCluster": 5, "parallelOutboundMigrationsPerNode": 2, "progressTimeout": 150, "allowAutoConverge": false, "allowPostCopy": false}, "resourceRequirements": {"vmiCPUAllocationRatio": 10}, "uninstallStrategy": "BlockUninstallIfWorkloadsExist", "virtualMachineOptions": {"disableFreePageReporting": false, "disableSerialConsoleLog": true}}
// +optional
Spec HyperConvergedSpec `json:"spec,omitempty"`
Status HyperConvergedStatus `json:"status,omitempty"`
Expand Down
4 changes: 4 additions & 0 deletions api/v1beta1/zz_generated.defaults.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions api/v1beta1/zz_generated.openapi.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ spec:
deployTektonTaskResources: false
deployVmConsoleProxy: false
disableMDevConfiguration: false
downwardMetrics: false
enableApplicationAwareQuota: false
enableCommonBootImageImport: true
enableManagedTenantQuota: false
Expand Down Expand Up @@ -1065,6 +1066,7 @@ spec:
deployTektonTaskResources: false
deployVmConsoleProxy: false
disableMDevConfiguration: false
downwardMetrics: false
enableApplicationAwareQuota: false
enableCommonBootImageImport: true
enableManagedTenantQuota: false
Expand Down Expand Up @@ -1110,6 +1112,7 @@ spec:
description: Disable mediated devices handling on KubeVirt
type: boolean
downwardMetrics:
default: false
description: Allow to expose a limited set of host metrics to
guests.
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,6 @@ var _ = Describe("HyperconvergedController", func() {
"HostDevices",
"WithHostModelCPU",
"HypervStrictCheck",
"DownwardMetrics",
"ExpandDisks",
"NUMA",
"WithHostPassthroughCPU",
Expand Down
5 changes: 1 addition & 4 deletions controllers/operands/kubevirt.go
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ func hcoConfig2KvConfig(hcoConfig hcov1beta1.HyperConvergedConfig, infrastructur
func getFeatureGateChecks(featureGates *hcov1beta1.HyperConvergedFeatureGates) []string {
fgs := make([]string, 0, 2)

if featureGates.DownwardMetrics == nil || *featureGates.DownwardMetrics {
if featureGates.DownwardMetrics != nil && *featureGates.DownwardMetrics {
fgs = append(fgs, kvDownwardMetrics)
}

Expand Down Expand Up @@ -889,11 +889,8 @@ func getMandatoryKvFeatureGates(isKVMEmulation bool) []string {
// get list of feature gates or KV FG list
func getKvFeatureGateList(fgs *hcov1beta1.HyperConvergedFeatureGates) []string {
checks := getFeatureGateChecks(fgs)

res := make([]string, 0, len(checks)+len(mandatoryKvFeatureGates)+1)

res = append(res, mandatoryKvFeatureGates...)

res = append(res, checks...)

return res
Expand Down
28 changes: 13 additions & 15 deletions controllers/operands/kubevirt_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ import (
)

var _ = Describe("KubeVirt Operand", func() {
// DownwardMetrics FG is not listed on any of these lists, but it is enabled by default
const enabledByDefaultFeatureGates = 1

var (
basicNumFgOnOpenshift = len(hardCodeKvFgs) + len(sspConditionKvFgs)
Expand Down Expand Up @@ -221,7 +219,7 @@ Version: 1.2.3`)
Expect(foundResource.Namespace).To(Equal(expectedResource.Namespace))

Expect(foundResource.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil())
Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(basicNumFgOnOpenshift + 1 + enabledByDefaultFeatureGates))
Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(basicNumFgOnOpenshift + 1))
Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(ContainElements(
hardCodeKvFgs,
))
Expand Down Expand Up @@ -425,7 +423,7 @@ Version: 1.2.3`)
foundResource),
).ToNot(HaveOccurred())
Expect(foundResource.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil())
Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(basicNumFgOnOpenshift + 1 + enabledByDefaultFeatureGates))
Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(basicNumFgOnOpenshift + 1))
Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(ContainElements(
hardCodeKvFgs,
))
Expand Down Expand Up @@ -1795,7 +1793,7 @@ Version: 1.2.3`)

Expect(existingResource.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil())
fgList := getKvFeatureGateList(&hco.Spec.FeatureGates)
Expect(fgList).To(HaveLen(basicNumFgOnOpenshift + enabledByDefaultFeatureGates))
Expect(fgList).To(HaveLen(basicNumFgOnOpenshift))
Expect(fgList).To(ContainElements(hardCodeKvFgs))
Expect(fgList).To(ContainElements(sspConditionKvFgs))
})
Expand All @@ -1813,7 +1811,7 @@ Version: 1.2.3`)
})
})

It("should add the DownwardMetrics if feature gate DownwardMetrics is not in HyperConverged CR", func() {
It("should no add the DownwardMetrics if feature gate DownwardMetrics is not in HyperConverged CR", func() {
hco.Spec.FeatureGates = hcov1beta1.HyperConvergedFeatureGates{
DownwardMetrics: nil,
}
Expand All @@ -1822,7 +1820,7 @@ Version: 1.2.3`)
Expect(err).ToNot(HaveOccurred())
By("KV CR should contain the DownwardMetrics feature gate", func() {
Expect(existingResource.Spec.Configuration.DeveloperConfiguration).NotTo(BeNil())
Expect(existingResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(ContainElement(kvDownwardMetrics))
Expect(existingResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).ToNot(ContainElement(kvDownwardMetrics))
})
})

Expand Down Expand Up @@ -1913,7 +1911,7 @@ Version: 1.2.3`)
mandatoryKvFeatureGates = getMandatoryKvFeatureGates(false)
Expect(foundResource.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil())
fgList := getKvFeatureGateList(&hco.Spec.FeatureGates)
Expect(fgList).To(HaveLen(basicNumFgOnOpenshift + enabledByDefaultFeatureGates))
Expect(fgList).To(HaveLen(basicNumFgOnOpenshift))
Expect(fgList).To(ContainElements(hardCodeKvFgs))
Expect(fgList).To(ContainElements(sspConditionKvFgs))
})
Expand Down Expand Up @@ -2097,7 +2095,7 @@ Version: 1.2.3`)
).ToNot(HaveOccurred())

Expect(foundResource.Spec.Configuration.DeveloperConfiguration).ToNot(BeNil())
Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(len(hardCodeKvFgs) + enabledByDefaultFeatureGates))
Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(HaveLen(len(hardCodeKvFgs)))
Expect(foundResource.Spec.Configuration.DeveloperConfiguration.FeatureGates).To(ContainElements(hardCodeKvFgs))
})
})
Expand Down Expand Up @@ -2128,37 +2126,37 @@ Version: 1.2.3`)
Entry("When not using kvm-emulation and FG is empty",
false,
&hcov1beta1.HyperConvergedFeatureGates{},
basicNumFgOnOpenshift+enabledByDefaultFeatureGates,
basicNumFgOnOpenshift,
[][]string{hardCodeKvFgs, sspConditionKvFgs},
),
Entry("When using kvm-emulation and FG is empty",
true,
&hcov1beta1.HyperConvergedFeatureGates{},
len(hardCodeKvFgs)+enabledByDefaultFeatureGates,
len(hardCodeKvFgs),
[][]string{hardCodeKvFgs},
),
Entry("When not using kvm-emulation and all FGs are disabled",
false,
&hcov1beta1.HyperConvergedFeatureGates{WithHostPassthroughCPU: ptr.To(false)},
basicNumFgOnOpenshift+enabledByDefaultFeatureGates,
basicNumFgOnOpenshift,
[][]string{hardCodeKvFgs, sspConditionKvFgs},
),
Entry("When using kvm-emulation all FGs are disabled",
true,
&hcov1beta1.HyperConvergedFeatureGates{WithHostPassthroughCPU: ptr.To(false)},
len(hardCodeKvFgs)+enabledByDefaultFeatureGates,
len(hardCodeKvFgs),
[][]string{hardCodeKvFgs},
),
Entry("When not using kvm-emulation and all FGs are enabled",
false,
&hcov1beta1.HyperConvergedFeatureGates{WithHostPassthroughCPU: ptr.To(true)},
basicNumFgOnOpenshift+1+enabledByDefaultFeatureGates,
basicNumFgOnOpenshift+1,
[][]string{hardCodeKvFgs, sspConditionKvFgs, {kvWithHostPassthroughCPU}},
),
Entry("When using kvm-emulation all FGs are enabled",
true,
&hcov1beta1.HyperConvergedFeatureGates{WithHostPassthroughCPU: ptr.To(true)},
len(hardCodeKvFgs)+1+enabledByDefaultFeatureGates,
len(hardCodeKvFgs)+1,
[][]string{hardCodeKvFgs, {kvWithHostPassthroughCPU}},
))
})
Expand Down
3 changes: 3 additions & 0 deletions deploy/crds/hco00.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ spec:
deployTektonTaskResources: false
deployVmConsoleProxy: false
disableMDevConfiguration: false
downwardMetrics: false
enableApplicationAwareQuota: false
enableCommonBootImageImport: true
enableManagedTenantQuota: false
Expand Down Expand Up @@ -1065,6 +1066,7 @@ spec:
deployTektonTaskResources: false
deployVmConsoleProxy: false
disableMDevConfiguration: false
downwardMetrics: false
enableApplicationAwareQuota: false
enableCommonBootImageImport: true
enableManagedTenantQuota: false
Expand Down Expand Up @@ -1110,6 +1112,7 @@ spec:
description: Disable mediated devices handling on KubeVirt
type: boolean
downwardMetrics:
default: false
description: Allow to expose a limited set of host metrics to
guests.
type: boolean
Expand Down
1 change: 1 addition & 0 deletions deploy/hco.cr.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ spec:
deployTektonTaskResources: false
deployVmConsoleProxy: false
disableMDevConfiguration: false
downwardMetrics: false
enableApplicationAwareQuota: false
enableCommonBootImageImport: true
enableManagedTenantQuota: false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ spec:
deployTektonTaskResources: false
deployVmConsoleProxy: false
disableMDevConfiguration: false
downwardMetrics: false
enableApplicationAwareQuota: false
enableCommonBootImageImport: true
enableManagedTenantQuota: false
Expand Down Expand Up @@ -1065,6 +1066,7 @@ spec:
deployTektonTaskResources: false
deployVmConsoleProxy: false
disableMDevConfiguration: false
downwardMetrics: false
enableApplicationAwareQuota: false
enableCommonBootImageImport: true
enableManagedTenantQuota: false
Expand Down Expand Up @@ -1110,6 +1112,7 @@ spec:
description: Disable mediated devices handling on KubeVirt
type: boolean
downwardMetrics:
default: false
description: Allow to expose a limited set of host metrics to
guests.
type: boolean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ spec:
deployTektonTaskResources: false
deployVmConsoleProxy: false
disableMDevConfiguration: false
downwardMetrics: false
enableApplicationAwareQuota: false
enableCommonBootImageImport: true
enableManagedTenantQuota: false
Expand Down Expand Up @@ -1065,6 +1066,7 @@ spec:
deployTektonTaskResources: false
deployVmConsoleProxy: false
disableMDevConfiguration: false
downwardMetrics: false
enableApplicationAwareQuota: false
enableCommonBootImageImport: true
enableManagedTenantQuota: false
Expand Down Expand Up @@ -1110,6 +1112,7 @@ spec:
description: Disable mediated devices handling on KubeVirt
type: boolean
downwardMetrics:
default: false
description: Allow to expose a limited set of host metrics to
guests.
type: boolean
Expand Down
Loading

0 comments on commit 8399021

Please sign in to comment.