Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make downwardMetric feature gate opt-in #2844

Merged
merged 1 commit into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jcanocan please also add the default value for the FeatureGates field (line 69), and for the spec field (line 824).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

// +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