Skip to content

Commit 99a4aa5

Browse files
soer3nsoer3n
authored
Change KubeVirt VM CPU assignment to not overwrite cpu alloc ratio (#1906)
* change kubevirt vm cpu assignment to not overwrite cpu alloc ratio Signed-off-by: soer3n <srenhenning@googlemail.com> * update kubevirt testdata Signed-off-by: soer3n <srenhenning@googlemail.com> * revert testdata change and add condition for vm cpu assignment Signed-off-by: soer3n <srenhenning@googlemail.com> * switch to providerSpec for enabling vcpu assignment Signed-off-by: soer3n <srenhenning@googlemail.com> * adapt kubevirt cpu struct * adapt kubevirt cpu struct for configuring vcpus for a virtual machine * modify logic in function for rendering resource requests and limits * modify validation accordingly to be a bit more specific regarding resources Signed-off-by: soer3n <srenhenning@googlemail.com> * revert unnessecarry changes to mocked kubevirt vm Signed-off-by: soer3n <srenhenning@googlemail.com> * changes after review Signed-off-by: soer3n <srenhenning@googlemail.com> --------- Signed-off-by: soer3n <srenhenning@googlemail.com>
1 parent 1427edd commit 99a4aa5

File tree

4 files changed

+142
-14
lines changed

4 files changed

+142
-14
lines changed

‎pkg/cloudprovider/provider/kubevirt/provider.go‎

Lines changed: 40 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ type Config struct {
9898
DNSConfig *corev1.PodDNSConfig
9999
DNSPolicy corev1.DNSPolicy
100100
CPUs string
101+
VCPUs *kubevirtcorev1.CPU
102+
Resources *corev1.ResourceList
101103
Memory string
102104
Namespace string
103105
OSImageSource *cdicorev1beta1.DataVolumeSource
@@ -273,14 +275,21 @@ func (p *provider) getConfig(provSpec clusterv1alpha1.ProviderSpec) (*Config, *p
273275
return nil, nil, fmt.Errorf("failed to decode kubeconfig: %w", err)
274276
}
275277

276-
config.CPUs, err = p.configVarResolver.GetConfigVarStringValue(rawConfig.VirtualMachine.Template.CPUs)
278+
cpus, err := p.configVarResolver.GetConfigVarStringValue(rawConfig.VirtualMachine.Template.CPUs)
277279
if err != nil {
278280
return nil, nil, fmt.Errorf(`failed to get value of "cpus" field: %w`, err)
279281
}
280-
config.Memory, err = p.configVarResolver.GetConfigVarStringValue(rawConfig.VirtualMachine.Template.Memory)
282+
283+
memory, err := p.configVarResolver.GetConfigVarStringValue(rawConfig.VirtualMachine.Template.Memory)
281284
if err != nil {
282285
return nil, nil, fmt.Errorf(`failed to get value of "memory" field: %w`, err)
283286
}
287+
288+
config.Resources, config.VCPUs, err = parseResources(cpus, memory, rawConfig.VirtualMachine.Template.VCPUs)
289+
if err != nil {
290+
return nil, nil, fmt.Errorf(`failed to configure resource requests and limits and vcpus: %w`, err)
291+
}
292+
284293
config.Namespace = getNamespace()
285294
if len(rawConfig.VirtualMachine.Template.PrimaryDisk.ExtraHeaders) > 0 {
286295
config.ExtraHeaders = rawConfig.VirtualMachine.Template.PrimaryDisk.ExtraHeaders
@@ -616,8 +625,16 @@ func (p *provider) Validate(ctx context.Context, _ *zap.SugaredLogger, spec clus
616625
// If instancetype is specified, skip CPU and Memory validation.
617626
// Values will come from instancetype.
618627
if c.Instancetype == nil {
619-
if _, err := parseResources(c.CPUs, c.Memory); err != nil {
620-
return err
628+
if c.Resources == nil {
629+
return fmt.Errorf("no resource requests set for the virtual machine")
630+
}
631+
632+
if c.VCPUs == nil && c.Resources.Cpu() == nil {
633+
return fmt.Errorf("no CPUs configured. Either vCPUs or CPUs have to be set.")
634+
}
635+
636+
if c.VCPUs != nil && c.Resources.Cpu() != nil {
637+
return fmt.Errorf("vCPUs and CPUs cannot be configured at the same time.")
621638
}
622639
}
623640

@@ -753,12 +770,8 @@ func (p *provider) newVirtualMachine(c *Config, pc *providerconfig.Config, machi
753770

754771
// if no instancetype, resources are from config.
755772
if c.Instancetype == nil {
756-
requestsAndLimits, err := parseResources(c.CPUs, c.Memory)
757-
if err != nil {
758-
return nil, err
759-
}
760-
resourceRequirements.Requests = *requestsAndLimits
761-
resourceRequirements.Limits = *requestsAndLimits
773+
resourceRequirements.Requests = *c.Resources
774+
resourceRequirements.Limits = *c.Resources
762775
}
763776

764777
// Add cluster labels
@@ -840,6 +853,13 @@ func (p *provider) newVirtualMachine(c *Config, pc *providerconfig.Config, machi
840853
DataVolumeTemplates: getDataVolumeTemplates(c, dataVolumeName, dvAnnotations),
841854
},
842855
}
856+
857+
if c.VCPUs != nil {
858+
virtualMachine.Spec.Template.Spec.Domain.CPU = &kubevirtcorev1.CPU{
859+
Cores: c.VCPUs.Cores,
860+
}
861+
}
862+
843863
return virtualMachine, nil
844864
}
845865

@@ -867,19 +887,25 @@ func (p *provider) Cleanup(ctx context.Context, _ *zap.SugaredLogger, machine *c
867887
return false, sigClient.Delete(ctx, vm)
868888
}
869889

870-
func parseResources(cpus, memory string) (*corev1.ResourceList, error) {
890+
func parseResources(cpus, memory string, vpcus kubevirttypes.VCPUs) (*corev1.ResourceList, *kubevirtcorev1.CPU, error) {
871891
memoryResource, err := resource.ParseQuantity(memory)
872892
if err != nil {
873-
return nil, fmt.Errorf("failed to parse memory requests: %w", err)
893+
return nil, nil, fmt.Errorf("failed to parse memory requests: %w", err)
874894
}
895+
896+
if vpcus.Cores != 0 {
897+
return &corev1.ResourceList{corev1.ResourceMemory: memoryResource}, &kubevirtcorev1.CPU{Cores: uint32(vpcus.Cores)}, nil
898+
}
899+
875900
cpuResource, err := resource.ParseQuantity(cpus)
876901
if err != nil {
877-
return nil, fmt.Errorf("failed to parse cpu request: %w", err)
902+
return nil, nil, fmt.Errorf("failed to parse cpu requests: %w", err)
878903
}
904+
879905
return &corev1.ResourceList{
880906
corev1.ResourceMemory: memoryResource,
881907
corev1.ResourceCPU: cpuResource,
882-
}, nil
908+
}, nil, nil
883909
}
884910

885911
func (p *provider) SetMetricsForMachines(_ clusterv1alpha1.MachineList) error {

‎pkg/cloudprovider/provider/kubevirt/provider_test.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ type kubevirtProviderSpecConf struct {
7272
ProviderNetwork *kubevirt.ProviderNetwork
7373
ExtraHeadersSet bool
7474
EvictStrategy string
75+
VCPUs uint32
7576
}
7677

7778
func (k kubevirtProviderSpecConf) rawProviderSpec(t *testing.T) []byte {
@@ -132,7 +133,13 @@ func (k kubevirtProviderSpecConf) rawProviderSpec(t *testing.T) []byte {
132133
},
133134
{{- end }}
134135
"template": {
136+
{{- if .VCPUs }}
137+
"vcpus": {
138+
"cores": {{ .VCPUs }}
139+
},
140+
{{- else }}
135141
"cpus": "2",
142+
{{- end }}
136143
"memory": "2Gi",
137144
{{- if .SecondaryDisks }}
138145
"secondaryDisks": [{
@@ -283,6 +290,10 @@ func TestNewVirtualMachine(t *testing.T) {
283290
name: "eviction-strategy-live-migrate",
284291
specConf: kubevirtProviderSpecConf{EvictStrategy: "LiveMigrate"},
285292
},
293+
{
294+
name: "dedicated-vcpus",
295+
specConf: kubevirtProviderSpecConf{VCPUs: 2},
296+
},
286297
}
287298
for _, tt := range tests {
288299
t.Run(tt.name, func(t *testing.T) {

‎pkg/cloudprovider/provider/kubevirt/testdata/dedicated-vcpus.yaml‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,80 @@
1+
apiVersion: kubevirt.io/v1
2+
kind: VirtualMachine
3+
metadata:
4+
annotations:
5+
labels:
6+
cluster.x-k8s.io/cluster-name: cluster-name
7+
cluster.x-k8s.io/role: worker
8+
kubevirt.io/vm: dedicated-vcpus
9+
md: md-name
10+
name: dedicated-vcpus
11+
namespace: test-namespace
12+
spec:
13+
dataVolumeTemplates:
14+
- metadata:
15+
name: dedicated-vcpus
16+
spec:
17+
storage:
18+
accessModes:
19+
- ReadWriteMany
20+
resources:
21+
requests:
22+
storage: 10Gi
23+
storageClassName: longhorn
24+
source:
25+
http:
26+
url: http://x.y.z.t/ubuntu.img
27+
runStrategy: Once
28+
template:
29+
metadata:
30+
creationTimestamp: null
31+
annotations:
32+
"kubevirt.io/allow-pod-bridge-network-live-migration": "true"
33+
"ovn.kubernetes.io/allow_live_migration": "true"
34+
labels:
35+
cluster.x-k8s.io/cluster-name: cluster-name
36+
cluster.x-k8s.io/role: worker
37+
kubevirt.io/vm: dedicated-vcpus
38+
md: md-name
39+
spec:
40+
affinity: {}
41+
domain:
42+
cpu:
43+
cores: 2
44+
devices:
45+
disks:
46+
- disk:
47+
bus: virtio
48+
name: datavolumedisk
49+
- disk:
50+
bus: virtio
51+
name: cloudinitdisk
52+
interfaces:
53+
- name: default
54+
bridge: {}
55+
networkInterfaceMultiqueue: true
56+
resources:
57+
limits:
58+
memory: 2Gi
59+
requests:
60+
memory: 2Gi
61+
networks:
62+
- name: default
63+
pod: {}
64+
terminationGracePeriodSeconds: 30
65+
topologyspreadconstraints:
66+
- maxskew: 1
67+
topologykey: kubernetes.io/hostname
68+
whenunsatisfiable: ScheduleAnyway
69+
labelselector:
70+
matchlabels:
71+
md: md-name
72+
volumes:
73+
- dataVolume:
74+
name: dedicated-vcpus
75+
name: datavolumedisk
76+
- cloudInitNoCloud:
77+
secretRef:
78+
name: udsn
79+
name: cloudinitdisk
80+
evictionStrategy: External

‎sdk/cloudprovider/kubevirt/types.go‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,23 @@ type Flavor struct {
7070

7171
// Template.
7272
type Template struct {
73+
// VCPUs is to configure vcpus used by a the virtual machine
74+
// when using kubevirts cpuAllocationRatio feature this leads to auto assignment of the
75+
// calculated ratio as resource cpu requests for the pod which launches the virtual machine
76+
VCPUs VCPUs `json:"vcpus,omitempty"`
77+
// CPUs is to configure cpu requests and limits directly for the pod which launches the virtual machine
78+
// and is related to the underlying hardware
7379
CPUs providerconfig.ConfigVarString `json:"cpus,omitempty"`
7480
Memory providerconfig.ConfigVarString `json:"memory,omitempty"`
7581
PrimaryDisk PrimaryDisk `json:"primaryDisk,omitempty"`
7682
SecondaryDisks []SecondaryDisks `json:"secondaryDisks,omitempty"`
7783
}
7884

85+
// VCPUs.
86+
type VCPUs struct {
87+
Cores int `json:"cores,omitempty"`
88+
}
89+
7990
// PrimaryDisk.
8091
type PrimaryDisk struct {
8192
Disk

0 commit comments

Comments
 (0)