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.

Sign up for GitHub

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

Introduce virtualMachineOptions to hco cr #2356

Merged
merged 1 commit into from
Jul 7, 2023
Merged

Introduce virtualMachineOptions to hco cr #2356

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 19 additions & 1 deletion api/v1beta1/hyperconverged_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,12 @@ type HyperConvergedSpec struct {
// The storage class must support RWX in filesystem mode.
// +optional
VMStateStorageClass *string `json:"vmStateStorageClass,omitempty"`

// VirtualMachineOptions holds the cluster level information regarding the virtual machine.
// +kubebuilder:default={"disableFreePageReporting": true}
// +default={"disableFreePageReporting": true}
fossedihelm marked this conversation as resolved.
Show resolved Hide resolved
// +optional
VirtualMachineOptions *VirtualMachineOptions `json:"virtualMachineOptions,omitempty"`
}

// CertRotateConfigCA contains the tunables for TLS certificates.
Expand Down Expand Up @@ -315,6 +321,18 @@ type LiveMigrationConfigurations struct {
AllowPostCopy *bool `json:"allowPostCopy,omitempty"`
}

// VirtualMachineOptions holds the cluster level information regarding the virtual machine.
type VirtualMachineOptions struct {
// DisableFreePageReporting disable the free page reporting of
// memory balloon device https://libvirt.org/formatdomain.html#memory-balloon-device.
// This will have effect only if AutoattachMemBalloon is not false and the vmi is not
// requesting any high performance feature (dedicatedCPU/realtime/hugePages), in which free page reporting is always disabled.
// +optional
// +kubebuilder:default=true
// +default=true
DisableFreePageReporting bool `json:"disableFreePageReporting,omitempty"`
fossedihelm marked this conversation as resolved.
Show resolved Hide resolved
}

// HyperConvergedFeatureGates is a set of optional feature gates to enable or disable new features that are not enabled
// by default yet.
// +k8s:openapi-gen=true
Expand Down Expand Up @@ -637,7 +655,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, "deployKubeSecondaryDNS": false, "nonRoot": true}, "liveMigrationConfig": {"completionTimeoutPerGiB": 800, "parallelMigrationsPerCluster": 5, "parallelOutboundMigrationsPerNode": 2, "progressTimeout": 150, "allowAutoConverge": false, "allowPostCopy": false}, "uninstallStrategy": "BlockUninstallIfWorkloadsExist"}
// +kubebuilder:default={"certConfig": {"ca": {"duration": "48h0m0s", "renewBefore": "24h0m0s"}, "server": {"duration": "24h0m0s", "renewBefore": "12h0m0s"}}, "virtualMachineOptions": {"disableFreePageReporting": true}, "featureGates": {"withHostPassthroughCPU": false, "enableCommonBootImageImport": true, "deployTektonTaskResources": false, "deployKubeSecondaryDNS": false, "nonRoot": true}, "liveMigrationConfig": {"completionTimeoutPerGiB": 800, "parallelMigrationsPerCluster": 5, "parallelOutboundMigrationsPerNode": 2, "progressTimeout": 150, "allowAutoConverge": false, "allowPostCopy": false}, "uninstallStrategy": "BlockUninstallIfWorkloadsExist"}
// +optional
Spec HyperConvergedSpec `json:"spec,omitempty"`
Status HyperConvergedStatus `json:"status,omitempty"`
Expand Down
21 changes: 21 additions & 0 deletions api/v1beta1/zz_generated.deepcopy.go
View file
Open in desktop

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

10 changes: 10 additions & 0 deletions api/v1beta1/zz_generated.defaults.go
View file
Open in desktop

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

9 changes: 8 additions & 1 deletion api/v1beta1/zz_generated.openapi.go
View file
Open in desktop

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

17 changes: 17 additions & 0 deletions config/crd/bases/hco.kubevirt.io_hyperconvergeds.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ spec:
parallelOutboundMigrationsPerNode: 2
progressTimeout: 150
uninstallStrategy: BlockUninstallIfWorkloadsExist
virtualMachineOptions:
disableFreePageReporting: true
description: HyperConvergedSpec defines the desired state of HyperConverged
properties:
certConfig:
Expand Down Expand Up @@ -2540,6 +2542,21 @@ spec:
description: VDDK Init Image eventually used to import VMs from external
providers
type: string
virtualMachineOptions:
default:
disableFreePageReporting: true
description: VirtualMachineOptions holds the cluster level information
regarding the virtual machine.
properties:
disableFreePageReporting:
default: true
description: DisableFreePageReporting disable the free page reporting
of memory balloon device https://libvirt.org/formatdomain.html#memory-balloon-device.
This will have effect only if AutoattachMemBalloon is not false
and the vmi is not requesting any high performance feature (dedicatedCPU/realtime/hugePages),
in which free page reporting is always disabled.
type: boolean
type: object
vmStateStorageClass:
description: VMStateStorageClass is the name of the storage class
to use for the PVCs created to preserve VM state, like TPM. The
Expand Down
4 changes: 4 additions & 0 deletions controllers/operands/kubevirt.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,6 +441,10 @@ func getKVConfig(hc *hcov1beta1.HyperConverged) (*kubevirtcorev1.KubeVirtConfigu
config.VMStateStorageClass = *hc.Spec.VMStateStorageClass
}

if hc.Spec.VirtualMachineOptions != nil && hc.Spec.VirtualMachineOptions.DisableFreePageReporting {
config.VirtualMachineOptions = &kubevirtcorev1.VirtualMachineOptions{DisableFreePageReporting: &kubevirtcorev1.DisableFreePageReporting{}}
}

Comment on lines +444 to +447
Copy link
Collaborator

Choose a reason for hiding this comment

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

We got a code smell here. The getKVConfig is too complex. Please consider to somehow refactor this function.

Not Blocking!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have some ideas, do you agree on addressing this with a followup PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes. Either that or we'll take it.

return config, nil
}

Expand Down
42 changes: 42 additions & 0 deletions controllers/operands/kubevirt_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2797,6 +2797,48 @@ Version: 1.2.3`)
})
})

Context("Virtual machine options", func() {
It("should set disableFreePageReporting by default", func() {
kv, err := NewKubeVirt(hco)
Expect(err).ToNot(HaveOccurred())
Expect(kv.Spec.Configuration).To(Not(BeNil()))
Expect(kv.Spec.Configuration.VirtualMachineOptions).To(Not(BeNil()))
Expect(kv.Spec.Configuration.VirtualMachineOptions.DisableFreePageReporting).To(Not(BeNil()))
fossedihelm marked this conversation as resolved.
Show resolved Hide resolved
})

DescribeTable("should modify disableFreePageReporting according to HCO CR", func(virtualMachineOptions *hcov1beta1.VirtualMachineOptions) {
existingResource, err := NewKubeVirt(hco)
Expect(err).ToNot(HaveOccurred())

By("Modify HCO's virtual machine options configuration")
hco.Spec.VirtualMachineOptions = virtualMachineOptions

cl := commontestutils.InitClient([]client.Object{hco, existingResource})
handler := (*genericOperand)(newKubevirtHandler(cl, commontestutils.GetScheme()))
res := handler.ensure(req)
Expect(res.UpgradeDone).To(BeFalse())
Expect(res.Updated).To(BeTrue())
Expect(res.Err).ToNot(HaveOccurred())

foundResource := &kubevirtcorev1.KubeVirt{}
Expect(
cl.Get(context.TODO(),
types.NamespacedName{Name: existingResource.Name, Namespace: existingResource.Namespace},
foundResource),
).ToNot(HaveOccurred())

Expect(existingResource.Spec.Configuration.VirtualMachineOptions).ToNot(BeNil())
Expect(existingResource.Spec.Configuration.VirtualMachineOptions.DisableFreePageReporting).ToNot(BeNil())
fossedihelm marked this conversation as resolved.
Show resolved Hide resolved

Expect(foundResource.Spec.Configuration.VirtualMachineOptions).To(BeNil())

Expect(req.Conditions).To(BeEmpty())
},
Entry("with virtualMachineOptions containing disableFreePageReporting false", &hcov1beta1.VirtualMachineOptions{DisableFreePageReporting: false}),
Entry("with empty virtualMachineOptions", &hcov1beta1.VirtualMachineOptions{}),
)
})

It("should handle conditions", func() {
expectedResource, err := NewKubeVirt(hco, commontestutils.Namespace)
Expect(err).ToNot(HaveOccurred())
Expand Down
17 changes: 17 additions & 0 deletions deploy/crds/hco00.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ spec:
parallelOutboundMigrationsPerNode: 2
progressTimeout: 150
uninstallStrategy: BlockUninstallIfWorkloadsExist
virtualMachineOptions:
disableFreePageReporting: true
description: HyperConvergedSpec defines the desired state of HyperConverged
properties:
certConfig:
Expand Down Expand Up @@ -2540,6 +2542,21 @@ spec:
description: VDDK Init Image eventually used to import VMs from external
providers
type: string
virtualMachineOptions:
default:
disableFreePageReporting: true
description: VirtualMachineOptions holds the cluster level information
regarding the virtual machine.
properties:
disableFreePageReporting:
default: true
description: DisableFreePageReporting disable the free page reporting
of memory balloon device https://libvirt.org/formatdomain.html#memory-balloon-device.
This will have effect only if AutoattachMemBalloon is not false
and the vmi is not requesting any high performance feature (dedicatedCPU/realtime/hugePages),
in which free page reporting is always disabled.
type: boolean
type: object
vmStateStorageClass:
description: VMStateStorageClass is the name of the storage class
to use for the PVCs created to preserve VM state, like TPM. The
Expand Down
2 changes: 2 additions & 0 deletions deploy/hco.cr.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ spec:
parallelOutboundMigrationsPerNode: 2
progressTimeout: 150
uninstallStrategy: BlockUninstallIfWorkloadsExist
virtualMachineOptions:
disableFreePageReporting: true
workloadUpdateStrategy:
batchEvictionInterval: 1m0s
batchEvictionSize: 10
Expand Down
17 changes: 17 additions & 0 deletions deploy/index-image/community-kubevirt-hyperconverged/1.10.0/manifests/hco00.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ spec:
parallelOutboundMigrationsPerNode: 2
progressTimeout: 150
uninstallStrategy: BlockUninstallIfWorkloadsExist
virtualMachineOptions:
disableFreePageReporting: true
description: HyperConvergedSpec defines the desired state of HyperConverged
properties:
certConfig:
Expand Down Expand Up @@ -2540,6 +2542,21 @@ spec:
description: VDDK Init Image eventually used to import VMs from external
providers
type: string
virtualMachineOptions:
default:
disableFreePageReporting: true
description: VirtualMachineOptions holds the cluster level information
regarding the virtual machine.
properties:
disableFreePageReporting:
default: true
description: DisableFreePageReporting disable the free page reporting
of memory balloon device https://libvirt.org/formatdomain.html#memory-balloon-device.
This will have effect only if AutoattachMemBalloon is not false
and the vmi is not requesting any high performance feature (dedicatedCPU/realtime/hugePages),
in which free page reporting is always disabled.
type: boolean
type: object
vmStateStorageClass:
description: VMStateStorageClass is the name of the storage class
to use for the PVCs created to preserve VM state, like TPM. The
Expand Down
17 changes: 17 additions & 0 deletions deploy/olm-catalog/community-kubevirt-hyperconverged/1.10.0/manifests/hco00.crd.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ spec:
parallelOutboundMigrationsPerNode: 2
progressTimeout: 150
uninstallStrategy: BlockUninstallIfWorkloadsExist
virtualMachineOptions:
disableFreePageReporting: true
description: HyperConvergedSpec defines the desired state of HyperConverged
properties:
certConfig:
Expand Down Expand Up @@ -2540,6 +2542,21 @@ spec:
description: VDDK Init Image eventually used to import VMs from external
providers
type: string
virtualMachineOptions:
default:
disableFreePageReporting: true
description: VirtualMachineOptions holds the cluster level information
regarding the virtual machine.
properties:
disableFreePageReporting:
default: true
description: DisableFreePageReporting disable the free page reporting
of memory balloon device https://libvirt.org/formatdomain.html#memory-balloon-device.
This will have effect only if AutoattachMemBalloon is not false
and the vmi is not requesting any high performance feature (dedicatedCPU/realtime/hugePages),
in which free page reporting is always disabled.
type: boolean
type: object
vmStateStorageClass:
description: VMStateStorageClass is the name of the storage class
to use for the PVCs created to preserve VM state, like TPM. The
Expand Down
Loading