Repository: kubermatic/machine-controller (go)

Issue:
kubevirt cpu allocation ratio is ignored for provisioned virtual machines
Currently the machine-controller sets requests and limits for cpu and memory resources of a kubevirt virtual machine. (https://github.com/kubermatic/machine-controller/blob/main/pkg/cloudprovider/provider/kubevirt/provider.go#L755-#L762) Due to this kubevirt ignores to calculate the [cpu ratio](https://kubevirt.io/user-guide/compute/node_overcommit/#node-cpu-allocation-ratio) for the launcher pod.

To let kubevirt calculate the cpu requests for a launcher pod the configured cpus needs to be set in `spec.domain.cpu` instead of configuring the requested cpus and the limit in `spec.domain.resources`. This leads then to another challenge because then no limit would be set to the launcher pod by default. For that the kubevirt need to have cpu [auto limit feature](https://kubevirt.io/user-guide/compute/resources_requests_and_limits/#auto-cpu-limits) enabled which also requires additional configuration by either [adding labels to namespaces](https://kubevirt.io/user-guide/compute/resources_requests_and_limits/#autocpulimitnamespacelabelselector-configuration) or [applying resource quotas](https://kubevirt.io/user-guide/compute/resources_requests_and_limits/#autoresourcelimitsgate-feature-gate) for namespaces.

It would be nice to be able to use this kubevirt feature and to have a documentation regarding the requirements to use it.


Pull request:
Change KubeVirt VM CPU assignment to not overwrite cpu alloc ratio
**What this PR does / why we need it**:
This pr changes the assignment of cpu resources for kubevirt virtual machines to be able to use kubevirt `cpuAllocationRatio` feature by introducing a new field to the providerSpec called vCPUs.

**Which issue(s) this PR fixes**:
<!--optional, in `fixes #<issue number>` format, will close the issue(s) when PR gets merged-->
Fixes #1905

**What type of PR is this?**
<!--
Add one of the following kinds:
/kind bug
/kind cleanup
/kind documentation
/kind feature
/kind design

Optionally add one or more of the following kinds if applicable:
/kind api-change
/kind deprecation
/kind failing-test
/kind flake
/kind regression
/kind chore
-->

/kind feature

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change? Then add your Release Note here**:
<!--
Write your release note:
1. Enter your extended release note in the below block. If the PR requires additional action from users switching to the new release, include the string "action required".
2. If no release note is required, just write "NONE".
-->
```release-note
A new field `cloudProviderSpec.virtualMachine.template.vcpus` was added to kubevirt provider. When `vcpus.cores` is set, the cpus for the virtual machine will be configured in `spec.template.spec.domain.cpu` instead of setting resource requests and limits explicit. The webhook validation will fail if both `vcpus` and `cpus` are configured for a machine.
```

**Documentation**:
<!--
Please do one of the following options:
- Add a link to the existing documentation
- Add a link to the kubermatic/docs pull request
- If no documentation change is applicable then add:
  - TBD (documentation will be added later)
  - NONE (no documentation needed for this PR)
-->
```documentation
NONE
```


Patch:
diff --git a/pkg/cloudprovider/provider/kubevirt/provider.go b/pkg/cloudprovider/provider/kubevirt/provider.go
index 8b8f1b61..78b14f03 100644
--- a/pkg/cloudprovider/provider/kubevirt/provider.go
+++ b/pkg/cloudprovider/provider/kubevirt/provider.go
@@ -98,6 +98,8 @@ type Config struct {
 	DNSConfig                 *corev1.PodDNSConfig
 	DNSPolicy                 corev1.DNSPolicy
 	CPUs                      string
+	VCPUs                     *kubevirtcorev1.CPU
+	Resources                 *corev1.ResourceList
 	Memory                    string
 	Namespace                 string
 	OSImageSource             *cdicorev1beta1.DataVolumeSource
@@ -273,14 +275,21 @@ func (p *provider) getConfig(provSpec clusterv1alpha1.ProviderSpec) (*Config, *p
 		return nil, nil, fmt.Errorf("failed to decode kubeconfig: %w", err)
 	}
 
-	config.CPUs, err = p.configVarResolver.GetConfigVarStringValue(rawConfig.VirtualMachine.Template.CPUs)
+	cpus, err := p.configVarResolver.GetConfigVarStringValue(rawConfig.VirtualMachine.Template.CPUs)
 	if err != nil {
 		return nil, nil, fmt.Errorf(`failed to get value of "cpus" field: %w`, err)
 	}
-	config.Memory, err = p.configVarResolver.GetConfigVarStringValue(rawConfig.VirtualMachine.Template.Memory)
+
+	memory, err := p.configVarResolver.GetConfigVarStringValue(rawConfig.VirtualMachine.Template.Memory)
 	if err != nil {
 		return nil, nil, fmt.Errorf(`failed to get value of "memory" field: %w`, err)
 	}
+
+	config.Resources, config.VCPUs, err = parseResources(cpus, memory, rawConfig.VirtualMachine.Template.VCPUs)
+	if err != nil {
+		return nil, nil, fmt.Errorf(`failed to configure resource requests and limits and vcpus: %w`, err)
+	}
+
 	config.Namespace = getNamespace()
 	if len(rawConfig.VirtualMachine.Template.PrimaryDisk.ExtraHeaders) > 0 {
 		config.ExtraHeaders = rawConfig.VirtualMachine.Template.PrimaryDisk.ExtraHeaders
@@ -616,8 +625,16 @@ func (p *provider) Validate(ctx context.Context, _ *zap.SugaredLogger, spec clus
 	// If instancetype is specified, skip CPU and Memory validation.
 	// Values will come from instancetype.
 	if c.Instancetype == nil {
-		if _, err := parseResources(c.CPUs, c.Memory); err != nil {
-			return err
+		if c.Resources == nil {
+			return fmt.Errorf("no resource requests set for the virtual machine")
+		}
+
+		if c.VCPUs == nil && c.Resources.Cpu() == nil {
+			return fmt.Errorf("no CPUs configured. Either vCPUs or CPUs have to be set.")
+		}
+
+		if c.VCPUs != nil && c.Resources.Cpu() != nil {
+			return fmt.Errorf("vCPUs and CPUs cannot be configured at the same time.")
 		}
 	}
 
@@ -753,12 +770,8 @@ func (p *provider) newVirtualMachine(c *Config, pc *providerconfig.Config, machi
 
 	// if no instancetype, resources are from config.
 	if c.Instancetype == nil {
-		requestsAndLimits, err := parseResources(c.CPUs, c.Memory)
-		if err != nil {
-			return nil, err
-		}
-		resourceRequirements.Requests = *requestsAndLimits
-		resourceRequirements.Limits = *requestsAndLimits
+		resourceRequirements.Requests = *c.Resources
+		resourceRequirements.Limits = *c.Resources
 	}
 
 	// Add cluster labels
@@ -840,6 +853,13 @@ func (p *provider) newVirtualMachine(c *Config, pc *providerconfig.Config, machi
 			DataVolumeTemplates: getDataVolumeTemplates(c, dataVolumeName, dvAnnotations),
 		},
 	}
+
+	if c.VCPUs != nil {
+		virtualMachine.Spec.Template.Spec.Domain.CPU = &kubevirtcorev1.CPU{
+			Cores: c.VCPUs.Cores,
+		}
+	}
+
 	return virtualMachine, nil
 }
 
@@ -867,19 +887,25 @@ func (p *provider) Cleanup(ctx context.Context, _ *zap.SugaredLogger, machine *c
 	return false, sigClient.Delete(ctx, vm)
 }
 
-func parseResources(cpus, memory string) (*corev1.ResourceList, error) {
+func parseResources(cpus, memory string, vpcus kubevirttypes.VCPUs) (*corev1.ResourceList, *kubevirtcorev1.CPU, error) {
 	memoryResource, err := resource.ParseQuantity(memory)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse memory requests: %w", err)
+		return nil, nil, fmt.Errorf("failed to parse memory requests: %w", err)
 	}
+
+	if vpcus.Cores != 0 {
+		return &corev1.ResourceList{corev1.ResourceMemory: memoryResource}, &kubevirtcorev1.CPU{Cores: uint32(vpcus.Cores)}, nil
+	}
+
 	cpuResource, err := resource.ParseQuantity(cpus)
 	if err != nil {
-		return nil, fmt.Errorf("failed to parse cpu request: %w", err)
+		return nil, nil, fmt.Errorf("failed to parse cpu requests: %w", err)
 	}
+
 	return &corev1.ResourceList{
 		corev1.ResourceMemory: memoryResource,
 		corev1.ResourceCPU:    cpuResource,
-	}, nil
+	}, nil, nil
 }
 
 func (p *provider) SetMetricsForMachines(_ clusterv1alpha1.MachineList) error {
diff --git a/sdk/cloudprovider/kubevirt/types.go b/sdk/cloudprovider/kubevirt/types.go
index cd90b07b..7025990a 100644
--- a/sdk/cloudprovider/kubevirt/types.go
+++ b/sdk/cloudprovider/kubevirt/types.go
@@ -70,12 +70,23 @@ type Flavor struct {
 
 // Template.
 type Template struct {
+	// VCPUs is to configure vcpus used by a the virtual machine
+	// when using kubevirts cpuAllocationRatio feature this leads to auto assignment of the
+	// calculated ratio as resource cpu requests for the pod which launches the virtual machine
+	VCPUs VCPUs `json:"vcpus,omitempty"`
+	// CPUs is to configure cpu requests and limits directly for the pod which launches the virtual machine
+	// and is related to the underlying hardware
 	CPUs           providerconfig.ConfigVarString `json:"cpus,omitempty"`
 	Memory         providerconfig.ConfigVarString `json:"memory,omitempty"`
 	PrimaryDisk    PrimaryDisk                    `json:"primaryDisk,omitempty"`
 	SecondaryDisks []SecondaryDisks               `json:"secondaryDisks,omitempty"`
 }
 
+// VCPUs.
+type VCPUs struct {
+	Cores int `json:"cores,omitempty"`
+}
+
 // PrimaryDisk.
 type PrimaryDisk struct {
 	Disk


Test patch:
diff --git a/pkg/cloudprovider/provider/kubevirt/provider_test.go b/pkg/cloudprovider/provider/kubevirt/provider_test.go
index 6647d84f..956a88bc 100644
--- a/pkg/cloudprovider/provider/kubevirt/provider_test.go
+++ b/pkg/cloudprovider/provider/kubevirt/provider_test.go
@@ -72,6 +72,7 @@ type kubevirtProviderSpecConf struct {
 	ProviderNetwork          *kubevirt.ProviderNetwork
 	ExtraHeadersSet          bool
 	EvictStrategy            string
+	VCPUs                    uint32
 }
 
 func (k kubevirtProviderSpecConf) rawProviderSpec(t *testing.T) []byte {
@@ -132,7 +133,13 @@ func (k kubevirtProviderSpecConf) rawProviderSpec(t *testing.T) []byte {
 			},
 			{{- end }}
 			"template": {
+				{{- if .VCPUs }}
+				"vcpus": {
+					"cores": {{ .VCPUs }}
+				},
+				{{- else }}
 				"cpus": "2",
+				{{- end }}
 				"memory": "2Gi",
 				{{- if .SecondaryDisks }}
 				"secondaryDisks": [{
@@ -283,6 +290,10 @@ func TestNewVirtualMachine(t *testing.T) {
 			name:     "eviction-strategy-live-migrate",
 			specConf: kubevirtProviderSpecConf{EvictStrategy: "LiveMigrate"},
 		},
+		{
+			name:     "dedicated-vcpus",
+			specConf: kubevirtProviderSpecConf{VCPUs: 2},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
diff --git a/pkg/cloudprovider/provider/kubevirt/testdata/dedicated-vcpus.yaml b/pkg/cloudprovider/provider/kubevirt/testdata/dedicated-vcpus.yaml
new file mode 100644
index 00000000..daae69a6
--- /dev/null
+++ b/pkg/cloudprovider/provider/kubevirt/testdata/dedicated-vcpus.yaml
@@ -0,0 +1,80 @@
+apiVersion: kubevirt.io/v1
+kind: VirtualMachine
+metadata:
+  annotations:
+  labels:
+    cluster.x-k8s.io/cluster-name: cluster-name
+    cluster.x-k8s.io/role: worker
+    kubevirt.io/vm: dedicated-vcpus
+    md: md-name
+  name: dedicated-vcpus
+  namespace: test-namespace
+spec:
+  dataVolumeTemplates:
+    - metadata:
+        name: dedicated-vcpus
+      spec:
+        storage:
+          accessModes:
+            - ReadWriteMany
+          resources:
+            requests:
+              storage: 10Gi
+          storageClassName: longhorn
+        source:
+          http:
+            url: http://x.y.z.t/ubuntu.img
+  runStrategy: Once
+  template:
+    metadata:
+      creationTimestamp: null
+      annotations:
+        "kubevirt.io/allow-pod-bridge-network-live-migration": "true"
+        "ovn.kubernetes.io/allow_live_migration": "true"
+      labels:
+        cluster.x-k8s.io/cluster-name: cluster-name
+        cluster.x-k8s.io/role: worker
+        kubevirt.io/vm: dedicated-vcpus
+        md: md-name
+    spec:
+      affinity: {}
+      domain:
+        cpu:
+          cores: 2
+        devices:
+          disks:
+            - disk:
+                bus: virtio
+              name: datavolumedisk
+            - disk:
+                bus: virtio
+              name: cloudinitdisk
+          interfaces:
+            - name: default
+              bridge: {}
+          networkInterfaceMultiqueue: true
+        resources:
+          limits:
+            memory: 2Gi
+          requests:
+            memory: 2Gi
+      networks:
+        - name: default
+          pod: {}
+      terminationGracePeriodSeconds: 30
+      topologyspreadconstraints:
+        - maxskew: 1
+          topologykey: kubernetes.io/hostname
+          whenunsatisfiable: ScheduleAnyway
+          labelselector:
+            matchlabels:
+              md: md-name
+      volumes:
+        - dataVolume:
+            name: dedicated-vcpus
+          name: datavolumedisk
+        - cloudInitNoCloud:
+            secretRef:
+              name: udsn
+          name: cloudinitdisk
+      evictionStrategy: External